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

Set correct cluster tag on EKS #8487

Merged
merged 3 commits into from
Nov 1, 2024
Merged

Set correct cluster tag on EKS #8487

merged 3 commits into from
Nov 1, 2024

Conversation

mikkeloscar
Copy link
Contributor

@mikkeloscar mikkeloscar commented Oct 31, 2024

This sets the correct cluster on EKS which is kubernetes.io/cluster/<eks-cluster-name>: owned. This fixes support for Service Type LoadBalancer because the EKS cloud-controller-manager was not able to find and attach nodes as they had the wrong cluster tag.

This is a bit messy because we use kubernetes.io/cluster/<cluster-id> on non-EKS and kubernetes.io/cluster/<eks-cluster-name> on EKS. So right now it's adding both tags on EKS to be compatible e.g. with e2e cleanup.

I propose to create an internal tech-depth issue to discuss how to clean up this mess with cluster ID vs. Cluster Name.

Depends on

@mikkeloscar mikkeloscar added the minor Minor changes, e.g. low risk config updates, changes that do not introduce a new API call. label Oct 31, 2024
@katyanna
Copy link
Member

👍

Signed-off-by: Mikkel Oscar Lyderik Larsen <[email protected]>
Signed-off-by: Mikkel Oscar Lyderik Larsen <[email protected]>
Signed-off-by: Mikkel Oscar Lyderik Larsen <[email protected]>
@@ -666,9 +623,6 @@ Resources:
GroupId: !Ref WorkerSecurityGroup
IpProtocol: tcp
SourceSecurityGroupId: !Ref MasterSecurityGroup
Tags:
- Key: 'kubernetes.io/cluster/{{.Cluster.ID}}'
Value: owned
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All these cases are dropped because we set the tag at stack level already

@mikkeloscar
Copy link
Contributor Author

👍

@demonCoder95
Copy link
Member

👍

@mikkeloscar mikkeloscar merged commit a48afc1 into eks Nov 1, 2024
10 checks passed
@mikkeloscar mikkeloscar deleted the eks-fix-cluster-tag branch November 1, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do-not-merge internal/merges-tagged minor Minor changes, e.g. low risk config updates, changes that do not introduce a new API call.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants