From a25433c394a15afd1e56ab62c3e49ba592c79804 Mon Sep 17 00:00:00 2001 From: Alex Szabo Date: Wed, 21 Aug 2024 23:10:41 +0200 Subject: [PATCH] [CI] Parallelize quick checks (#190401) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary Use typescript's async processes to start quick checks in parallel 👍 Check out these for runs: - happy case: https://buildkite.com/elastic/kibana-pull-request/builds/227443#01914ca3-1f0d-4178-b539-263fbc588e98 - some broken checks: https://buildkite.com/elastic/kibana-pull-request/builds/228957#01917607-f7bd-4e08-8c70-7fdc3f9c12d1 Benefits: - with this (+more CPU) we can speed up the quick-check step's runtime, from ~15m to ~7m. - the added benefit is that all checks run so that we won't bail on the 1st one Disadvantage: - uglier error output, since we collect the logs asynchronously, and print it only upon failure - ~no output printed for happy checks (can be changed)~ Extra: - additionally, `yarn quick-checks` will now allow devs to run these checks locally (adjustments made so that the checks won't fail in local dev) - added the option to declare a 'context' for tooling loggers, so we can identify which script logs Solves 2/3 of https://github.com/elastic/kibana-operations/issues/124 (+speedup) --- .buildkite/pipelines/on_merge.yml | 2 +- .buildkite/pipelines/pull_request/base.yml | 2 +- .buildkite/scripts/common/util.sh | 2 +- .buildkite/scripts/steps/checks/event_log.sh | 20 +- .../scripts/steps/checks/quick_checks.txt | 19 ++ .buildkite/scripts/steps/quick_checks.sh | 26 +-- package.json | 1 + scripts/quick_checks.js | 10 + src/dev/run_quick_checks.ts | 196 ++++++++++++++++++ 9 files changed, 253 insertions(+), 25 deletions(-) create mode 100644 .buildkite/scripts/steps/checks/quick_checks.txt create mode 100644 scripts/quick_checks.js create mode 100644 src/dev/run_quick_checks.ts diff --git a/.buildkite/pipelines/on_merge.yml b/.buildkite/pipelines/on_merge.yml index 4eb15c16970ef3..7719b338b13efa 100644 --- a/.buildkite/pipelines/on_merge.yml +++ b/.buildkite/pipelines/on_merge.yml @@ -37,7 +37,7 @@ steps: image: family/kibana-ubuntu-2004 imageProject: elastic-images-prod provider: gcp - machineType: n2-standard-2 + machineType: n2-highcpu-8 preemptible: true key: quick_checks timeout_in_minutes: 60 diff --git a/.buildkite/pipelines/pull_request/base.yml b/.buildkite/pipelines/pull_request/base.yml index e5da8ce788e5b6..2f2e0a739a3043 100644 --- a/.buildkite/pipelines/pull_request/base.yml +++ b/.buildkite/pipelines/pull_request/base.yml @@ -23,7 +23,7 @@ steps: - command: .buildkite/scripts/steps/quick_checks.sh label: 'Quick Checks' agents: - machineType: n2-standard-2 + machineType: n2-highcpu-8 preemptible: true key: quick_checks timeout_in_minutes: 60 diff --git a/.buildkite/scripts/common/util.sh b/.buildkite/scripts/common/util.sh index d50fafad6967b9..dce418180c107d 100755 --- a/.buildkite/scripts/common/util.sh +++ b/.buildkite/scripts/common/util.sh @@ -12,7 +12,7 @@ is_pr_with_label() { IFS=',' read -ra labels <<< "${GITHUB_PR_LABELS:-}" - for label in "${labels[@]}" + for label in "${labels[@]:-}" do if [ "$label" == "$match" ]; then return diff --git a/.buildkite/scripts/steps/checks/event_log.sh b/.buildkite/scripts/steps/checks/event_log.sh index dc9c01902c0109..930908bb2d0ece 100755 --- a/.buildkite/scripts/steps/checks/event_log.sh +++ b/.buildkite/scripts/steps/checks/event_log.sh @@ -8,7 +8,25 @@ echo --- Check Event Log Schema # event log schema is pinned to a specific version of ECS ECS_STABLE_VERSION=1.8 -git clone --depth 1 -b $ECS_STABLE_VERSION https://github.com/elastic/ecs.git ../ecs + +# we can potentially skip this check on a local env, if ../ecs is present, and modified by the developer +if [[ "${CI:-false}" =~ ^(0|false)$ ]] && [[ -d '../ecs' ]]; then + LOCAL_ECS_BRANCH=$(git -C ../ecs branch --show-current) + if [[ "$LOCAL_ECS_BRANCH" != "$ECS_STABLE_VERSION" ]]; then + echo "Skipping event log schema check because ECS schema is not on $ECS_STABLE_VERSION." + exit 0 + fi + + TOUCHED_FILES=$(git -C ../ecs status --porcelain) + if [[ -n "$TOUCHED_FILES" ]]; then + echo "Skipping event log schema check because ECS schema files have been modified." + exit 0 + fi + + echo "../ecs is already cloned and @ $ECS_STABLE_VERSION" +else + git clone --depth 1 -b $ECS_STABLE_VERSION https://github.com/elastic/ecs.git ../ecs +fi node x-pack/plugins/event_log/scripts/create_schemas.js diff --git a/.buildkite/scripts/steps/checks/quick_checks.txt b/.buildkite/scripts/steps/checks/quick_checks.txt new file mode 100644 index 00000000000000..028aa47ff9567a --- /dev/null +++ b/.buildkite/scripts/steps/checks/quick_checks.txt @@ -0,0 +1,19 @@ +.buildkite/scripts/steps/checks/precommit_hook.sh +.buildkite/scripts/steps/checks/ts_projects.sh +.buildkite/scripts/steps/checks/packages.sh +.buildkite/scripts/steps/checks/bazel_packages.sh +.buildkite/scripts/steps/checks/verify_notice.sh +.buildkite/scripts/steps/checks/plugin_list_docs.sh +.buildkite/scripts/steps/checks/event_log.sh +.buildkite/scripts/steps/checks/telemetry.sh +.buildkite/scripts/steps/checks/jest_configs.sh +.buildkite/scripts/steps/checks/bundle_limits.sh +.buildkite/scripts/steps/checks/i18n.sh +.buildkite/scripts/steps/checks/file_casing.sh +.buildkite/scripts/steps/checks/licenses.sh +.buildkite/scripts/steps/checks/test_projects.sh +.buildkite/scripts/steps/checks/test_hardening.sh +.buildkite/scripts/steps/checks/ftr_configs.sh +.buildkite/scripts/steps/checks/yarn_deduplicate.sh +.buildkite/scripts/steps/checks/prettier_topology.sh +.buildkite/scripts/steps/checks/renovate.sh diff --git a/.buildkite/scripts/steps/quick_checks.sh b/.buildkite/scripts/steps/quick_checks.sh index eb14209f97f5a3..1b1613d42dc8dc 100755 --- a/.buildkite/scripts/steps/quick_checks.sh +++ b/.buildkite/scripts/steps/quick_checks.sh @@ -2,25 +2,9 @@ set -euo pipefail -export DISABLE_BOOTSTRAP_VALIDATION=false -.buildkite/scripts/bootstrap.sh +if [[ "${CI:-}" =~ ^(1|true)$ ]]; then + export DISABLE_BOOTSTRAP_VALIDATION=false + .buildkite/scripts/bootstrap.sh +fi -.buildkite/scripts/steps/checks/precommit_hook.sh -.buildkite/scripts/steps/checks/ts_projects.sh -.buildkite/scripts/steps/checks/packages.sh -.buildkite/scripts/steps/checks/bazel_packages.sh -.buildkite/scripts/steps/checks/verify_notice.sh -.buildkite/scripts/steps/checks/plugin_list_docs.sh -.buildkite/scripts/steps/checks/event_log.sh -.buildkite/scripts/steps/checks/telemetry.sh -.buildkite/scripts/steps/checks/jest_configs.sh -.buildkite/scripts/steps/checks/bundle_limits.sh -.buildkite/scripts/steps/checks/i18n.sh -.buildkite/scripts/steps/checks/file_casing.sh -.buildkite/scripts/steps/checks/licenses.sh -.buildkite/scripts/steps/checks/test_projects.sh -.buildkite/scripts/steps/checks/test_hardening.sh -.buildkite/scripts/steps/checks/ftr_configs.sh -.buildkite/scripts/steps/checks/yarn_deduplicate.sh -.buildkite/scripts/steps/checks/prettier_topology.sh -.buildkite/scripts/steps/checks/renovate.sh +node scripts/quick_checks --file .buildkite/scripts/steps/checks/quick_checks.txt diff --git a/package.json b/package.json index ee6ebf60ce1878..0b93cb96d4e475 100644 --- a/package.json +++ b/package.json @@ -55,6 +55,7 @@ "lint:es": "node scripts/eslint", "lint:style": "node scripts/stylelint", "makelogs": "node scripts/makelogs", + "quick-checks": "node scripts/quick_checks", "serverless": "node scripts/kibana --dev --serverless", "serverless-es": "node scripts/kibana --dev --serverless=es", "serverless-oblt": "node scripts/kibana --dev --serverless=oblt", diff --git a/scripts/quick_checks.js b/scripts/quick_checks.js new file mode 100644 index 00000000000000..db6a3b1daeef4c --- /dev/null +++ b/scripts/quick_checks.js @@ -0,0 +1,10 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +require('../src/setup_node_env'); +require('../src/dev/run_quick_checks'); diff --git a/src/dev/run_quick_checks.ts b/src/dev/run_quick_checks.ts new file mode 100644 index 00000000000000..cdb59bdce3cb27 --- /dev/null +++ b/src/dev/run_quick_checks.ts @@ -0,0 +1,196 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License + * 2.0 and the Server Side Public License, v 1; you may not use this file except + * in compliance with, at your election, the Elastic License 2.0 or the Server + * Side Public License, v 1. + */ + +import { exec } from 'child_process'; +import { availableParallelism } from 'os'; +import { join, isAbsolute } from 'path'; +import { readdirSync, readFileSync } from 'fs'; + +import { run, RunOptions } from '@kbn/dev-cli-runner'; +import { REPO_ROOT } from '@kbn/repo-info'; +import { ToolingLog } from '@kbn/tooling-log'; + +const MAX_PARALLELISM = availableParallelism(); +const buildkiteQuickchecksFolder = join('.buildkite', 'scripts', 'steps', 'checks'); +const quickChecksList = join(buildkiteQuickchecksFolder, 'quick_checks.txt'); +const sleep = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + +interface CheckResult { + success: boolean; + script: string; + output: string; + durationMs: number; +} + +const scriptOptions: RunOptions = { + description: ` + Runs sanity-testing quick-checks in parallel. + - arguments (--file, --dir, --checks) are exclusive - only one can be used at a time. + `, + flags: { + string: ['dir', 'checks', 'file'], + help: ` + --file Run all checks from a given file. (default='${quickChecksList}') + --dir Run all checks in a given directory. + --checks Runs all scripts given in this parameter. (comma or newline delimited) + `, + }, + log: { + context: 'quick-checks', + defaultLevel: process.env.CI === 'true' ? 'debug' : 'info', + }, +}; + +let logger: ToolingLog; +void run(async ({ log, flagsReader }) => { + logger = log; + + const scriptsToRun = collectScriptsToRun({ + targetFile: flagsReader.string('file'), + targetDir: flagsReader.string('dir'), + checks: flagsReader.string('checks'), + }); + + logger.write( + `--- Running ${scriptsToRun.length} checks, with parallelism ${MAX_PARALLELISM}...`, + scriptsToRun + ); + const startTime = Date.now(); + const results = await runAllChecks(scriptsToRun); + + logger.write('--- All checks finished.'); + printResults(startTime, results); + + const failedChecks = results.filter((check) => !check.success); + if (failedChecks.length > 0) { + logger.write(`--- ${failedChecks.length} quick check(s) failed. ❌`); + logger.write(`See above for details.`); + process.exitCode = 1; + } else { + logger.write('--- All checks passed. ✅'); + return results; + } +}, scriptOptions); + +function collectScriptsToRun(inputOptions: { + targetFile: string | undefined; + targetDir: string | undefined; + checks: string | undefined; +}) { + const { targetFile, targetDir, checks } = inputOptions; + if ([targetFile, targetDir, checks].filter(Boolean).length > 1) { + throw new Error('Only one of --file, --dir, or --checks can be used at a time.'); + } + + if (targetDir) { + const targetDirAbsolute = isAbsolute(targetDir) ? targetDir : join(REPO_ROOT, targetDir); + return readdirSync(targetDirAbsolute).map((file) => join(targetDir, file)); + } else if (checks) { + return checks + .trim() + .split(/[,\n]/) + .map((script) => script.trim()); + } else { + const targetFileWithDefault = targetFile || quickChecksList; + const targetFileAbsolute = isAbsolute(targetFileWithDefault) + ? targetFileWithDefault + : join(REPO_ROOT, targetFileWithDefault); + + return readFileSync(targetFileAbsolute, 'utf-8') + .trim() + .split('\n') + .map((line) => line.trim()); + } +} + +async function runAllChecks(scriptsToRun: string[]) { + const checksRunning: Array> = []; + const checksFinished: CheckResult[] = []; + + while (scriptsToRun.length > 0 || checksRunning.length > 0) { + while (scriptsToRun.length > 0 && checksRunning.length < MAX_PARALLELISM) { + const script = scriptsToRun.shift(); + if (!script) { + continue; + } + + const check = runCheckAsync(script); + checksRunning.push(check); + check.then((result) => { + checksRunning.splice(checksRunning.indexOf(check), 1); + checksFinished.push(result); + }); + } + + await sleep(1000); + } + + return checksFinished; +} + +async function runCheckAsync(script: string): Promise { + logger.info(`Starting check: ${script}`); + const startTime = Date.now(); + + return new Promise((resolve) => { + const scriptProcess = exec(script); + let output = ''; + const appendToOutput = (data: string | Buffer) => (output += data); + + scriptProcess.stdout?.on('data', appendToOutput); + scriptProcess.stderr?.on('data', appendToOutput); + + scriptProcess.on('exit', (code) => { + const result: CheckResult = { + success: code === 0, + script, + output, + durationMs: Date.now() - startTime, + }; + if (code === 0) { + logger.info(`Passed check: ${script} in ${humanizeTime(result.durationMs)}`); + } else { + logger.warning(`Failed check: ${script} in ${humanizeTime(result.durationMs)}`); + } + + resolve(result); + }); + }); +} + +function printResults(startTimestamp: number, results: CheckResult[]) { + const totalDuration = results.reduce((acc, result) => acc + result.durationMs, 0); + const total = humanizeTime(totalDuration); + const effective = humanizeTime(Date.now() - startTimestamp); + logger.info(`- Total time: ${total}, effective: ${effective}`); + + results.forEach((result) => { + logger.write( + `--- ${result.success ? '✅' : '❌'} ${result.script}: ${humanizeTime(result.durationMs)}` + ); + if (result.success) { + logger.debug(result.output); + } else { + logger.warning(result.output); + } + }); +} + +function humanizeTime(ms: number) { + if (ms < 1000) { + return `${ms}ms`; + } + + const minutes = Math.floor(ms / 1000 / 60); + const seconds = Math.floor((ms - minutes * 60 * 1000) / 1000); + if (minutes === 0) { + return `${seconds}s`; + } else { + return `${minutes}m ${seconds}s`; + } +}