chore(cli): revert concurrency PR (#20230)

Reverts #19378 It breaks 4 integ tests: ``` ● security related changes without a CLI are expected to fail ... (node:7140) UnhandledPromiseRejectionWarning: Error: \"--require-approval\" is enabled and stack includes security-sensitive updates, but terminal (TTY) is not attached so we are unable to get a confirmation from the user ● update to stack in ROLLBACK_COMPLETE state will delete stack and create a new one expect(received).rejects.toThrow() Received promise resolved instead of rejected Resolved to value: "" at expect (../node_modules/expect/build/index.js:128:15) at expect (cli.integtest.ts:267:9) at block (../helpers/cdk.ts:131:13) at runMicrotasks (<anonymous>) at ResourcePool.using (../helpers/resource-pool.ts:44:14) at ../helpers/test-helpers.ts:36:14 ● stack in UPDATE_ROLLBACK_COMPLETE state can be updated expect(received).rejects.toThrow() Received promise resolved instead of rejected Resolved to value: "" at expect (../node_modules/expect/build/index.js:128:15) at expect (cli.integtest.ts:320:9) at runMicrotasks (<anonymous>) at ../helpers/cdk.ts:131:7 at ResourcePool.using (../helpers/resource-pool.ts:44:14) at ../helpers/test-helpers.ts:36:14 ● failed deploy does not hang ... (node:8336) UnhandledPromiseRejectionWarning: Error: The stack named cdktest-0m9jeq4rpbp-failed failed creation, it may need to be manually deleted from the AWS console: ROLLBACK_COMPLETE: Property PolicyDocument cannot be empty. ``` Something around the handling of errors in the new async deployment.
aws · May 5, 2022 · 8795b85 · 8795b85
1 parent f4f6e40
commit 8795b85
Show file tree

Hide file tree

Showing 8 changed files with 10 additions and 178 deletions.
diff --git a/packages/aws-cdk/README.md b/packages/aws-cdk/README.md
@@ -158,8 +158,6 @@ 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
@@ -186,8 +184,6 @@ 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

diff --git a/packages/aws-cdk/THIRD_PARTY_LICENSES b/packages/aws-cdk/THIRD_PARTY_LICENSES
@@ -1143,32 +1143,6 @@ ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
 THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 
-----------------
-
-** eventemitter3@4.0.7 - 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.
-
-
 ----------------
 
 ** fast-deep-equal@3.1.3 - https://www.npmjs.com/package/fast-deep-equal/v/3.1.3 | MIT
@@ -2296,60 +2270,6 @@ ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF OR
 IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
 
 
-----------------
-
-** p-finally@1.0.0 - https://www.npmjs.com/package/p-finally/v/1.0.0 | MIT
-The MIT License (MIT)
-
-Copyright (c) Sindre Sorhus <sindresorhus@gmail.com> (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.
-
-
-----------------
-
-** p-queue@6.6.2 - https://www.npmjs.com/package/p-queue/v/6.6.2 | MIT
-MIT License
-
-Copyright (c) Sindre Sorhus <sindresorhus@gmail.com> (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.
-
-
-----------------
-
-** p-timeout@3.2.0 - https://www.npmjs.com/package/p-timeout/v/3.2.0 | MIT
-MIT License
-
-Copyright (c) Sindre Sorhus <sindresorhus@gmail.com> (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.
-
-
 ----------------
 
 ** pac-proxy-agent@5.0.0 - https://www.npmjs.com/package/pac-proxy-agent/v/5.0.0 | MIT

diff --git a/packages/aws-cdk/lib/cdk-toolkit.ts b/packages/aws-cdk/lib/cdk-toolkit.ts
@@ -4,7 +4,6 @@ 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';
@@ -129,7 +128,7 @@ export class CdkToolkit {
     }
 
     const startSynthTime = new Date().getTime();
-    const stackCollection = await this.selectStacksForDeploy(options.selector, options.exclusively, options.cacheCloudAssembly);
+    const stacks = await this.selectStacksForDeploy(options.selector, options.exclusively, options.cacheCloudAssembly);
     const elapsedSynthTime = new Date().getTime() - startSynthTime;
     print('\n✨  Synthesis time: %ss\n', formatTime(elapsedSynthTime));
 
@@ -155,24 +154,11 @@ export class CdkToolkit {
       warning('⚠️ It should only be used for development - never use it for your production Stacks!');
     }
 
-    const stacks = stackCollection.stackArtifacts;
-
     const stackOutputs: { [key: string]: any } = { };
     const outputsFile = options.outputsFile;
 
-    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); }
+    for (const stack of stacks.stackArtifacts) {
+      if (stacks.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.`);
@@ -191,7 +177,7 @@ export class CdkToolkit {
             fromDeploy: true,
           });
         }
-        return;
+        continue;
       }
 
       if (requireApproval !== RequireApproval.Never) {
@@ -205,13 +191,6 @@ 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'); }
         }
@@ -240,7 +219,7 @@ export class CdkToolkit {
           force: options.force,
           parameters: Object.assign({}, parameterMap['*'], parameterMap[stack.stackName]),
           usePreviousParameters: options.usePreviousParameters,
-          progress,
+          progress: options.progress,
           ci: options.ci,
           rollback: options.rollback,
           hotswap: options.hotswap,
@@ -257,11 +236,10 @@ 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)));
@@ -289,33 +267,8 @@ 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 () => {
-      stacks.forEach((stack) => {
-        if (isStackUnblocked(stack)) {
-          // Find current index due to stacks list changing within loop
-          const index = stacks.indexOf(stack);
-          stacks.splice(index, 1);
-
-          void queue.add(async () => {
-            await deployStack(stack);
-            await enqueueStackDeploys();
-          });
-        }
-      });
-    };
-
-    await enqueueStackDeploys();
-    await queue.onIdle();
+    }
   }
 
   public async watch(options: WatchOptions) {
@@ -976,14 +929,6 @@ export interface DeployOptions extends CfnDeployOptions, 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
-   */
-  readonly concurrency?: number;
 }
 
 export interface ImportOptions extends CfnDeployOptions {

diff --git a/packages/aws-cdk/lib/cli.ts b/packages/aws-cdk/lib/cli.ts
@@ -146,8 +146,7 @@ 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('import [STACK]', 'Import existing resource(s) into the given STACK', (yargs: Argv) => yargs
       .option('execute', { type: 'boolean', desc: 'Whether to execute ChangeSet (--no-execute will NOT execute the ChangeSet)', default: true })
@@ -473,7 +472,6 @@ async function initCommandLine() {
           hotswap: args.hotswap,
           watch: args.watch,
           traceLogs: args.logs,
-          concurrency: args.concurrency,
         });
 
       case 'import':

diff --git a/packages/aws-cdk/package.json b/packages/aws-cdk/package.json
@@ -105,7 +105,6 @@
     "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.6",

diff --git a/packages/aws-cdk/test/cdk-toolkit.test.ts b/packages/aws-cdk/test/cdk-toolkit.test.ts
@@ -455,14 +455,6 @@ 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'];

diff --git a/packages/aws-cdk/test/integ/cli/cli.integtest.ts b/packages/aws-cdk/test/integ/cli/cli.integtest.ts
@@ -100,14 +100,6 @@ 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',
@@ -151,17 +143,7 @@ integTest('deploy', withDefaultFixture(async (fixture) => {
 integTest('deploy all', withDefaultFixture(async (fixture) => {
   const arns = await fixture.cdkDeploy('test-*', { captureStderr: false });
 
-  // 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-*', {
-    captureStderr: false,
-    options: ['--concurrency', '2'],
-  });
-
-  // verify that we only deployed both stacks (there are 2 ARNs in the output)
+  // verify that we only deployed a single stack (there's a single ARN in the output)
   expect(arns.split('\n').length).toEqual(2);
 }));
 

diff --git a/packages/aws-cdk/tsconfig.json b/packages/aws-cdk/tsconfig.json
@@ -15,7 +15,7 @@
         "resolveJsonModule": true,
         "composite": true,
         "incremental": true
-      },
+    },
     "include": [
         "**/*.ts",
         "**/*.d.ts",