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: incorrectly defaulting advertise address #153

Merged
merged 1 commit into from
Jun 30, 2023
Merged

fix: incorrectly defaulting advertise address #153

merged 1 commit into from
Jun 30, 2023

Conversation

richardcase
Copy link
Contributor

What this PR does / why we need it:

With the recent change to introduce new registration methods we started to default the advertiseAddress to the "registration address" if the registration method of address was used. This caused the kube-api server to be startedw tith that address. If you then used a VIP/LB solution like kube-vip or metalb that runs within cluster it caused pods to not start start as there is a chicken and egg scenario.

This change removes that defaulting and also adds a sample that uses kube-vip.

Which issue(s) this PR fixes (optional, in fixes #<issue number>(, fixes #<issue_number>, ...) format, will close the issue(s) when PR gets merged):
Fixes #

Special notes for your reviewer:

Checklist:

  • squashed commits into logical changes
  • includes documentation
  • adds unit tests
  • adds or updates e2e tests

@richardcase richardcase added the kind/bug Something isn't working label Jun 30, 2023
@richardcase
Copy link
Contributor Author

@tmmorin
Copy link

tmmorin commented Jun 30, 2023

This looks very good!

We'll test this out as soon as this ships.

With the recent change to introduce new registration methods we started
to default the `advertiseAddress` to the "registration address" if the
registration method of `address` was used. This caused the kube-api
server to be startedw tith that address. If you then used a VIP/LB
solution like kube-vip or metalb that runs within cluster it caused
pods to not start start as there is a chicken and egg scenario.

This change removes that defaulting and also adds a sample that uses
kube-vip.

Signed-off-by: Richard Case <[email protected]>
@richardcase richardcase merged commit af4782b into rancher:main Jun 30, 2023
6 checks passed
@richardcase
Copy link
Contributor Author

NOTE: merged directly instead of waiting for reviews so that a release could be done asap.

Copy link
Contributor

@furkatgofurov7 furkatgofurov7 left a comment

Choose a reason for hiding this comment

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

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants