Skip to content
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

feat(cli): --concurrency option #19378

Merged
merged 18 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/aws-cdk/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ currently deployed stack to the template and tags that are about to be deployed
will skip deployment if they are identical. Use `--force` to override this behavior
and always deploy the stack.



#### Disabling Rollback

If a resource fails to be created or updated, the deployment will *roll back* before the CLI returns. All changes made
Expand All @@ -183,6 +185,8 @@ In order to deploy them, you can list the stacks you want to deploy. If your app

If you want to deploy all of them, you can use the flag `--all` or the wildcard `*` to deploy all stacks in an app. Please note that, if you have a hierarchy of stacks as described above, `--all` and `*` will only match the stacks on the top level. If you want to match all the stacks in the hierarchy, use `**`. You can also combine these patterns. For example, if you want to deploy all stacks in the `Prod` stage, you can use `cdk deploy PipelineStack/Prod/**`.

`--concurrency N` allows deploying multiple stacks in parallel while respecting inter-stack dependencies to speed up deployments. It does not protect against CloudFormation and other AWS account rate limiting.

#### Parameters

Pass parameters to your template during deployment by using `--parameters
Expand Down
80 changes: 80 additions & 0 deletions packages/aws-cdk/THIRD_PARTY_LICENSES
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,32 @@ ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.


----------------

** [email protected] - https://www.npmjs.com/package/eventemitter3/v/4.0.7 | MIT
The MIT License (MIT)

Copyright (c) 2014 Arnout Kazemier

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all
copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
SOFTWARE.


----------------

** [email protected] - https://www.npmjs.com/package/fast-deep-equal/v/3.1.3 | MIT
Expand Down Expand Up @@ -2270,6 +2296,60 @@ ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR
IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.


----------------

** [email protected] - https://www.npmjs.com/package/p-finally/v/1.0.0 | MIT
The MIT License (MIT)

Copyright (c) Sindre Sorhus <[email protected]> (sindresorhus.com)

Permission is hereby granted, free of charge, to any person obtaining a copy
of this software and associated documentation files (the "Software"), to deal
in the Software without restriction, including without limitation the rights
to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
copies of the Software, and to permit persons to whom the Software is
furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in
all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
THE SOFTWARE.


----------------

** [email protected] - https://www.npmjs.com/package/p-queue/v/6.6.2 | MIT
MIT License

Copyright (c) Sindre Sorhus <[email protected]> (https://sindresorhus.com)

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.


----------------

** [email protected] - https://www.npmjs.com/package/p-timeout/v/3.2.0 | MIT
MIT License

Copyright (c) Sindre Sorhus <[email protected]> (sindresorhus.com)

Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions:

The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.


----------------

** [email protected] - https://www.npmjs.com/package/pac-proxy-agent/v/5.0.0 | MIT
Expand Down
66 changes: 59 additions & 7 deletions packages/aws-cdk/lib/cdk-toolkit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import * as cxapi from '@aws-cdk/cx-api';
import * as chalk from 'chalk';
import * as chokidar from 'chokidar';
import * as fs from 'fs-extra';
import PQueue from 'p-queue';
import * as promptly from 'promptly';
import { environmentsFromDescriptors, globEnvironmentsFromStacks, looksLikeGlob } from '../lib/api/cxapp/environments';
import { SdkProvider } from './api/aws-auth';
Expand Down Expand Up @@ -127,7 +128,7 @@ export class CdkToolkit {
}

const startSynthTime = new Date().getTime();
const stacks = await this.selectStacksForDeploy(options.selector, options.exclusively, options.cacheCloudAssembly);
const stackCollection = await this.selectStacksForDeploy(options.selector, options.exclusively, options.cacheCloudAssembly);
const elapsedSynthTime = new Date().getTime() - startSynthTime;
print('\n✨ Synthesis time: %ss\n', formatTime(elapsedSynthTime));

Expand All @@ -153,11 +154,24 @@ export class CdkToolkit {
warning('⚠️ It should only be used for development - never use it for your production Stacks!');
}

const stacks = stackCollection.stackArtifacts;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all works because stackArtifacts is pre-sorted based on stack dependencies


const stackOutputs: { [key: string]: any } = { };
const outputsFile = options.outputsFile;

for (const stack of stacks.stackArtifacts) {
if (stacks.stackCount !== 1) { highlight(stack.displayName); }
const concurrency = options.concurrency || 1;
const queue = new PQueue({ concurrency });
const stacksAwaitingDeploy = stacks.reduce((acc, stack) => ({
...acc, [stack.id]: true,
}), {} as Record<string, boolean>);

const progress = concurrency > 1 ? StackActivityProgress.EVENTS : options.progress;
if (concurrency > 1 && options.progress && options.progress != StackActivityProgress.EVENTS) {
warning('⚠️ The --concurrency flag only supports --progress "events". Switching to "events".');
}

const deployStack = async (stack: cxapi.CloudFormationStackArtifact) => {
if (stackCollection.stackCount !== 1) { highlight(stack.displayName); }
if (!stack.environment) {
// eslint-disable-next-line max-len
throw new Error(`Stack ${stack.displayName} does not define an environment, and AWS credentials could not be obtained from standard locations or no region was configured.`);
Expand All @@ -176,7 +190,7 @@ export class CdkToolkit {
fromDeploy: true,
});
}
continue;
return;
}

if (requireApproval !== RequireApproval.Never) {
Expand All @@ -190,6 +204,13 @@ export class CdkToolkit {
'but terminal (TTY) is not attached so we are unable to get a confirmation from the user');
}

// only talk to user if concurreny is 1 (otherwise, fail)
if (concurrency > 1) {
throw new Error(
'"--require-approval" is enabled and stack includes security-sensitive updates, ' +
'but concurrency is greater than 1 so we are unable to get a confirmation from the user');
}

const confirmed = await promptly.confirm('Do you wish to deploy these changes (y/n)?');
if (!confirmed) { throw new Error('Aborted by user'); }
}
Expand Down Expand Up @@ -218,7 +239,7 @@ export class CdkToolkit {
force: options.force,
parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]),
usePreviousParameters: options.usePreviousParameters,
progress: options.progress,
progress,
ci: options.ci,
rollback: options.rollback,
hotswap: options.hotswap,
Expand All @@ -235,10 +256,11 @@ export class CdkToolkit {

if (Object.keys(result.outputs).length > 0) {
print('Outputs:');

stackOutputs[stack.stackName] = result.outputs;
}

stacksAwaitingDeploy[stack.id] = false;

for (const name of Object.keys(result.outputs).sort()) {
const value = result.outputs[name];
print('%s.%s = %s', chalk.cyan(stack.id), chalk.cyan(name), chalk.underline(chalk.cyan(value)));
Expand Down Expand Up @@ -266,8 +288,30 @@ export class CdkToolkit {
});
}
}

print('\n✨ Total time: %ss\n', formatTime(elapsedSynthTime + elapsedDeployTime));
}
};

const isStackUnblocked = (stack: cxapi.CloudFormationStackArtifact) =>
stack.dependencies
.map(({ id }) => id)
.filter((id) => !id.endsWith('.assets'))
.every((id) => !stacksAwaitingDeploy[id]);

const enqueueStackDeploys = async () => {
while (stacks[0] && isStackUnblocked(stacks[0])) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're leaving some concurrency on the table here by only considering the first element from the list.

If we had dependencies:

C -> A
D -> B

And the list happened to toposort as [A, B, C, D], then with this implementation even if B finishes we still can't start D because we are forced to wait for C to start first.

This can be changed to a for loop -- on every iteration, consider all stacks.


It should probably be a forEach, otherwise we might run afoul of the "closing over a loop variable" gotcha.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch! Updated to a forEach loop

const stack = stacks.shift();
if (stack) {
void queue.add(async () => {
await deployStack(stack);
await enqueueStackDeploys();
});
}
}
};

await enqueueStackDeploys();
await queue.onIdle();
}

public async watch(options: WatchOptions) {
Expand Down Expand Up @@ -849,6 +893,14 @@ export interface DeployOptions extends WatchOptions {
* @default - not monitoring CloudWatch logs
*/
readonly cloudWatchLogMonitor?: CloudWatchLogEventMonitor;

/**
* Maximum number of simulatenous deployments (dependency permitting) to execute.
* The default is '1', which executes all deployments serially.
*
* @default - 1
relm923 marked this conversation as resolved.
Show resolved Hide resolved
*/
readonly concurrency?: number;
}

export interface DestroyOptions {
Expand Down
4 changes: 3 additions & 1 deletion packages/aws-cdk/lib/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,8 @@ async function parseCommandLineArguments() {
desc: 'Show CloudWatch log events from all resources in the selected Stacks in the terminal. ' +
"'true' by default, use --no-logs to turn off. " +
"Only in effect if specified alongside the '--watch' option",
}),
})
.option('concurrency', { type: 'number', desc: 'Maximum number of simulatenous deployments (dependency permitting) to execute.', default: 1, requiresArg: true }),
)
.command('watch [STACKS..]', "Shortcut for 'deploy --watch'", (yargs: Argv) => yargs
// I'm fairly certain none of these options, present for 'deploy', make sense for 'watch':
Expand Down Expand Up @@ -443,6 +444,7 @@ async function initCommandLine() {
hotswap: args.hotswap,
watch: args.watch,
traceLogs: args.logs,
concurrency: args.concurrency,
});

case 'watch':
Expand Down
1 change: 1 addition & 0 deletions packages/aws-cdk/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@
"glob": "^7.2.0",
"json-diff": "^0.7.3",
"minimatch": ">=3.1",
"p-queue": "^6.6.2",
"promptly": "^3.2.0",
"proxy-agent": "^5.0.0",
"semver": "^7.3.5",
Expand Down
8 changes: 8 additions & 0 deletions packages/aws-cdk/test/cdk-toolkit.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,14 @@ describe('deploy', () => {
await toolkit.deploy({ selector: { patterns: ['*'] } });
});

test('with concurrency', async () => {
// GIVEN
const toolkit = defaultToolkitSetup();

// WHEN
await toolkit.deploy({ concurrency: 2, selector: { patterns: ['Test-Stack-A', 'Test-Stack-B'] } });
});

test('with sns notification arns', async () => {
// GIVEN
const notificationArns = ['arn:aws:sns:::cfn-notifications', 'arn:aws:sns:::my-cool-topic'];
Expand Down
20 changes: 19 additions & 1 deletion packages/aws-cdk/test/integ/cli/cli.integtest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,14 @@ integTest('automatic ordering', withDefaultFixture(async (fixture) => {
await fixture.cdkDestroy('order-providing');
}));

integTest('automatic ordering with concurrency', withDefaultFixture(async (fixture) => {
// Deploy the consuming stack which will include the producing stack
await fixture.cdkDeploy('order-consuming', { options: ['--concurrency', '2'] });

// Destroy the providing stack which will include the consuming stack
await fixture.cdkDestroy('order-providing');
}));

integTest('context setting', withDefaultFixture(async (fixture) => {
await fs.writeFile(path.join(fixture.integTestDir, 'cdk.context.json'), JSON.stringify({
contextkey: 'this is the context value',
Expand Down Expand Up @@ -143,7 +151,17 @@ integTest('deploy', withDefaultFixture(async (fixture) => {
integTest('deploy all', withDefaultFixture(async (fixture) => {
const arns = await fixture.cdkDeploy('test-*', { captureStderr: false });

// verify that we only deployed a single stack (there's a single ARN in the output)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pre-existing incorrect comment

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

// verify that we only deployed both stacks (there are 2 ARNs in the output)
expect(arns.split('\n').length).toEqual(2);
}));

integTest('deploy all concurrently', withDefaultFixture(async (fixture) => {
const arns = await fixture.cdkDeploy('test-*', {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure this is safe, many stacks in the integ app were designed to be deployed in a "just-so" way. Please pick a well-defined subset of like 5 stacks or so.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was mainly basing this off the pre-existing deploy-all test case -

integTest('deploy all', withDefaultFixture(async (fixture) => {

Happy to pick a different set if you'd prefer

captureStderr: false,
options: ['--concurrency', '2'],
});

// verify that we only deployed both stacks (there are 2 ARNs in the output)
expect(arns.split('\n').length).toEqual(2);
}));

Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
"resolveJsonModule": true,
"composite": true,
"incremental": true
},
},
"include": [
"**/*.ts",
"**/*.d.ts",
Expand Down