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

fix(cli): excessive stack event polling during deployment #32196

Merged
merged 11 commits into from
Nov 20, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
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 @@ -52,6 +52,7 @@ import {
type DescribeResourceScanCommandInput,
type DescribeResourceScanCommandOutput,
type DescribeStackEventsCommandInput,
DescribeStackEventsCommandOutput,
DescribeStackResourcesCommand,
DescribeStackResourcesCommandInput,
DescribeStackResourcesCommandOutput,
Expand Down Expand Up @@ -91,7 +92,6 @@ import {
RollbackStackCommand,
RollbackStackCommandInput,
RollbackStackCommandOutput,
StackEvent,
StackResourceSummary,
StartResourceScanCommand,
type StartResourceScanCommandInput,
Expand Down Expand Up @@ -311,7 +311,7 @@ import { GetCallerIdentityCommand, STSClient } from '@aws-sdk/client-sts';
import { Upload } from '@aws-sdk/lib-storage';
import { getEndpointFromInstructions } from '@smithy/middleware-endpoint';
import type { NodeHttpHandlerOptions } from '@smithy/node-http-handler';
import { AwsCredentialIdentity, Logger } from '@smithy/types';
import { AwsCredentialIdentity, Logger, Paginator } from '@smithy/types';
import { ConfiguredRetryStrategy } from '@smithy/util-retry';
import { WaiterResult } from '@smithy/util-waiter';
import { AccountAccessKeyCache } from './account-cache';
Expand Down Expand Up @@ -404,7 +404,7 @@ export interface ICloudFormationClient {
input: UpdateTerminationProtectionCommandInput,
): Promise<UpdateTerminationProtectionCommandOutput>;
// Pagination functions
describeStackEvents(input: DescribeStackEventsCommandInput): Promise<StackEvent[]>;
describeStackEventsPaginated(input: DescribeStackEventsCommandInput): Paginator<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;
describeStackEventsPaginated: (input: DescribeStackEventsCommandInput): Paginator<DescribeStackEventsCommandOutput> => {
return paginateDescribeStackEvents({ client }, input);
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
},
listStackResources: async (input: ListStackResourcesCommandInput): Promise<StackResourceSummary[]> => {
const stackResources = Array<StackResourceSummary>();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,65 +88,61 @@ export class StackEventPoller {
private async doPoll(): Promise<ResourceEvent[]> {
const events: ResourceEvent[] = [];
try {
const eventList = await this.cfn.describeStackEvents({
const paginator = this.cfn.describeStackEventsPaginated({
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;
}

// 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;
for await (const page of paginator) {
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;

/* istanbul ignore next */
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
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)
) {
/* istanbul ignore next */
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
// 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 ?? '']);
}

/* istanbul ignore next */
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
if (isParentStackEvent && isStackTerminalState(event.ResourceStatus)) {
this.complete = true;
}
}
}
} catch (e: any) {
/* istanbul ignore next */
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
if (!(e.name === 'ValidationError' && e.message === `Stack [${this.props.stackName}] does not exist`)) {
throw e;
}
}
// // Also poll all nested stacks we're currently tracking
iliapolo marked this conversation as resolved.
Show resolved Hide resolved
// 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 ? [postDeployEvent2] : [postDeployEvent1],
NextToken: input.NextToken ? 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 ? undefined : 'token', // simulate a two page event stream.
};

});

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

await poller.poll();

});

});
Loading