-
-
Notifications
You must be signed in to change notification settings - Fork 359
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
Nuke EKS clusters #43
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great, thanks Yori. BTW, don't forget to update the README!
aws/eks_test.go
Outdated
}) | ||
if err != nil { | ||
require.Fail(t, errors.WithStackTrace(err).Error()) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use require.NoError
here and below on similar if
/Fail
combos.
aws/eks_utils_for_test.go
Outdated
} | ||
rand.Seed(time.Now().UnixNano()) | ||
randIndex := rand.Intn(len(supportedRegions)) | ||
return supportedRegions[randIndex] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a terratest GetRandomRegion
helper that takes in a whitelist and/or blacklist of regions you can use...
) | ||
|
||
// getAllEksClusters returns a list of strings of EKS Cluster Names that uniquely identify each cluster. | ||
func getAllEksClusters(awsSession *session.Session, excludeAfter time.Time) ([]*string, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you need to call this EKS code from somewhere? I think there's a struct of some sort you need to add and then some extra API calls in the main CLI app...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup... I can't believe I missed that...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was so tied up in the tests and it was getting late that I got a bit too trigger happy I guess.
d791082
to
7578319
Compare
One downside of this: the tests now have an extra 20 minutes because launching and tearing down EKS clusters take that long... |
Does it run in parallel with the other tests so the whole build takes 20 min? Or is it sequential? |
It runs parallel, so the whole build takes 20 minutes. For comparison, it was ~5 mins before. |
Understood. Not much we can do, TBH. Eventually, we'll want to nuke RDS, Elasticsearch, and other slow stuff too, so the build will only get slower. Such is DevOps. |
Can anything be done to finish this? |
@rfvermut Thanks for the inquiry! I plan on getting back to this sometime later this month or early next month. The main issue right now is that our CI build has gotten unstable with the introduction of the new tests here so need to get a handle on that. |
We don't seem to have verbose output in the tests. Are there any particular tests that keep failing? |
Thanks @tonerdo for the test fixes! I took a quick scan and I believe I addressed all the comments, so I am going to go ahead and merge this and release! |
Given how expensive EKS is (one cluster = $0.20/hr), and how frequent the tests fail right now as I actively develop, I am afraid I might forget to clean up one of these so decided to go ahead and implement the nuking.
Note that this required upgrading
aws-sdk-go
. I tried the latest and there were issues, so downgraded to what we have in terratest.