This is untested locally. I need to look into what it takes to use my local branch in a venv or something rather than the aws cli I already have installed. Will comment back when I've run this locally and tested a few invocations with --disable-rollback, --no-disable-rollback, and neither option.

Hmm.. testing this locally it is only working like expected for update changesets. If aws cloudformation deploy is creating a new stack, it is rolling back when an error is encountered. I'll have to dig in a little more to see if change set execution happens differently for create vs update.

Edit: I was testing this in us-gov-west-1 and seeing the problem above. I just tested this in us-east-1 instead and both create and update change sets are working as expected.

How I've tested this within an AWS account in us-east-1 Region:

1. Deploy 3 CloudFormation stacks using a template with an expected error. CfnDeploy is deploy with no rollback options specified, CfnDeployNoDisableRollback is deployed with --no-disable-rollback, and CfnDeployDisableRollback is deployed with --disable-rollback:

   ./bin/aws cloudformation deploy --stack-name CfnDeploy --template-file ~/tmp/cfn-error.yml
   ./bin/aws cloudformation deploy --stack-name CfnDeployNoDisableRollback --template-file ~/tmp/cfn-error.yml --no-disable-rollback
   ./bin/aws cloudformation deploy --stack-name CfnDeployDisableRollback --template-file ~/tmp/cfn-error.yml --disable-rollback

   Stack statuses after these deploys:

   Stack Name	Stack Status
   CfnDeploy	ROLLBACK_COMPLETE
   CfnDeployNoDisableRollback	ROLLBACK_COMPLETE
   CfnDeployDisableRollback	CREATE_FAILED

2. Deploy 3 CloudFormation stacks using a template with no error. After the stacks have been created successfully, update the stacks using a template with an expected error. CfnDeployUpdate is deploy with no rollback options specified, CfnDeployUpdateNoDisableRollback is deployed with --no-disable-rollback, and CfnDeployUpdateDisableRollback is deployed with --disable-rollback:

   # First deploy the stacks successfully using a template with no errors
   ./bin/aws cloudformation deploy --stack-name CfnDeployUpdate --template-file ~/tmp/no-resources.yml
   ./bin/aws cloudformation deploy --stack-name CfnDeployUpdateNoDisableRollback --template-file ~/tmp/no-resources.yml --no-disable-rollback
   ./bin/aws cloudformation deploy --stack-name CfnDeployUpdateDisableRollback --template-file ~/tmp/no-resources.yml --disable-rollback
   
   # Then deploy the stacks using a template with an expected error
   ./bin/aws cloudformation deploy --stack-name CfnDeployUpdate --template-file ~/tmp/cfn-error.yml
   ./bin/aws cloudformation deploy --stack-name CfnDeployUpdateNoDisableRollback --template-file ~/tmp/cfn-error.yml --no-disable-rollback
   ./bin/aws cloudformation deploy --stack-name CfnDeployUpdateDisableRollback --template-file ~/tmp/cfn-error.yml --disable-rollback

   Stack statuses after these deploys:

   Stack Name	Stack Status
   CfnDeployUpdate	UPDATE_ROLLBACK_COMPLETE
   CfnDeployUpdateNoDisableRollback	UPDATE_ROLLBACK_COMPLETE
   CfnDeployUpdateDisableRollback	UPDATE_FAILED

This seems to be working as I expect. I have updated unit tests in the code and tested manually within an account. This change is ready to be reviewed and merged. Please let me know if any updates are needed to these changes.

Anything I can do to get some attention on this?

Hi @atheiman,

Checked your end to end test results and all look good.

I left some comments on your commit, no major concern but mostly comments related with customer-facing doc updates. For some of them I will come back tomorrow with update after discussing internally with my team.

Thanks !

@SeveroeHe - I believe I have implemented or replied to all your suggestions. Local testing still working as I expect.

If you have other comments, can you leave them on the Files changed page of this PR, rather than on a specific commit? They are harder to track on a commit page.

Thanks for reviewing! Let me know what else is needed

Discussed with our doc writer and our PM, there are two more changes we want to make, besides all good.

@SeveroeHe updated, let me know if any other changes are needed or if i missed anything. Thanks!

bleh, looks like unit tests broke last time I was making changes as well. I will get the tests fixed up and respond to the comments here.

Fixed tests locally, thanks for approving the CI workflow. I replied to the only outstanding conversation i see about using both --disable-rollback --no-disable-rollback in one command. Let me know how you want it handled

Codecov Report

Merging #6453 (365a789) into develop (408c70a) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop    #6453   +/-   ##
========================================
  Coverage    92.85%   92.85%           
========================================
  Files          204      204           
  Lines        16298    16298           
========================================
  Hits         15133    15133           
  Misses        1165     1165           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 408c70a...365a789. Read the comment docs.

As far as I can tell, all conversations are resolved. @nateprewitt @SeveroeHe let me know if more is needed from me. Thanks!

@nateprewitt @SeveroeHe is this ready to merge? Anything needed from me?

@nateprewitt @SeveroeHe can we get this merged in? Anything else needed from me?