From 2a113424fc45f2d9cf3e1f8d818bc2b1dedda98d Mon Sep 17 00:00:00 2001 From: SataQiu Date: Wed, 5 Jan 2022 16:04:55 +0800 Subject: [PATCH] kubeadm: use build tags and split the Windows releated logic into separate files for kubelet component config --- cmd/kubeadm/app/componentconfigs/kubelet.go | 57 ------------ .../app/componentconfigs/kubelet_test.go | 64 ------------- .../app/componentconfigs/kubelet_unix.go | 25 ++++++ .../app/componentconfigs/kubelet_windows.go | 76 ++++++++++++++++ .../componentconfigs/kubelet_windows_test.go | 90 +++++++++++++++++++ 5 files changed, 191 insertions(+), 121 deletions(-) create mode 100644 cmd/kubeadm/app/componentconfigs/kubelet_unix.go create mode 100644 cmd/kubeadm/app/componentconfigs/kubelet_windows.go create mode 100644 cmd/kubeadm/app/componentconfigs/kubelet_windows_test.go diff --git a/cmd/kubeadm/app/componentconfigs/kubelet.go b/cmd/kubeadm/app/componentconfigs/kubelet.go index bc230df45e9..31b2e94a15b 100644 --- a/cmd/kubeadm/app/componentconfigs/kubelet.go +++ b/cmd/kubeadm/app/componentconfigs/kubelet.go @@ -17,10 +17,7 @@ limitations under the License. package componentconfigs import ( - "os" "path/filepath" - "runtime" - "strings" "github.com/pkg/errors" "k8s.io/apimachinery/pkg/util/version" @@ -233,60 +230,6 @@ func (kc *kubeletConfig) Default(cfg *kubeadmapi.ClusterConfiguration, _ *kubead } } -// Mutate modifies absolute path fields in the KubeletConfiguration to be Windows compatible absolute paths. -func (kc *kubeletConfig) Mutate() error { - // TODO: use build tags and move the Windows related logic to _windows.go files - // once the kubeadm code base is unit tested for Windows as part of CI - "GOOS=windows go test ...". - if runtime.GOOS != "windows" { - return nil - } - - // When "kubeadm join" downloads the KubeletConfiguration from the cluster on Windows - // nodes, it would contain absolute paths that may lack drive letters, since the config - // could have been generated on a Linux control-plane node. On Windows the - // Golang path.IsAbs() function returns false unless the path contains a drive letter. - // This trips client-go and the kubelet, creating problems on Windows nodes. - // Fixing it in client-go or the kubelet is a breaking change to existing Windows - // users that rely on relative paths: - // https://github.com/kubernetes/kubernetes/pull/77710#issuecomment-491989621 - // - // Thus, a workaround here is to adapt the KubeletConfiguration paths for Windows. - // Note this is currently bound to KubeletConfiguration v1beta1. - klog.V(2).Infoln("[componentconfig] Adapting the paths in the KubeletConfiguration for Windows...") - - // Get the drive from where the kubeadm binary was called. - exe, err := os.Executable() - if err != nil { - return errors.Wrap(err, "could not obtain information about the kubeadm executable") - } - drive := filepath.VolumeName(filepath.Dir(exe)) - klog.V(2).Infof("[componentconfig] Assuming Windows drive %q", drive) - - // Mutate the paths in the config. - mutatePathsOnWindows(&kc.config, drive) - return nil -} - -func mutatePathsOnWindows(cfg *kubeletconfig.KubeletConfiguration, drive string) { - mutateStringField := func(name string, field *string) { - // path.IsAbs() is not reliable here in the Windows runtime, so check if the - // path starts with "/" instead. This means the path originated from a Unix node and - // is an absolute path. - if !strings.HasPrefix(*field, "/") { - return - } - // Prepend the drive letter to the path and update the field. - *field = filepath.Join(drive, *field) - klog.V(2).Infof("[componentconfig] kubelet/Windows: adapted path for field %q to %q", name, *field) - } - - // Mutate the fields we care about. - klog.V(2).Infof("[componentconfig] kubelet/Windows: changing field \"resolverConfig\" to empty") - cfg.ResolverConfig = utilpointer.String("") - mutateStringField("staticPodPath", &cfg.StaticPodPath) - mutateStringField("authentication.x509.clientCAFile", &cfg.Authentication.X509.ClientCAFile) -} - // isServiceActive checks whether the given service exists and is running func isServiceActive(name string) (bool, error) { initSystem, err := initsystem.GetInitSystem() diff --git a/cmd/kubeadm/app/componentconfigs/kubelet_test.go b/cmd/kubeadm/app/componentconfigs/kubelet_test.go index 2002fca8c89..3b2513df9fd 100644 --- a/cmd/kubeadm/app/componentconfigs/kubelet_test.go +++ b/cmd/kubeadm/app/componentconfigs/kubelet_test.go @@ -298,67 +298,3 @@ func TestKubeletFromCluster(t *testing.T) { return kubeletHandler.FromCluster(client, testClusterCfg(legacyKubeletConfigMap)) }) } - -func TestMutatePathsOnWindows(t *testing.T) { - const drive = "C:" - var fooResolverConfig string = "/foo/resolver" - - tests := []struct { - name string - cfg *kubeletconfig.KubeletConfiguration - expected *kubeletconfig.KubeletConfiguration - }{ - { - name: "valid: all fields are absolute paths", - cfg: &kubeletconfig.KubeletConfiguration{ - ResolverConfig: &fooResolverConfig, - StaticPodPath: "/foo/staticpods", - Authentication: kubeletconfig.KubeletAuthentication{ - X509: kubeletconfig.KubeletX509Authentication{ - ClientCAFile: "/foo/ca.crt", - }, - }, - }, - expected: &kubeletconfig.KubeletConfiguration{ - ResolverConfig: utilpointer.String(""), - StaticPodPath: filepath.Join(drive, "/foo/staticpods"), - Authentication: kubeletconfig.KubeletAuthentication{ - X509: kubeletconfig.KubeletX509Authentication{ - ClientCAFile: filepath.Join(drive, "/foo/ca.crt"), - }, - }, - }, - }, - { - name: "valid: some fields are not absolute paths", - cfg: &kubeletconfig.KubeletConfiguration{ - ResolverConfig: &fooResolverConfig, - StaticPodPath: "./foo/staticpods", // not an absolute Unix path - Authentication: kubeletconfig.KubeletAuthentication{ - X509: kubeletconfig.KubeletX509Authentication{ - ClientCAFile: "/foo/ca.crt", - }, - }, - }, - expected: &kubeletconfig.KubeletConfiguration{ - ResolverConfig: utilpointer.String(""), - StaticPodPath: "./foo/staticpods", - Authentication: kubeletconfig.KubeletAuthentication{ - X509: kubeletconfig.KubeletX509Authentication{ - ClientCAFile: filepath.Join(drive, "/foo/ca.crt"), - }, - }, - }, - }, - } - - for _, test := range tests { - t.Run(test.name, func(t *testing.T) { - mutatePathsOnWindows(test.cfg, drive) - if !reflect.DeepEqual(test.cfg, test.expected) { - t.Errorf("Missmatch between expected and got:\nExpected:\n%+v\n---\nGot:\n%+v", - test.expected, test.cfg) - } - }) - } -} diff --git a/cmd/kubeadm/app/componentconfigs/kubelet_unix.go b/cmd/kubeadm/app/componentconfigs/kubelet_unix.go new file mode 100644 index 00000000000..b5534079e50 --- /dev/null +++ b/cmd/kubeadm/app/componentconfigs/kubelet_unix.go @@ -0,0 +1,25 @@ +//go:build !windows +// +build !windows + +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package componentconfigs + +// Mutate allows applying pre-defined modifications to the config before it's marshaled. +func (kc *kubeletConfig) Mutate() error { + return nil +} diff --git a/cmd/kubeadm/app/componentconfigs/kubelet_windows.go b/cmd/kubeadm/app/componentconfigs/kubelet_windows.go new file mode 100644 index 00000000000..50d4794a9f2 --- /dev/null +++ b/cmd/kubeadm/app/componentconfigs/kubelet_windows.go @@ -0,0 +1,76 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package componentconfigs + +import ( + "os" + "path/filepath" + "strings" + + "github.com/pkg/errors" + "k8s.io/klog/v2" + kubeletconfig "k8s.io/kubelet/config/v1beta1" + utilpointer "k8s.io/utils/pointer" +) + +// Mutate modifies absolute path fields in the KubeletConfiguration to be Windows compatible absolute paths. +func (kc *kubeletConfig) Mutate() error { + // When "kubeadm join" downloads the KubeletConfiguration from the cluster on Windows + // nodes, it would contain absolute paths that may lack drive letters, since the config + // could have been generated on a Linux control-plane node. On Windows the + // Golang path.IsAbs() function returns false unless the path contains a drive letter. + // This trips client-go and the kubelet, creating problems on Windows nodes. + // Fixing it in client-go or the kubelet is a breaking change to existing Windows + // users that rely on relative paths: + // https://github.com/kubernetes/kubernetes/pull/77710#issuecomment-491989621 + // + // Thus, a workaround here is to adapt the KubeletConfiguration paths for Windows. + // Note this is currently bound to KubeletConfiguration v1beta1. + klog.V(2).Infoln("[componentconfig] Adapting the paths in the KubeletConfiguration for Windows...") + + // Get the drive from where the kubeadm binary was called. + exe, err := os.Executable() + if err != nil { + return errors.Wrap(err, "could not obtain information about the kubeadm executable") + } + drive := filepath.VolumeName(filepath.Dir(exe)) + klog.V(2).Infof("[componentconfig] Assuming Windows drive %q", drive) + + // Mutate the paths in the config. + mutatePaths(&kc.config, drive) + return nil +} + +func mutatePaths(cfg *kubeletconfig.KubeletConfiguration, drive string) { + mutateStringField := func(name string, field *string) { + // path.IsAbs() is not reliable here in the Windows runtime, so check if the + // path starts with "/" instead. This means the path originated from a Unix node and + // is an absolute path. + if !strings.HasPrefix(*field, "/") { + return + } + // Prepend the drive letter to the path and update the field. + *field = filepath.Join(drive, *field) + klog.V(2).Infof("[componentconfig] kubelet/Windows: adapted path for field %q to %q", name, *field) + } + + // Mutate the fields we care about. + klog.V(2).Infof("[componentconfig] kubelet/Windows: changing field \"resolverConfig\" to empty") + cfg.ResolverConfig = utilpointer.String("") + mutateStringField("staticPodPath", &cfg.StaticPodPath) + mutateStringField("authentication.x509.clientCAFile", &cfg.Authentication.X509.ClientCAFile) +} diff --git a/cmd/kubeadm/app/componentconfigs/kubelet_windows_test.go b/cmd/kubeadm/app/componentconfigs/kubelet_windows_test.go new file mode 100644 index 00000000000..09c8d25a6f7 --- /dev/null +++ b/cmd/kubeadm/app/componentconfigs/kubelet_windows_test.go @@ -0,0 +1,90 @@ +/* +Copyright 2021 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package componentconfigs + +import ( + "path/filepath" + "reflect" + "testing" + + kubeletconfig "k8s.io/kubelet/config/v1beta1" + utilpointer "k8s.io/utils/pointer" +) + +func TestMutatePaths(t *testing.T) { + const drive = "C:" + var fooResolverConfig string = "/foo/resolver" + + tests := []struct { + name string + cfg *kubeletconfig.KubeletConfiguration + expected *kubeletconfig.KubeletConfiguration + }{ + { + name: "valid: all fields are absolute paths", + cfg: &kubeletconfig.KubeletConfiguration{ + ResolverConfig: &fooResolverConfig, + StaticPodPath: "/foo/staticpods", + Authentication: kubeletconfig.KubeletAuthentication{ + X509: kubeletconfig.KubeletX509Authentication{ + ClientCAFile: "/foo/ca.crt", + }, + }, + }, + expected: &kubeletconfig.KubeletConfiguration{ + ResolverConfig: utilpointer.String(""), + StaticPodPath: filepath.Join(drive, "/foo/staticpods"), + Authentication: kubeletconfig.KubeletAuthentication{ + X509: kubeletconfig.KubeletX509Authentication{ + ClientCAFile: filepath.Join(drive, "/foo/ca.crt"), + }, + }, + }, + }, + { + name: "valid: some fields are not absolute paths", + cfg: &kubeletconfig.KubeletConfiguration{ + ResolverConfig: &fooResolverConfig, + StaticPodPath: "./foo/staticpods", // not an absolute Unix path + Authentication: kubeletconfig.KubeletAuthentication{ + X509: kubeletconfig.KubeletX509Authentication{ + ClientCAFile: "/foo/ca.crt", + }, + }, + }, + expected: &kubeletconfig.KubeletConfiguration{ + ResolverConfig: utilpointer.String(""), + StaticPodPath: "./foo/staticpods", + Authentication: kubeletconfig.KubeletAuthentication{ + X509: kubeletconfig.KubeletX509Authentication{ + ClientCAFile: filepath.Join(drive, "/foo/ca.crt"), + }, + }, + }, + }, + } + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + mutatePaths(test.cfg, drive) + if !reflect.DeepEqual(test.cfg, test.expected) { + t.Errorf("Missmatch between expected and got:\nExpected:\n%+v\n---\nGot:\n%+v", + test.expected, test.cfg) + } + }) + } +}