From 151d61e122367aaa2ba1f234f540cff19dadcad4 Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 18 Aug 2022 18:23:44 +0200 Subject: [PATCH 1/4] fix(cli): `--exclusively` leads to 0 stacks being selected The new concurrency logic introduced in #20345 was checking for all dependencies to a stack to have been deployed previously for the stack to be unblocked, but if not all stacks are selected, they will never become unblocked. Because there was no check on being "complete" at the end, this silently leads to no stacks being deployed and the CLI no-opping. Fix by only waiting for selected stacks among the dependencies, and adding a safeguard at the end to make sure we deployed all stacks successfully (or failed halfway through). --- packages/aws-cdk/lib/deploy.ts | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/deploy.ts b/packages/aws-cdk/lib/deploy.ts index 95f45e8d93299..9de65b16095d7 100644 --- a/packages/aws-cdk/lib/deploy.ts +++ b/packages/aws-cdk/lib/deploy.ts @@ -16,7 +16,7 @@ export const deployStacks = async (stacks: cxapi.CloudFormationStackArtifact[], stack.dependencies .map(({ id }) => id) .filter((id) => !id.endsWith('.assets')) - .every((id) => deploymentStates[id] === 'completed'); + .every((id) => !deploymentStates[id] || deploymentStates[id] === 'completed'); // Dependency not selected or already finished const hasAnyStackFailed = (states: Record) => Object.values(states).includes('failed'); @@ -57,4 +57,10 @@ export const deployStacks = async (stacks: cxapi.CloudFormationStackArtifact[], if (deploymentErrors.length) { throw Error(`Stack Deployments Failed: ${deploymentErrors}`); } + + // We shouldn't be able to get here, but check it anyway + const neverUnblocked = Object.entries(deploymentStates).filter(([_, s]) => s === 'pending').map(([n, _]) => n); + if (neverUnblocked.length > 0) { + throw new Error(`The following stacks never became unblocked: ${neverUnblocked.join(', ')}`); + } }; \ No newline at end of file From 64f2247ef7ba6514bba4af03e8e4b3f26c224d4b Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 19 Aug 2022 11:00:47 +0200 Subject: [PATCH 2/4] Add unit test --- packages/aws-cdk/test/deploy.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/aws-cdk/test/deploy.test.ts b/packages/aws-cdk/test/deploy.test.ts index 600f47531dd95..9a8a83b0d1c13 100644 --- a/packages/aws-cdk/test/deploy.test.ts +++ b/packages/aws-cdk/test/deploy.test.ts @@ -110,6 +110,14 @@ describe('DeployStacks', () => { ], expected: ['C', 'D', 'A', 'B'], }, + { + scenario: 'A -> B, A not selected', + concurrency: 1, + toDeploy: [ + { id: 'B', dependencies: [{ id: 'A' }] }, + ], + expected: ['B'], + }, ])('Success - Concurrency: $concurrency - $scenario', async ({ concurrency, expected, toDeploy }) => { await expect(deployStacks(toDeploy as unknown as Stack[], { concurrency, deployStack })).resolves.toBeUndefined(); From bab6c1499dde3a38fdcddd43272ad2fdcf36c5ae Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 19 Aug 2022 11:03:29 +0200 Subject: [PATCH 3/4] Prompt for bug report --- packages/aws-cdk/lib/deploy.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/aws-cdk/lib/deploy.ts b/packages/aws-cdk/lib/deploy.ts index 9de65b16095d7..75cc91b025c94 100644 --- a/packages/aws-cdk/lib/deploy.ts +++ b/packages/aws-cdk/lib/deploy.ts @@ -61,6 +61,6 @@ export const deployStacks = async (stacks: cxapi.CloudFormationStackArtifact[], // We shouldn't be able to get here, but check it anyway const neverUnblocked = Object.entries(deploymentStates).filter(([_, s]) => s === 'pending').map(([n, _]) => n); if (neverUnblocked.length > 0) { - throw new Error(`The following stacks never became unblocked: ${neverUnblocked.join(', ')}`); + throw new Error(`The following stacks never became unblocked: ${neverUnblocked.join(', ')}. Please report this at https://github.com/aws/aws-cdk/issues`); } }; \ No newline at end of file From 742e345b0bf676622da66c958f8131fbcfffde6d Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Fri, 19 Aug 2022 11:29:34 +0200 Subject: [PATCH 4/4] Add integ tests for --exclusively --- packages/aws-cdk/test/integ/cli/app/app.js | 9 +++++++- .../aws-cdk/test/integ/cli/cli.integtest.ts | 23 +++++++++++++++++++ 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/packages/aws-cdk/test/integ/cli/app/app.js b/packages/aws-cdk/test/integ/cli/app/app.js index 8c6a722bdb674..131e759e10495 100755 --- a/packages/aws-cdk/test/integ/cli/app/app.js +++ b/packages/aws-cdk/test/integ/cli/app/app.js @@ -279,6 +279,9 @@ class DockerStackWithCustomFile extends cdk.Stack { } } +/** + * A stack that will never succeed deploying (done in a way that CDK cannot detect but CFN will complain about) + */ class FailedStack extends cdk.Stack { constructor(parent, id, props) { @@ -400,7 +403,11 @@ switch (stackSet) { new LambdaHotswapStack(app, `${stackPrefix}-lambda-hotswap`); new DockerStack(app, `${stackPrefix}-docker`); new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`); - new FailedStack(app, `${stackPrefix}-failed`) + const failed = new FailedStack(app, `${stackPrefix}-failed`) + + // A stack that depends on the failed stack -- used to test that '-e' does not deploy the failing stack + const dependsOnFailed = new OutputsStack(app, `${stackPrefix}-depends-on-failed`); + dependsOnFailed.addDependency(failed); if (process.env.ENABLE_VPC_TESTING) { // Gating so we don't do context fetching unless that's what we are here for const env = { account: process.env.CDK_DEFAULT_ACCOUNT, region: process.env.CDK_DEFAULT_REGION }; diff --git a/packages/aws-cdk/test/integ/cli/cli.integtest.ts b/packages/aws-cdk/test/integ/cli/cli.integtest.ts index 77832d9df2a45..89477861966d1 100644 --- a/packages/aws-cdk/test/integ/cli/cli.integtest.ts +++ b/packages/aws-cdk/test/integ/cli/cli.integtest.ts @@ -143,6 +143,29 @@ integTest('automatic ordering with concurrency', withDefaultFixture(async (fixtu await fixture.cdkDestroy('order-providing'); })); +integTest('--exclusively selects only selected stack', withDefaultFixture(async (fixture) => { + // Deploy the "depends-on-failed" stack, with --exclusively. It will NOT fail (because + // of --exclusively) and it WILL create an output we can check for to confirm that it did + // get deployed. + const outputsFile = path.join(fixture.integTestDir, 'outputs', 'outputs.json'); + await fs.mkdir(path.dirname(outputsFile), { recursive: true }); + + await fixture.cdkDeploy('depends-on-failed', { + options: [ + '--exclusively', + '--outputs-file', outputsFile, + ], + }); + + // Verify the output to see that the stack deployed + const outputs = JSON.parse((await fs.readFile(outputsFile, { encoding: 'utf-8' })).toString()); + expect(outputs).toEqual({ + [`${fixture.stackNamePrefix}-depends-on-failed`]: { + TopicName: `${fixture.stackNamePrefix}-depends-on-failedMyTopic`, + }, + }); +})); + integTest('context setting', withDefaultFixture(async (fixture) => { await fs.writeFile(path.join(fixture.integTestDir, 'cdk.context.json'), JSON.stringify({ contextkey: 'this is the context value',