✨ Add server name for the Machine InternalDNS#1715
✨ Add server name for the Machine InternalDNS#1715k8s-ci-robot merged 1 commit intokubernetes-sigs:mainfrom
Conversation
✅ Deploy Preview for kubernetes-sigs-cluster-api-openstack ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
Hi @MaysaMacedo. Thanks for your PR. I'm waiting for a kubernetes-sigs member to verify that this patch is reasonable to test. If it is, they should reply with 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/test-infra repository. |
|
/ok-to-test |
7193b9b to
344ab51
Compare
|
/retest |
mdbooth
left a comment
There was a problem hiding this comment.
@MaysaMacedo Expanded comment looks good, thanks! Can I ask you to just clarify that the specific behaviour described is OpenShift's?
@lentzi90 I've worked on this with @MaysaMacedo so I'd appreciate an independent look if you get a moment.
/approve
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: MaysaMacedo, mdbooth The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
lentzi90
left a comment
There was a problem hiding this comment.
Do I understand correctly that the idea is that the server name will in most cases be the same as the node name, especially if migrating from the in-tree provider?
Any plans to support other cases, or is this enough?
I'd expect node name to be the same as server name only in cases where it's been overridden in order to be compatible with the in-tree provider. Honestly, outside of OpenShift I'd expect this to be an unusual configuration now that the in-tree provider has been removed. I expect the normal case to be where nodename is the same as hostname, which this doesn't support. The plan is to add a second InternalDNS entry containing hostname, but only when the server supports microversion 2.90. This may end up being one of the first consumers of your microversion patch. So:
|
For a node to join the cluster, a CSR generated by kubelet with the node name must be approved. The approval happens if the value of the NodeInternalDNS entry on the Machine matches the node name. When using legacy cloud provider the value of that node name is the Server name. This commit ensures the nodes handled with legacy cloud provides get approved by adding the server name to the list of Machine addresses.
344ab51 to
de8aaa4
Compare
|
/test all |
|
/hold cancel We tested it and it worked. |
What this PR does / why we need it:
For a node to join the cluster, a CSR generated by kubelet with the node name must be approved. The approval happens if the value of the NodeInternalDNS entry on the Machine matches the node name. When using legacy cloud provider the value of that node name is the Server name. This commit ensures the nodes handled with legacy cloud provides get approved by adding the server name to the list of Machine addresses.
Fixes #1689
/hold