From 5c30255b70b51ef760a72a64769614c0297e6c94 Mon Sep 17 00:00:00 2001 From: Tom Wright Date: Wed, 27 Mar 2024 08:02:31 +1100 Subject: [PATCH] fix(cli): ecs hotswap deployment waits correctly for success or failure (#28448) Reraising https://github.com/aws/aws-cdk/pull/27943 as it was incorrectly closed as stale. See original PR for details. Closes https://github.com/aws/aws-cdk/issues/27882. See linked issue for further details. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../@aws-cdk-testing/cli-integ/lib/aws.ts | 2 + .../cli-integ/resources/cdk-apps/app/app.js | 57 ++++++++++++++ .../tests/cli-integ-tests/cli.integtest.ts | 77 +++++++++++++++++++ .../aws-cdk/lib/api/hotswap/ecs-services.ts | 24 ++++-- 4 files changed, 153 insertions(+), 7 deletions(-) diff --git a/packages/@aws-cdk-testing/cli-integ/lib/aws.ts b/packages/@aws-cdk-testing/cli-integ/lib/aws.ts index 69598777662d4..73f8c9bf0782a 100644 --- a/packages/@aws-cdk-testing/cli-integ/lib/aws.ts +++ b/packages/@aws-cdk-testing/cli-integ/lib/aws.ts @@ -18,6 +18,7 @@ export class AwsClients { public readonly cloudFormation: AwsCaller; public readonly s3: AwsCaller; public readonly ecr: AwsCaller; + public readonly ecs: AwsCaller; public readonly sns: AwsCaller; public readonly iam: AwsCaller; public readonly lambda: AwsCaller; @@ -34,6 +35,7 @@ export class AwsClients { this.cloudFormation = makeAwsCaller(AWS.CloudFormation, this.config); this.s3 = makeAwsCaller(AWS.S3, this.config); this.ecr = makeAwsCaller(AWS.ECR, this.config); + this.ecs = makeAwsCaller(AWS.ECS, this.config); this.sns = makeAwsCaller(AWS.SNS, this.config); this.iam = makeAwsCaller(AWS.IAM, this.config); this.lambda = makeAwsCaller(AWS.Lambda, this.config); diff --git a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js index a5524ae7a8e26..bdcc4457fbee9 100755 --- a/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js +++ b/packages/@aws-cdk-testing/cli-integ/resources/cdk-apps/app/app.js @@ -4,6 +4,7 @@ var constructs = require('constructs'); if (process.env.PACKAGE_LAYOUT_VERSION === '1') { var cdk = require('@aws-cdk/core'); var ec2 = require('@aws-cdk/aws-ec2'); + var ecs = require('@aws-cdk/aws-ecs'); var s3 = require('@aws-cdk/aws-s3'); var ssm = require('@aws-cdk/aws-ssm'); var iam = require('@aws-cdk/aws-iam'); @@ -17,6 +18,7 @@ if (process.env.PACKAGE_LAYOUT_VERSION === '1') { DefaultStackSynthesizer, LegacyStackSynthesizer, aws_ec2: ec2, + aws_ecs: ecs, aws_s3: s3, aws_ssm: ssm, aws_iam: iam, @@ -357,6 +359,60 @@ class LambdaHotswapStack extends cdk.Stack { } } +class EcsHotswapStack extends cdk.Stack { + constructor(parent, id, props) { + super(parent, id, props); + + // define a simple vpc and cluster + const vpc = new ec2.Vpc(this, 'vpc', { + natGateways: 0, + subnetConfiguration: [ + { + cidrMask: 24, + name: 'Public', + subnetType: ec2.SubnetType.PUBLIC, + }, + ], + maxAzs: 1, + }); + const cluster = new ecs.Cluster(this, 'cluster', { + vpc, + }); + + // allow stack to be used to test failed deployments + const image = + process.env.USE_INVALID_ECS_HOTSWAP_IMAGE == 'true' + ? 'nginx:invalidtag' + : 'nginx:alpine'; + + // deploy basic service + const taskDefinition = new ecs.FargateTaskDefinition( + this, + 'task-definition' + ); + taskDefinition.addContainer('nginx', { + image: ecs.ContainerImage.fromRegistry(image), + environment: { + SOME_VARIABLE: process.env.DYNAMIC_ECS_PROPERTY_VALUE ?? 'environment', + }, + healthCheck: { + command: ['CMD-SHELL', 'exit 0'], // fake health check to speed up deployment + interval: cdk.Duration.seconds(5), + }, + }); + const service = new ecs.FargateService(this, 'service', { + cluster, + taskDefinition, + assignPublicIp: true, // required without NAT to pull image + circuitBreaker: { rollback: false }, + desiredCount: 1, + }); + + new cdk.CfnOutput(this, 'ClusterName', { value: cluster.clusterName }); + new cdk.CfnOutput(this, 'ServiceName', { value: service.serviceName }); + } +} + class DockerStack extends cdk.Stack { constructor(parent, id, props) { super(parent, id, props); @@ -532,6 +588,7 @@ switch (stackSet) { new LambdaStack(app, `${stackPrefix}-lambda`); new LambdaHotswapStack(app, `${stackPrefix}-lambda-hotswap`); + new EcsHotswapStack(app, `${stackPrefix}-ecs-hotswap`); new DockerStack(app, `${stackPrefix}-docker`); new DockerStackWithCustomFile(app, `${stackPrefix}-docker-with-custom-file`); const failed = new FailedStack(app, `${stackPrefix}-failed`) diff --git a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts index 3c69673e775ba..86350567a11af 100644 --- a/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts +++ b/packages/@aws-cdk-testing/cli-integ/tests/cli-integ-tests/cli.integtest.ts @@ -1571,6 +1571,83 @@ integTest('hotswap deployment supports Fn::ImportValue intrinsic', withDefaultFi } })); +integTest('hotswap deployment supports ecs service', withDefaultFixture(async (fixture) => { + // GIVEN + const stackArn = await fixture.cdkDeploy('ecs-hotswap', { + captureStderr: false, + }); + + // WHEN + const deployOutput = await fixture.cdkDeploy('ecs-hotswap', { + options: ['--hotswap'], + captureStderr: true, + onlyStderr: true, + modEnv: { + DYNAMIC_ECS_PROPERTY_VALUE: 'new value', + }, + }); + + const response = await fixture.aws.cloudFormation('describeStacks', { + StackName: stackArn, + }); + const serviceName = response.Stacks?.[0].Outputs?.find(output => output.OutputKey == 'ServiceName')?.OutputValue; + + // THEN + + // The deployment should not trigger a full deployment, thus the stack's status must remains + // "CREATE_COMPLETE" + expect(response.Stacks?.[0].StackStatus).toEqual('CREATE_COMPLETE'); + expect(deployOutput).toContain(`ECS Service '${serviceName}' hotswapped!`); +})); + +integTest('hotswap deployment for ecs service waits for deployment to complete', withDefaultFixture(async (fixture) => { + // GIVEN + const stackArn = await fixture.cdkDeploy('ecs-hotswap', { + captureStderr: false, + }); + + // WHEN + await fixture.cdkDeploy('ecs-hotswap', { + options: ['--hotswap'], + modEnv: { + DYNAMIC_ECS_PROPERTY_VALUE: 'new value', + }, + }); + + const describeStacksResponse = await fixture.aws.cloudFormation('describeStacks', { + StackName: stackArn, + }); + const clusterName = describeStacksResponse.Stacks?.[0].Outputs?.find(output => output.OutputKey == 'ClusterName')?.OutputValue!; + const serviceName = describeStacksResponse.Stacks?.[0].Outputs?.find(output => output.OutputKey == 'ServiceName')?.OutputValue!; + + // THEN + + const describeServicesResponse = await fixture.aws.ecs('describeServices', { + cluster: clusterName, + services: [serviceName], + }); + expect(describeServicesResponse.services?.[0].deployments).toHaveLength(1); // only one deployment present + +})); + +integTest('hotswap deployment for ecs service detects failed deployment and errors', withDefaultFixture(async (fixture) => { + // GIVEN + await fixture.cdkDeploy('ecs-hotswap'); + + // WHEN + const deployOutput = await fixture.cdkDeploy('ecs-hotswap', { + options: ['--hotswap'], + modEnv: { + USE_INVALID_ECS_HOTSWAP_IMAGE: 'true', + }, + allowErrExit: true, + }); + + // THEN + expect(deployOutput).toContain(`❌ ${fixture.stackNamePrefix}-ecs-hotswap failed: ResourceNotReady: Resource is not in the state deploymentCompleted`); + expect(deployOutput).not.toContain('hotswapped!'); +})); + async function listChildren(parent: string, pred: (x: string) => Promise) { const ret = new Array(); for (const child of await fs.readdir(parent, { encoding: 'utf-8' })) { diff --git a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts index 175c35be5bdac..266bd9e7e460b 100644 --- a/packages/aws-cdk/lib/api/hotswap/ecs-services.ts +++ b/packages/aws-cdk/lib/api/hotswap/ecs-services.ts @@ -119,11 +119,11 @@ export async function isHotswappableEcsServiceChange( // Step 3 - wait for the service deployments triggered in Step 2 to finish // configure a custom Waiter - (sdk.ecs() as any).api.waiters.deploymentToFinish = { - name: 'DeploymentToFinish', + (sdk.ecs() as any).api.waiters.deploymentCompleted = { + name: 'DeploymentCompleted', operation: 'describeServices', - delay: 10, - maxAttempts: 60, + delay: 6, + maxAttempts: 100, acceptors: [ { matcher: 'pathAny', @@ -143,16 +143,26 @@ export async function isHotswappableEcsServiceChange( expected: 'INACTIVE', state: 'failure', }, + + // failure if any services report a deployment with status FAILED + { + matcher: 'path', + argument: "length(services[].deployments[? rolloutState == 'FAILED'][]) > `0`", + expected: true, + state: 'failure', + }, + + // wait for all services to report only a single deployment { matcher: 'path', - argument: "length(services[].deployments[? status == 'PRIMARY' && runningCount < desiredCount][]) == `0`", + argument: 'length(services[? length(deployments) > `1`]) == `0`', expected: true, state: 'success', }, ], }; - // create a custom Waiter that uses the deploymentToFinish configuration added above - const deploymentWaiter = new (AWS as any).ResourceWaiter(sdk.ecs(), 'deploymentToFinish'); + // create a custom Waiter that uses the deploymentCompleted configuration added above + const deploymentWaiter = new (AWS as any).ResourceWaiter(sdk.ecs(), 'deploymentCompleted'); // wait for all of the waiters to finish await Promise.all(Object.entries(servicePerClusterUpdates).map(([clusterName, serviceUpdates]) => { return deploymentWaiter.wait({