diff --git a/cmd/kubeadm/app/apis/kubeadm/v1beta2/defaults_unix.go b/cmd/kubeadm/app/apis/kubeadm/v1beta2/defaults_unix.go index 2ec1487c5a7..e87626a28f2 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1beta2/defaults_unix.go +++ b/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 - DefaultUrlScheme = "unix" + // DefaultContainerRuntimeURLScheme defines default socket url prefix + DefaultContainerRuntimeURLScheme = "unix" ) diff --git a/cmd/kubeadm/app/apis/kubeadm/v1beta2/defaults_windows.go b/cmd/kubeadm/app/apis/kubeadm/v1beta2/defaults_windows.go index 3f61b23bbeb..2d770e39c15 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1beta2/defaults_windows.go +++ b/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 - DefaultUrlScheme = "npipe" + // DefaultContainerRuntimeURLScheme defines default socket url prefix + DefaultContainerRuntimeURLScheme = "npipe" ) diff --git a/cmd/kubeadm/app/apis/kubeadm/v1beta2/doc.go b/cmd/kubeadm/app/apis/kubeadm/v1beta2/doc.go index 071bd6567e5..be5ea5bbf9b 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: "/var/run/dockershim.sock" +// criSocket: "unix:///var/run/dockershim.sock" // taints: // - key: "kubeadmNode" // value: "master" diff --git a/cmd/kubeadm/app/apis/kubeadm/v1beta3/defaults_unix.go b/cmd/kubeadm/app/apis/kubeadm/v1beta3/defaults_unix.go index 673e4f3784c..7779e64021f 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1beta3/defaults_unix.go +++ b/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 - DefaultUrlScheme = "unix" + // DefaultContainerRuntimeURLScheme defines default socket url prefix + DefaultContainerRuntimeURLScheme = "unix" ) diff --git a/cmd/kubeadm/app/apis/kubeadm/v1beta3/defaults_windows.go b/cmd/kubeadm/app/apis/kubeadm/v1beta3/defaults_windows.go index f41b3d02cd5..c2f424efc62 100644 --- a/cmd/kubeadm/app/apis/kubeadm/v1beta3/defaults_windows.go +++ b/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 - DefaultUrlScheme = "npipe" + // DefaultContainerRuntimeURLScheme defines default socket url prefix + DefaultContainerRuntimeURLScheme = "npipe" ) diff --git a/cmd/kubeadm/app/apis/kubeadm/v1beta3/doc.go b/cmd/kubeadm/app/apis/kubeadm/v1beta3/doc.go index 7234f2e4cca..37a861885c9 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: "/var/run/dockershim.sock" +// criSocket: "unix:///var/run/dockershim.sock" // taints: // - key: "kubeadmNode" // value: "master" diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation.go index 356c505a60d..5bdd8864360 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation.go +++ b/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 == "" { - if !filepath.IsAbs(u.Path) { - return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("path is not absolute: %s", socket))) - } - } else if u.Scheme != kubeadmapiv1.DefaultUrlScheme { - return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("URL scheme %s is not supported", u.Scheme))) + // static and dynamic defaulting should have ensured that an URL scheme is used + if u.Scheme != kubeadmapiv1.DefaultContainerRuntimeURLScheme { + return append(allErrs, field.Invalid(fldPath, socket, fmt.Sprintf("only URL scheme %q is supported, got %q", kubeadmapiv1.DefaultContainerRuntimeURLScheme, u.Scheme))) } return allErrs diff --git a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go b/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go index 2d0063a5739..9a87bcb721d 100644 --- a/cmd/kubeadm/app/apis/kubeadm/validation/validation_test.go +++ b/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.DefaultUrlScheme + "://" + "/some/path", expectedErrors: false}, - {name: "unsupported url scheme", criSocket: "bla:///some/path", expectedErrors: true}, - {name: "unparseable url", criSocket: ":::", expectedErrors: true}, - {name: "invalid CRISocket (path is not absolute)", criSocket: "some/path", expectedErrors: true}, - {name: "empty CRISocket (path is not absolute)", criSocket: "", expectedErrors: true}, + {name: "valid socket URL", criSocket: kubeadmapiv1.DefaultContainerRuntimeURLScheme + "://" + "/some/path", expectedErrors: false}, + {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: "empty CRISocket", criSocket: "", expectedErrors: true}, } for _, tc := range tests { actual := ValidateSocketPath(tc.criSocket, field.NewPath("criSocket")) diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/data.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/data.go index b4f24861eb0..c6ab05a970f 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/data.go +++ b/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 } diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go index d4602fea94d..a31e2c55173 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go +++ b/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 diff --git a/cmd/kubeadm/app/cmd/upgrade/node.go b/cmd/kubeadm/app/cmd/upgrade/node.go index 08b4bf07c46..7072312a0b8 100644 --- a/cmd/kubeadm/app/cmd/upgrade/node.go +++ b/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 +} diff --git a/cmd/kubeadm/app/constants/constants.go b/cmd/kubeadm/app/constants/constants.go index c2b8f6e64be..20a8bd36dae 100644 --- a/cmd/kubeadm/app/constants/constants.go +++ b/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" diff --git a/cmd/kubeadm/app/constants/constants_unix.go b/cmd/kubeadm/app/constants/constants_unix.go index 812faf064b5..3d2ecec90f3 100644 --- a/cmd/kubeadm/app/constants/constants_unix.go +++ b/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" ) diff --git a/cmd/kubeadm/app/phases/kubelet/flags_test.go b/cmd/kubeadm/app/phases/kubelet/flags_test.go index 3e810ac0bc4..0b4256d7106 100644 --- a/cmd/kubeadm/app/phases/kubelet/flags_test.go +++ b/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", }, }, } diff --git a/cmd/kubeadm/app/phases/patchnode/patchnode_test.go b/cmd/kubeadm/app/phases/patchnode/patchnode_test.go index cae45bb3ed5..f0a5656e3ad 100644 --- a/cmd/kubeadm/app/phases/patchnode/patchnode_test.go +++ b/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", - expectedPatch: `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"/run/containerd/containerd.sock"}}}`, + newCRISocketAnnotation: "unix:///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", - newCRISocketAnnotation: "/run/containerd/containerd.sock", + currentCRISocketAnnotation: "unix:///run/containerd/containerd.sock", + newCRISocketAnnotation: "unix:///run/containerd/containerd.sock", expectedPatch: `{}`, }, { name: "CRI-socket annotation needs to be updated", - currentCRISocketAnnotation: "/var/run/dockershim.sock", - newCRISocketAnnotation: "/run/containerd/containerd.sock", - expectedPatch: `{"metadata":{"annotations":{"kubeadm.alpha.kubernetes.io/cri-socket":"/run/containerd/containerd.sock"}}}`, + currentCRISocketAnnotation: "unix:///var/run/dockershim.sock", + 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/phases/upgrade/postupgrade.go b/cmd/kubeadm/app/phases/upgrade/postupgrade.go index d388194c276..c3e169489de 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade.go +++ b/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) +} diff --git a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go b/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go index 04ede0a0e3e..c5c9472f408 100644 --- a/cmd/kubeadm/app/phases/upgrade/postupgrade_test.go +++ b/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) + } + }) + } +} diff --git a/cmd/kubeadm/app/util/config/cluster.go b/cmd/kubeadm/app/util/config/cluster.go index 730728b374c..242d82940b9 100644 --- a/cmd/kubeadm/app/util/config/cluster.go +++ b/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 -func getNodeRegistration(kubeconfigDir string, client clientset.Interface, nodeRegistration *kubeadmapi.NodeRegistrationOptions) error { +// GetNodeRegistration returns the nodeRegistration for the current node +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 diff --git a/cmd/kubeadm/app/util/config/cluster_test.go b/cmd/kubeadm/app/util/config/cluster_test.go index e1736401736..019997d3f94 100644 --- a/cmd/kubeadm/app/util/config/cluster_test.go +++ b/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 diff --git a/cmd/kubeadm/app/util/config/initconfiguration.go b/cmd/kubeadm/app/util/config/initconfiguration.go index b461b2388bf..2c691491523 100644 --- a/cmd/kubeadm/app/util/config/initconfiguration.go +++ b/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 diff --git a/cmd/kubeadm/app/util/marshal_test.go b/cmd/kubeadm/app/util/marshal_test.go index 4e7b02fc20d..e31bd3b0b44 100644 --- a/cmd/kubeadm/app/util/marshal_test.go +++ b/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{ { diff --git a/cmd/kubeadm/app/util/runtime/runtime.go b/cmd/kubeadm/app/util/runtime/runtime.go index e422cef9e59..11cd8bbc1fe 100644 --- a/cmd/kubeadm/app/util/runtime/runtime.go +++ b/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) { diff --git a/cmd/kubeadm/app/util/runtime/runtime_test.go b/cmd/kubeadm/app/util/runtime/runtime_test.go index ca0c5a502b1..2995e82e8a1 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_test.go +++ b/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", - "/var/run/crio/crio.sock", + "unix:///var/run/docker.sock", + "unix:///var/run/crio/crio.sock", }, expectedError: true, }, { name: "Docker and containerd lead to Docker being used", existingSockets: []string{ - "/var/run/docker.sock", - "/run/containerd/containerd.sock", + "unix:///var/run/docker.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", - "/run/containerd/containerd.sock", + "unix:///var/run/crio/crio.sock", + "unix:///run/containerd/containerd.sock", }, expectedError: true, }, diff --git a/cmd/kubeadm/app/util/runtime/runtime_unix.go b/cmd/kubeadm/app/util/runtime/runtime_unix.go index 11bc059dea7..b0695324ee0 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_unix.go +++ b/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 - containerdSocket = "/run/containerd/containerd.sock" + 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 { - 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 } diff --git a/cmd/kubeadm/app/util/runtime/runtime_windows.go b/cmd/kubeadm/app/util/runtime/runtime_windows.go index 35a84cd3855..b82f6b8f727 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_windows.go +++ b/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 - containerdSocket = "//./pipe/containerd-containerd" // Proposed containerd named pipe for Windows + 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 { - _, 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 }