aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.68k stars 3.92k forks source link

(integ-runner): Runner builds only the old version of integration tests for metadata-only changes #21496

Closed laurelmay closed 2 years ago

laurelmay commented 2 years ago

Describe the bug

yarn integ-runner --update-on-failed does not result in the new stack being built in CloudFormation; instead only the old snapshot is built because the UpdateStack operation does nothing in this case.

Expected Behavior

yarn integ-runner --update-on-failed tests the new code and the deployed CloudFormation template shown in the CloudFormation console matches the created snapshot.

Current Behavior

yarn integ-runner --update-on-failed builds a CloudFormation stack using the existing snapshot in version control and not one with the new synthesized template when changes to the template don't impact resource properties, such changes that only impact DependsOn.

This occurs for tests with a CHANGED status; NEW tests are built using the template and the point is mostly moot for UNCHANGED templates.

Reproduction Steps

  1. Checkout a PR that makes results in changes to a synthesized CloudFormation template but only to metadata-ish attributes like DependsOn. For example, #21495. This PR has only a code change with no changes to tests.
  2. Run yarn build+test
  3. See that there are integration tests with a CHANGED status, resulting in a test failure
  4. Attempt to run yarn integ --update-on-failed
  5. The stacks that are created match the old synthesized CloudFormation templates, not the new ones that will be checked in.

Possible Solution

When DependsOn is the only thing that changes, CloudFormation returns a result with a ValidationException and the message No updates are to be performed.

When that happens, it may be best to build the stack again fresh based off the new CloudFormation template as well. This ensures that updates are no-ops for existing users as expected and that the stack operates as expected for users building it the first time.

Additional Information/Context

This is the command and the output I used when testing the provided PR, #21495, myself.

$ yarn integ --update-on-failed --parallel-regions us-east-1

yarn run v1.22.19
$ integ-runner --update-on-failed --parallel-regions us-east-1

Verifying integration test snapshots...

  UNCHANGED  integ.import-default-vpc.lit 2.451s
  UNCHANGED  integ.graviton3 3.092s
  UNCHANGED  integ.bastion-host 3.242s
  UNCHANGED  integ.instance-multipart-userdata 3.162s
  UNCHANGED  integ.instance 3.306s
  UNCHANGED  integ.share-vpcs.lit 3.567s
  CHANGED    integ.ports 4.274s
      Resources
[~] AWS::EC2::NatGateway VPCPublicSubnet1NATGatewayE0556630 
 └─ [+] DependsOn
     └─ ["VPCPublicSubnet1DefaultRoute91CEF279"]
[~] AWS::EC2::NatGateway VPCPublicSubnet2NATGateway3C070193 
 └─ [+] DependsOn
     └─ ["VPCPublicSubnet2DefaultRouteB7481BBA"]

  CHANGED    integ.vpc-flow-logs 4.654s
      Resources
[~] AWS::EC2::NatGateway VPCPublicSubnet1NATGatewayE0556630 
 └─ [+] DependsOn
     └─ ["VPCPublicSubnet1DefaultRoute91CEF279"]
[~] AWS::EC2::NatGateway VPCPublicSubnet2NATGateway3C070193 
 └─ [+] DependsOn
     └─ ["VPCPublicSubnet2DefaultRouteB7481BBA"]

  CHANGED    integ.vpc-flow-logs 4.662s
      Resources
[~] AWS::EC2::NatGateway VPCPublicSubnet1NATGatewayE0556630 
 └─ [+] DependsOn
     └─ ["VPCPublicSubnet1DefaultRoute91CEF279"]
[~] AWS::EC2::NatGateway VPCPublicSubnet2NATGateway3C070193 
 └─ [+] DependsOn
     └─ ["VPCPublicSubnet2DefaultRouteB7481BBA"]

  UNCHANGED  integ.client-vpn-endpoint 6.748s
  CHANGED    integ.bastion-host-arm-support 6.901s
      Resources
[~] AWS::EC2::NatGateway VPCPublicSubnet1NATGatewayE0556630 
 └─ [+] DependsOn
     └─ ["VPCPublicSubnet1DefaultRoute91CEF279"]
[~] AWS::EC2::NatGateway VPCPublicSubnet2NATGateway3C070193 
 └─ [+] DependsOn
     └─ ["VPCPublicSubnet2DefaultRouteB7481BBA"]

  UNCHANGED  integ.reserved-private-subnet 6.831s
  CHANGED    integ.vpc-azs 6.695s
      Resources
[~] AWS::EC2::NatGateway MyVpcPublicSubnet1NATGatewayAD3400C1 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet1DefaultRoute95FDF9EB"]

  UNCHANGED  integ.vpc-gateway 6.829s
  CHANGED    integ.instance-init 7.363s
      Resources
[~] AWS::EC2::NatGateway IntegInitVpcPublicSubnet1NATGateway46F32F7F 
 └─ [+] DependsOn
     └─ ["IntegInitVpcPublicSubnet1DefaultRoute5BB90E8C"]
[~] AWS::EC2::NatGateway IntegInitVpcPublicSubnet2NATGateway9CCB4A9C 
 └─ [+] DependsOn
     └─ ["IntegInitVpcPublicSubnet2DefaultRoute2393995F"]

  UNCHANGED  integ.nat-instances.lit 7.377s
  CHANGED    integ.vpc-endpoint.lit 7.135s
      Resources
[~] AWS::EC2::NatGateway MyVpcPublicSubnet1NATGatewayAD3400C1 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet1DefaultRoute95FDF9EB"]
[~] AWS::EC2::NatGateway MyVpcPublicSubnet2NATGateway91BFBEC9 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet2DefaultRoute052936F6"]

  CHANGED    integ.vpc-networkacl 5.875s
      Resources
[~] AWS::EC2::NatGateway MyVpcPublicSubnet1NATGatewayAD3400C1 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet1DefaultRoute95FDF9EB"]
[~] AWS::EC2::NatGateway MyVpcPublicSubnet2NATGateway91BFBEC9 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet2DefaultRoute052936F6"]

  CHANGED    integ.vpn 5.277s
      Resources
[~] AWS::EC2::NatGateway MyVpcPublicSubnet1NATGatewayAD3400C1 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet1DefaultRoute95FDF9EB"]
[~] AWS::EC2::NatGateway MyVpcPublicSubnet2NATGateway91BFBEC9 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet2DefaultRoute052936F6"]

  CHANGED    integ.vpc 5.419s
      Resources
[~] AWS::EC2::NatGateway MyVpcPublicSubnet1NATGatewayAD3400C1 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet1DefaultRoute95FDF9EB"]
[~] AWS::EC2::NatGateway MyVpcPublicSubnet2NATGateway91BFBEC9 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet2DefaultRoute052936F6"]

  CHANGED    integ.vpn-pre-shared-key-token 5.337s
      Resources
[~] AWS::EC2::NatGateway MyVpcPublicSubnet1NATGatewayAD3400C1 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet1DefaultRoute95FDF9EB"]
[~] AWS::EC2::NatGateway MyVpcPublicSubnet2NATGateway91BFBEC9 
 └─ [+] DependsOn
     └─ ["MyVpcPublicSubnet2DefaultRoute052936F6"]

Clearly, all of these should result in CloudFormation stacks that have a DependsOn with the listed resources. And that's exactly what does appear in the created template files (such as test/vpn.integ.snapshot/aws-cdk-ec2-vpn.template.json); however, the CloudFormation stack that got built is https://gist.github.com/kylelaker/84816bbfbecdecce9d8d72a947b25eef; the newly created snapshot was https://gist.github.com/09782fc5fbc6afa8b40c01c358f8878e.

CDK CLI Version

2.33.0

Framework Version

No response

Node.js Version

16

OS

Linux

Language

Typescript

Language Version

No response

Other information

This may also impact users who change properties such as the stack Description without changes to the template; however, I am unsure if integ-runner is meant to handle that use case.

Arguably, this is a CloudFormation bug/request. Ideally, I'd be able to change the order in which my resources will be deleted without having to make any changes the resources' properties.

corymhall commented 2 years ago

For this use case I would use the method described here and disable the update workflow (i.e. yarn integ --disable-update-workflow).

github-actions[bot] commented 2 years ago

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see. If you need more assistance, please either tag a team member or open a new issue that references this one. If you wish to keep having a conversation with other community members under this issue feel free to do so.