Skip to content

Commit

Permalink
fix(cli): excessive stack event polling during deployment (#32196)
Browse files Browse the repository at this point in the history
Closes #32186

### Reason for this change

<!--What is the bug or use case behind this change?-->

The CLI will poll for all stack events from the beginning of time, even
though it will end up using only a fraction of them to display in the
terminal. This can cause a significant slowdown of `cdk deploy`.

### Description of changes

<!--What code changes did you make? Have you made any important design
decisions?-->

Moved the pagination to the caller side, so it can decide whether or not
to poll the next page. This is how it was before `2.167.0`


https://github.com/aws/aws-cdk/blob/7bb9203eb95fe894c0d40942ff49c782a9fec251/packages/aws-cdk/lib/api/util/cloudformation/stack-event-poller.ts#L73-L74

### Description of how you validated changes

<!--Have you added any unit tests and/or integration tests?-->

Added unit test.

### Notes

- There are several functions in the
[sdk.ts](https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk/lib/api/aws-auth/sdk.ts)
that perform implicit pagination to retrieve all results. From a quick
glance, none of them seem to be misusing it though (at least with
respect to how it always was). I will investigate those functions
further in a followup PR.

### Checklist
- [x] My code adheres to the [CONTRIBUTING
GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and
[DESIGN
GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

*By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license*
  • Loading branch information
iliapolo authored Nov 20, 2024
1 parent 6317a2a commit a8bc46d
Show file tree
Hide file tree
Showing 3 changed files with 136 additions and 59 deletions.
15 changes: 5 additions & 10 deletions packages/aws-cdk/lib/api/aws-auth/sdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,9 @@ import {
DescribeResourceScanCommand,
type DescribeResourceScanCommandInput,
type DescribeResourceScanCommandOutput,
DescribeStackEventsCommand,
type DescribeStackEventsCommandInput,
DescribeStackEventsCommandOutput,
DescribeStackResourcesCommand,
DescribeStackResourcesCommandInput,
DescribeStackResourcesCommandOutput,
Expand Down Expand Up @@ -86,12 +88,10 @@ import {
ListStacksCommand,
ListStacksCommandInput,
ListStacksCommandOutput,
paginateDescribeStackEvents,
paginateListStackResources,
RollbackStackCommand,
RollbackStackCommandInput,
RollbackStackCommandOutput,
StackEvent,
StackResourceSummary,
StartResourceScanCommand,
type StartResourceScanCommandInput,
Expand Down Expand Up @@ -404,7 +404,7 @@ export interface ICloudFormationClient {
input: UpdateTerminationProtectionCommandInput,
): Promise<UpdateTerminationProtectionCommandOutput>;
// Pagination functions
describeStackEvents(input: DescribeStackEventsCommandInput): Promise<StackEvent[]>;
describeStackEvents(input: DescribeStackEventsCommandInput): Promise<DescribeStackEventsCommandOutput>;
listStackResources(input: ListStackResourcesCommandInput): Promise<StackResourceSummary[]>;
}

Expand Down Expand Up @@ -664,13 +664,8 @@ export class SDK {
input: UpdateTerminationProtectionCommandInput,
): Promise<UpdateTerminationProtectionCommandOutput> =>
client.send(new UpdateTerminationProtectionCommand(input)),
describeStackEvents: async (input: DescribeStackEventsCommandInput): Promise<StackEvent[]> => {
const stackEvents = Array<StackEvent>();
const paginator = paginateDescribeStackEvents({ client }, input);
for await (const page of paginator) {
stackEvents.push(...(page?.StackEvents || []));
}
return stackEvents;
describeStackEvents: (input: DescribeStackEventsCommandInput): Promise<DescribeStackEventsCommandOutput> => {
return client.send(new DescribeStackEventsCommand(input));
},
listStackResources: async (input: ListStackResourcesCommandInput): Promise<StackResourceSummary[]> => {
const stackResources = Array<StackResourceSummary>();
Expand Down
97 changes: 48 additions & 49 deletions packages/aws-cdk/lib/api/util/cloudformation/stack-event-poller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,65 +88,64 @@ export class StackEventPoller {
private async doPoll(): Promise<ResourceEvent[]> {
const events: ResourceEvent[] = [];
try {
const eventList = await this.cfn.describeStackEvents({
StackName: this.props.stackName,
});
for (const event of eventList) {
// Event from before we were interested in 'em
if (this.props.startTime !== undefined && event.Timestamp!.valueOf() < this.props.startTime) {
return events;
}

// Already seen this one
if (this.eventIds.has(event.EventId!)) {
return events;
}
this.eventIds.add(event.EventId!);

// The events for the stack itself are also included next to events about resources; we can test for them in this way.
const isParentStackEvent = event.PhysicalResourceId === event.StackId;

if (isParentStackEvent && this.props.stackStatuses?.includes(event.ResourceStatus ?? '')) {
return events;
let nextToken: string | undefined;
let finished = false;

while (!finished) {
const page = await this.cfn.describeStackEvents({ StackName: this.props.stackName, NextToken: nextToken });
for (const event of page?.StackEvents ?? []) {
// Event from before we were interested in 'em
if (this.props.startTime !== undefined && event.Timestamp!.valueOf() < this.props.startTime) {
return events;
}

// Already seen this one
if (this.eventIds.has(event.EventId!)) {
return events;
}
this.eventIds.add(event.EventId!);

// The events for the stack itself are also included next to events about resources; we can test for them in this way.
const isParentStackEvent = event.PhysicalResourceId === event.StackId;

if (isParentStackEvent && this.props.stackStatuses?.includes(event.ResourceStatus ?? '')) {
return events;
}

// Fresh event
const resEvent: ResourceEvent = {
event: event,
parentStackLogicalIds: this.props.parentStackLogicalIds ?? [],
isStackEvent: isParentStackEvent,
};
events.push(resEvent);

if (
!isParentStackEvent &&
event.ResourceType === 'AWS::CloudFormation::Stack' &&
isStackBeginOperationState(event.ResourceStatus)
) {
// If the event is not for `this` stack and has a physical resource Id, recursively call for events in the nested stack
this.trackNestedStack(event, [...(this.props.parentStackLogicalIds ?? []), event.LogicalResourceId ?? '']);
}

if (isParentStackEvent && isStackTerminalState(event.ResourceStatus)) {
this.complete = true;
}
}

// Fresh event
const resEvent: ResourceEvent = {
event: event,
parentStackLogicalIds: this.props.parentStackLogicalIds ?? [],
isStackEvent: isParentStackEvent,
};
events.push(resEvent);

if (
!isParentStackEvent &&
event.ResourceType === 'AWS::CloudFormation::Stack' &&
isStackBeginOperationState(event.ResourceStatus)
) {
// If the event is not for `this` stack and has a physical resource Id, recursively call for events in the nested stack
this.trackNestedStack(event, [...(this.props.parentStackLogicalIds ?? []), event.LogicalResourceId ?? '']);
nextToken = page?.NextToken;
if (nextToken === undefined) {
finished = true;
}

if (isParentStackEvent && isStackTerminalState(event.ResourceStatus)) {
this.complete = true;
}
}
} catch (e: any) {
if (!(e.name === 'ValidationError' && e.message === `Stack [${this.props.stackName}] does not exist`)) {
throw e;
}
}
// // Also poll all nested stacks we're currently tracking
// for (const [logicalId, poller] of Object.entries(this.nestedStackPollers)) {
// events.push(...(await poller.poll()));
// if (poller.complete) {
// delete this.nestedStackPollers[logicalId];
// }
// }

// // Return what we have so far
// events.sort((a, b) => a.event.Timestamp!.valueOf() - b.event.Timestamp!.valueOf());
// this.events.push(...events);

return events;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
import { DescribeStackEventsCommand, DescribeStackEventsCommandInput, StackEvent } from '@aws-sdk/client-cloudformation';
import { StackEventPoller } from '../../../../lib/api/util/cloudformation/stack-event-poller';
import { MockSdk, mockCloudFormationClient } from '../../../util/mock-sdk';

beforeEach(() => {
jest.resetAllMocks();
});

describe('poll', () => {

test('polls all necessary pages', async () => {

const deployTime = Date.now();

const postDeployEvent1: StackEvent = {
Timestamp: new Date(deployTime + 1000),
EventId: 'event-1',
StackId: 'stack-id',
StackName: 'stack',
};

const postDeployEvent2: StackEvent = {
Timestamp: new Date(deployTime + 2000),
EventId: 'event-2',
StackId: 'stack-id',
StackName: 'stack',
};

const sdk = new MockSdk();
mockCloudFormationClient.on(DescribeStackEventsCommand).callsFake((input: DescribeStackEventsCommandInput) => {
const result = {
StackEvents: input.NextToken === 'token' ? [postDeployEvent2] : [postDeployEvent1],
NextToken: input.NextToken === 'token' ? undefined : 'token', // simulate a two page event stream.
};

return result;
});

const poller = new StackEventPoller(sdk.cloudFormation(), {
stackName: 'stack',
startTime: new Date().getTime(),
});

const events = await poller.poll();
expect(events.length).toEqual(2);

});

test('does not poll unnecessary pages', async () => {

const deployTime = Date.now();

const preDeployTimeEvent: StackEvent = {
Timestamp: new Date(deployTime - 1000),
EventId: 'event-1',
StackId: 'stack-id',
StackName: 'stack',
};

const sdk = new MockSdk();
mockCloudFormationClient.on(DescribeStackEventsCommand).callsFake((input: DescribeStackEventsCommandInput) => {

// the first event we return should stop the polling. we therefore
// do not expect a second page to be polled.
expect(input.NextToken).toBe(undefined);

return {
StackEvents: [preDeployTimeEvent],
NextToken: input.NextToken === 'token' ? undefined : 'token', // simulate a two page event stream.
};

});

const poller = new StackEventPoller(sdk.cloudFormation(), {
stackName: 'stack',
startTime: new Date().getTime(),
});

await poller.poll();

});

});

0 comments on commit a8bc46d

Please sign in to comment.