From 936e12c9307d144183f9c3bc21cc39126f1b65fe Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Tue, 4 Jan 2022 22:28:53 +0200 Subject: [PATCH 1/3] kubeadm: do not special case Docker as a container runtime crictl already works with the current state of dockershim. Using the docker CLI is not required and the DockerRuntime can be removed from kubeadm. This means that crictl can connect at the dockershim (or cri-dockerd) socket and be used to list containers, pull images, remove containers, and all actions that the kubelet can otherwise perform with the socket. Ensure that crictl is now required for all supported container runtimes in checks.go. In the help text in waitcontrolplane.go show only the crictl example. Remove the check for the docker service from checks.go. Remove the DockerValidor check from checks.go. These two checks were special casing Docker as CR and compensating for the lack of the same checks in dockershim. With the extraction of dockershim to cri-dockerd, ideally cri-dockerd should perform the required checks whether it can support a given Docker config / version running on a host. --- .../app/cmd/phases/init/waitcontrolplane.go | 20 ++--- cmd/kubeadm/app/preflight/checks.go | 20 +---- cmd/kubeadm/app/util/runtime/runtime.go | 85 ++----------------- cmd/kubeadm/app/util/runtime/runtime_test.go | 23 ++--- 4 files changed, 23 insertions(+), 125 deletions(-) diff --git a/cmd/kubeadm/app/cmd/phases/init/waitcontrolplane.go b/cmd/kubeadm/app/cmd/phases/init/waitcontrolplane.go index e9f2b9ee1ca..a0af2810b36 100644 --- a/cmd/kubeadm/app/cmd/phases/init/waitcontrolplane.go +++ b/cmd/kubeadm/app/cmd/phases/init/waitcontrolplane.go @@ -29,7 +29,6 @@ import ( "k8s.io/klog/v2" "k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow" - kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants" "k8s.io/kubernetes/cmd/kubeadm/app/util/apiclient" dryrunutil "k8s.io/kubernetes/cmd/kubeadm/app/util/dryrun" ) @@ -49,17 +48,10 @@ var ( Additionally, a control plane component may have crashed or exited when started by the container runtime. To troubleshoot, list all containers using your preferred container runtimes CLI. -{{ if .IsDocker }} - Here is one example how you may list all Kubernetes containers running in docker: - - 'docker ps -a | grep kube | grep -v pause' - Once you have found the failing container, you can inspect its logs with: - - 'docker logs CONTAINERID' -{{ else }} - Here is one example how you may list all Kubernetes containers running in cri-o/containerd using crictl: + Here is one example how you may list all running Kubernetes containers by using crictl: - 'crictl --runtime-endpoint {{ .Socket }} ps -a | grep kube | grep -v pause' Once you have found the failing container, you can inspect its logs with: - 'crictl --runtime-endpoint {{ .Socket }} logs CONTAINERID' -{{ end }} `))) ) @@ -105,13 +97,11 @@ func runWaitControlPlanePhase(c workflow.RunData) error { if err := waiter.WaitForKubeletAndFunc(waiter.WaitForAPI); err != nil { context := struct { - Error string - Socket string - IsDocker bool + Error string + Socket string }{ - Error: fmt.Sprintf("%v", err), - Socket: data.Cfg().NodeRegistration.CRISocket, - IsDocker: data.Cfg().NodeRegistration.CRISocket == kubeadmconstants.DefaultDockerCRISocket, + Error: fmt.Sprintf("%v", err), + Socket: data.Cfg().NodeRegistration.CRISocket, } kubeletFailTempl.Execute(data.OutputWriter(), context) diff --git a/cmd/kubeadm/app/preflight/checks.go b/cmd/kubeadm/app/preflight/checks.go index dc4ac9f17b3..58ae981c8bc 100644 --- a/cmd/kubeadm/app/preflight/checks.go +++ b/cmd/kubeadm/app/preflight/checks.go @@ -507,9 +507,7 @@ func (subnet HTTPProxyCIDRCheck) Check() (warnings, errorList []error) { } // SystemVerificationCheck defines struct used for running the system verification node check in test/e2e_node/system -type SystemVerificationCheck struct { - IsDocker bool -} +type SystemVerificationCheck struct{} // Name will return SystemVerification as name for SystemVerificationCheck func (SystemVerificationCheck) Name() string { @@ -530,11 +528,6 @@ func (sysver SystemVerificationCheck) Check() (warnings, errorList []error) { var validators = []system.Validator{ &system.KernelValidator{Reporter: reporter}} - // run the docker validator only with docker runtime - if sysver.IsDocker { - validators = append(validators, &system.DockerValidator{Reporter: reporter}) - } - if runtime.GOOS == "linux" { //add linux validators validators = append(validators, @@ -1025,26 +1018,19 @@ func RunJoinNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.JoinConfigura // kubeadm init and join commands func addCommonChecks(execer utilsexec.Interface, k8sVersion string, nodeReg *kubeadmapi.NodeRegistrationOptions, checks []Checker) []Checker { containerRuntime, err := utilruntime.NewContainerRuntime(execer, nodeReg.CRISocket) - isDocker := false if err != nil { fmt.Printf("[preflight] WARNING: Couldn't create the interface used for talking to the container runtime: %v\n", err) } else { checks = append(checks, ContainerRuntimeCheck{runtime: containerRuntime}) - if containerRuntime.IsDocker() { - isDocker = true - checks = append(checks, ServiceCheck{Service: "docker", CheckIfActive: true}) - } } // non-windows checks if runtime.GOOS == "linux" { - if !isDocker { - checks = append(checks, InPathCheck{executable: "crictl", mandatory: true, exec: execer}) - } checks = append(checks, FileContentCheck{Path: bridgenf, Content: []byte{'1'}}, FileContentCheck{Path: ipv4Forward, Content: []byte{'1'}}, SwapCheck{}, + InPathCheck{executable: "crictl", mandatory: true, exec: execer}, InPathCheck{executable: "conntrack", mandatory: true, exec: execer}, InPathCheck{executable: "ip", mandatory: true, exec: execer}, InPathCheck{executable: "iptables", mandatory: true, exec: execer}, @@ -1057,7 +1043,7 @@ func addCommonChecks(execer utilsexec.Interface, k8sVersion string, nodeReg *kub InPathCheck{executable: "touch", mandatory: false, exec: execer}) } checks = append(checks, - SystemVerificationCheck{IsDocker: isDocker}, + SystemVerificationCheck{}, HostnameCheck{nodeName: nodeReg.Name}, KubeletVersionCheck{KubernetesVersion: k8sVersion, exec: execer}, ServiceCheck{Service: "kubelet", CheckIfActive: false}, diff --git a/cmd/kubeadm/app/util/runtime/runtime.go b/cmd/kubeadm/app/util/runtime/runtime.go index 11cd8bbc1fe..08821591908 100644 --- a/cmd/kubeadm/app/util/runtime/runtime.go +++ b/cmd/kubeadm/app/util/runtime/runtime.go @@ -29,7 +29,7 @@ import ( // ContainerRuntime is an interface for working with container runtimes type ContainerRuntime interface { - IsDocker() bool + Socket() string IsRunning() error ListKubeContainers() ([]string, error) RemoveContainers(containers []string) error @@ -43,39 +43,19 @@ type CRIRuntime struct { criSocket string } -// DockerRuntime is a struct that interfaces with the Docker daemon -type DockerRuntime struct { - exec utilsexec.Interface -} - // NewContainerRuntime sets up and returns a ContainerRuntime struct func NewContainerRuntime(execer utilsexec.Interface, criSocket string) (ContainerRuntime, error) { - var toolName string - var runtime ContainerRuntime - - if criSocket != constants.DefaultDockerCRISocket { - toolName = "crictl" - runtime = &CRIRuntime{execer, criSocket} - } else { - toolName = "docker" - runtime = &DockerRuntime{execer} - } - + toolName := "crictl" + runtime := &CRIRuntime{execer, criSocket} if _, err := execer.LookPath(toolName); err != nil { - return nil, errors.Wrapf(err, "%s is required for container runtime", toolName) + return nil, errors.Wrapf(err, "%s is required by the container runtime", toolName) } - return runtime, nil } -// IsDocker returns true if the runtime is docker -func (runtime *CRIRuntime) IsDocker() bool { - return false -} - -// IsDocker returns true if the runtime is docker -func (runtime *DockerRuntime) IsDocker() bool { - return true +// Socket returns the CRI socket endpoint +func (runtime *CRIRuntime) Socket() string { + return runtime.criSocket } // IsRunning checks if runtime is running @@ -86,14 +66,6 @@ func (runtime *CRIRuntime) IsRunning() error { return nil } -// IsRunning checks if runtime is running -func (runtime *DockerRuntime) IsRunning() error { - if out, err := runtime.exec.Command("docker", "info").CombinedOutput(); err != nil { - return errors.Wrapf(err, "container runtime is not running: output: %s, error", string(out)) - } - return nil -} - // ListKubeContainers lists running k8s CRI pods func (runtime *CRIRuntime) ListKubeContainers() ([]string, error) { out, err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "pods", "-q").CombinedOutput() @@ -105,12 +77,6 @@ func (runtime *CRIRuntime) ListKubeContainers() ([]string, error) { return pods, nil } -// ListKubeContainers lists running k8s containers -func (runtime *DockerRuntime) ListKubeContainers() ([]string, error) { - output, err := runtime.exec.Command("docker", "ps", "-a", "--filter", "name=k8s_", "-q").CombinedOutput() - return strings.Fields(string(output)), err -} - // RemoveContainers removes running k8s pods func (runtime *CRIRuntime) RemoveContainers(containers []string) error { errs := []error{} @@ -129,24 +95,6 @@ func (runtime *CRIRuntime) RemoveContainers(containers []string) error { return errorsutil.NewAggregate(errs) } -// RemoveContainers removes running containers -func (runtime *DockerRuntime) RemoveContainers(containers []string) error { - errs := []error{} - for _, container := range containers { - out, err := runtime.exec.Command("docker", "stop", container).CombinedOutput() - if err != nil { - // don't stop on errors, try to remove as many containers as possible - errs = append(errs, errors.Wrapf(err, "failed to stop running container %s: output: %s, error", container, string(out))) - } else { - out, err = runtime.exec.Command("docker", "rm", "--volumes", container).CombinedOutput() - if err != nil { - errs = append(errs, errors.Wrapf(err, "failed to remove running container %s: output: %s, error", container, string(out))) - } - } - } - return errorsutil.NewAggregate(errs) -} - // PullImage pulls the image func (runtime *CRIRuntime) PullImage(image string) error { var err error @@ -160,31 +108,12 @@ func (runtime *CRIRuntime) PullImage(image string) error { return errors.Wrapf(err, "output: %s, error", out) } -// PullImage pulls the image -func (runtime *DockerRuntime) PullImage(image string) error { - var err error - var out []byte - for i := 0; i < constants.PullImageRetry; i++ { - out, err = runtime.exec.Command("docker", "pull", image).CombinedOutput() - if err == nil { - return nil - } - } - return errors.Wrapf(err, "output: %s, error", out) -} - // ImageExists checks to see if the image exists on the system func (runtime *CRIRuntime) ImageExists(image string) (bool, error) { err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "inspecti", image).Run() return err == nil, nil } -// ImageExists checks to see if the image exists on the system -func (runtime *DockerRuntime) ImageExists(image string) (bool, error) { - err := runtime.exec.Command("docker", "inspect", image).Run() - return err == nil, nil -} - // detectCRISocketImpl is separated out only for test purposes, DON'T call it directly, use DetectCRISocket instead func detectCRISocketImpl(isSocket func(string) bool) (string, error) { foundCRISockets := []string{} diff --git a/cmd/kubeadm/app/util/runtime/runtime_test.go b/cmd/kubeadm/app/util/runtime/runtime_test.go index 2995e82e8a1..242505ef157 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_test.go +++ b/cmd/kubeadm/app/util/runtime/runtime_test.go @@ -40,32 +40,25 @@ func TestNewContainerRuntime(t *testing.T) { LookPathFunc: func(cmd string) (string, error) { return "", errors.Errorf("%s not found", cmd) }, } cases := []struct { - name string - execer fakeexec.FakeExec - criSocket string - isDocker bool - isError bool + name string + execer fakeexec.FakeExec + isError bool }{ - {"valid: default cri socket", execLookPathOK, constants.DefaultDockerCRISocket, true, false}, - {"valid: cri-o socket url", execLookPathOK, "unix:///var/run/crio/crio.sock", false, false}, - {"invalid: no crictl", execLookPathErr, "unix:///var/run/crio/crio.sock", false, true}, + {"valid: crictl present", execLookPathOK, false}, + {"invalid: no crictl", execLookPathErr, true}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - runtime, err := NewContainerRuntime(&tc.execer, tc.criSocket) + _, err := NewContainerRuntime(&tc.execer, "unix:///some/socket.sock") if err != nil { if !tc.isError { - t.Fatalf("unexpected NewContainerRuntime error. criSocket: %s, error: %v", tc.criSocket, err) + t.Fatalf("unexpected NewContainerRuntime error. error: %v", err) } return // expected error occurs, impossible to test runtime further } if tc.isError && err == nil { - t.Fatalf("unexpected NewContainerRuntime success. criSocket: %s", tc.criSocket) - } - isDocker := runtime.IsDocker() - if tc.isDocker != isDocker { - t.Fatalf("unexpected isDocker() result %v for the criSocket %s", isDocker, tc.criSocket) + t.Fatal("unexpected NewContainerRuntime success") } }) } From ea2c948799a0031720aee21f41afdd680accf146 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Tue, 4 Jan 2022 23:15:12 +0200 Subject: [PATCH 2/3] kubeadm: change the default CRI socket to containerd Change the default container runtime CRI socket endpoint to the one of containerd. Previously it was the one for Docker - Rename constants.DefaultDockerCRISocket to DefaultCRISocket - Make the constants files include the endpoints for all supported container runtimes for Unix/Windows. - Update unit tests related to docker runtime testing. - In kubelet/flags.go hardcode the legacy docker socket as a check to allow kubeadm 1.24 to run against kubelet 1.23 if the user explicitly sets the criSocket field to "npipe:////./pipe/dockershim" on Windows or "unix:///var/run/dockershim.sock" on Linux. --- cmd/kubeadm/app/apis/kubeadm/v1beta2/doc.go | 2 +- cmd/kubeadm/app/apis/kubeadm/v1beta3/doc.go | 2 +- .../kubeadm/validation/validation_test.go | 8 ++--- cmd/kubeadm/app/cmd/certs_test.go | 2 +- cmd/kubeadm/app/cmd/config.go | 2 +- cmd/kubeadm/app/cmd/config_test.go | 4 +-- .../app/cmd/phases/reset/cleanupnode.go | 2 ++ cmd/kubeadm/app/constants/constants_unix.go | 11 +++++-- .../app/constants/constants_windows.go | 12 +++++-- cmd/kubeadm/app/phases/kubelet/flags.go | 9 +++++- .../app/phases/patchnode/patchnode_test.go | 2 +- cmd/kubeadm/app/preflight/checks_test.go | 4 +-- .../app/util/config/initconfiguration.go | 2 +- cmd/kubeadm/app/util/runtime/runtime.go | 4 +-- cmd/kubeadm/app/util/runtime/runtime_test.go | 31 +++---------------- 15 files changed, 50 insertions(+), 47 deletions(-) diff --git a/cmd/kubeadm/app/apis/kubeadm/v1beta2/doc.go b/cmd/kubeadm/app/apis/kubeadm/v1beta2/doc.go index be5ea5bbf9b..b66059cfb1c 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1beta2/doc.go +++ b/cmd/kubeadm/app/apis/kubeadm/v1beta2/doc.go @@ -169,7 +169,7 @@ limitations under the License. // - system:bootstrappers:kubeadm:default-node-token // nodeRegistration: // name: "ec2-10-100-0-1" -// criSocket: "unix:///var/run/dockershim.sock" +// criSocket: "unix:///var/run/containerd/containerd.sock" // taints: // - key: "kubeadmNode" // value: "master" diff --git a/cmd/kubeadm/app/apis/kubeadm/v1beta3/doc.go b/cmd/kubeadm/app/apis/kubeadm/v1beta3/doc.go index 37a861885c9..4ba87609d38 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1beta3/doc.go +++ b/cmd/kubeadm/app/apis/kubeadm/v1beta3/doc.go @@ -173,7 +173,7 @@ limitations under the License. // - system:bootstrappers:kubeadm:default-node-token // nodeRegistration: // name: "ec2-10-100-0-1" -// criSocket: "unix:///var/run/dockershim.sock" +// criSocket: "unix:///var/run/containerd/containerd.sock" // taints: // - key: "kubeadmNode" // value: "master" diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go index 9a87bcb721d..db8a22c5637 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go +++ b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go @@ -579,7 +579,7 @@ func TestValidateJoinConfiguration(t *testing.T) { }, NodeRegistration: kubeadmapi.NodeRegistrationOptions{ Name: "aaa", - CRISocket: "unix:///var/run/dockershim.sock", + CRISocket: "unix:///var/run/containerd/containerd.sock", }, }, true}, {&kubeadmapi.JoinConfiguration{ // Pass with JoinControlPlane @@ -594,7 +594,7 @@ func TestValidateJoinConfiguration(t *testing.T) { }, NodeRegistration: kubeadmapi.NodeRegistrationOptions{ Name: "aaa", - CRISocket: "unix:///var/run/dockershim.sock", + CRISocket: "unix:///var/run/containerd/containerd.sock", }, ControlPlane: &kubeadmapi.JoinControlPlane{ LocalAPIEndpoint: kubeadmapi.APIEndpoint{ @@ -615,7 +615,7 @@ func TestValidateJoinConfiguration(t *testing.T) { }, NodeRegistration: kubeadmapi.NodeRegistrationOptions{ Name: "aaa", - CRISocket: "unix:///var/run/dockershim.sock", + CRISocket: "unix:///var/run/containerd/containerd.sock", }, ControlPlane: &kubeadmapi.JoinControlPlane{ LocalAPIEndpoint: kubeadmapi.APIEndpoint{ @@ -636,7 +636,7 @@ func TestValidateJoinConfiguration(t *testing.T) { }, NodeRegistration: kubeadmapi.NodeRegistrationOptions{ Name: "aaa", - CRISocket: "/var/run/dockershim.sock", + CRISocket: "unix:///var/run/containerd/containerd.sock", }, ControlPlane: &kubeadmapi.JoinControlPlane{ LocalAPIEndpoint: kubeadmapi.APIEndpoint{ diff --git a/cmd/kubeadm/app/cmd/certs_test.go b/cmd/kubeadm/app/cmd/certs_test.go index 18ec56694d2..8efa58c7456 100644 --- a/cmd/kubeadm/app/cmd/certs_test.go +++ b/cmd/kubeadm/app/cmd/certs_test.go @@ -359,7 +359,7 @@ kind: InitConfiguration localAPIEndpoint: advertiseAddress: 192.0.2.1 nodeRegistration: - criSocket: /path/to/dockershim.sock + criSocket: "unix:///var/run/containerd/containerd.sock" --- apiVersion: %[1]s kind: ClusterConfiguration diff --git a/cmd/kubeadm/app/cmd/config.go b/cmd/kubeadm/app/cmd/config.go index 6c875a8bb37..db0488c9990 100644 --- a/cmd/kubeadm/app/cmd/config.go +++ b/cmd/kubeadm/app/cmd/config.go @@ -211,7 +211,7 @@ func getDefaultNodeConfigBytes() ([]byte, error) { }, }, NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{ - CRISocket: constants.DefaultDockerCRISocket, // avoid CRI detection + CRISocket: constants.DefaultCRISocket, // avoid CRI detection }, }) if err != nil { diff --git a/cmd/kubeadm/app/cmd/config_test.go b/cmd/kubeadm/app/cmd/config_test.go index d7d88878ccd..27dd17b75c3 100644 --- a/cmd/kubeadm/app/cmd/config_test.go +++ b/cmd/kubeadm/app/cmd/config_test.go @@ -363,10 +363,10 @@ func TestImagesPull(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, - LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/docker", nil }, + LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/crictl", nil }, } - containerRuntime, err := utilruntime.NewContainerRuntime(&fexec, constants.DefaultDockerCRISocket) + containerRuntime, err := utilruntime.NewContainerRuntime(&fexec, constants.DefaultCRISocket) if err != nil { t.Errorf("unexpected NewContainerRuntime error: %v", err) } diff --git a/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go b/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go index a387ff7cac6..866689715d3 100644 --- a/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go +++ b/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go @@ -84,6 +84,8 @@ func runCleanupNode(c workflow.RunData) error { klog.Warningf("[reset] Failed to remove containers: %v\n", err) } + // TODO: remove the dockershim directory cleanup in 1.25 + // https://github.com/kubernetes/kubeadm/issues/2626 r.AddDirsToClean("/var/lib/dockershim", "/var/run/kubernetes", "/var/lib/cni") // Remove contents from the config and pki directories diff --git a/cmd/kubeadm/app/constants/constants_unix.go b/cmd/kubeadm/app/constants/constants_unix.go index 3d2ecec90f3..0943d0610c0 100644 --- a/cmd/kubeadm/app/constants/constants_unix.go +++ b/cmd/kubeadm/app/constants/constants_unix.go @@ -20,6 +20,13 @@ limitations under the License. package constants const ( - // DefaultDockerCRISocket defines the default Docker CRI socket - DefaultDockerCRISocket = "unix:///var/run/dockershim.sock" + // CRISocketContainerd is the containerd CRI endpoint + CRISocketContainerd = "unix:///var/run/containerd/containerd.sock" + // CRISocketCRIO is the cri-o CRI endpoint + CRISocketCRIO = "unix:///var/run/crio/crio.sock" + // CRISocketDocker is the cri-dockerd CRI endpoint + CRISocketDocker = "unix:///var/run/cri-dockerd.sock" + + // DefaultCRISocket defines the default CRI socket + DefaultCRISocket = CRISocketContainerd ) diff --git a/cmd/kubeadm/app/constants/constants_windows.go b/cmd/kubeadm/app/constants/constants_windows.go index 1a44a82723d..48e870023bc 100644 --- a/cmd/kubeadm/app/constants/constants_windows.go +++ b/cmd/kubeadm/app/constants/constants_windows.go @@ -20,6 +20,14 @@ limitations under the License. package constants const ( - // DefaultDockerCRISocket defines the default Docker CRI socket - DefaultDockerCRISocket = "npipe:////./pipe/docker_engine" + // CRISocketContainerd is the containerd CRI endpoint + CRISocketContainerd = "npipe:////./pipe/containerd-containerd" + // CRISocketCRIO is the cri-o CRI endpoint + // NOTE: this is a placeholder as CRI-O does not support Windows + CRISocketCRIO = "npipe:////./pipe/cri-o" + // CRISocketDocker is the cri-dockerd CRI endpoint + CRISocketDocker = "npipe:////./pipe/cri-dockerd" + + // DefaultCRISocket defines the default CRI socket + DefaultCRISocket = CRISocketContainerd ) diff --git a/cmd/kubeadm/app/phases/kubelet/flags.go b/cmd/kubeadm/app/phases/kubelet/flags.go index de24a57ad3f..6b420869cd7 100644 --- a/cmd/kubeadm/app/phases/kubelet/flags.go +++ b/cmd/kubeadm/app/phases/kubelet/flags.go @@ -21,6 +21,7 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "strings" "github.com/pkg/errors" @@ -102,7 +103,13 @@ func buildKubeletArgMapCommon(opts kubeletFlagsOpts) map[string]string { // Once that happens only the "remote" branch option should be left. // TODO: https://github.com/kubernetes/kubeadm/issues/2626 hasDockershim := opts.kubeletVersion.Major() == 1 && opts.kubeletVersion.Minor() < 24 - if opts.nodeRegOpts.CRISocket == constants.DefaultDockerCRISocket && hasDockershim { + var dockerSocket string + if runtime.GOOS == "windows" { + dockerSocket = "npipe:////./pipe/dockershim" + } else { + dockerSocket = "unix:///var/run/dockershim.sock" + } + if opts.nodeRegOpts.CRISocket == dockerSocket && hasDockershim { kubeletFlags["network-plugin"] = "cni" } else { kubeletFlags["container-runtime"] = "remote" diff --git a/cmd/kubeadm/app/phases/patchnode/patchnode_test.go b/cmd/kubeadm/app/phases/patchnode/patchnode_test.go index f0a5656e3ad..35dad449e3c 100644 --- a/cmd/kubeadm/app/phases/patchnode/patchnode_test.go +++ b/cmd/kubeadm/app/phases/patchnode/patchnode_test.go @@ -52,7 +52,7 @@ func TestAnnotateCRISocket(t *testing.T) { }, { name: "CRI-socket annotation needs to be updated", - currentCRISocketAnnotation: "unix:///var/run/dockershim.sock", + currentCRISocketAnnotation: "unix:///foo/bar", newCRISocketAnnotation: "unix:///run/containerd/containerd.sock", expectedPatch: `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"unix:///run/containerd/containerd.sock"}}}`, }, diff --git a/cmd/kubeadm/app/preflight/checks_test.go b/cmd/kubeadm/app/preflight/checks_test.go index 26ef8b5d004..f672d713e68 100644 --- a/cmd/kubeadm/app/preflight/checks_test.go +++ b/cmd/kubeadm/app/preflight/checks_test.go @@ -917,10 +917,10 @@ func TestImagePullCheck(t *testing.T) { func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) }, }, - LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/docker", nil }, + LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/crictl", nil }, } - containerRuntime, err := utilruntime.NewContainerRuntime(&fexec, constants.DefaultDockerCRISocket) + containerRuntime, err := utilruntime.NewContainerRuntime(&fexec, constants.DefaultCRISocket) if err != nil { t.Errorf("unexpected NewContainerRuntime error: %v", err) } diff --git a/cmd/kubeadm/app/util/config/initconfiguration.go b/cmd/kubeadm/app/util/config/initconfiguration.go index 2c691491523..8129ed4aed1 100644 --- a/cmd/kubeadm/app/util/config/initconfiguration.go +++ b/cmd/kubeadm/app/util/config/initconfiguration.go @@ -196,7 +196,7 @@ func DefaultedStaticInitConfiguration() (*kubeadmapi.InitConfiguration, error) { LocalAPIEndpoint: kubeadmapiv1.APIEndpoint{AdvertiseAddress: "1.2.3.4"}, BootstrapTokens: []bootstraptokenv1.BootstrapToken{PlaceholderToken}, NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{ - CRISocket: kubeadmconstants.DefaultDockerCRISocket, // avoid CRI detection + CRISocket: kubeadmconstants.DefaultCRISocket, // avoid CRI detection Name: "node", }, } diff --git a/cmd/kubeadm/app/util/runtime/runtime.go b/cmd/kubeadm/app/util/runtime/runtime.go index 08821591908..74007ca58f6 100644 --- a/cmd/kubeadm/app/util/runtime/runtime.go +++ b/cmd/kubeadm/app/util/runtime/runtime.go @@ -124,7 +124,7 @@ func detectCRISocketImpl(isSocket func(string) bool) (string, error) { if isSocket(dockerSocket) { // the path in dockerSocket is not CRI compatible, hence we should replace it with a CRI compatible socket - foundCRISockets = append(foundCRISockets, constants.DefaultDockerCRISocket) + foundCRISockets = append(foundCRISockets, constants.DefaultCRISocket) } else if isSocket(containerdSocket) { // Docker 18.09 gets bundled together with containerd, thus having both dockerSocket and containerdSocket present. // For compatibility reasons, we use the containerd socket only if Docker is not detected. @@ -140,7 +140,7 @@ func detectCRISocketImpl(isSocket func(string) bool) (string, error) { switch len(foundCRISockets) { case 0: // Fall back to Docker if no CRI is detected, we can error out later on if we need it - return constants.DefaultDockerCRISocket, nil + return constants.DefaultCRISocket, nil case 1: // Precisely one CRI found, use that return foundCRISockets[0], nil diff --git a/cmd/kubeadm/app/util/runtime/runtime_test.go b/cmd/kubeadm/app/util/runtime/runtime_test.go index 242505ef157..8e36cf17ac2 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_test.go +++ b/cmd/kubeadm/app/util/runtime/runtime_test.go @@ -89,11 +89,6 @@ func TestIsRunning(t *testing.T) { LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/crictl", nil }, } - dockerExecer := fakeexec.FakeExec{ - CommandScript: genFakeActions(&fcmd, len(fcmd.CombinedOutputScript)), - LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/docker", nil }, - } - cases := []struct { name string criSocket string @@ -102,8 +97,6 @@ func TestIsRunning(t *testing.T) { }{ {"valid: CRI-O is running", "unix:///var/run/crio/crio.sock", criExecer, false}, {"invalid: CRI-O is not running", "unix:///var/run/crio/crio.sock", criExecer, true}, - {"valid: docker is running", constants.DefaultDockerCRISocket, dockerExecer, false}, - {"invalid: docker is not running", constants.DefaultDockerCRISocket, dockerExecer, true}, } for _, tc := range cases { @@ -143,7 +136,6 @@ func TestListKubeContainers(t *testing.T) { }{ {"valid: list containers using CRI socket url", "unix:///var/run/crio/crio.sock", false}, {"invalid: list containers using CRI socket url", "unix:///var/run/crio/crio.sock", true}, - {"valid: list containers using docker", constants.DefaultDockerCRISocket, false}, } for _, tc := range cases { @@ -197,9 +189,6 @@ func TestRemoveContainers(t *testing.T) { {"valid: remove containers using CRI", "unix:///var/run/crio/crio.sock", []string{"k8s_p1", "k8s_p2", "k8s_p3"}, false}, // Test case 1 {"invalid: CRI rmp failure", "unix:///var/run/crio/crio.sock", []string{"k8s_p1", "k8s_p2", "k8s_p3"}, true}, {"invalid: CRI stopp failure", "unix:///var/run/crio/crio.sock", []string{"k8s_p1", "k8s_p2", "k8s_p3"}, true}, - {"valid: remove containers using docker", constants.DefaultDockerCRISocket, []string{"k8s_c1", "k8s_c2", "k8s_c3"}, false}, - {"invalid: docker rm failure", constants.DefaultDockerCRISocket, []string{"k8s_c1", "k8s_c2", "k8s_c3"}, true}, - {"invalid: docker stop failure", constants.DefaultDockerCRISocket, []string{"k8s_c1", "k8s_c2", "k8s_c3"}, true}, } for _, tc := range cases { @@ -252,8 +241,6 @@ func TestPullImage(t *testing.T) { }{ {"valid: pull image using CRI", "unix:///var/run/crio/crio.sock", "image1", false}, {"invalid: CRI pull error", "unix:///var/run/crio/crio.sock", "image2", true}, - {"valid: pull image using docker", constants.DefaultDockerCRISocket, "image1", false}, - {"invalid: docker pull error", constants.DefaultDockerCRISocket, "image2", true}, } for _, tc := range cases { @@ -295,9 +282,7 @@ func TestImageExists(t *testing.T) { result bool }{ {"valid: test if image exists using CRI", "unix:///var/run/crio/crio.sock", "image1", false}, - {"invalid: CRI inspecti failure", "unix:///var/run/crio/crio.sock", "image2", true}, - {"valid: test if image exists using docker", constants.DefaultDockerCRISocket, "image1", false}, - {"invalid: docker inspect failure", constants.DefaultDockerCRISocket, "image2", true}, + {"invalid: CRI inspect failure", "unix:///var/run/crio/crio.sock", "image2", true}, } for _, tc := range cases { @@ -388,10 +373,10 @@ func TestDetectCRISocketImpl(t *testing.T) { expectedSocket string }{ { - name: "No existing sockets, use Docker", + name: "No existing sockets, use default", existingSockets: []string{}, expectedError: false, - expectedSocket: constants.DefaultDockerCRISocket, + expectedSocket: constants.DefaultCRISocket, }, { name: "One valid CRI socket leads to success", @@ -399,12 +384,6 @@ func TestDetectCRISocketImpl(t *testing.T) { expectedError: false, expectedSocket: "unix:///var/run/crio/crio.sock", }, - { - name: "Correct Docker CRI socket is returned", - existingSockets: []string{"unix:///var/run/docker.sock"}, - expectedError: false, - expectedSocket: constants.DefaultDockerCRISocket, - }, { name: "CRI and Docker sockets lead to an error", existingSockets: []string{ @@ -420,10 +399,10 @@ func TestDetectCRISocketImpl(t *testing.T) { "unix:///run/containerd/containerd.sock", }, expectedError: false, - expectedSocket: constants.DefaultDockerCRISocket, + expectedSocket: constants.DefaultCRISocket, }, { - name: "A couple of CRI sockets lead to an error", + name: "Multiple CRI sockets lead to an error", existingSockets: []string{ "unix:///var/run/crio/crio.sock", "unix:///run/containerd/containerd.sock", From f3f1332223667200f85af84cb297c743d72556de Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Tue, 4 Jan 2022 23:36:45 +0200 Subject: [PATCH 3/3] kubeadm: update the CRI socket detection logic - Throw an error if there is more than one known socket on the host. - Remove the special handling for docker+containerd. - Remove the local instances of constants for endpoints for Windows / Unix and use the defaultKnownCRISockets variable which is populated from OS specific constants. - Update error message in detectCRISocketImpl to have more details. - Make detectCRISocketImpl accept a list of "known" sockets - Update unit tests for detectCRISocketImpl and make them use generic paths such as "unix:///foo/bar.sock". --- cmd/kubeadm/app/util/runtime/runtime.go | 30 ++++++++----------- cmd/kubeadm/app/util/runtime/runtime_test.go | 29 ++++-------------- cmd/kubeadm/app/util/runtime/runtime_unix.go | 5 ---- .../app/util/runtime/runtime_windows.go | 5 ---- 4 files changed, 19 insertions(+), 50 deletions(-) diff --git a/cmd/kubeadm/app/util/runtime/runtime.go b/cmd/kubeadm/app/util/runtime/runtime.go index 74007ca58f6..95697d7c656 100644 --- a/cmd/kubeadm/app/util/runtime/runtime.go +++ b/cmd/kubeadm/app/util/runtime/runtime.go @@ -27,6 +27,13 @@ import ( "k8s.io/kubernetes/cmd/kubeadm/app/constants" ) +// defaultKnownCRISockets holds the set of known CRI endpoints +var defaultKnownCRISockets = []string{ + constants.CRISocketContainerd, + constants.CRISocketCRIO, + constants.CRISocketDocker, +} + // ContainerRuntime is an interface for working with container runtimes type ContainerRuntime interface { Socket() string @@ -115,21 +122,8 @@ func (runtime *CRIRuntime) ImageExists(image string) (bool, error) { } // detectCRISocketImpl is separated out only for test purposes, DON'T call it directly, use DetectCRISocket instead -func detectCRISocketImpl(isSocket func(string) bool) (string, error) { +func detectCRISocketImpl(isSocket func(string) bool, knownCRISockets []string) (string, error) { foundCRISockets := []string{} - knownCRISockets := []string{ - // Docker and containerd sockets are special cased below, hence not to be included here - "unix:///var/run/crio/crio.sock", - } - - if isSocket(dockerSocket) { - // the path in dockerSocket is not CRI compatible, hence we should replace it with a CRI compatible socket - foundCRISockets = append(foundCRISockets, constants.DefaultCRISocket) - } else if isSocket(containerdSocket) { - // Docker 18.09 gets bundled together with containerd, thus having both dockerSocket and containerdSocket present. - // For compatibility reasons, we use the containerd socket only if Docker is not detected. - foundCRISockets = append(foundCRISockets, containerdSocket) - } for _, socket := range knownCRISockets { if isSocket(socket) { @@ -139,18 +133,20 @@ func detectCRISocketImpl(isSocket func(string) bool) (string, error) { switch len(foundCRISockets) { case 0: - // Fall back to Docker if no CRI is detected, we can error out later on if we need it + // Fall back to the default socket if no CRI is detected, we can error out later on if we need it return constants.DefaultCRISocket, nil case 1: // Precisely one CRI found, use that return foundCRISockets[0], nil default: // Multiple CRIs installed? - return "", errors.Errorf("Found multiple CRI sockets, please use --cri-socket to select one: %s", strings.Join(foundCRISockets, ", ")) + return "", errors.Errorf("Found multiple CRI endpoints on the host. Please define which one do you wish "+ + "to use by setting the 'criSocket' field in the kubeadm configuration file: %s", + strings.Join(foundCRISockets, ", ")) } } // DetectCRISocket uses a list of known CRI sockets to detect one. If more than one or none is discovered, an error is returned. func DetectCRISocket() (string, error) { - return detectCRISocketImpl(isExistingSocket) + return detectCRISocketImpl(isExistingSocket, defaultKnownCRISockets) } diff --git a/cmd/kubeadm/app/util/runtime/runtime_test.go b/cmd/kubeadm/app/util/runtime/runtime_test.go index 8e36cf17ac2..62e237364bb 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_test.go +++ b/cmd/kubeadm/app/util/runtime/runtime_test.go @@ -380,32 +380,15 @@ func TestDetectCRISocketImpl(t *testing.T) { }, { name: "One valid CRI socket leads to success", - existingSockets: []string{"unix:///var/run/crio/crio.sock"}, + existingSockets: []string{"unix:///foo/bar.sock"}, expectedError: false, - expectedSocket: "unix:///var/run/crio/crio.sock", - }, - { - name: "CRI and Docker sockets lead to an error", - existingSockets: []string{ - "unix:///var/run/docker.sock", - "unix:///var/run/crio/crio.sock", - }, - expectedError: true, - }, - { - name: "Docker and containerd lead to Docker being used", - existingSockets: []string{ - "unix:///var/run/docker.sock", - "unix:///run/containerd/containerd.sock", - }, - expectedError: false, - expectedSocket: constants.DefaultCRISocket, + expectedSocket: "unix:///foo/bar.sock", }, { name: "Multiple CRI sockets lead to an error", existingSockets: []string{ - "unix:///var/run/crio/crio.sock", - "unix:///run/containerd/containerd.sock", + "unix:///foo/bar.sock", + "unix:///foo/baz.sock", }, expectedError: true, }, @@ -419,9 +402,9 @@ func TestDetectCRISocketImpl(t *testing.T) { return true } } - return false - }) + }, test.existingSockets) + if (err != nil) != test.expectedError { t.Fatalf("detectCRISocketImpl returned unexpected result\n\tExpected error: %t\n\tGot error: %t", test.expectedError, err != nil) } diff --git a/cmd/kubeadm/app/util/runtime/runtime_unix.go b/cmd/kubeadm/app/util/runtime/runtime_unix.go index b0695324ee0..5989c14290f 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_unix.go +++ b/cmd/kubeadm/app/util/runtime/runtime_unix.go @@ -24,11 +24,6 @@ import ( "net/url" ) -const ( - dockerSocket = "unix:///var/run/docker.sock" // The Docker socket is not CRI compatible - containerdSocket = "unix:///run/containerd/containerd.sock" -) - // isExistingSocket checks if path exists and is domain socket func isExistingSocket(path string) bool { u, err := url.Parse(path) diff --git a/cmd/kubeadm/app/util/runtime/runtime_windows.go b/cmd/kubeadm/app/util/runtime/runtime_windows.go index b82f6b8f727..460bfcd0388 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_windows.go +++ b/cmd/kubeadm/app/util/runtime/runtime_windows.go @@ -25,11 +25,6 @@ import ( winio "github.com/Microsoft/go-winio" ) -const ( - dockerSocket = "npipe:////./pipe/docker_engine" // The Docker socket is not CRI compatible - containerdSocket = "npipe:////./pipe/containerd-containerd" // Proposed containerd named pipe for Windows -) - // isExistingSocket checks if path exists and is domain socket func isExistingSocket(path string) bool { u, err := url.Parse(path)