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

kubeadm: Don't error out on join with --cri-socket override #76505

Merged
merged 1 commit into from Apr 14, 2019

Conversation

@rosti
Copy link
Member

commented Apr 12, 2019

What type of PR is this?
/kind bug

What this PR does / why we need it:

In the case where newControlPlane is true we don't go through
getNodeRegistration() and initcfg.NodeRegistration.CRISocket is empty.
This forces DetectCRISocket() to be called later on, and if there is more than
one CRI installed on the system, it will error out, while asking for the user
to provide an override for the CRI socket. Even if the user provides an
override, the call to DetectCRISocket() can happen too early and thus ignore it
(while still erroring out).
However, if newControlPlane == true, initcfg.NodeRegistration is not used at
all and it's overwritten later on.
Thus it's necessary to supply some default value, that will avoid the call to
DetectCRISocket() and as initcfg.NodeRegistration is discarded, setting
whatever value here is harmless.

Which issue(s) this PR fixes:

Fixes kubernetes/kubeadm#1495

Special notes for your reviewer:

/cc @kubernetes/sig-cluster-lifecycle-pr-reviews
/area kubeadm
/priority critical-urgent
/assign @neolit123
/assign @fabriziopandini

Does this PR introduce a user-facing change?:

kubeadm: Fix a bug where if couple of CRIs are installed a user override of the CRI during join (via kubeadm join --cri-socket ...) is ignored and kubeadm bails out with an error
kubeadm: Don't error out on join with --cri-socket override
In the case where newControlPlane is true we don't go through
getNodeRegistration() and initcfg.NodeRegistration.CRISocket is empty.
This forces DetectCRISocket() to be called later on, and if there is more than
one CRI installed on the system, it will error out, while asking for the user
to provide an override for the CRI socket. Even if the user provides an
override, the call to DetectCRISocket() can happen too early and thus ignore it
(while still erroring out).
However, if newControlPlane == true, initcfg.NodeRegistration is not used at
all and it's overwritten later on.
Thus it's necessary to supply some default value, that will avoid the call to
DetectCRISocket() and as initcfg.NodeRegistration is discarded, setting
whatever value here is harmless.

Signed-off-by: Rostislav M. Georgiev <rostislavg@vmware.com>
@k8s-ci-robot

This comment has been minimized.

Copy link
Contributor

commented Apr 12, 2019

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rosti

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@neolit123
Copy link
Member

left a comment

thanks for debugging this @rosti
let me know if you want me to send the cherry pick for 1.14.

/lgtm
/hold

@rosti

This comment has been minimized.

Copy link
Member Author

commented Apr 12, 2019

@neolit123 we'll have to. I also checked 1.13, but the issue is not present in there.

@neolit123

This comment has been minimized.

Copy link
Member

commented Apr 12, 2019

sent cherry pick: #76510

@SataQiu

This comment has been minimized.

Copy link
Member

commented Apr 14, 2019

/lgtm
👍

@neolit123

This comment has been minimized.

Copy link
Member

commented Apr 14, 2019

/hold cancel

@k8s-ci-robot k8s-ci-robot merged commit 299aa5d into kubernetes:master Apr 14, 2019

18 checks passed

cla/linuxfoundation rosti authorized
Details
pull-kubernetes-bazel-build Job succeeded.
Details
pull-kubernetes-bazel-test Job succeeded.
Details
pull-kubernetes-conformance-image-test Skipped.
pull-kubernetes-cross Skipped.
pull-kubernetes-dependencies Job succeeded.
Details
pull-kubernetes-e2e-gce Job succeeded.
Details
pull-kubernetes-e2e-gce-100-performance Job succeeded.
Details
pull-kubernetes-e2e-gce-device-plugin-gpu Job succeeded.
Details
pull-kubernetes-godeps Skipped.
pull-kubernetes-integration Job succeeded.
Details
pull-kubernetes-kubemark-e2e-gce-big Job succeeded.
Details
pull-kubernetes-local-e2e Skipped.
pull-kubernetes-node-e2e Job succeeded.
Details
pull-kubernetes-typecheck Job succeeded.
Details
pull-kubernetes-verify Job succeeded.
Details
pull-publishing-bot-validate Skipped.
tide In merge pool.
Details

k8s-ci-robot added a commit that referenced this pull request May 1, 2019

Merge pull request #76510 from neolit123/automated-cherry-pick-of-#76…
…505-origin-release-1.14

Automated cherry pick of #76505: kubeadm: Don't error out on join with --cri-socket override
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.