From 7594f0ef9017d14088fb04076b41779abfd1d6fe Mon Sep 17 00:00:00 2001 From: pacoxu Date: Fri, 11 Jun 2021 16:05:41 +0800 Subject: [PATCH 1/3] kubeadm: detect runtime socket as URL format - Update defaults for v1beta2 and 3 to have URL scheme - Raname DefaultUrlScheme to DefaultContainerRuntimeURLScheme - Prepend a missing URL scheme to user sockets and warn them that this might not be supported in the future - Update socket validation to exclude IsAbs() testing (This is broken on Windows). Assume the path is not empty and has URL scheme at this point (validation happens after defaulting). - Use net.Dial to open Unix sockets - Update all related unit tests Signed-off-by: pacoxu Signed-off-by: Lubomir I. Ivanov --- .../app/apis/kubeadm/v1beta2/defaults_unix.go | 4 +-- .../apis/kubeadm/v1beta2/defaults_windows.go | 4 +-- cmd/kubeadm/app/apis/kubeadm/v1beta2/doc.go | 2 +- .../app/apis/kubeadm/v1beta3/defaults_unix.go | 4 +-- .../apis/kubeadm/v1beta3/defaults_windows.go | 4 +-- cmd/kubeadm/app/apis/kubeadm/v1beta3/doc.go | 2 +- .../app/apis/kubeadm/validation/validation.go | 13 +++++---- .../kubeadm/validation/validation_test.go | 29 +++++++++---------- cmd/kubeadm/app/constants/constants.go | 2 +- cmd/kubeadm/app/constants/constants_unix.go | 2 +- cmd/kubeadm/app/phases/kubelet/flags_test.go | 22 +++++++------- .../app/phases/patchnode/patchnode_test.go | 14 ++++----- .../app/util/config/initconfiguration.go | 8 +++++ cmd/kubeadm/app/util/marshal_test.go | 2 +- cmd/kubeadm/app/util/runtime/runtime.go | 12 ++------ cmd/kubeadm/app/util/runtime/runtime_test.go | 23 +++++++-------- cmd/kubeadm/app/util/runtime/runtime_unix.go | 19 ++++++++---- .../app/util/runtime/runtime_windows.go | 17 ++++++++--- 18 files changed, 99 insertions(+), 84 deletions(-) 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/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/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 } From 207556e057d4cdb9b0fad35aef0733ec0479e4f0 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Wed, 29 Dec 2021 01:59:07 +0200 Subject: [PATCH 2/3] kubeadm: make "upgrade node" include URL scheme in socket paths The CRI socket that kubeadm writes as an annotation on a particular Node object can include an endpoint that does not have an URL scheme. This is undesired as long term the kubelet can stop allowing endpoints without URL scheme. For control plane nodes "kubeadm upgrade apply" takes the locally defaulted / populated NodeRegistration and refreshes the CRI socket in PerformPostUpgradeTasks. But for secondary nodes "kubeadm upgrade node" does not. Adapt "upgrade node" to fetch the NodeRegistration for this node and fix the CRI socket missing URL scheme if needed in the Node annotation. --- .../app/cmd/phases/upgrade/node/data.go | 1 + .../cmd/phases/upgrade/node/kubeletconfig.go | 35 +++++++++++++++++++ cmd/kubeadm/app/cmd/upgrade/node.go | 7 ++++ cmd/kubeadm/app/util/config/cluster.go | 12 +++---- cmd/kubeadm/app/util/config/cluster_test.go | 4 +-- 5 files changed, 51 insertions(+), 8 deletions(-) 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..04ddeff3ff0 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,34 @@ 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") + } + } + 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/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 From 39330c4278b4d7581685c12f6c973f00905eb5f3 Mon Sep 17 00:00:00 2001 From: "Lubomir I. Ivanov" Date: Mon, 3 Jan 2022 20:23:18 +0200 Subject: [PATCH 3/3] kubeadm: ensure CRI URL scheme is present in the kubelet env file During "upgrade node" and "upgrade apply" read the kubelet env file from /var/lib/kubelet/kubeadm-flags.env patch the --container-runtime-endpoint flag value to have the appropriate URL scheme prefix (e.g. unix:// on Linux) and write the file back to disk. This is a temporary workaround that should be kept only for 1 release cycle - i.e. remove this in 1.25. --- .../cmd/phases/upgrade/node/kubeletconfig.go | 6 ++ cmd/kubeadm/app/phases/upgrade/postupgrade.go | 75 +++++++++++++++++++ .../app/phases/upgrade/postupgrade_test.go | 44 +++++++++++ 3 files changed, 125 insertions(+) diff --git a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go index 04ddeff3ff0..a31e2c55173 100644 --- a/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go +++ b/cmd/kubeadm/app/cmd/phases/upgrade/node/kubeletconfig.go @@ -122,6 +122,12 @@ func runKubeletConfigPhase() func(c workflow.RunData) error { } } + // 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/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) + } + }) + } +}