Merge pull request #110022 from neolit123/1.25-handle-kubelet-flags-w…

…ithout-dockershim kubeadm: perform dockershim cleanup for 1.25
kubernetes · May 13, 2022 · 9d85e18 · 9d85e18
2 parents 340ba56 + 6efdcfd
commit 9d85e18
Show file tree

Hide file tree

Showing 3 changed files with 7 additions and 124 deletions.
diff --git a/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go b/cmd/kubeadm/app/cmd/phases/reset/cleanupnode.go
@@ -96,10 +96,6 @@ func runCleanupNode(c workflow.RunData) error {
 		fmt.Println("[reset] Would remove Kubernetes-managed containers")
 	}
 
-	// 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
 	if certsDir != kubeadmapiv1.DefaultCertificatesDir {
 		klog.Warningf("[reset] WARNING: Cleaning a non-default certificates directory: %q\n", certsDir)

diff --git a/cmd/kubeadm/app/phases/kubelet/flags.go b/cmd/kubeadm/app/phases/kubelet/flags.go
@@ -20,30 +20,22 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"runtime"
 	"strings"
 
 	"github.com/pkg/errors"
 
-	versionutil "k8s.io/apimachinery/pkg/util/version"
-	componentversion "k8s.io/component-base/version"
 	"k8s.io/klog/v2"
-	utilsexec "k8s.io/utils/exec"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/images"
-	preflight "k8s.io/kubernetes/cmd/kubeadm/app/preflight"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 )
 
 type kubeletFlagsOpts struct {
 	nodeRegOpts              *kubeadmapi.NodeRegistrationOptions
 	pauseImage               string
 	registerTaintsUsingFlags bool
-	// This is a temporary measure until kubeadm no longer supports a kubelet version with built-in dockershim.
-	// TODO: https://github.com/kubernetes/kubeadm/issues/2626
-	kubeletVersion *versionutil.Version
 }
 
 // GetNodeNameAndHostname obtains the name for this Node using the following precedence
@@ -67,24 +59,10 @@ func GetNodeNameAndHostname(cfg *kubeadmapi.NodeRegistrationOptions) (string, st
 // WriteKubeletDynamicEnvFile writes an environment file with dynamic flags to the kubelet.
 // Used at "kubeadm init" and "kubeadm join" time.
 func WriteKubeletDynamicEnvFile(cfg *kubeadmapi.ClusterConfiguration, nodeReg *kubeadmapi.NodeRegistrationOptions, registerTaintsUsingFlags bool, kubeletDir string) error {
-	// This is a temporary measure until kubeadm no longer supports a kubelet version with built-in dockershim.
-	// TODO: https://github.com/kubernetes/kubeadm/issues/2626
-	kubeletVersion, err := preflight.GetKubeletVersion(utilsexec.New())
-	if err != nil {
-		// We cannot return an error here, due to the k/k CI, where /cmd/kubeadm/test tests run without
-		// a kubelet built on the host. On error, we assume a kubelet version equal to the version
-		// of the kubeadm binary. During normal cluster creation this should not happens as kubeadm needs
-		// the kubelet binary for init / join.
-		kubeletVersion = versionutil.MustParseSemantic(componentversion.Get().GitVersion)
-		klog.Warningf("cannot obtain the version of the kubelet while writing dynamic environment file: %v."+
-			" Using the version of the kubeadm binary: %s", err, kubeletVersion.String())
-	}
-
 	flagOpts := kubeletFlagsOpts{
 		nodeRegOpts:              nodeReg,
 		pauseImage:               images.GetPauseImage(cfg),
 		registerTaintsUsingFlags: registerTaintsUsingFlags,
-		kubeletVersion:           kubeletVersion,
 	}
 	stringMap := buildKubeletArgMap(flagOpts)
 	argList := kubeadmutil.BuildArgumentListFromMap(stringMap, nodeReg.KubeletExtraArgs)
@@ -97,23 +75,7 @@ func WriteKubeletDynamicEnvFile(cfg *kubeadmapi.ClusterConfiguration, nodeReg *k
 //that are common to both Linux and Windows
 func buildKubeletArgMapCommon(opts kubeletFlagsOpts) map[string]string {
 	kubeletFlags := map[string]string{}
-
-	// This is a temporary measure until kubeadm no longer supports a kubelet version with built-in dockershim.
-	// 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
-	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"
-		kubeletFlags["container-runtime-endpoint"] = opts.nodeRegOpts.CRISocket
-	}
+	kubeletFlags["container-runtime-endpoint"] = opts.nodeRegOpts.CRISocket
 
 	// This flag passes the pod infra container image (e.g. "pause" image) to the kubelet
 	// and prevents its garbage collection

diff --git a/cmd/kubeadm/app/phases/kubelet/flags_test.go b/cmd/kubeadm/app/phases/kubelet/flags_test.go
@@ -21,73 +21,27 @@ import (
 	"testing"
 
 	v1 "k8s.io/api/core/v1"
-	"k8s.io/apimachinery/pkg/util/version"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 )
 
 func TestBuildKubeletArgMap(t *testing.T) {
-	// Tests must be updated once kubeadm no longer supports a kubelet version with built-in dockershim.
-	// TODO: https://github.com/kubernetes/kubeadm/issues/2626
 	tests := []struct {
 		name     string
 		opts     kubeletFlagsOpts
 		expected map[string]string
 	}{
 		{
-			name: "the simplest case",
+			name: "hostname override",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "unix:///var/run/dockershim.sock",
-					Taints: []v1.Taint{ // This should be ignored as registerTaintsUsingFlags is false
-						{
-							Key:    "foo",
-							Value:  "bar",
-							Effect: "baz",
-						},
-					},
-				},
-			},
-			expected: map[string]string{
-				"network-plugin": "cni",
-			},
-		},
-		{
-			name: "hostname override from NodeRegistrationOptions.Name",
-			opts: kubeletFlagsOpts{
-				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "unix:///var/run/dockershim.sock",
-					Name:      "override-name",
-				},
-			},
-			expected: map[string]string{
-				"network-plugin":    "cni",
-				"hostname-override": "override-name",
-			},
-		},
-		{
-			name: "hostname override from NodeRegistrationOptions.KubeletExtraArgs",
-			opts: kubeletFlagsOpts{
-				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket:        "unix:///var/run/dockershim.sock",
+					CRISocket:        "unix:///var/run/containerd/containerd.sock",
 					KubeletExtraArgs: map[string]string{"hostname-override": "override-name"},
 				},
 			},
 			expected: map[string]string{
-				"network-plugin":    "cni",
-				"hostname-override": "override-name",
-			},
-		},
-		{
-			name: "external CRI runtime",
-			opts: kubeletFlagsOpts{
-				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "unix:///var/run/containerd/containerd.sock",
-				},
-			},
-			expected: map[string]string{
-				"container-runtime":          "remote",
 				"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
+				"hostname-override":          "override-name",
 			},
 		},
 		{
@@ -111,7 +65,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				registerTaintsUsingFlags: true,
 			},
 			expected: map[string]string{
-				"container-runtime":          "remote",
 				"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
 				"register-with-taints":       "foo=bar:baz,key=val:eff",
 			},
@@ -120,47 +73,19 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			name: "pause image is set",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "unix:///var/run/dockershim.sock",
+					CRISocket: "unix:///var/run/containerd/containerd.sock",
 				},
 				pauseImage: "k8s.gcr.io/pause:3.7",
 			},
 			expected: map[string]string{
-				"network-plugin":            "cni",
-				"pod-infra-container-image": "k8s.gcr.io/pause:3.7",
-			},
-		},
-		{
-			name: "dockershim socket and kubelet version with built-in dockershim",
-			opts: kubeletFlagsOpts{
-				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "unix:///var/run/dockershim.sock",
-				},
-				kubeletVersion: version.MustParseSemantic("v1.23.6"),
-			},
-			expected: map[string]string{
-				"network-plugin": "cni",
-			},
-		},
-		{
-			name: "dockershim socket but kubelet version is without built-in dockershim",
-			opts: kubeletFlagsOpts{
-				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "unix:///var/run/dockershim.sock",
-				},
-				kubeletVersion: version.MustParseSemantic("v1.24.0-alpha.1"),
-			},
-			expected: map[string]string{
-				"container-runtime":          "remote",
-				"container-runtime-endpoint": "unix:///var/run/dockershim.sock",
+				"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
+				"pod-infra-container-image":  "k8s.gcr.io/pause:3.7",
 			},
 		},
 	}
 
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
-			if test.opts.kubeletVersion == nil {
-				test.opts.kubeletVersion = version.MustParseSemantic("v1.0.0")
-			}
 			actual := buildKubeletArgMap(test.opts)
 			if !reflect.DeepEqual(actual, test.expected) {
 				t.Errorf(