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

Initial 1.19 support upgrade #204

Merged
merged 12 commits into from
Mar 29, 2021
Merged

Initial 1.19 support upgrade #204

merged 12 commits into from
Mar 29, 2021

Conversation

ettiee
Copy link
Contributor

@ettiee ettiee commented Mar 12, 2021

Ref #197

  • Upgrade cluster / node group versions
  • Upgrade core addons to recommended versions (coredns / kube-proxy)
  • Upgrade other addons to latest compatible versions

For discussion

EKS no longer adds kubernetes.io/cluster/ tags to subnets

We are on the latest load-balancer-controller so we do not need to support these tags for compatibility. However, these tags are required for auto discovery on load balancer creation. I think this is the way we should encourage teams to operate (and is how they do now). The implication for PR changes:

  • Add tags to subnets in vpc module
  • remove lifecycle change ignore on tags in vpc module
  • Recommend users to codify these tags if they are not using the vpc module

@ettiee ettiee marked this pull request as ready for review March 12, 2021 16:37
@ettiee ettiee requested a review from a team as a code owner March 12, 2021 16:37
@ettiee ettiee requested review from errm and kenju and removed request for a team March 12, 2021 16:37
@ettiee
Copy link
Contributor Author

ettiee commented Mar 12, 2021

@aidy @dan-slinky-ckpd @errm let me know your opinion on

EKS no longer adds kubernetes.io/cluster/ tags to subnets

If you agree we should default to keeping these tags on subnets I'll fix the tests and make the tags optional in the VPC module.

modules/vpc/variables.tf Outdated Show resolved Hide resolved
@errm
Copy link
Member

errm commented Mar 16, 2021

However, these tags are required for auto discovery on load balancer creation.

Could you explain some more about this? Is there a link to documentation about the situations where it would be needed?

@ettiee
Copy link
Contributor Author

ettiee commented Mar 17, 2021

Could you explain some more about this? Is there a link to documentation about the situations where it would be needed?

Thanks @errm 🙏 it's for the auto discovery for aws load balancer controller https://kubernetes-sigs.github.io/aws-load-balancer-controller/latest/deploy/subnet_discovery/
In practice - without those subnet tags users will need to specify the tag alb.ingress.kubernetes.io/subnets on their ingress definitions.

@aidy
Copy link
Contributor

aidy commented Mar 22, 2021

EKS no longer adds kubernetes.io/cluster/ tags to subnets

I think that makes sense to me.

@ettiee
Copy link
Contributor Author

ettiee commented Mar 23, 2021

From yesterday's discussions with @aidy - plan to introduce cert-manager to avoid generating self-signed certs for the aws-load-balancer will be done in #205
I've implemented the more opinionated VPC / subnet tags in the VPC module example - and upgrade notes for users to add them if they terraform their own VPC.
Remaining review can focus on the 1.19 upgrade + updates to critical addons

Copy link
Contributor

@dan-slinky-ckpd dan-slinky-ckpd left a comment

Choose a reason for hiding this comment

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

LGTM. Gimme an RC :)

@ettiee ettiee merged commit 8b749a9 into master Mar 29, 2021
@ettiee ettiee deleted the ee/upgrade-1-19 branch March 29, 2021 10:34
@ettiee ettiee mentioned this pull request Mar 29, 2021
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

4 participants