daemon: get the apiserver url from the kubelet's kubeconfig #2978
Conversation
|
/retest |
cmd/machine-config-daemon/start.go
Outdated
There was a problem hiding this comment.
One thing to be aware of is that setenv() is unsafe in the presence of threads that may be executing C code. https://internals.rust-lang.org/t/synchronized-ffi-access-to-posix-environment-variable-functions/15475 is a Rust thread on this; which links to e.g. https://sourceware.org/bugzilla/show_bug.cgi?id=15607
The Go runtime uses a middle ground trick of only calling the C setenv if cgo is in use which...I think will happen with us when linking to openssl at least.
So we should (at least eventually) fix the client API to allow overriding these things without setenv().
But...for now it's probably OK, I would think (hope) that we're not running any other active goroutines at this point.
|
This is a bit related to #2190 too right? /approve |
openshift-ci
bot
added
the
approved
Yes. For the same purpose, decouple the MCO from the network provider. |
kikisdeliveryservice
left a comment
kikisdeliveryservice
left a comment
There was a problem hiding this comment.
one comment
|
Also this sounds like a bug? Is there an existing bz? |
In ovnkube based cluster, the connectivity between MCD pods and kube-api-server relies on the openflow rules injected by ovnkube. If due to some reason, the ovnkube-node pod cannot start after the reboot of applying new MC. The MCD will not be able to reach the api-server. This PR let the kubeclient in MCD use the kube-api-server url of the node kubeconfig file instead. It eliminates the dependency on ovnkube-node pod from MCD.
No. I found this issue in a cluster with DPU (a smart NIC), which we plan to support in a future release. I suppose it might also happen on a regular cluster. |
|
/retest |
|
Looks good to me |
|
/retest |
|
I was looking at logs from an unrelated PR and saw e.g.: over there, and then realized that this PR will probably fix it, and indeed it seems to. We're no longer depending on the SDN pod for the MCD, which indeed seems like a huge reliability improvement. /lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: cgwalters, pliurh 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 |
|
/skip |
|
OK let's give this one more spin of the wheel of fortune, but if that fails I think we should override. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
4 similar comments
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
|
@pliurh: The following tests failed, say
Full PR test history. Your PR dashboard. 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. I understand the commands that are listed here. |
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
|
/retest-required Please review the full test history for this PR and help us cut down flakes. |
6 participants
In ovnkube based cluster, the connectivity between MCD pods and
kube-api-server relies on the openflow rules injected by ovnkube.
If due to some reason, the ovnkube-node pod cannot start after
the reboot of applying new MC. The MCD will not be able to reach
the api-server.
This PR lets the kubeclient in MCD use the kube-api-server url of
the node kubeconfig file instead. It eliminates the dependency
on ovnkube-node pod from MCD.