fix(cli): unable to update stacks in ROLLBACK_COMPLETE #8779

eladb · 2020-06-29T10:35:54Z

The CLI mistakingly determined that a stack in ROLLBACK_COMPLETE status is not updatable.

This change cleans up this logic so that a stack update will fail only if the stack is in _FAILED status, which is a non-updatable state.

Fixes #8126 #5151

TODO:

Unit tests + integration test
I think we also have a major issue with isCreationFailed. It appears that if a stack is in ROLLBACK_XXX status, it will be determined that it's creation was failed, and it will be deleted (??). Is that a bug?

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

The CLI mistakingly determined that a stack in `ROLLBACK_COMPLETE` status is not updatable. This change cleans up this logic so that a stack update will fail only if the stack is in `_FAILED` status, which is a non-updatable state. Fixes #8126

BryanPan342 · 2020-06-30T17:06:16Z

I think this also fixes #5151

Removal of isRollback function would fix the Update_Rollback_Complete bug as mentioned in #5151 and would also fix the unaddressed bug of Import_Rollback_Complete which would have incorrectly been in a failed state!

cursive-ide · 2020-07-01T08:40:43Z

I currently have a production environment stuck in this state and I would really love to use this fix!

eladb · 2020-07-01T11:26:06Z

@cursive-ide wrote:

I currently have a production environment stuck in this state and I would really love to use this fix!

If you feel adventurous, you can try to patch up your local CLI. Update the isRollback property in stack-status.js file from this:

get isRollback() {
  return this.name.indexOf('ROLLBACK') !== -1;
}

To this:

get isRollback() {
  return this.name.indexOf('ROLLBACK_FAILED') !== -1;
}

LMK if this helps.

cursive-ide · 2020-07-01T21:22:51Z

LMK if this helps.

Yes, I actually did this last night (I applied the whole patch rather than just that one change - the JS is mostly the same as the TypeScript) and it allowed me to get my system out of the stuck state - thanks!

shivlaks · 2020-07-02T01:15:07Z

picking this up, will update the PR as I go

…back_in_progress

…ing rollback_in_progress" This reverts commit 5b67fa9.

shivlaks · 2020-07-08T03:21:19Z

@eladb - looked into this a bit more as I was trying to get rid of isCreationFailure. However, I think it's the right behaviour. I stepped into a gotcha of confusing ROLLBACK_COMPLETE with UPDATE_ROLLBACK_COMPLETE as I was writing the integ test.

per the Stack status codes

ROLLBACK_COMPLETE

Successful removal of one or more stacks after a failed stack creation or after an explicitly canceled stack creation. Any resources that were created during the create stack action are deleted.

This status exists only after a failed stack creation. It signifies that all operations from the partially created stack have been appropriately cleaned up. When in this state, only a delete operation can be performed.

UPDATE_ROLLBACK_COMPLETE

Successful return of one or more stacks to a previous working state after a failed stack update.

The bug this addresses is the UPDATE_ROLLBACK_COMPLETE scenario which we currently preclude from updating, but should allow.

updating the tests to verify theUPDATE_ROLLBACK_COMPLETE behaviour. Let me know if you had something different in mind.

shivlaks · 2020-07-08T04:15:21Z

@eladb - think I addressed the TODO list, can you take a look when you get a chance?

aws-cdk-automation · 2020-07-08T04:32:56Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: 0aaa465
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

BryanPan342 · 2020-07-08T03:53:20Z

packages/aws-cdk/lib/api/util/cloudformation/stack-status.ts

-  get isSuccess(): boolean {
-    return !this.isNotFound && !this.isRollback && !this.isFailure;
+  get isComplete(): boolean {
+    return this.name.endsWith('_COMPLETE');
  }


What's the use case for the isComplete function? I think this function might be deceiving because Rollback_Complete state isn't technically fully functional... I think it might be best to just remove it or add some documentation to each function.

If this function is used so our customers can check on their state then I think we should add some documentation to this!

Also, if isComplete is added as a function for customers to use.. I think then I think isRollback can arguably be put in as well but never called.

users don't have programmatic access to the CLI yet, so these methods are only used internally. However, I think you have a good point about whether it's deceiving what isComplete means and maybe it's better to leave it off for now since we're not using it.

might be handy to have a method indicate whether a stack is updateable. What do you think?

If we dont use it might as well just remove it I think.. might be confusing when we look back and get confused as to why we have it in the code anyways..

isComplete doesn't indicate if a stack is updateable though. If it is Rollback_Complete then it can only be deleted right?

shivlaks · 2020-07-08T16:45:10Z

superseded by #8948

Supersedes #8779 The CLI determined that a stack in `UPDATE_ROLLBACK_COMPLETE` status is not updateable. related [comment](#8779 (comment)) This change modifies this logic by splitting up `waitForStack` into `waitForStackDelete` and `waitForStackDeploy` which evaluate stack status after it reaches a stable state depending on the operation that was performed. Closes #8126 #5151 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Supersedes aws#8779 The CLI determined that a stack in `UPDATE_ROLLBACK_COMPLETE` status is not updateable. related [comment](aws#8779 (comment)) This change modifies this logic by splitting up `waitForStack` into `waitForStackDelete` and `waitForStackDeploy` which evaluate stack status after it reaches a stable state depending on the operation that was performed. Closes aws#8126 aws#5151 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

eladb assigned shivlaks Jun 29, 2020

eladb requested review from shivlaks and rix0rrr June 29, 2020 12:16

shivlaks added the in-progress This issue is being actively worked on. label Jul 2, 2020

shivlaks added 2 commits July 6, 2020 22:49

only delete stack in failed status. added unit test for updating roll…

5b67fa9

…back_in_progress

Revert "only delete stack in failed status. added unit test for updat…

b3c65a5

…ing rollback_in_progress" This reverts commit 5b67fa9.

shivlaks added 2 commits July 7, 2020 20:38

add unit test

b234bfb

add integ test

0aaa465

shivlaks marked this pull request as ready for review July 8, 2020 04:14

BryanPan342 reviewed Jul 8, 2020

View reviewed changes

shivlaks mentioned this pull request Jul 8, 2020

fix(cli): unable to update stacks in UPDATE_ROLLBACK_COMPLETE #8948

Merged

shivlaks closed this Jul 8, 2020

shivlaks deleted the benisrae/cli-update-rollback branch July 28, 2020 18:23

renovate bot mentioned this pull request Aug 27, 2020

Update aws cdk to v1.149.0 cohalz/cirrocumulus#130

Open

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(cli): unable to update stacks in ROLLBACK_COMPLETE #8779

fix(cli): unable to update stacks in ROLLBACK_COMPLETE #8779

eladb commented Jun 29, 2020 •

edited by shivlaks

Loading

BryanPan342 commented Jun 30, 2020

cursive-ide commented Jul 1, 2020

eladb commented Jul 1, 2020

cursive-ide commented Jul 1, 2020

shivlaks commented Jul 2, 2020

shivlaks commented Jul 8, 2020

shivlaks commented Jul 8, 2020

aws-cdk-automation commented Jul 8, 2020

BryanPan342 Jul 8, 2020

shivlaks Jul 8, 2020

BryanPan342 Jul 8, 2020

shivlaks commented Jul 8, 2020

fix(cli): unable to update stacks in ROLLBACK_COMPLETE #8779

fix(cli): unable to update stacks in ROLLBACK_COMPLETE #8779

Conversation

eladb commented Jun 29, 2020 • edited by shivlaks Loading

BryanPan342 commented Jun 30, 2020

cursive-ide commented Jul 1, 2020

eladb commented Jul 1, 2020

cursive-ide commented Jul 1, 2020

shivlaks commented Jul 2, 2020

shivlaks commented Jul 8, 2020

shivlaks commented Jul 8, 2020

aws-cdk-automation commented Jul 8, 2020

AWS CodeBuild CI Report

BryanPan342 Jul 8, 2020

Choose a reason for hiding this comment

shivlaks Jul 8, 2020

Choose a reason for hiding this comment

BryanPan342 Jul 8, 2020

Choose a reason for hiding this comment

shivlaks commented Jul 8, 2020

eladb commented Jun 29, 2020 •

edited by shivlaks

Loading