Repository navigation
refactor(hcl2cdk): global setup/teardown along with parallel tests to speed the b… #41
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+385
−78
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,134 @@ | ||
| # CI Optimization: Reduce hcl2cdk Test Runtime & Scoped Builds | ||
|
|
||
| ## Summary | ||
|
|
||
| Three changes that together save ~16 min of aggregate CI compute per run: | ||
|
|
||
| 1. **Scoped builds** — Replace full monorepo `yarn build` with per-package `lerna run --scope` builds, saving ~35s per job across 14 non-hcl2cdk jobs. | ||
| 2. **Parallel test execution** — Remove `--runInBand` from hcl2cdk tests. A new Jest `globalSetup` pre-generates all provider bindings and base projects once, eliminating the per-worker race conditions that originally forced serialization. | ||
| 3. **Provider schema pre-caching** — The `globalSetup` also pre-caches all 12 provider schemas using `CDKTF_EXPERIMENTAL_PROVIDER_SCHEMA_CACHE_PATH`, so test workers never shell out to `terraform init` / `terraform providers schema`. This eliminates the Terraform plugin cache collisions that caused test failures under parallelism. | ||
|
so0k marked this conversation as resolved.
Resolved
|
||
|
|
||
| **Result**: hcl2cdk test step drops from ~11m27s (serial) to ~7 min (parallel), with 0 Terraform-related flaky failures. | ||
|
|
||
| ## Problem | ||
|
|
||
| The `unit_test (@cdktn/hcl2cdk)` CI job took ~14m54s per run, with the test step alone at ~11m27s (77% of job time). Two main issues: | ||
|
|
||
| 1. **Every unit test job compiled the full monorepo** (~63s each, 16 jobs = ~15 min aggregate waste) | ||
| 2. **hcl2cdk tests ran single-threaded** (`--runInBand`) on a 16-core depot runner | ||
|
|
||
| ### Why `--runInBand` existed | ||
|
|
||
| In September 2023, parallel test workers were each independently running `cdktf get` and `cdktf init`, causing Terraform plugin cache collisions and filesystem race conditions. After several attempts to fix the parallelism (`41a2e7f`, `f2e06fd`), the workaround was to serialize all tests with `--runInBand` (`5ba985c`). | ||
|
|
||
| ## Changes | ||
|
|
||
| ### 1. Jest `globalSetup` / `globalTeardown` (new files) | ||
|
|
||
| **`packages/@cdktn/hcl2cdk/test/globalSetup.ts`** | ||
|
|
||
| Runs once before any test worker starts: | ||
|
|
||
| - Generates all 12 provider bindings via `cdktn get` (in parallel) | ||
| - Initializes base projects for required languages via `cdktn init` (in parallel) | ||
| - Pre-caches provider schemas via `readSchema()` (sequentially, one provider at a time) into a schema cache directory, using the `CDKTF_EXPERIMENTAL_PROVIDER_SCHEMA_CACHE_PATH` mechanism — this eliminates all `terraform init` / `terraform providers schema` calls from test workers | ||
| - Writes a JSON manifest mapping provider FQNs, languages, and schema cache path to their generated paths | ||
| - Sets env vars (`HCL2CDK_FIXTURES_MANIFEST`, `CDKTF_EXPERIMENTAL_PROVIDER_SCHEMA_CACHE_PATH`) for workers | ||
|
|
||
| **`packages/@cdktn/hcl2cdk/test/globalTeardown.ts`** | ||
|
|
||
| Runs once after all workers finish: | ||
|
|
||
| - Cleans up the temp fixtures directory | ||
|
|
||
| ### 2. Refactored test helper to use pre-generated fixtures | ||
|
|
||
| **`packages/@cdktn/hcl2cdk/test/helpers/convert.ts`** | ||
|
|
||
| - Removed module-level `prepareBaseProject()` calls and `generateBindings()` / `providerBindingCache` — these ran expensive `cdktn init`/`cdktn get` per-worker | ||
| - Added `getManifest()` that lazily reads the fixtures manifest from the env var | ||
| - `copyBindingsForProvider()` now copies from pre-generated paths in the manifest | ||
| - `getProjectDirectory()` now reads base project paths from the manifest instead of awaiting per-worker promises | ||
| - `getProviderSchema()` now uses the schema cache (via env var or manifest), so `readSchema()` gets cache hits and never shells out to Terraform | ||
|
|
||
| ### 3. Enabled file-level parallelism | ||
|
|
||
| **`packages/@cdktn/hcl2cdk/package.json`** | ||
|
|
||
| Removed `--runInBand` from `test`, `test:ci`, and `test:update` scripts. This is now safe because: | ||
|
so0k marked this conversation as resolved.
Resolved
|
||
|
|
||
| - Provider bindings, base projects, and schemas are generated once in `globalSetup` (no more cache collisions) | ||
| - Workers never run `terraform init` or `terraform providers schema` — all schema reads are cache hits | ||
| - `process.chdir()` is isolated per Jest worker process | ||
| - 19 test files can run across multiple cores simultaneously | ||
|
|
||
| **`packages/@cdktn/hcl2cdk/jest.config.js`** | ||
|
|
||
| Added `globalSetup` and `globalTeardown` entries pointing to the new files. | ||
|
|
||
| ### 4. Scoped compile step in CI | ||
|
|
||
| **`.github/workflows/unit.yml`** | ||
|
|
||
| Replaced the full monorepo build: | ||
|
|
||
| ```yaml | ||
| # Before | ||
| - name: compile | ||
| run: | | ||
| tools/align-version.sh | ||
| yarn build | ||
| yarn package | ||
| ``` | ||
|
|
||
| With a scoped build per package, plus a conditional step for hcl2cdk: | ||
|
|
||
| ```yaml | ||
| # After | ||
| - name: compile | ||
| run: | | ||
| tools/align-version.sh | ||
| npx lerna run --scope '${{ inputs.package }}' build | ||
|
|
||
| - name: compile cdktn-cli and package | ||
| if: inputs.package == '@cdktn/hcl2cdk' | ||
| run: | | ||
| npx lerna run --scope 'cdktn-cli' build | ||
| yarn package | ||
| ``` | ||
|
|
||
| Nx integration (`build.dependsOn: ["^build"]`) automatically resolves and builds transitive dependencies. The `@cdktn/hcl2cdk` tests additionally need the `cdktn-cli` binary and `dist/` packages. | ||
|
|
||
| ## Local Verification | ||
|
|
||
| - Full build + package: ~61s | ||
| - hcl2cdk tests (parallelized): **~7 min wall clock** (includes globalSetup schema caching) | ||
| - 27/28 suites passed (1 skipped), 279/280 tests passed (1 skipped), 202 snapshots matched | ||
| - 0 Terraform cache collision failures (previously 15 failures without schema pre-caching) | ||
| - `globalSetup` and `globalTeardown` ran and cleaned up successfully | ||
|
|
||
| ## Efficiency Gains | ||
|
|
||
| ### Before (baseline) | ||
|
|
||
| | Metric | Value | | ||
| | ------------------------- | ------------------------------------------------------------------- | | ||
| | hcl2cdk CI job wall clock | ~14m54s | | ||
| | hcl2cdk test step alone | ~11m27s (single-threaded `--runInBand`) | | ||
| | Compile step per CI job | full monorepo `yarn build` (~63s each, 16 jobs = ~15 min aggregate) | | ||
|
|
||
| ### After | ||
|
|
||
| | Metric | Value | | ||
| | --------------------------------------- | ------------------------------------------------------------------- | | ||
| | hcl2cdk test step (local, parallelized) | ~7 min wall clock (was ~11m27s) | | ||
| | Terraform calls during tests | 0 (all cached in globalSetup) | | ||
| | Compile step per CI job | scoped `lerna run --scope` build (~35s savings per non-hcl2cdk job) | | ||
|
|
||
| ### Estimated CI Impact | ||
|
|
||
| | Optimization | Estimated Savings | | ||
| | ----------------------------------------- | -------------------------------------------------- | | ||
| | Scoped builds (non-hcl2cdk jobs) | ~35s avg x 14 jobs = **~8 min aggregate** | | ||
| | hcl2cdk test parallelism + schema caching | ~4 min per job x 2 terraform versions = **~8 min** | | ||
| | **Total** | **~16 min aggregate compute saved per CI run** | | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,196 @@ | ||
| // Copyright (c) HashiCorp, Inc | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
| import * as fs from "fs-extra"; | ||
| import * as path from "path"; | ||
| import * as os from "os"; | ||
| import execa from "execa"; | ||
| import { readSchema } from "@cdktn/provider-schema"; | ||
| import { | ||
| TerraformProviderConstraint, | ||
| TerraformModuleConstraint, | ||
| } from "@cdktn/commons"; | ||
|
|
||
| const cdktnBin = path.join(__dirname, "../../../cdktn-cli/bundle/bin/cdktn"); | ||
| const cdktnDist = path.join(__dirname, "../../../../dist"); | ||
|
|
||
| const includeSynthTests = Boolean(process.env.CI); | ||
|
|
||
| enum ProviderType { | ||
| provider, | ||
| module, | ||
| } | ||
|
|
||
| type ProviderDefinition = { | ||
| fqn: string; | ||
| type: ProviderType; | ||
| path: string; | ||
| }; | ||
|
|
||
| const bindings: Record<string, ProviderDefinition> = { | ||
| aws: { | ||
| fqn: "hashicorp/aws@=5.11.0", | ||
| type: ProviderType.provider, | ||
| path: "providers/aws", | ||
| }, | ||
| docker: { | ||
| fqn: "kreuzwerker/docker@=3.0.1", | ||
| type: ProviderType.provider, | ||
| path: "providers/docker", | ||
| }, | ||
| null: { | ||
| fqn: "hashicorp/null@=3.2.1", | ||
| type: ProviderType.provider, | ||
| path: "providers/null", | ||
| }, | ||
| google: { | ||
| fqn: "hashicorp/google@=4.55.0", | ||
| type: ProviderType.provider, | ||
| path: "providers/google", | ||
| }, | ||
| azuread: { | ||
| fqn: "hashicorp/azuread@=2.36.0", | ||
| type: ProviderType.provider, | ||
| path: "providers/azuread", | ||
| }, | ||
| local: { | ||
| fqn: "hashicorp/local@=2.3.0", | ||
| type: ProviderType.provider, | ||
| path: "providers/local", | ||
| }, | ||
| auth0: { | ||
| fqn: "alexkappa/auth0@=0.26.2", | ||
| type: ProviderType.provider, | ||
| path: "providers/auth0", | ||
| }, | ||
| datadog: { | ||
| fqn: "DataDog/datadog@=3.21.0", | ||
| type: ProviderType.provider, | ||
| path: "providers/datadog", | ||
| }, | ||
| kubernetes: { | ||
| fqn: "hashicorp/kubernetes@=2.18.0", | ||
| type: ProviderType.provider, | ||
| path: "providers/kubernetes", | ||
| }, | ||
| scaleway: { | ||
| fqn: "scaleway/scaleway@ ~>2.10.0", | ||
| type: ProviderType.provider, | ||
| path: "providers/scaleway", | ||
| }, | ||
| external: { | ||
| fqn: "hashicorp/external@=2.3.1", | ||
| type: ProviderType.provider, | ||
| path: "providers/external", | ||
| }, | ||
| awsVpc: { | ||
| fqn: "terraform-aws-modules/vpc/aws@=3.19.0", | ||
| type: ProviderType.module, | ||
| path: "modules/terraform-aws-modules/aws", | ||
| }, | ||
| }; | ||
|
|
||
| async function generateBinding(binding: ProviderDefinition): Promise<string> { | ||
| const tempDir = await fs.mkdtemp(path.join(os.tmpdir(), "cdktf-provider-")); | ||
| await fs.writeFile( | ||
| path.resolve(tempDir, "cdktf.json"), | ||
| JSON.stringify({ | ||
| language: "typescript", | ||
| app: "npx ts-node main.ts", | ||
| terraformProviders: | ||
| binding.type === ProviderType.provider ? [binding.fqn] : [], | ||
| terraformModules: | ||
| binding.type === ProviderType.module ? [binding.fqn] : [], | ||
| }), | ||
| ); | ||
| await execa(cdktnBin, ["get"], { cwd: tempDir }); | ||
|
|
||
| return path.resolve(tempDir, ".gen", binding.path); | ||
| } | ||
|
|
||
| async function prepareBaseProject(language: string): Promise<string> { | ||
| const projectDir = await fs.mkdtemp( | ||
| path.join(os.tmpdir(), "cdktf-convert-base-"), | ||
| ); | ||
| await execa( | ||
| cdktnBin, | ||
| [ | ||
| "init", | ||
| "--local", | ||
| `--dist=${cdktnDist}`, | ||
| "--project-name='hello'", | ||
| "--project-description='world'", | ||
| `--template=${language}`, | ||
| "--enable-crash-reporting=false", | ||
| ], | ||
| { | ||
| cwd: projectDir, | ||
| }, | ||
| ); | ||
| return projectDir; | ||
| } | ||
|
|
||
| export interface FixturesManifest { | ||
| providerBindings: Record<string, string>; // fqn -> absolute path to generated binding dir | ||
| baseProjects: Record<string, string>; // language -> absolute path to base project dir | ||
| schemaCacheDir: string; // absolute path to pre-populated provider schema cache | ||
| } | ||
|
|
||
| module.exports = async function globalSetup() { | ||
| const fixturesDir = await fs.mkdtemp( | ||
| path.join(os.tmpdir(), "hcl2cdk-fixtures-"), | ||
| ); | ||
|
|
||
| console.log("[globalSetup] Generating provider bindings..."); | ||
| const bindingEntries = Object.values(bindings); | ||
| const bindingResults = await Promise.all( | ||
| bindingEntries.map(async (b) => { | ||
| const absolutePath = await generateBinding(b); | ||
| return [b.fqn, absolutePath] as [string, string]; | ||
| }), | ||
| ); | ||
| const providerBindings = Object.fromEntries(bindingResults); | ||
|
|
||
| const requiredLanguages = includeSynthTests | ||
| ? ["typescript", "python", "csharp"] | ||
| : ["typescript"]; | ||
|
|
||
| console.log( | ||
| `[globalSetup] Initializing base projects for: ${requiredLanguages.join(", ")}...`, | ||
| ); | ||
| const baseProjectEntries = await Promise.all( | ||
| requiredLanguages.map(async (lang) => { | ||
| const projectDir = await prepareBaseProject(lang); | ||
| return [lang, projectDir] as [string, string]; | ||
| }), | ||
| ); | ||
| const baseProjects = Object.fromEntries(baseProjectEntries); | ||
|
|
||
| // Pre-cache provider schemas so test workers never need to run terraform | ||
| const schemaCacheDir = path.join(fixturesDir, "schema-cache"); | ||
| await fs.mkdirp(schemaCacheDir); | ||
|
|
||
| console.log("[globalSetup] Pre-caching provider schemas..."); | ||
| for (const entry of bindingEntries) { | ||
| const constraint = | ||
| entry.type === ProviderType.provider | ||
| ? new TerraformProviderConstraint(entry.fqn) | ||
| : new TerraformModuleConstraint(entry.fqn); | ||
| await readSchema([constraint], schemaCacheDir); | ||
| } | ||
|
|
||
| const manifest: FixturesManifest = { | ||
| providerBindings, | ||
| baseProjects, | ||
| schemaCacheDir, | ||
| }; | ||
| const manifestPath = path.join(fixturesDir, "manifest.json"); | ||
| await fs.writeFile(manifestPath, JSON.stringify(manifest, null, 2)); | ||
|
|
||
| // Pass to test workers via environment variables | ||
| // (Jest 29 propagates parent env to forked workers) | ||
| process.env.HCL2CDK_FIXTURES_MANIFEST = manifestPath; | ||
| process.env.CDKTF_EXPERIMENTAL_PROVIDER_SCHEMA_CACHE_PATH = schemaCacheDir; | ||
| process.env.HCL2CDK_FIXTURES_DIR = fixturesDir; | ||
|
|
||
| console.log(`[globalSetup] Fixtures manifest written to ${manifestPath}`); | ||
| }; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| // Copyright (c) HashiCorp, Inc | ||
| // SPDX-License-Identifier: MPL-2.0 | ||
| import * as fs from "fs-extra"; | ||
|
|
||
| module.exports = async function globalTeardown() { | ||
| const fixturesDir = process.env.HCL2CDK_FIXTURES_DIR; | ||
| if (fixturesDir) { | ||
| console.log(`[globalTeardown] Cleaning up fixtures at ${fixturesDir}`); | ||
| await fs.remove(fixturesDir); | ||
| } | ||
| }; |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.