ANXKUBE-1251: Allow setting hostname via annotation instead of IPs #355
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Code Climate has analyzed commit c2aab4c and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 0.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 56.8% (-0.6% change). View more on Code Climate. |
LittleFox94
reviewed
Dec 17, 2024
LittleFox94
reviewed
Dec 17, 2024
nachtjasmin
force-pushed
the
ANXKUBE-1251/hostname-from-annotation
branch
2 times, most recently
from
579b446 to
8d6d143
Compare
nachtjasmin
reviewed
Jan 13, 2025
nachtjasmin
force-pushed
the
ANXKUBE-1251/hostname-from-annotation
branch
from
8d6d143 to
d8130db
Compare
nachtjasmin
force-pushed
the
ANXKUBE-1251/hostname-from-annotation
branch
from
d8130db to
c379f2d
Compare
|
Manually tested the changes with a From my point of view, this PR is ready for another review. |
LittleFox94
reviewed
Jan 13, 2025
|
Verified works :) |
By setting the hostname via an annotation on the given Service, we can workaround the shenanigans of kube-proxy to enable support for the PROXY protocol. In addition to the annotation being set, there's a configuration change needed by the LBaaS operators which is done manually right now. This change is also done by other CCM implementations, like the one of [DigitalOcean]. [DigitalOcean]: https://github.com/digitalocean/digitalocean-cloud-controller-manager/blob/d274be6e44f650311e4b960960b23001e6e513b4/cloud-controller-manager/do/loadbalancers.go#L236 Right now, this implementation is lacking proper tests to test this behaviour, which were skipped due to missing test infrastructure and a lack of time. Closes: ANXKUBE-1251 Co-authored-by: Zofia Hagenguth <[email protected]>
LittleFox94
force-pushed
the
ANXKUBE-1251/hostname-from-annotation
branch
from
c379f2d to
c2aab4c
Compare
LittleFox94
approved these changes
Jan 14, 2025
nachtjasmin
added a commit
that referenced
this pull request
Jan 15, 2025
Because the IP of the LoadBalancer Service is no longer exposed, it would get harder to setup DNS records. The current workaround would be: * Wait for the LoadBalancer to be reconciled without the annotation. * Write down the IP address to configure it elsewhere. * Annotate the service. This only works because there's a limitation of only one LoadBalancer per cluster at the moment. Yet, it can turn out to get problematic for any kind of automation. As such, we're exposing the hostname together with the IP. This also removes a lot of the code introduced in #355 and moves the code to its previous implementation. Closes: ANXKUBE-1280 See also: ANXKUBE-1251, #355
1 task
nachtjasmin
added a commit
that referenced
this pull request
Jan 15, 2025
Because the IP of the LoadBalancer Service is no longer exposed, it would get harder to setup DNS records. The current workaround would be: * Wait for the LoadBalancer to be reconciled without the annotation. * Write down the IP address to configure it elsewhere. * Annotate the service. This only works because there's a limitation of only one LoadBalancer per cluster at the moment. Yet, it can turn out to get problematic for any kind of automation. As such, we're exposing the hostname together with the IP. This also removes a lot of the code introduced in #355 and moves the code to its previous implementation. Closes: ANXKUBE-1280 See also: ANXKUBE-1251, #355
nachtjasmin
added a commit
that referenced
this pull request
Jan 15, 2025
Because the IP of the LoadBalancer Service is no longer exposed, it would get harder to setup DNS records. The current workaround would be: * Wait for the LoadBalancer to be reconciled without the annotation. * Write down the IP address to configure it elsewhere. * Annotate the service. This only works because there's a limitation of only one LoadBalancer per cluster at the moment. Yet, it can turn out to get problematic for any kind of automation. As such, we're exposing the hostname together with the IP. This also removes a lot of the code introduced in #355 and moves the code to its previous implementation. Closes: ANXKUBE-1280 See also: ANXKUBE-1251, #355
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We want to allow setting a hostname on the LB instead of IPs, this is useful when using proxy-protocol in LBaaS.
Checklist
Unreleasedsection in CHANGELOG.mdReferences
Community Note