Merge pull request #107270 from SataQiu/refactor-20211231

kubeadm: use build tags and split the Windows releated logic into separate files for kubelet component config

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()

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)
-			}
-		})
-	}
-}

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
+}

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)
+}

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)
+			}
+		})
+	}
+}