-
Notifications
You must be signed in to change notification settings - Fork 41
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
Manage vpc-cni, coredns and kube-proxy via eks addons #276
Conversation
5c19db2
to
989ef33
Compare
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.
lgtm - could you update the PR description before merging?
989ef33 removes a variable so probably worth documenting in |
hmm, so - my normal approach is to have sane, atomic, commits - and then to merge the commits, rather than squash. Perhaps it's worth rewording 821d95f to have some reasoning for the motivation for this change. Would that work for you? |
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.
lgtm - be good to include #276 (comment)
Oh, yes. (That should probably have been a whole different PR - but we're here now) |
For the switch to using eks_addons, we'll need to ensure that nodes are up and running before trying to provision them. Otherwise we end up in a race condition and get errors of the kind: ``` Error: unexpected EKS Add-On (terraform-aws-eks-testing-jPfg0J:coredns) state returned during creation: unexpected state 'DEGRADED', wanted target 'ACTIVE'. last error: %!!(MISSING)s(<nil>) ```
https://docs.aws.amazon.com/eks/latest/userguide/eks-add-ons.html This makes a start on our reconsideration of addons - #224 Where available, we'd prefer to use eks addons rather than set them up ourselves.
EKS and the node AMIs are using the same logic to calculate it.
No description provided.