use CRICheck in "kubeadm reset" #64332
Conversation
k8s-ci-robot
added
release-note-none
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: bart0sh Assign the PR to them by writing 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 |
k8s-ci-robot
added
area/kubeadm
sig/cluster-lifecycle
|
/ok-to-test |
k8s-ci-robot
removed
the
needs-ok-to-test
cmd/kubeadm/app/cmd/reset.go
Outdated
| @@ -33,6 +33,7 @@ import ( | |||
| kubeadmapiv1alpha2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha2" | |||
| "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/validation" | |||
| kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" | |||
| kubeadmdefaults "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1alpha1" | |||
There was a problem hiding this comment.
why not to use already imported kubeadmapiv1alpha2 ?
|
/retest |
bart0sh
force-pushed
the
PR0016-kubeadm-reset-check-CRI-correctly
branch
from
718f4ba
to
6cea24e
Compare
|
/test pull-kubernetes-e2e-gce-device-plugin-gpu |
2 similar comments
|
/test pull-kubernetes-e2e-gce-device-plugin-gpu |
|
/test pull-kubernetes-e2e-gce-device-plugin-gpu |
|
/retest |
"kubeadm reset" tries to use CRI if it finds crictl in PATH It's better to use the same approach as in preflight checks: use CRICheck only if cri socket is provided by user.
bart0sh
force-pushed
the
PR0016-kubeadm-reset-check-CRI-correctly
branch
from
6cea24e
to
ab3adbb
Compare
| if err == nil { | ||
| resetWithCrictl(execer, dockerCheck, criSocketPath, crictlPath) | ||
| func reset(execer utilsexec.Interface, dockerCheck preflight.Checker, criCheck preflight.Checker, criSocketPath string) { | ||
| if criSocketPath != kubeadmapiv1alpha2.DefaultCRISocket { |
There was a problem hiding this comment.
Having a lot of detection for docker vs. crictrl spread across source code is not good in my opinion.
It will be a lot better to refactor this code a bit, move all detection to helper function in preflight/checks.go and here keep only simple code resetWithDocker/resetWithCrictl based on output of helper function.
|
Closing. will address this different way. Thanks for the review! |
What this PR does / why we need it:
"kubeadm reset" tries to use CRI if it finds crictl in PATH
It's better to use the same approach as in preflight checks:
use CRICheck only if cri socket is provided by user.
Release note: