Skip to content

Commit

Permalink
[ftr] handle unexpected Kibana/ES shutdowns better (elastic#131767)
Browse files Browse the repository at this point in the history
  • Loading branch information
Spencer authored and kertal committed May 24, 2022
1 parent 9d3c667 commit 02a4165
Show file tree
Hide file tree
Showing 12 changed files with 108 additions and 29 deletions.
3 changes: 3 additions & 0 deletions packages/kbn-dev-utils/src/proc_runner/proc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -156,5 +156,8 @@ export function startProc(name: string, options: ProcOptions, log: ToolingLog) {
outcome$,
outcomePromise,
stop,
stopWasCalled() {
return stopCalled;
},
};
}
31 changes: 21 additions & 10 deletions packages/kbn-dev-utils/src/proc_runner/proc_runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ const noop = () => {};
interface RunOptions extends ProcOptions {
wait: true | RegExp;
waitTimeout?: number | false;
onEarlyExit?: (msg: string) => void;
}

/**
Expand All @@ -47,16 +48,6 @@ export class ProcRunner {

/**
* Start a process, tracking it by `name`
* @param {String} name
* @param {Object} options
* @property {String} options.cmd executable to run
* @property {Array<String>?} options.args arguments to provide the executable
* @property {String?} options.cwd current working directory for the process
* @property {RegExp|Boolean} options.wait Should start() wait for some time? Use
* `true` will wait until the proc exits,
* a `RegExp` will wait until that log line
* is found
* @return {Promise<undefined>}
*/
async run(name: string, options: RunOptions) {
const {
Expand All @@ -66,6 +57,7 @@ export class ProcRunner {
wait = false,
waitTimeout = 15 * MINUTE,
env = process.env,
onEarlyExit,
} = options;
const cmd = options.cmd === 'node' ? process.execPath : options.cmd;

Expand All @@ -89,6 +81,25 @@ export class ProcRunner {
stdin,
});

if (onEarlyExit) {
proc.outcomePromise
.then(
(code) => {
if (!proc.stopWasCalled()) {
onEarlyExit(`[${name}] exitted early with ${code}`);
}
},
(error) => {
if (!proc.stopWasCalled()) {
onEarlyExit(`[${name}] exitted early: ${error.message}`);
}
}
)
.catch((error) => {
throw new Error(`Error handling early exit: ${error.stack}`);
});
}

try {
if (wait instanceof RegExp) {
// wait for process to log matching line
Expand Down
19 changes: 19 additions & 0 deletions packages/kbn-es/src/cluster.js
Original file line number Diff line number Diff line change
Expand Up @@ -215,6 +215,25 @@ exports.Cluster = class Cluster {
}),
]);
});

if (options.onEarlyExit) {
this._outcome
.then(
() => {
if (!this._stopCalled) {
options.onEarlyExit(`ES exitted unexpectedly`);
}
},
(error) => {
if (!this._stopCalled) {
options.onEarlyExit(`ES exitted unexpectedly: ${error.stack}`);
}
}
)
.catch((error) => {
throw new Error(`failure handling early exit: ${error.stack}`);
});
}
}

/**
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-es/src/cluster_exec_options.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,5 @@ export interface EsClusterExecOptions {
password?: string;
skipReadyCheck?: boolean;
readyTimeout?: number;
onEarlyExit?: (msg: string) => void;
}
7 changes: 7 additions & 0 deletions packages/kbn-test/src/es/test_es_cluster.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,11 @@ export interface CreateTestEsClusterOptions {
* defaults to the transport port from `packages/kbn-test/src/es/es_test_config.ts`
*/
transportPort?: number | string;
/**
* Report to the creator of the es-test-cluster that the es node has exitted before stop() was called, allowing
* this caller to react appropriately. If this is not passed then an uncatchable exception will be thrown
*/
onEarlyExit?: (msg: string) => void;
}

export function createTestEsCluster<
Expand All @@ -165,6 +170,7 @@ export function createTestEsCluster<
clusterName: customClusterName = 'es-test-cluster',
ssl,
transportPort,
onEarlyExit,
} = options;

const clusterName = `${CI_PARALLEL_PROCESS_PREFIX}${customClusterName}`;
Expand Down Expand Up @@ -258,6 +264,7 @@ export function createTestEsCluster<
// set it up after the last node is started.
skipNativeRealmSetup: this.nodes.length > 1 && i < this.nodes.length - 1,
skipReadyCheck: this.nodes.length > 1 && i < this.nodes.length - 1,
onEarlyExit,
});
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export interface Test {
export interface Runner extends EventEmitter {
abort(): void;
failures: any[];
uncaught: (error: Error) => void;
}

export interface Mocha {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export class FunctionalTestRunner {
: new EsVersion(esVersion);
}

async run() {
async run(abortSignal?: AbortSignal) {
const testStats = await this.getTestStats();

return await this.runHarness(async (config, lifecycle, coreProviders) => {
Expand Down Expand Up @@ -106,10 +106,19 @@ export class FunctionalTestRunner {
return this.simulateMochaDryRun(mocha);
}

if (abortSignal?.aborted) {
this.log.warning('run aborted');
return;
}

await lifecycle.beforeTests.trigger(mocha.suite);
this.log.info('Starting tests');
if (abortSignal?.aborted) {
this.log.warning('run aborted');
return;
}

return await runTests(lifecycle, mocha);
this.log.info('Starting tests');
return await runTests(lifecycle, mocha, abortSignal);
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
* Side Public License, v 1.
*/

import * as Rx from 'rxjs';
import { Lifecycle } from '../lifecycle';
import { Mocha } from '../../fake_mocha_types';

Expand All @@ -18,14 +19,23 @@ import { Mocha } from '../../fake_mocha_types';
* @param {Mocha} mocha
* @return {Promise<Number>} resolves to the number of test failures
*/
export async function runTests(lifecycle: Lifecycle, mocha: Mocha) {
export async function runTests(lifecycle: Lifecycle, mocha: Mocha, abortSignal?: AbortSignal) {
let runComplete = false;
const runner = mocha.run(() => {
runComplete = true;
});

lifecycle.cleanup.add(() => {
if (!runComplete) runner.abort();
Rx.race(
lifecycle.cleanup.before$,
abortSignal ? Rx.fromEvent(abortSignal, 'abort').pipe(Rx.take(1)) : Rx.NEVER
).subscribe({
next() {
if (!runComplete) {
runComplete = true;
runner.uncaught(new Error('Forcing mocha to abort'));
runner.abort();
}
},
});

return new Promise((resolve) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface RunElasticsearchOptions {
log: ToolingLog;
esFrom?: string;
config: Config;
onEarlyExit?: (msg: string) => void;
}

interface CcsConfig {
Expand Down Expand Up @@ -92,7 +93,8 @@ export async function runElasticsearch(
async function startEsNode(
log: ToolingLog,
name: string,
config: EsConfig & { transportPort?: number }
config: EsConfig & { transportPort?: number },
onEarlyExit?: (msg: string) => void
) {
const cluster = createTestEsCluster({
clusterName: `cluster-${name}`,
Expand All @@ -112,6 +114,7 @@ async function startEsNode(
},
],
transportPort: config.transportPort,
onEarlyExit,
});

await cluster.start();
Expand Down
18 changes: 9 additions & 9 deletions packages/kbn-test/src/functional_tests/lib/run_ftr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ async function createFtr({
};
}

export async function assertNoneExcluded({ configPath, options }: CreateFtrParams) {
const { config, ftr } = await createFtr({ configPath, options });
export async function assertNoneExcluded(params: CreateFtrParams) {
const { config, ftr } = await createFtr(params);

if (config.get('testRunner')) {
// tests with custom test runners are not included in this check
Expand All @@ -95,30 +95,30 @@ export async function assertNoneExcluded({ configPath, options }: CreateFtrParam
}
if (stats.testsExcludedByTag.length > 0) {
throw new CliError(`
${stats.testsExcludedByTag.length} tests in the ${configPath} config
${stats.testsExcludedByTag.length} tests in the ${params.configPath} config
are excluded when filtering by the tags run on CI. Make sure that all suites are
tagged with one of the following tags:
${JSON.stringify(options.suiteTags)}
${JSON.stringify(params.options.suiteTags)}
- ${stats.testsExcludedByTag.join('\n - ')}
`);
}
}

export async function runFtr({ configPath, options }: CreateFtrParams) {
const { ftr } = await createFtr({ configPath, options });
export async function runFtr(params: CreateFtrParams, signal?: AbortSignal) {
const { ftr } = await createFtr(params);

const failureCount = await ftr.run();
const failureCount = await ftr.run(signal);
if (failureCount > 0) {
throw new CliError(
`${failureCount} functional test ${failureCount === 1 ? 'failure' : 'failures'}`
);
}
}

export async function hasTests({ configPath, options }: CreateFtrParams) {
const { ftr, config } = await createFtr({ configPath, options });
export async function hasTests(params: CreateFtrParams) {
const { ftr, config } = await createFtr(params);

if (config.get('testRunner')) {
// configs with custom test runners are assumed to always have tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ export async function runKibanaServer({
procs,
config,
options,
onEarlyExit,
}: {
procs: ProcRunner;
config: Config;
options: { installDir?: string; extraKbnOpts?: string[] };
onEarlyExit?: (msg: string) => void;
}) {
const runOptions = config.get('kbnTestServer.runOptions');
const installDir = runOptions.alwaysUseSource ? undefined : options.installDir;
Expand All @@ -51,6 +53,7 @@ export async function runKibanaServer({
},
cwd: installDir || KIBANA_ROOT,
wait: runOptions.wait,
onEarlyExit,
});
}

Expand Down
18 changes: 15 additions & 3 deletions packages/kbn-test/src/functional_tests/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,26 @@ export async function runTests(options: RunTestsParams) {

await withProcRunner(log, async (procs) => {
const config = await readConfigFile(log, options.esVersion, configPath);
const abortCtrl = new AbortController();

const onEarlyExit = (msg: string) => {
log.error(msg);
abortCtrl.abort();
};

let shutdownEs;
try {
if (process.env.TEST_ES_DISABLE_STARTUP !== 'true') {
shutdownEs = await runElasticsearch({ ...options, log, config });
shutdownEs = await runElasticsearch({ ...options, log, config, onEarlyExit });
if (abortCtrl.signal.aborted) {
return;
}
}
await runKibanaServer({ procs, config, options, onEarlyExit });
if (abortCtrl.signal.aborted) {
return;
}
await runKibanaServer({ procs, config, options });
await runFtr({ configPath, options: { ...options, log } });
await runFtr({ configPath, options: { ...options, log } }, abortCtrl.signal);
} finally {
try {
const delay = config.get('kbnTestServer.delayShutdown');
Expand Down

0 comments on commit 02a4165

Please sign in to comment.