Skip to content

Commit

Permalink
fix: deploy monitor count is off if there are > 100 changes (#20067)
Browse files Browse the repository at this point in the history
The deployment monitor failed to account for the fact the
`DescribeChangeSet` operation returns a paginated output, and only
represents up to 100 changes per pages.

Since only the first page was considered, the progress bar would always
start with a value of 101 (or less), instead of the correct count.

This aggregates the `Changes` arrays from all pages of the change set
description so the count is correct.

Fixes #11805

----

### All Submissions:

* [X] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/master/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/master/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
RomainMuller authored Apr 25, 2022
1 parent f0aff23 commit fd306ee
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 10 deletions.
7 changes: 5 additions & 2 deletions packages/aws-cdk/lib/api/deploy-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,12 @@ async function prepareAndExecuteChangeSet(
Capabilities: ['CAPABILITY_IAM', 'CAPABILITY_NAMED_IAM', 'CAPABILITY_AUTO_EXPAND'],
Tags: options.tags,
}).promise();

const execute = options.execute ?? true;

debug('Initiated creation of changeset: %s; waiting for it to finish creating...', changeSet.Id);
const changeSetDescription = await waitForChangeSet(cfn, deployName, changeSetName);
// Fetching all pages if we'll execute, so we can have the correct change count when monitoring.
const changeSetDescription = await waitForChangeSet(cfn, deployName, changeSetName, { fetchAll: execute });

// Update termination protection only if it has changed.
const terminationProtection = stackArtifact.terminationProtection ?? false;
Expand All @@ -352,7 +356,6 @@ async function prepareAndExecuteChangeSet(
return { noOp: true, outputs: cloudFormationStack.outputs, stackArn: changeSet.StackId! };
}

const execute = options.execute === undefined ? true : options.execute;
if (execute) {
debug('Initiating execution of changeset %s on stack %s', changeSet.Id, deployName);

Expand Down
48 changes: 40 additions & 8 deletions packages/aws-cdk/lib/api/util/cloudformation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,14 +170,40 @@ export class CloudFormationStack {
/**
* Describe a changeset in CloudFormation, regardless of its current state.
*
* @param cfn a CloudFormation client
* @param stackName the name of the Stack the ChangeSet belongs to
* @param cfn a CloudFormation client
* @param stackName the name of the Stack the ChangeSet belongs to
* @param changeSetName the name of the ChangeSet
* @param fetchAll if true, fetches all pages of the change set description.
*
* @returns CloudFormation information about the ChangeSet
*/
async function describeChangeSet(cfn: CloudFormation, stackName: string, changeSetName: string): Promise<CloudFormation.DescribeChangeSetOutput> {
async function describeChangeSet(
cfn: CloudFormation,
stackName: string,
changeSetName: string,
{ fetchAll }: { fetchAll: boolean },
): Promise<CloudFormation.DescribeChangeSetOutput> {
const response = await cfn.describeChangeSet({ StackName: stackName, ChangeSetName: changeSetName }).promise();

// If fetchAll is true, traverse all pages from the change set description.
while (fetchAll && response.NextToken != null) {
const nextPage = await cfn.describeChangeSet({
StackName: stackName,
ChangeSetName: response.ChangeSetId ?? changeSetName,
NextToken: response.NextToken,
}).promise();

// Consolidate the changes
if (nextPage.Changes != null) {
response.Changes = response.Changes != null
? response.Changes.concat(nextPage.Changes)
: nextPage.Changes;
}

// Forward the new NextToken
response.NextToken = nextPage.NextToken;
}

return response;
}

Expand Down Expand Up @@ -207,17 +233,23 @@ async function waitFor<T>(valueProvider: () => Promise<T | null | undefined>, ti
* Will return a changeset that is either ready to be executed or has no changes.
* Will throw in other cases.
*
* @param cfn a CloudFormation client
* @param stackName the name of the Stack that the ChangeSet belongs to
* @param cfn a CloudFormation client
* @param stackName the name of the Stack that the ChangeSet belongs to
* @param changeSetName the name of the ChangeSet
* @param fetchAll if true, fetches all pages of the ChangeSet before returning.
*
* @returns the CloudFormation description of the ChangeSet
*/
// eslint-disable-next-line max-len
export async function waitForChangeSet(cfn: CloudFormation, stackName: string, changeSetName: string): Promise<CloudFormation.DescribeChangeSetOutput> {
export async function waitForChangeSet(
cfn: CloudFormation,
stackName: string,
changeSetName: string,
{ fetchAll }: { fetchAll: boolean },
): Promise<CloudFormation.DescribeChangeSetOutput> {
debug('Waiting for changeset %s on stack %s to finish creating...', changeSetName, stackName);
const ret = await waitFor(async () => {
const description = await describeChangeSet(cfn, stackName, changeSetName);
const description = await describeChangeSet(cfn, stackName, changeSetName, { fetchAll });
// The following doesn't use a switch because tsc will not allow fall-through, UNLESS it is allows
// EVERYWHERE that uses this library directly or indirectly, which is undesirable.
if (description.Status === 'CREATE_PENDING' || description.Status === 'CREATE_IN_PROGRESS') {
Expand Down Expand Up @@ -453,4 +485,4 @@ export class ParameterValues {
}
}

export type ParameterChanges = boolean | 'ssm';
export type ParameterChanges = boolean | 'ssm';

0 comments on commit fd306ee

Please sign in to comment.