-
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
Initial 1.19 support upgrade #204
Conversation
@aidy @dan-slinky-ckpd @errm let me know your opinion on
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. |
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/ |
I think that makes sense to me. |
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 |
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. Gimme an RC :)
Ref #197
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: