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

fix: Prevents setting LoadBalancerIP when AWS-specific annotations are present #707

Closed
wants to merge 1 commit into from

Conversation

Pandry
Copy link

@Pandry Pandry commented Mar 2, 2025

Addresses #688, this commit adds a check to the ControlPlane service's annotations.
If the spec.loadBalancerIP property is set in the service, the AWS cloud controller will complain and causes issues.

The controller now checks for the presence of these specific annotations and avoids setting the LoadBalancerIP to ensure that the service is correctly configured according to AWS requirements.

Copy link

netlify bot commented Mar 2, 2025

Deploy Preview for kamaji-documentation canceled.

Name Link
🔨 Latest commit 7d7f0f6
🔍 Latest deploy log https://app.netlify.com/sites/kamaji-documentation/deploys/67c48ddbfed6890008ad789b

Addresses clastix#688, this commit adds a check to the ControlPlane service's
annotations.
If the `spec.loadBalancerIP` property is set in the service, the AWS
cloud controller will complain and causes issues.

The controller now checks for the presence of these specific annotations
and avoids setting the LoadBalancerIP to ensure that the service is correctly
configured according to AWS requirements.
Copy link
Member

@prometherion prometherion left a comment

Choose a reason for hiding this comment

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

Of course we'll need to rename the PR and amend commit subject, but this is fine.

if len(address) > 0 {

// hack: Load Balancer IP should not be added to avoid the AWS Load Balancer get stuck
_, hasLbAwsEipAllocations := r.resource.Annotations["service.beta.kubernetes.io/aws-load-balancer-eip-allocations"]
Copy link
Member

Choose a reason for hiding this comment

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

We have a better option, here:

The.spec.loadBalancerIP field for a Service was deprecated in Kubernetes v1.24.

source: https://kubernetes.io/docs/concepts/services-networking/service/#loadbalancer

Let's remove at all the r.resource.Spec.LoadBalancerIP assignment and we'll be safe.

Copy link
Author

Choose a reason for hiding this comment

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

Oh, wow, good to know!
I think it might make more sense to close this then and open a new PR

@Pandry
Copy link
Author

Pandry commented Mar 5, 2025

Closing in favor of #713

@Pandry Pandry closed this Mar 5, 2025
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.

2 participants