Repository navigation
[occm] fix bootstrap dns - #3259
jihyun-huh wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Hi @jihyun-huh. Thanks for your PR. I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with Tip We noticed you've done this a few times! Consider joining the org to skip this step and gain Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions 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-sigs/prow repository. |
|
/assign |
| # Use node DNS so OCCM can initialise nodes before cluster DNS is available. | ||
| # If cluster DNS is already available, use ClusterFirstWithHostNet with hostNetwork. | ||
| # Use ClusterFirst for cluster DNS when hostNetwork is disabled. | ||
| dnsPolicy: Default |
There was a problem hiding this comment.
I would change this value back to ClusterFirst, because:
- If someone is using this value as ClusterFirst, it wouldn't have much surprises when upgrading to a newer version;
ClusterFirstworks unlessHostNetworkis false. I would document what the implications are.
Don't know what others think.
There was a problem hiding this comment.
thaks for your review, and that makes sense.
With hostNetwork: true and ClusterFirst also restores node DNS in the same way as the Default, so I’m happy with this approach.
131aac3 to
83c8e12
Compare
winiciusallan
left a comment
There was a problem hiding this comment.
/lgtm
Can you please update the PR's description and the release-note to reflect ClusterFirst instead of Default?
|
@winiciusallan: changing LGTM is restricted to collaborators DetailsIn 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-sigs/prow repository. |
|
@winiciusallan thanks, i updated the description! |
I'm not an org member/reviewer, so I don't have that power. Sorry. We'd need someone else. |
|
oh i see, @winiciusallan but also thanks for your review and feedback! |
|
Hi @jihyun-huh ! The rendering tests you added, cover the intended policy combinations well. |
|
hi @IlanZuckerman, thanks for your comment! and also im thinking about adding bootstrap test for OCCM, especially using DNS server outside the Kubernetes cluster. if that so, i will mention you when i open the test PR |
Yes, that addresses my concern well. /lgtm |
|
@IlanZuckerman: changing LGTM is restricted to collaborators DetailsIn 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-sigs/prow repository. |
What this PR does / why we need it:
The OCCM Helm chart currently defaults to
hostNetwork: trueanddnsPolicy: ClusterFirstWithHostNet.and this makes OpenStackAPI hostname resolution depend on cluster DNS, including when the API hostname is external to the cluster.
so during it's bootstraping, CoreDNS may be waiting for OCCM to initialize nodes and remove the
node.cloudprovider.kubernetes.io/uninitializedtaint. if OCCM needs CoreDNS to resolve its openstack api endpoint, neither component can make progress.This PR is to change the chart default value and standalone manifests to
dnsPolicy: ClusterFirst, with the defaulthostNetwork: true, and this restores the node DNS behavior that the chart had before #3097. (the node resolver must be able to resolve openstack api endpoints independently of cluster DNS)With
hostNetwork: false, ClusterFirst uses cluster DNS by default.I found the same bootstrap previous failure was reported in #2611 and addressed by #2621.
also found Kubespray restored node DNS behavior in kubernetes-sigs/kubespray#11168 after encountering the same dependency cycle in 2024.
Which issue this PR fixes(if applicable):
Related to #2611 and #2621
fix #3258
Special notes for reviewers:
Release note: