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
Consider Hetzner network set by cluster spec #8872
Consider Hetzner network set by cluster spec #8872
Conversation
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
/cherry-pick release/v2.20 |
@embik: once the present PR merges, I will cherry-pick it on top of release/v2.20 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/cherry-pick release/v2.19 |
@embik: once the present PR merges, I will cherry-pick it on top of release/v2.19 in a new PR and assign it to you. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Signed-off-by: Marvin Beckers <marvin@kubermatic.com>
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.
Oh thank god someone fixed this. I experienced some weirdness myself but never got around to investigating where I fucked up.
/approve
LGTM label has been added. Git tree hash: 49c031aecb701e8cb2508b11ada0c219a0aeac68
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: embik, xrstf The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
1 similar comment
/retest |
/retest Review the full test history Silence the bot with an |
@embik: new pull request created: #8891 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
@embik: new pull request created: #8892 In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
What does this PR do / Why do we need it:
#6588 introduced a
network
field for Hetzner on various levels (datacenter and cluster spec). The description suggests that the field on the cluster spec takes precedence over the datacenter field.Unfortunately, it looks like the implementation back then did not consider the field on the cluster spec level for the generated
MachineDeployments
and only applied the datacenter-level value.This can result in
MachineDeployments
failing to deploy if the datacenter value is not given or set to an empty string, or a mismatch from the network that is defined in CCM settings (as that does consider the cluster spec value):kubermatic/pkg/resources/cloudcontroller/hetzner.go
Lines 79 to 82 in e234573
Does this PR close any issues?:
Fixes #8868
Special notes for your reviewer:
Documentation:
Does this PR introduce a user-facing change?:
Signed-off-by: Marvin Beckers marvin@kubermatic.com