Skip to content

feat: env deploy --no-rollback flag#4168

Merged
mergify[bot] merged 2 commits intoaws:mainlinefrom
dannyrandall:feat/env-rollback
Nov 16, 2022
Merged

feat: env deploy --no-rollback flag#4168
mergify[bot] merged 2 commits intoaws:mainlinefrom
dannyrandall:feat/env-rollback

Conversation

@dannyrandall
Copy link
Copy Markdown
Contributor

should help with some debugging.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.

@dannyrandall dannyrandall requested a review from a team as a code owner November 10, 2022 22:48
@dannyrandall dannyrandall requested review from huanjani and removed request for a team November 10, 2022 22:48
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 10, 2022

🍕 Here are the new binary sizes!

Name New size (kiB) v1.23.0 size (kiB) Delta (%)
macOS (amd) 47672 47548 +0.26
macOS (arm) 47672 48200 ❤️ -1.10
linux (amd) 41928 41812 +0.28
linux (arm) 41928 41220 🥺 +1.72
windows (amd) 38764 38664 +0.26

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #4168 (52fa4a7) into mainline (b956464) will decrease coverage by 0.01%.
The diff coverage is 42.10%.

@@             Coverage Diff              @@
##           mainline    #4168      +/-   ##
============================================
- Coverage     69.24%   69.22%   -0.02%     
============================================
  Files           250      250              
  Lines         35927    35945      +18     
  Branches        264      264              
============================================
+ Hits          24876    24883       +7     
- Misses         9856     9866      +10     
- Partials       1195     1196       +1     
Impacted Files Coverage Δ
internal/pkg/cli/env_deploy.go 54.31% <8.33%> (-2.99%) ⬇️
internal/pkg/cli/deploy/env.go 75.19% <100.00%> (+0.58%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

}
if o.disableRollback {
stackName := stack.NameForEnv(o.targetApp.Name, o.targetEnv.Name)
rollbackCmd := fmt.Sprintf("aws cloudformation rollback-stack --stack-name %s --role-arn %s", stackName, o.targetEnv.ExecutionRoleARN)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice to include the role ARN! 💖

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: This is nice so you don't have to change it, I'm just thinking out loud here.

I'd have expected this log to be in either the (1) RecommendedActions method, or, (2) we implement a custom error type that implements the RecommendActions method.

I don't think (1) fits since we are not exiting Execute successfully, so I think (2) is the better route 💭

So perhaps something like:

err := fmt.Errorf("deploy environment %s: %w", o.name, err)
if o.disableRollback{ 
  return &errWithDisabledRollback{
     parentErr: err,
     stackName: stack.NameForEnv(o.targetApp.Name, o.targetEnv.Name),
     cmd: fmt.Sprintf("aws cloudformation rollback-stack --stack-name %s --role-arn %s", stackName, o.targetEnv.ExecutionRoleARN)
  }
}
return err

Copy link
Copy Markdown
Contributor

@iamhopaul123 iamhopaul123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is awesome! One quick question: shall we give a different hint to them if their env deploy fails because they did env deploy --no-rollback before and they haven't manually rollback the deployment?

@dannyrandall
Copy link
Copy Markdown
Contributor Author

This is what the error looks like right now - I think it's probably good enough, what do you think? The second command is the specific case you're talking about:

➜ cl env deploy --no-rollback
✔ Proposing infrastructure changes for the app-env environment.
- Creating the infrastructure for the app-env environment.              [update failed]    [23.1s]
  The following resource(s) failed to create: [DeleteMeRouteTable].                               
  - A Virtual Private Cloud to control networking of your AWS resources        [update complete]  [0.0s]
It seems like you have disabled automatic stack rollback for this deployment.
To debug, you can visit the AWS console to inspect the errors.
After fixing the deployment, you can:
1. Run `aws cloudformation rollback-stack --stack-name app-env --role-arn arn:aws:iam::123456789:role/app-env-CFNExecutionRole` to rollback the deployment.
2. Run `copilot env deploy` to make a new deployment.
✘ deploy environment env: stack app-env did not complete successfully and exited with status UPDATE_FAILED

➜ cl env deploy                                                                                                                                                                                              
✔ Proposing infrastructure changes for the app-env environment.
✘ deploy environment pdx: execute change set arn:aws:cloudformation:us-west-2:123456789:changeSet/copilot-id/id for stack app-env: ValidationError: This stack is currently in a non-terminal [UPDATE_FAILED] state. To update the stack from this state, please use the disable-rollback parameter with update-stack API. To rollback to the last known good state, use the rollback-stack API
        status code: 400, request id: id

@mergify mergify Bot merged commit 5781c53 into aws:mainline Nov 16, 2022
@dannyrandall dannyrandall deleted the feat/env-rollback branch November 16, 2022 17:10
KollaAdithya pushed a commit to KollaAdithya/copilot-cli that referenced this pull request Nov 21, 2022
should help with some debugging.
- https://gitter.im/aws/copilot-cli?at=636cb648473cf96648dbcbb9
- https://gitter.im/aws/copilot-cli?at=636c024ca34b51121122d168

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the Apache 2.0 License.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants