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

Bug fixes in eks cleanup code #104

Merged
merged 2 commits into from
Dec 10, 2020
Merged

Bug fixes in eks cleanup code #104

merged 2 commits into from
Dec 10, 2020

Conversation

rhoboat
Copy link
Contributor

@rhoboat rhoboat commented Dec 10, 2020

Test run on terraform-aws-eks revealed a bug in the logic in our cleanup. This attempts to fix that.

yorinasub17
yorinasub17 previously approved these changes Dec 10, 2020
Copy link
Contributor

@yorinasub17 yorinasub17 left a comment

Choose a reason for hiding this comment

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

This LGTM, assuming my interpretation is correct. Basically the bug was that we were wrapping the error in the stack traced error struct, so the IsAwsErr checks were failing?

@rhoboat
Copy link
Contributor Author

rhoboat commented Dec 10, 2020

The bug was that we were throwing errors that the ENI was NotFound, from within requestDetach, when actually, we have a place that catches that error in the calling function:

kubergrunt/eks/cleanup.go

Lines 183 to 185 in ca93476

case isNIAttachmentNotFoundErr(err):
logger.Infof("Network interface %s is detached.", aws.StringValue(ni.NetworkInterfaceId))
continue

@rhoboat
Copy link
Contributor Author

rhoboat commented Dec 10, 2020

Sorry, need another approval, couldn't resist improving some comments.

@yorinasub17
Copy link
Contributor

The bug was that we were throwing errors that the ENI was NotFound, from within requestDetach, when actually, we have a place that catches that error in the calling function:

Got it! I think we are saying the same thing.

@rhoboat rhoboat merged commit 34293ea into master Dec 10, 2020
@rhoboat rhoboat deleted the bug-fixes branch December 10, 2020 19:27
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.

None yet

2 participants