Merge pull request #107295 from neolit123/1.24-kubeadm-add-scheme-to-cri-sockets

kubeadm: ensure CRI endpoints are managed with URL schemes

cmd/kubeadm/app/apis/kubeadm/v1beta2/defaults_unix.go

@@ -22,6 +22,6 @@ package v1beta2
 const (
 	// DefaultCACertPath defines default location of CA certificate on Linux
 	DefaultCACertPath = "/etc/kubernetes/pki/ca.crt"
-	// DefaultUrlScheme defines default socket url prefix
+	// DefaultContainerRuntimeURLScheme defines default socket url prefix
-	DefaultUrlScheme = "unix"
+	DefaultContainerRuntimeURLScheme = "unix"
 )

cmd/kubeadm/app/apis/kubeadm/v1beta2/defaults_windows.go

@@ -22,6 +22,6 @@ package v1beta2
 const (
 	// DefaultCACertPath defines default location of CA certificate on Windows
 	DefaultCACertPath = "C:/etc/kubernetes/pki/ca.crt"
-	// DefaultUrlScheme defines default socket url prefix
+	// DefaultContainerRuntimeURLScheme defines default socket url prefix
-	DefaultUrlScheme = "npipe"
+	DefaultContainerRuntimeURLScheme = "npipe"
 )

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: "/var/run/dockershim.sock"
+// 	  criSocket: "unix:///var/run/dockershim.sock"
 // 	  taints:
 // 	  - key: "kubeadmNode"
 // 	    value: "master"

cmd/kubeadm/app/apis/kubeadm/v1beta3/defaults_unix.go

@@ -22,6 +22,6 @@ package v1beta3
 const (
 	// DefaultCACertPath defines default location of CA certificate on Linux
 	DefaultCACertPath = "/etc/kubernetes/pki/ca.crt"
-	// DefaultUrlScheme defines default socket url prefix
+	// DefaultContainerRuntimeURLScheme defines default socket url prefix
-	DefaultUrlScheme = "unix"
+	DefaultContainerRuntimeURLScheme = "unix"
 )

cmd/kubeadm/app/apis/kubeadm/v1beta3/defaults_windows.go

@@ -22,6 +22,6 @@ package v1beta3
 const (
 	// DefaultCACertPath defines default location of CA certificate on Windows
 	DefaultCACertPath = "C:/etc/kubernetes/pki/ca.crt"
-	// DefaultUrlScheme defines default socket url prefix
+	// DefaultContainerRuntimeURLScheme defines default socket url prefix
-	DefaultUrlScheme = "npipe"
+	DefaultContainerRuntimeURLScheme = "npipe"
 )

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: "/var/run/dockershim.sock"
+// 	  criSocket: "unix:///var/run/dockershim.sock"
 // 	  taints:
 // 	  - key: "kubeadmNode"
 // 	    value: "master"

cmd/kubeadm/app/apis/kubeadm/validation/validation.go

@@ -624,17 +624,18 @@ func ValidateIgnorePreflightErrors(ignorePreflightErrorsFromCLI, ignorePreflight
 func ValidateSocketPath(socket string, fldPath *field.Path) field.ErrorList {
 	allErrs := field.ErrorList{}
 
+	if len(socket) == 0 { // static and dynamic defaulting should have added a value to the field already
+		return append(allErrs, field.Invalid(fldPath, socket, "empty CRI socket"))
+	}
+
 	u, err := url.Parse(socket)
 	if err != nil {
 		return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("URL parsing error: %v", err)))
 	}
 
-	if u.Scheme == "" {
+	// static and dynamic defaulting should have ensured that an URL scheme is used
-		if !filepath.IsAbs(u.Path) {
+	if u.Scheme != kubeadmapiv1.DefaultContainerRuntimeURLScheme {
-			return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("path is not absolute: %s", socket)))
+		return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("only URL scheme %q is supported, got %q", kubeadmapiv1.DefaultContainerRuntimeURLScheme, u.Scheme)))
-		}
-	} else if u.Scheme != kubeadmapiv1.DefaultUrlScheme {
-		return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("URL scheme %s is not supported", u.Scheme)))
 	}
 
 	return allErrs

cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go

@@ -111,7 +111,7 @@ func TestValidateNodeRegistrationOptions(t *testing.T) {
 		// test cases for criSocket are covered in TestValidateSocketPath
 	}
 	for _, rt := range tests {
-		nro := kubeadmapi.NodeRegistrationOptions{Name: rt.nodeName, CRISocket: "/some/path"}
+		nro := kubeadmapi.NodeRegistrationOptions{Name: rt.nodeName, CRISocket: "unix:///some/path"}
 		actual := ValidateNodeRegistrationOptions(&nro, field.NewPath("nodeRegistration"))
 		actualErrors := len(actual) > 0
 		if actualErrors != rt.expectedErrors {
@@ -448,7 +448,7 @@ func TestValidateInitConfiguration(t *testing.T) {
 					},
 					CertificatesDir: "/some/cert/dir",
 				},
-				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
 			}, false},
 		{"invalid missing token with IPv6 service subnet",
 			&kubeadmapi.InitConfiguration{
@@ -463,7 +463,7 @@ func TestValidateInitConfiguration(t *testing.T) {
 					},
 					CertificatesDir: "/some/cert/dir",
 				},
-				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
 			}, false},
 		{"invalid missing node name",
 			&kubeadmapi.InitConfiguration{
@@ -493,7 +493,7 @@ func TestValidateInitConfiguration(t *testing.T) {
 					},
 					CertificatesDir: "/some/other/cert/dir",
 				},
-				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
 			}, false},
 		{"valid InitConfiguration with IPv4 service subnet",
 			&kubeadmapi.InitConfiguration{
@@ -514,7 +514,7 @@ func TestValidateInitConfiguration(t *testing.T) {
 					},
 					CertificatesDir: "/some/other/cert/dir",
 				},
-				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
 			}, true},
 		{"valid InitConfiguration using IPv6 service subnet",
 			&kubeadmapi.InitConfiguration{
@@ -534,7 +534,7 @@ func TestValidateInitConfiguration(t *testing.T) {
 					},
 					CertificatesDir: "/some/other/cert/dir",
 				},
-				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "/some/path"},
+				NodeRegistration: kubeadmapi.NodeRegistrationOptions{Name: nodename, CRISocket: "unix:///some/path"},
 			}, true},
 	}
 	for _, rt := range tests {
@@ -579,7 +579,7 @@ func TestValidateJoinConfiguration(t *testing.T) {
 			},
 			NodeRegistration: kubeadmapi.NodeRegistrationOptions{
 				Name:      "aaa",
-				CRISocket: "/var/run/dockershim.sock",
+				CRISocket: "unix:///var/run/dockershim.sock",
 			},
 		}, true},
 		{&kubeadmapi.JoinConfiguration{ // Pass with JoinControlPlane
@@ -594,7 +594,7 @@ func TestValidateJoinConfiguration(t *testing.T) {
 			},
 			NodeRegistration: kubeadmapi.NodeRegistrationOptions{
 				Name:      "aaa",
-				CRISocket: "/var/run/dockershim.sock",
+				CRISocket: "unix:///var/run/dockershim.sock",
 			},
 			ControlPlane: &kubeadmapi.JoinControlPlane{
 				LocalAPIEndpoint: kubeadmapi.APIEndpoint{
@@ -615,7 +615,7 @@ func TestValidateJoinConfiguration(t *testing.T) {
 			},
 			NodeRegistration: kubeadmapi.NodeRegistrationOptions{
 				Name:      "aaa",
-				CRISocket: "/var/run/dockershim.sock",
+				CRISocket: "unix:///var/run/dockershim.sock",
 			},
 			ControlPlane: &kubeadmapi.JoinControlPlane{
 				LocalAPIEndpoint: kubeadmapi.APIEndpoint{
@@ -963,12 +963,11 @@ func TestValidateSocketPath(t *testing.T) {
 		criSocket      string
 		expectedErrors bool
 	}{
-		{name: "valid path", criSocket: "/some/path", expectedErrors: false},
+		{name: "valid socket URL", criSocket: kubeadmapiv1.DefaultContainerRuntimeURLScheme + "://" + "/some/path", expectedErrors: false},
-		{name: "valid socket url", criSocket: kubeadmapiv1.DefaultUrlScheme + "://" + "/some/path", expectedErrors: false},
+		{name: "unsupported URL scheme", criSocket: "bla:///some/path", expectedErrors: true},
-		{name: "unsupported url scheme", criSocket: "bla:///some/path", expectedErrors: true},
+		{name: "missing URL scheme", criSocket: "/some/path", expectedErrors: true},
-		{name: "unparseable url", criSocket: ":::", expectedErrors: true},
+		{name: "unparseable URL", criSocket: ":::", expectedErrors: true},
-		{name: "invalid CRISocket (path is not absolute)", criSocket: "some/path", expectedErrors: true},
+		{name: "empty CRISocket", criSocket: "", expectedErrors: true},
-		{name: "empty CRISocket (path is not absolute)", criSocket: "", expectedErrors: true},
 	}
 	for _, tc := range tests {
 		actual := ValidateSocketPath(tc.criSocket, field.NewPath("criSocket"))

cmd/kubeadm/app/cmd/phases/upgrade/node/data.go

@@ -34,4 +34,5 @@ type Data interface {
 	Client() clientset.Interface
 	IgnorePreflightErrors() sets.String
 	PatchesDir() string
+	KubeConfigPath() string
 }

cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go

@@ -20,15 +20,22 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/pkg/errors"
 
+	"k8s.io/klog/v2"
+
+	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3"
 	"k8s.io/kubernetes/cmd/kubeadm/app/cmd/options"
 	"k8s.io/kubernetes/cmd/kubeadm/app/cmd/phases/workflow"
 	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	kubeletphase "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet"
+	patchnodephase "k8s.io/kubernetes/cmd/kubeadm/app/phases/patchnode"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/upgrade"
+	configutil "k8s.io/kubernetes/cmd/kubeadm/app/util/config"
 	dryrunutil "k8s.io/kubernetes/cmd/kubeadm/app/util/dryrun"
 )
 
@@ -87,6 +94,40 @@ func runKubeletConfigPhase() func(c workflow.RunData) error {
 			return nil
 		}
 
+		// Handle a missing URL scheme in the Node CRI socket.
+		// Older versions of kubeadm tolerate CRI sockets without URL schemes (/var/run/foo without unix://).
+		// During "upgrade node" for worker nodes the cfg.NodeRegistration would be left empty.
+		// This requires to call GetNodeRegistration on demand and fetch the node name and CRI socket.
+		// If the NodeRegistration (nro) contains a socket without a URL scheme, update it.
+		//
+		// TODO: this workaround can be removed in 1.25 once all user node sockets have a URL scheme:
+		// https://github.com/kubernetes/kubeadm/issues/2426
+		var nro *kubeadmapi.NodeRegistrationOptions
+		var missingURLScheme bool
+		if !dryRun {
+			if err := configutil.GetNodeRegistration(data.KubeConfigPath(), data.Client(), nro); err != nil {
+				return errors.Wrap(err, "could not retrieve the node registration options for this node")
+			}
+			missingURLScheme = strings.HasPrefix(nro.CRISocket, kubeadmapiv1.DefaultContainerRuntimeURLScheme)
+		}
+		if missingURLScheme {
+			if !dryRun {
+				newSocket := kubeadmapiv1.DefaultContainerRuntimeURLScheme + "://" + nro.CRISocket
+				klog.V(2).Infof("ensuring that Node %q has a CRI socket annotation with URL scheme %q", nro.Name, newSocket)
+				if err := patchnodephase.AnnotateCRISocket(data.Client(), nro.Name, newSocket); err != nil {
+					return errors.Wrapf(err, "error updating the CRI socket for Node %q", nro.Name)
+				}
+			} else {
+				fmt.Println("[dryrun] would update the node CRI socket path to include an URL scheme")
+			}
+		}
+
+		// TODO: Temporary workaround. Remove in 1.25:
+		// https://github.com/kubernetes/kubeadm/issues/2426
+		if err := upgrade.UpdateKubeletDynamicEnvFileWithURLScheme(dryRun); err != nil {
+			return err
+		}
+
 		fmt.Println("[upgrade] The configuration for this node was successfully updated!")
 		fmt.Println("[upgrade] Now you should go ahead and upgrade the kubelet package using your package manager.")
 		return nil

cmd/kubeadm/app/cmd/upgrade/node.go

@@ -61,6 +61,7 @@ type nodeData struct {
 	client                clientset.Interface
 	patchesDir            string
 	ignorePreflightErrors sets.String
+	kubeConfigPath        string
 }
 
 // newCmdNode returns the cobra command for `kubeadm upgrade node`
@@ -159,6 +160,7 @@ func newNodeData(cmd *cobra.Command, args []string, options *nodeOptions) (*node
 		isControlPlaneNode:    isControlPlaneNode,
 		patchesDir:            options.patchesDir,
 		ignorePreflightErrors: ignorePreflightErrorsSet,
+		kubeConfigPath:        options.kubeConfigPath,
 	}, nil
 }
 
@@ -201,3 +203,8 @@ func (d *nodeData) PatchesDir() string {
 func (d *nodeData) IgnorePreflightErrors() sets.String {
 	return d.ignorePreflightErrors
 }
+
+// KubeconfigPath returns the path to the user kubeconfig file.
+func (d *nodeData) KubeConfigPath() string {
+	return d.kubeConfigPath
+}

cmd/kubeadm/app/constants/constants.go

@@ -245,7 +245,7 @@ const (
 	AnnotationKubeadmCRISocket = "kubeadm.alpha.kubernetes.io/cri-socket"
 
 	// UnknownCRISocket defines the undetected or unknown CRI socket
-	UnknownCRISocket = "/var/run/unknown.sock"
+	UnknownCRISocket = "unix:///var/run/unknown.sock"
 
 	// KubeadmConfigConfigMap specifies in what ConfigMap in the kube-system namespace the `kubeadm init` configuration should be stored
 	KubeadmConfigConfigMap = "kubeadm-config"

cmd/kubeadm/app/constants/constants_unix.go

@@ -21,5 +21,5 @@ package constants
 
 const (
 	// DefaultDockerCRISocket defines the default Docker CRI socket
-	DefaultDockerCRISocket = "/var/run/dockershim.sock"
+	DefaultDockerCRISocket = "unix:///var/run/dockershim.sock"
 )

cmd/kubeadm/app/phases/kubelet/flags_test.go

@@ -38,7 +38,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			name: "the simplest case",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/dockershim.sock",
+					CRISocket: "unix:///var/run/dockershim.sock",
 					Taints: []v1.Taint{ // This should be ignored as registerTaintsUsingFlags is false
 						{
 							Key:    "foo",
@@ -56,7 +56,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			name: "hostname override from NodeRegistrationOptions.Name",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/dockershim.sock",
+					CRISocket: "unix:///var/run/dockershim.sock",
 					Name:      "override-name",
 				},
 			},
@@ -69,7 +69,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			name: "hostname override from NodeRegistrationOptions.KubeletExtraArgs",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket:        "/var/run/dockershim.sock",
+					CRISocket:        "unix:///var/run/dockershim.sock",
 					KubeletExtraArgs: map[string]string{"hostname-override": "override-name"},
 				},
 			},
@@ -82,19 +82,19 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			name: "external CRI runtime",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/containerd.sock",
+					CRISocket: "unix:///var/run/containerd/containerd.sock",
 				},
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
-				"container-runtime-endpoint": "/var/run/containerd.sock",
+				"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
 			},
 		},
 		{
 			name: "register with taints",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/containerd.sock",
+					CRISocket: "unix:///var/run/containerd/containerd.sock",
 					Taints: []v1.Taint{
 						{
 							Key:    "foo",
@@ -112,7 +112,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
-				"container-runtime-endpoint": "/var/run/containerd.sock",
+				"container-runtime-endpoint": "unix:///var/run/containerd/containerd.sock",
 				"register-with-taints":       "foo=bar:baz,key=val:eff",
 			},
 		},
@@ -120,7 +120,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			name: "pause image is set",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/dockershim.sock",
+					CRISocket: "unix:///var/run/dockershim.sock",
 				},
 				pauseImage: "k8s.gcr.io/pause:3.6",
 			},
@@ -133,7 +133,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			name: "dockershim socket and kubelet version with built-in dockershim",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/dockershim.sock",
+					CRISocket: "unix:///var/run/dockershim.sock",
 				},
 				kubeletVersion: version.MustParseSemantic("v1.23.6"),
 			},
@@ -145,13 +145,13 @@ func TestBuildKubeletArgMap(t *testing.T) {
 			name: "dockershim socket but kubelet version is without built-in dockershim",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/dockershim.sock",
+					CRISocket: "unix:///var/run/dockershim.sock",
 				},
 				kubeletVersion: version.MustParseSemantic("v1.24.0-alpha.1"),
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
-				"container-runtime-endpoint": "/var/run/dockershim.sock",
+				"container-runtime-endpoint": "unix:///var/run/dockershim.sock",
 			},
 		},
 	}

cmd/kubeadm/app/phases/patchnode/patchnode_test.go

@@ -41,20 +41,20 @@ func TestAnnotateCRISocket(t *testing.T) {
 		{
 			name:                       "CRI-socket annotation missing",
 			currentCRISocketAnnotation: "",
-			newCRISocketAnnotation:     "/run/containerd/containerd.sock",
+			newCRISocketAnnotation:     "unix:///run/containerd/containerd.sock",
-			expectedPatch:              `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"/run/containerd/containerd.sock"}}}`,
+			expectedPatch:              `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"unix:///run/containerd/containerd.sock"}}}`,
 		},
 		{
 			name:                       "CRI-socket annotation already exists",
-			currentCRISocketAnnotation: "/run/containerd/containerd.sock",
+			currentCRISocketAnnotation: "unix:///run/containerd/containerd.sock",
-			newCRISocketAnnotation:     "/run/containerd/containerd.sock",
+			newCRISocketAnnotation:     "unix:///run/containerd/containerd.sock",
 			expectedPatch:              `{}`,
 		},
 		{
 			name:                       "CRI-socket annotation needs to be updated",
-			currentCRISocketAnnotation: "/var/run/dockershim.sock",
+			currentCRISocketAnnotation: "unix:///var/run/dockershim.sock",
-			newCRISocketAnnotation:     "/run/containerd/containerd.sock",
+			newCRISocketAnnotation:     "unix:///run/containerd/containerd.sock",
-			expectedPatch:              `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"/run/containerd/containerd.sock"}}}`,
+			expectedPatch:              `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"unix:///run/containerd/containerd.sock"}}}`,
 		},
 	}
 

cmd/kubeadm/app/phases/upgrade/postupgrade.go

@@ -18,7 +18,11 @@ package upgrade
 
 import (
 	"context"
+	"fmt"
+	"io/ioutil"
 	"os"
+	"path/filepath"
+	"strings"
 
 	"github.com/pkg/errors"
 
@@ -31,6 +35,7 @@ import (
 	"k8s.io/klog/v2"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3"
 	kubeadmconstants "k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/dns"
 	"k8s.io/kubernetes/cmd/kubeadm/app/phases/addons/proxy"
@@ -65,6 +70,12 @@ func PerformPostUpgradeTasks(client clientset.Interface, cfg *kubeadmapi.InitCon
 		errs = append(errs, err)
 	}
 
+	// TODO: Temporary workaround. Remove in 1.25:
+	// https://github.com/kubernetes/kubeadm/issues/2426
+	if err := UpdateKubeletDynamicEnvFileWithURLScheme(dryRun); err != nil {
+		return err
+	}
+
 	// Annotate the node with the crisocket information, sourced either from the InitConfiguration struct or
 	// --cri-socket.
 	// TODO: In the future we want to use something more official like NodeStatus or similar for detecting this properly
@@ -231,3 +242,67 @@ func LabelOldControlPlaneNodes(client clientset.Interface) error {
 	}
 	return nil
 }
+
+// UpdateKubeletDynamicEnvFileWithURLScheme reads the kubelet dynamic environment file
+// from disk, ensure that the CRI endpoint flag has a scheme prefix and writes it
+// back to disk.
+// TODO: Temporary workaround. Remove in 1.25:
+// https://github.com/kubernetes/kubeadm/issues/2426
+func UpdateKubeletDynamicEnvFileWithURLScheme(dryRun bool) error {
+	filePath := filepath.Join(kubeadmconstants.KubeletRunDirectory, kubeadmconstants.KubeletEnvFileName)
+	if dryRun {
+		fmt.Printf("[dryrun] Would ensure that %q includes a CRI endpoint URL scheme\n", filePath)
+		return nil
+	}
+	klog.V(2).Infof("Ensuring that %q includes a CRI endpoint URL scheme", filePath)
+	bytes, err := ioutil.ReadFile(filePath)
+	if err != nil {
+		return errors.Wrapf(err, "failed to read kubelet configuration from file %q", filePath)
+	}
+	updated := updateKubeletDynamicEnvFileWithURLScheme(string(bytes))
+	if err := ioutil.WriteFile(filePath, []byte(updated), 0644); err != nil {
+		return errors.Wrapf(err, "failed to write kubelet configuration to the file %q", filePath)
+	}
+	return nil
+}
+
+func updateKubeletDynamicEnvFileWithURLScheme(str string) string {
+	const (
+		flag   = "container-runtime-endpoint"
+		scheme = kubeadmapiv1.DefaultContainerRuntimeURLScheme + "://"
+	)
+	// Trim the prefix
+	str = strings.TrimLeft(str, fmt.Sprintf("%s=\"", kubeadmconstants.KubeletEnvFileVariableName))
+
+	// Flags are managed by kubeadm as pairs of key=value separated by space.
+	// Split them, find the one containing the flag of interest and update
+	// its value to have the scheme prefix.
+	split := strings.Split(str, " ")
+	for i, s := range split {
+		if !strings.Contains(s, flag) {
+			continue
+		}
+		keyValue := strings.Split(s, "=")
+		if len(keyValue) < 2 {
+			// Post init/join, the user may have edited the file and has flags that are not
+			// followed by "=". If that is the case the next argument must be the value
+			// of the endpoint flag and if its not a flag itself. Update that argument with
+			// the scheme instead.
+			if i+1 < len(split) {
+				nextArg := split[i+1]
+				if !strings.HasPrefix(nextArg, "-") && !strings.HasPrefix(nextArg, scheme) {
+					split[i+1] = scheme + nextArg
+				}
+			}
+			continue
+		}
+		if len(keyValue[1]) == 0 || strings.HasPrefix(keyValue[1], scheme) {
+			continue // The flag value already has the URL scheme prefix or is empty
+		}
+		// Missing prefix. Add it and update the key=value pair
+		keyValue[1] = scheme + keyValue[1]
+		split[i] = strings.Join(keyValue, "=")
+	}
+	str = strings.Join(split, " ")
+	return fmt.Sprintf("%s=\"%s", kubeadmconstants.KubeletEnvFileVariableName, str)
+}

cmd/kubeadm/app/phases/upgrade/postupgrade_test.go

@@ -17,6 +17,7 @@ limitations under the License.
 package upgrade
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -24,6 +25,7 @@ import (
 
 	"github.com/pkg/errors"
 
+	kubeadmapiv1 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta3"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	testutil "k8s.io/kubernetes/cmd/kubeadm/test"
 )
@@ -101,3 +103,45 @@ func TestRollbackFiles(t *testing.T) {
 		t.Fatalf("Expected error contains %q, got %v", errString, err)
 	}
 }
+
+func TestUpdateKubeletDynamicEnvFileWithURLScheme(t *testing.T) {
+	tcases := []struct {
+		name     string
+		input    string
+		expected string
+	}{
+		{
+			name:     "missing flag of interest",
+			input:    fmt.Sprintf("%s=\"--foo=abc --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --bar=def\"", constants.KubeletEnvFileVariableName),
+		},
+		{
+			name:     "add missing URL scheme",
+			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint=/some/endpoint --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint=%s:///some/endpoint --bar=def\"", constants.KubeletEnvFileVariableName, kubeadmapiv1.DefaultContainerRuntimeURLScheme),
+		},
+		{
+			name:     "add missing URL scheme if there is no '=' after the flag name",
+			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint /some/endpoint --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint %s:///some/endpoint --bar=def\"", constants.KubeletEnvFileVariableName, kubeadmapiv1.DefaultContainerRuntimeURLScheme),
+		},
+		{
+			name:     "empty flag of interest value following '='",
+			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint= --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint= --bar=def\"", constants.KubeletEnvFileVariableName),
+		},
+		{
+			name:     "empty flag of interest value without '='",
+			input:    fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint --bar=def\"", constants.KubeletEnvFileVariableName),
+			expected: fmt.Sprintf("%s=\"--foo=abc --container-runtime-endpoint --bar=def\"", constants.KubeletEnvFileVariableName),
+		},
+	}
+	for _, tt := range tcases {
+		t.Run(tt.name, func(t *testing.T) {
+			output := updateKubeletDynamicEnvFileWithURLScheme(tt.input)
+			if output != tt.expected {
+				t.Errorf("expected output: %q, got: %q", tt.expected, output)
+			}
+		})
+	}
+}

cmd/kubeadm/app/util/config/cluster.go

@@ -97,7 +97,8 @@ func getInitConfigurationFromCluster(kubeconfigDir string, client clientset.Inte
 	// get nodes specific information as well
 	if !newControlPlane {
 		// gets the nodeRegistration for the current from the node object
-		if err := getNodeRegistration(kubeconfigDir, client, &initcfg.NodeRegistration); err != nil {
+		kubeconfigFile := filepath.Join(kubeconfigDir, constants.KubeletKubeConfigFileName)
+		if err := GetNodeRegistration(kubeconfigFile, client, &initcfg.NodeRegistration); err != nil {
 			return nil, errors.Wrap(err, "failed to get node registration")
 		}
 		// gets the APIEndpoint for the current node
@@ -117,10 +118,10 @@ func getInitConfigurationFromCluster(kubeconfigDir string, client clientset.Inte
 	return initcfg, nil
 }
 
-// getNodeRegistration returns the nodeRegistration for the current node
+// GetNodeRegistration returns the nodeRegistration for the current node
-func getNodeRegistration(kubeconfigDir string, client clientset.Interface, nodeRegistration *kubeadmapi.NodeRegistrationOptions) error {
+func GetNodeRegistration(kubeconfigFile string, client clientset.Interface, nodeRegistration *kubeadmapi.NodeRegistrationOptions) error {
 	// gets the name of the current node
-	nodeName, err := getNodeNameFromKubeletConfig(kubeconfigDir)
+	nodeName, err := getNodeNameFromKubeletConfig(kubeconfigFile)
 	if err != nil {
 		return errors.Wrap(err, "failed to get node name from kubelet config")
 	}
@@ -149,9 +150,8 @@ func getNodeRegistration(kubeconfigDir string, client clientset.Interface, nodeR
 // getNodeNameFromKubeletConfig gets the node name from a kubelet config file
 // TODO: in future we want to switch to a more canonical way for doing this e.g. by having this
 //       information in the local kubelet config.yaml
-func getNodeNameFromKubeletConfig(kubeconfigDir string) (string, error) {
+func getNodeNameFromKubeletConfig(fileName string) (string, error) {
 	// loads the kubelet.conf file
-	fileName := filepath.Join(kubeconfigDir, constants.KubeletKubeConfigFileName)
 	config, err := clientcmd.LoadFromFile(fileName)
 	if err != nil {
 		return "", err

cmd/kubeadm/app/util/config/cluster_test.go

@@ -261,7 +261,7 @@ func TestGetNodeNameFromKubeletConfig(t *testing.T) {
 				return
 			}
 
-			name, err := getNodeNameFromKubeletConfig(tmpdir)
+			name, err := getNodeNameFromKubeletConfig(kubeconfigPath)
 			if rt.expectedError != (err != nil) {
 				t.Errorf("unexpected return err from getNodeRegistration: %v", err)
 				return
@@ -338,7 +338,7 @@ func TestGetNodeRegistration(t *testing.T) {
 			}
 
 			cfg := &kubeadmapi.InitConfiguration{}
-			err = getNodeRegistration(tmpdir, client, &cfg.NodeRegistration)
+			err = GetNodeRegistration(cfgPath, client, &cfg.NodeRegistration)
 			if rt.expectedError != (err != nil) {
 				t.Errorf("unexpected return err from getNodeRegistration: %v", err)
 				return

cmd/kubeadm/app/util/config/initconfiguration.go

@@ -22,6 +22,7 @@ import (
 	"io/ioutil"
 	"net"
 	"strconv"
+	"strings"
 
 	"github.com/pkg/errors"
 
@@ -115,6 +116,13 @@ func SetNodeRegistrationDynamicDefaults(cfg *kubeadmapi.NodeRegistrationOptions,
 			return err
 		}
 		klog.V(1).Infof("detected and using CRI socket: %s", cfg.CRISocket)
+	} else {
+		if !strings.HasPrefix(cfg.CRISocket, kubeadmapiv1.DefaultContainerRuntimeURLScheme) {
+			klog.Warningf("Usage of CRI endpoints without URL scheme is deprecated and can cause kubelet errors "+
+				"in the future. Automatically prepending scheme %q to the \"criSocket\" with value %q. "+
+				"Please update your configuration!", kubeadmapiv1.DefaultContainerRuntimeURLScheme, cfg.CRISocket)
+			cfg.CRISocket = kubeadmapiv1.DefaultContainerRuntimeURLScheme + "://" + cfg.CRISocket
+		}
 	}
 
 	return nil

cmd/kubeadm/app/util/marshal_test.go

@@ -119,7 +119,7 @@ func TestMarshalUnmarshalToYamlForCodecs(t *testing.T) {
 		},
 		NodeRegistration: kubeadmapiv1.NodeRegistrationOptions{
 			Name:      "testNode",
-			CRISocket: "/var/run/cri.sock",
+			CRISocket: "unix:///var/run/cri.sock",
 		},
 		BootstrapTokens: []bootstraptokenv1.BootstrapToken{
 			{

cmd/kubeadm/app/util/runtime/runtime.go

@@ -14,11 +14,9 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package util
+package runtime
 
 import (
-	"path/filepath"
-	goruntime "runtime"
 	"strings"
 
 	"github.com/pkg/errors"
@@ -57,12 +55,6 @@ func NewContainerRuntime(execer utilsexec.Interface, criSocket string) (Containe
 
 	if criSocket != constants.DefaultDockerCRISocket {
 		toolName = "crictl"
-		// !!! temporary work around crictl warning:
-		// Using "/var/run/crio/crio.sock" as endpoint is deprecated,
-		// please consider using full url format "unix:///var/run/crio/crio.sock"
-		if filepath.IsAbs(criSocket) && goruntime.GOOS != "windows" {
-			criSocket = "unix://" + criSocket
-		}
 		runtime = &CRIRuntime{execer, criSocket}
 	} else {
 		toolName = "docker"
@@ -198,7 +190,7 @@ func detectCRISocketImpl(isSocket func(string) bool) (string, error) {
 	foundCRISockets := []string{}
 	knownCRISockets := []string{
 		// Docker and containerd sockets are special cased below, hence not to be included here
-		"/var/run/crio/crio.sock",
+		"unix:///var/run/crio/crio.sock",
 	}
 
 	if isSocket(dockerSocket) {

cmd/kubeadm/app/util/runtime/runtime_test.go

@@ -14,7 +14,7 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package util
+package runtime
 
 import (
 	"io/ioutil"
@@ -48,7 +48,6 @@ func TestNewContainerRuntime(t *testing.T) {
 	}{
 		{"valid: default cri socket", execLookPathOK, constants.DefaultDockerCRISocket, true, false},
 		{"valid: cri-o socket url", execLookPathOK, "unix:///var/run/crio/crio.sock", false, false},
-		{"valid: cri-o socket path", execLookPathOK, "/var/run/crio/crio.sock", false, false},
 		{"invalid: no crictl", execLookPathErr, "unix:///var/run/crio/crio.sock", false, true},
 	}
 
@@ -351,7 +350,7 @@ func TestIsExistingSocket(t *testing.T) {
 				}
 				defer con.Close()
 
-				if !isExistingSocket(theSocket) {
+				if !isExistingSocket("unix://" + theSocket) {
 					t.Fatalf("isExistingSocket(%q) gave unexpected result. Should have been true, instead of false", theSocket)
 				}
 			},
@@ -403,29 +402,29 @@ func TestDetectCRISocketImpl(t *testing.T) {
 		},
 		{
 			name:            "One valid CRI socket leads to success",
-			existingSockets: []string{"/var/run/crio/crio.sock"},
+			existingSockets: []string{"unix:///var/run/crio/crio.sock"},
 			expectedError:   false,
-			expectedSocket:  "/var/run/crio/crio.sock",
+			expectedSocket:  "unix:///var/run/crio/crio.sock",
 		},
 		{
 			name:            "Correct Docker CRI socket is returned",
-			existingSockets: []string{"/var/run/docker.sock"},
+			existingSockets: []string{"unix:///var/run/docker.sock"},
 			expectedError:   false,
 			expectedSocket:  constants.DefaultDockerCRISocket,
 		},
 		{
 			name: "CRI and Docker sockets lead to an error",
 			existingSockets: []string{
-				"/var/run/docker.sock",
+				"unix:///var/run/docker.sock",
-				"/var/run/crio/crio.sock",
+				"unix:///var/run/crio/crio.sock",
 			},
 			expectedError: true,
 		},
 		{
 			name: "Docker and containerd lead to Docker being used",
 			existingSockets: []string{
-				"/var/run/docker.sock",
+				"unix:///var/run/docker.sock",
-				"/run/containerd/containerd.sock",
+				"unix:///run/containerd/containerd.sock",
 			},
 			expectedError:  false,
 			expectedSocket: constants.DefaultDockerCRISocket,
@@ -433,8 +432,8 @@ func TestDetectCRISocketImpl(t *testing.T) {
 		{
 			name: "A couple of CRI sockets lead to an error",
 			existingSockets: []string{
-				"/var/run/crio/crio.sock",
+				"unix:///var/run/crio/crio.sock",
-				"/run/containerd/containerd.sock",
+				"unix:///run/containerd/containerd.sock",
 			},
 			expectedError: true,
 		},

cmd/kubeadm/app/util/runtime/runtime_unix.go

@@ -17,23 +17,30 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package util
+package runtime
 
 import (
-	"os"
+	"net"
+	"net/url"
 )
 
 const (
-	dockerSocket     = "/var/run/docker.sock" // The Docker socket is not CRI compatible
+	dockerSocket     = "unix:///var/run/docker.sock" // The Docker socket is not CRI compatible
-	containerdSocket = "/run/containerd/containerd.sock"
+	containerdSocket = "unix:///run/containerd/containerd.sock"
 )
 
 // isExistingSocket checks if path exists and is domain socket
 func isExistingSocket(path string) bool {
-	fileInfo, err := os.Stat(path)
+	u, err := url.Parse(path)
 	if err != nil {
+		// should not happen, since we are trying to access known / hardcoded sockets
 		return false
 	}
 
-	return fileInfo.Mode()&os.ModeSocket != 0
+	c, err := net.Dial(u.Scheme, u.Path)
+	if err != nil {
+		return false
+	}
+	defer c.Close()
+	return true
 }

cmd/kubeadm/app/util/runtime/runtime_windows.go

@@ -17,20 +17,29 @@ See the License for the specific language governing permissions and
 limitations under the License.
 */
 
-package util
+package runtime
 
 import (
+	"net/url"
+
 	winio "github.com/Microsoft/go-winio"
 )
 
 const (
-	dockerSocket     = "//./pipe/docker_engine"         // The Docker socket is not CRI compatible
+	dockerSocket     = "npipe:////./pipe/docker_engine"         // The Docker socket is not CRI compatible
-	containerdSocket = "//./pipe/containerd-containerd" // Proposed containerd named pipe for Windows
+	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 {
-	_, err := winio.DialPipe(path, nil)
+	u, err := url.Parse(path)
+	if err != nil {
+		// should not happen, since we are trying to access known / hardcoded sockets
+		return false
+	}
+
+	// the dial path must be without "npipe://"
+	_, err = winio.DialPipe(u.Path, nil)
 	if err != nil {
 		return false
 	}