test(e2e): clean redundant task stack for task e2e test#1267
test(e2e): clean redundant task stack for task e2e test#1267mergify[bot] merged 2 commits intoaws:mainlinefrom
Conversation
|
Does the env get deleted, too? |
yeah |
78e0093 to
34247ed
Compare
| err := command.Run("aws", []string{"ecr", "delete-repository", "--repository-name", repoName, "--force"}) | ||
| Expect(err).NotTo(HaveOccurred(), "delete ecr repo") | ||
| // Delete task stack. | ||
| err = command.Run("aws", []string{"cloudformation", "delete-stack", "--stack-name", taskStackName}) |
There was a problem hiding this comment.
So I'm trying to understand based on the error that you saw yesterday while trying to delete from console:
Role arn:aws:iam::xxx:role/e2e-task-1597338546-test-CFNExecutionRole is invalid or cannot be assumed
Will this have the necessary permissions? how come?
There was a problem hiding this comment.
yeah it will have. I tested that. So it's because we still have the CFN execution role since we haven't deleted the environment yet.
There was a problem hiding this comment.
Ok thanks that totally makes sense!
I think one thing that these tests made us realize is that if a customer uses the same task group name to run their tasks both in a Copilot env and then in default, they won't be able to delete the stack in case the Copilot environment is deleted which is slightly surprising:
It is not possible to remove a service role attached to a stack after the stack is created.
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-iam-servicerole.html
Maybe, when we implement task delete we can provide a nicer err message with action items when this err occurs.
| err := command.Run("aws", []string{"ecr", "delete-repository", "--repository-name", repoName, "--force"}) | ||
| Expect(err).NotTo(HaveOccurred(), "delete ecr repo") | ||
| // Delete task stack. | ||
| err = command.Run("aws", []string{"cloudformation", "delete-stack", "--stack-name", taskStackName}) |
There was a problem hiding this comment.
Ok thanks that totally makes sense!
I think one thing that these tests made us realize is that if a customer uses the same task group name to run their tasks both in a Copilot env and then in default, they won't be able to delete the stack in case the Copilot environment is deleted which is slightly surprising:
It is not possible to remove a service role attached to a stack after the stack is created.
https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-iam-servicerole.html
Maybe, when we implement task delete we can provide a nicer err message with action items when this err occurs.
<!-- Provide summary of changes --> Currently our task e2e test doesn't delete the task stack after running the task e2e test. And the task stack is very difficult to remove because the CFN Execution Role that used to manage the task stack is deleted when we delete copilot environment. Also, the remaining task stacks piles up in our Copilot pipeline account. This PR fixes this issue by removing task stack before we delete copilot app/env. <!-- Issue number, if available. E.g. "Fixes aws#31", "Addresses aws#42, 77" --> By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Currently our task e2e test doesn't delete the task stack after running the task e2e test. And the task stack is very difficult to remove because the CFN Execution Role that used to manage the task stack is deleted when we delete copilot environment. Also, the remaining task stacks piles up in our Copilot pipeline account.
This PR fixes this issue by removing task stack before we delete copilot app/env.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.