Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cleanupAws in integration test is out of date #208

Closed
errordeveloper opened this issue Sep 13, 2018 · 3 comments
Closed

cleanupAws in integration test is out of date #208

errordeveloper opened this issue Sep 13, 2018 · 3 comments
Labels
good first issue Good for newcomers help wanted Extra attention is needed kind/bug

Comments

@errordeveloper
Copy link
Contributor

errordeveloper commented Sep 13, 2018

This got me very puzzled actually, I nearly panicked that I found a bug in EKS. I found CloudFormation stacks in my account, but couldn't see actual EKS clusters.

Turns out we delete the cluster directly first:

https://github.com/weaveworks/eksctl/blob/142fd2b8fbec89678aec657e612482e1a46ef689/tests/integration/cleanup.go#L12-L14

Then we try to delete some stack, but using the old naming convention:

https://github.com/weaveworks/eksctl/blob/142fd2b8fbec89678aec657e612482e1a46ef689/tests/integration/cleanup.go#L16-L34

Obviously something that got overlooked earlier, easy to fix though. Probably it should just call eksctl delete cluster, wait and verify that both stacks get deleted using CloudFormation API (basically mirror what we do when we setup the integration tests).

@errordeveloper errordeveloper added help wanted Extra attention is needed good first issue Good for newcomers kind/bug labels Sep 13, 2018
@errordeveloper errordeveloper added this to the 0.1.x – improvements milestone Sep 13, 2018
@errordeveloper
Copy link
Contributor Author

...wait and verify that both stacks get deleted using CloudFormation API (basically mirror what we do when we setup the integration tests).

Waiting for cluster stack to get delete may take too long actually, perhaps we don't have wait for that. Just check if eksctl delete cluster resulted in nodegroup stack going away, and cluster stack status becomes DELETE_IN_PROGRESS. If we decide to make eksctl delete cluster wait for all resource to get deleted, so be it, but for now we shouldn't have to make tests run too long before there is a need for that.

@richardcase
Copy link
Contributor

richardcase commented Sep 13, 2018

In my head the integration test was always supposed to be full life cycle:
create->list->delete
and the cleanup code was only there if the test failed and we just needed to cleanup.

I never got around to completeing the full lifecycle.

@errordeveloper
Copy link
Contributor Author

Fixed by @Lazyshot via #255.

torredil pushed a commit to torredil/eksctl that referenced this issue May 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers help wanted Extra attention is needed kind/bug
Projects
None yet
Development

No branches or pull requests

2 participants