Merge pull request #90513 from SataQiu/fix-kubeadm-20200427

kubeadm: do not set deprecated '--cgroup-driver' flag in kubeadm-flags.env, it will be set in config.yaml

cmd/kubeadm/app/apis/kubeadm/types.go

@@ -444,7 +444,7 @@ type ComponentConfig interface {
 	Unmarshal(docmap DocumentMap) error
 
 	// Default patches the component config with kubeadm preferred defaults
-	Default(cfg *ClusterConfiguration, localAPIEndpoint *APIEndpoint)
+	Default(cfg *ClusterConfiguration, localAPIEndpoint *APIEndpoint, nodeRegOpts *NodeRegistrationOptions)
 }
 
 // ComponentConfigMap is a map between a group name (as in GVK group) and a ComponentConfig

cmd/kubeadm/app/componentconfigs/BUILD

@@ -32,6 +32,7 @@ go_library(
         "//staging/src/k8s.io/kubelet/config/v1beta1:go_default_library",
         "//vendor/github.com/pkg/errors:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
+        "//vendor/k8s.io/utils/exec:go_default_library",
         "//vendor/k8s.io/utils/pointer:go_default_library",
     ],
 )

cmd/kubeadm/app/componentconfigs/configset.go

@@ -139,17 +139,17 @@ func ensureInitializedComponentConfigs(clusterCfg *kubeadmapi.ClusterConfigurati
 }
 
 // Default sets up defaulted component configs in the supplied ClusterConfiguration
-func Default(clusterCfg *kubeadmapi.ClusterConfiguration, localAPIEndpoint *kubeadmapi.APIEndpoint) {
+func Default(clusterCfg *kubeadmapi.ClusterConfiguration, localAPIEndpoint *kubeadmapi.APIEndpoint, nodeRegOpts *kubeadmapi.NodeRegistrationOptions) {
 	ensureInitializedComponentConfigs(clusterCfg)
 
 	for _, handler := range known {
 		// If the component config exists, simply default it. Otherwise, create it before defaulting.
 		group := handler.GroupVersion.Group
 		if componentCfg, ok := clusterCfg.ComponentConfigs[group]; ok {
-			componentCfg.Default(clusterCfg, localAPIEndpoint)
+			componentCfg.Default(clusterCfg, localAPIEndpoint, nodeRegOpts)
 		} else {
 			componentCfg := handler.CreateEmpty()
-			componentCfg.Default(clusterCfg, localAPIEndpoint)
+			componentCfg.Default(clusterCfg, localAPIEndpoint, nodeRegOpts)
 			clusterCfg.ComponentConfigs[group] = componentCfg
 		}
 	}

cmd/kubeadm/app/componentconfigs/configset_test.go

@@ -35,8 +35,9 @@ import (
 func TestDefault(t *testing.T) {
 	clusterCfg := &kubeadmapi.ClusterConfiguration{}
 	localAPIEndpoint := &kubeadmapi.APIEndpoint{}
+	nodeRegOps := &kubeadmapi.NodeRegistrationOptions{}
 
-	Default(clusterCfg, localAPIEndpoint)
+	Default(clusterCfg, localAPIEndpoint, nodeRegOps)
 
 	if len(clusterCfg.ComponentConfigs) != len(known) {
 		t.Errorf("missmatch between supported and defaulted type numbers:\n\tgot: %d\n\texpected: %d", len(clusterCfg.ComponentConfigs), len(known))

cmd/kubeadm/app/componentconfigs/kubelet.go

@@ -24,12 +24,14 @@ import (
 	"k8s.io/klog"
 	kubeletconfig "k8s.io/kubelet/config/v1beta1"
 	"k8s.io/kubernetes/cmd/kubeadm/app/util/initsystem"
+	utilsexec "k8s.io/utils/exec"
 	utilpointer "k8s.io/utils/pointer"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	kubeadmapiv1beta2 "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm/v1beta2"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/features"
+	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
 )
 
 const (
@@ -96,7 +98,7 @@ func (kc *kubeletConfig) Unmarshal(docmap kubeadmapi.DocumentMap) error {
 	return kubeletHandler.Unmarshal(docmap, &kc.config)
 }
 
-func (kc *kubeletConfig) Default(cfg *kubeadmapi.ClusterConfiguration, _ *kubeadmapi.APIEndpoint) {
+func (kc *kubeletConfig) Default(cfg *kubeadmapi.ClusterConfiguration, _ *kubeadmapi.APIEndpoint, nodeRegOpts *kubeadmapi.NodeRegistrationOptions) {
 	const kind = "KubeletConfiguration"
 
 	if kc.config.FeatureGates == nil {
@@ -179,6 +181,21 @@ func (kc *kubeletConfig) Default(cfg *kubeadmapi.ClusterConfiguration, _ *kubead
 	// There is no way to determine if the user has set this or not, given the field is a non-pointer.
 	kc.config.RotateCertificates = kubeletRotateCertificates
 
+	// TODO: Conditionally set CgroupDriver to either `systemd` or `cgroupfs` for CRI other than Docker
+	if nodeRegOpts.CRISocket == constants.DefaultDockerCRISocket {
+		driver, err := kubeadmutil.GetCgroupDriverDocker(utilsexec.New())
+		if err != nil {
+			klog.Warningf("cannot automatically set CgroupDriver when starting the Kubelet: %v", err)
+		} else {
+			// if we can parse the right cgroup driver from docker info,
+			// we should always override CgroupDriver here no matter user specifies this value explicitly or not
+			if kc.config.CgroupDriver != "" && kc.config.CgroupDriver != driver {
+				klog.Warningf("detected %q as the Docker cgroup driver, the provided value %q in %q will be overrided", driver, kc.config.CgroupDriver, kind)
+			}
+			kc.config.CgroupDriver = driver
+		}
+	}
+
 	ok, err := isServiceActive("systemd-resolved")
 	if err != nil {
 		klog.Warningf("cannot determine if systemd-resolved is active: %v", err)

cmd/kubeadm/app/componentconfigs/kubelet_test.go

@@ -323,7 +323,7 @@ func TestKubeletDefault(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			got := &kubeletConfig{}
-			got.Default(&test.clusterCfg, &kubeadmapi.APIEndpoint{})
+			got.Default(&test.clusterCfg, &kubeadmapi.APIEndpoint{}, &kubeadmapi.NodeRegistrationOptions{})
 			if !reflect.DeepEqual(got, &test.expected) {
 				t.Fatalf("Missmatch between expected and got:\nExpected:\n%v\n---\nGot:\n%v", test.expected, got)
 			}

cmd/kubeadm/app/componentconfigs/kubeproxy.go

@@ -77,7 +77,7 @@ func kubeProxyDefaultBindAddress(localAdvertiseAddress string) string {
 	return kubeadmapiv1beta2.DefaultProxyBindAddressv6
 }
 
-func (kp *kubeProxyConfig) Default(cfg *kubeadmapi.ClusterConfiguration, localAPIEndpoint *kubeadmapi.APIEndpoint) {
+func (kp *kubeProxyConfig) Default(cfg *kubeadmapi.ClusterConfiguration, localAPIEndpoint *kubeadmapi.APIEndpoint, _ *kubeadmapi.NodeRegistrationOptions) {
 	const kind = "KubeProxyConfiguration"
 
 	// The below code is necessary because while KubeProxy may be defined, the user may not

cmd/kubeadm/app/componentconfigs/kubeproxy_test.go

@@ -297,7 +297,7 @@ func TestKubeProxyDefault(t *testing.T) {
 	for _, test := range tests {
 		t.Run(test.name, func(t *testing.T) {
 			got := &kubeProxyConfig{}
-			got.Default(&test.clusterCfg, &test.endpoint)
+			got.Default(&test.clusterCfg, &test.endpoint, &kubeadmapi.NodeRegistrationOptions{})
 			if !reflect.DeepEqual(got, &test.expected) {
 				t.Fatalf("Missmatch between expected and got:\nExpected:\n%v\n---\nGot:\n%v", test.expected, got)
 			}

cmd/kubeadm/app/phases/kubelet/BUILD

@@ -6,8 +6,6 @@ go_library(
         "config.go",
         "dynamic.go",
         "flags.go",
-        "flags_unix.go",
-        "flags_windows.go",
         "kubelet.go",
     ],
     importpath = "k8s.io/kubernetes/cmd/kubeadm/app/phases/kubelet",
@@ -29,7 +27,6 @@ go_library(
         "//staging/src/k8s.io/client-go/kubernetes:go_default_library",
         "//vendor/github.com/pkg/errors:go_default_library",
         "//vendor/k8s.io/klog:go_default_library",
-        "//vendor/k8s.io/utils/exec:go_default_library",
     ],
 )
 
@@ -52,8 +49,6 @@ go_test(
         "//staging/src/k8s.io/apimachinery/pkg/util/version:go_default_library",
         "//staging/src/k8s.io/client-go/kubernetes/fake:go_default_library",
         "//staging/src/k8s.io/client-go/testing:go_default_library",
-        "//vendor/github.com/pkg/errors:go_default_library",
-        "//vendor/k8s.io/utils/exec:go_default_library",
     ],
 )
 

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

@@ -18,18 +18,19 @@ package kubelet
 
 import (
 	"fmt"
-	"github.com/pkg/errors"
 	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
+
+	"github.com/pkg/errors"
 	"k8s.io/klog"
+
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
 	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 	"k8s.io/kubernetes/cmd/kubeadm/app/images"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
-	utilsexec "k8s.io/utils/exec"
-	"os"
-	"path/filepath"
-	"strings"
 )
 
 type kubeletFlagsOpts struct {
@@ -37,7 +38,6 @@ type kubeletFlagsOpts struct {
 	featureGates             map[string]bool
 	pauseImage               string
 	registerTaintsUsingFlags bool
-	execer                   utilsexec.Interface
 }
 
 // GetNodeNameAndHostname obtains the name for this Node using the following precedence
@@ -66,7 +66,6 @@ func WriteKubeletDynamicEnvFile(cfg *kubeadmapi.ClusterConfiguration, nodeReg *k
 		featureGates:             cfg.FeatureGates,
 		pauseImage:               images.GetPauseImage(cfg),
 		registerTaintsUsingFlags: registerTaintsUsingFlags,
-		execer:                   utilsexec.New(),
 	}
 	stringMap := buildKubeletArgMap(flagOpts)
 	argList := kubeadmutil.BuildArgumentListFromMap(stringMap, nodeReg.KubeletExtraArgs)
@@ -133,3 +132,9 @@ func writeKubeletFlagBytesToDisk(b []byte, kubeletDir string) error {
 	}
 	return nil
 }
+
+// buildKubeletArgMap takes a kubeletFlagsOpts object and builds based on that a string-string map with flags
+// that should be given to the local kubelet daemon.
+func buildKubeletArgMap(opts kubeletFlagsOpts) map[string]string {
+	return buildKubeletArgMapCommon(opts)
+}

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

@@ -17,76 +17,11 @@ limitations under the License.
 package kubelet
 
 import (
-	"context"
-	"io"
 	"reflect"
-	"strings"
 	"testing"
 
-	"github.com/pkg/errors"
-
 	v1 "k8s.io/api/core/v1"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
-	"k8s.io/utils/exec"
-)
-
-type fakeCmd struct {
-	b   []byte
-	err error
-}
-
-func (f fakeCmd) Run() error                         { return f.err }
-func (f fakeCmd) CombinedOutput() ([]byte, error)    { return f.b, f.err }
-func (f fakeCmd) Output() ([]byte, error)            { return f.b, f.err }
-func (f fakeCmd) SetDir(dir string)                  {}
-func (f fakeCmd) SetStdin(in io.Reader)              {}
-func (f fakeCmd) SetStdout(out io.Writer)            {}
-func (f fakeCmd) SetStderr(out io.Writer)            {}
-func (f fakeCmd) SetEnv([]string)                    {}
-func (f fakeCmd) Stop()                              {}
-func (f fakeCmd) Start() error                       { return nil }
-func (f fakeCmd) Wait() error                        { return nil }
-func (f fakeCmd) StdoutPipe() (io.ReadCloser, error) { return nil, nil }
-func (f fakeCmd) StderrPipe() (io.ReadCloser, error) { return nil, nil }
-
-type fakeExecer struct {
-	ioMap map[string]fakeCmd
-}
-
-func (f fakeExecer) Command(cmd string, args ...string) exec.Cmd {
-	cmds := []string{cmd}
-	cmds = append(cmds, args...)
-	return f.ioMap[strings.Join(cmds, " ")]
-}
-func (f fakeExecer) CommandContext(ctx context.Context, cmd string, args ...string) exec.Cmd {
-	return f.Command(cmd, args...)
-}
-func (f fakeExecer) LookPath(file string) (string, error) { return "", errors.New("unknown binary") }
-
-var (
-	systemdCgroupExecer = fakeExecer{
-		ioMap: map[string]fakeCmd{
-			"docker info -f {{.CgroupDriver}}": {
-				b: []byte(`systemd`),
-			},
-		},
-	}
-
-	cgroupfsCgroupExecer = fakeExecer{
-		ioMap: map[string]fakeCmd{
-			"docker info -f {{.CgroupDriver}}": {
-				b: []byte(`cgroupfs`),
-			},
-		},
-	}
-
-	errCgroupExecer = fakeExecer{
-		ioMap: map[string]fakeCmd{
-			"docker info -f {{.CgroupDriver}}": {
-				err: errors.New("no such binary: docker"),
-			},
-		},
-	}
 )
 
 func TestBuildKubeletArgMap(t *testing.T) {
@@ -109,7 +44,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 						},
 					},
 				},
-				execer: errCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin": "cni",
@@ -122,7 +56,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 					CRISocket: "/var/run/dockershim.sock",
 					Name:      "override-name",
 				},
-				execer: errCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin":    "cni",
@@ -136,46 +69,18 @@ func TestBuildKubeletArgMap(t *testing.T) {
 					CRISocket:        "/var/run/dockershim.sock",
 					KubeletExtraArgs: map[string]string{"hostname-override": "override-name"},
 				},
-				execer: errCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin":    "cni",
 				"hostname-override": "override-name",
 			},
 		},
-		{
-			name: "systemd cgroup driver",
-			opts: kubeletFlagsOpts{
-				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/dockershim.sock",
-				},
-				execer: systemdCgroupExecer,
-			},
-			expected: map[string]string{
-				"network-plugin": "cni",
-				"cgroup-driver":  "systemd",
-			},
-		},
-		{
-			name: "cgroupfs cgroup driver",
-			opts: kubeletFlagsOpts{
-				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/dockershim.sock",
-				},
-				execer: cgroupfsCgroupExecer,
-			},
-			expected: map[string]string{
-				"network-plugin": "cni",
-				"cgroup-driver":  "cgroupfs",
-			},
-		},
 		{
 			name: "external CRI runtime",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/containerd.sock",
 				},
-				execer: cgroupfsCgroupExecer,
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
@@ -201,7 +106,6 @@ func TestBuildKubeletArgMap(t *testing.T) {
 					},
 				},
 				registerTaintsUsingFlags: true,
-				execer:                   cgroupfsCgroupExecer,
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
@@ -216,11 +120,9 @@ func TestBuildKubeletArgMap(t *testing.T) {
 					CRISocket: "/var/run/dockershim.sock",
 				},
 				pauseImage: "gcr.io/pause:3.2",
-				execer:     cgroupfsCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin":            "cni",
-				"cgroup-driver":             "cgroupfs",
 				"pod-infra-container-image": "gcr.io/pause:3.2",
 			},
 		},

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

@@ -1,43 +0,0 @@
-// +build !windows
-
-/*
-Copyright 2020 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 kubelet
-
-import (
-	"k8s.io/klog"
-	"k8s.io/kubernetes/cmd/kubeadm/app/constants"
-	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
-)
-
-// buildKubeletArgMap takes a kubeletFlagsOpts object and builds based on that a string-string map with flags
-// that should be given to the local Linux kubelet daemon.
-func buildKubeletArgMap(opts kubeletFlagsOpts) map[string]string {
-	kubeletFlags := buildKubeletArgMapCommon(opts)
-
-	// TODO: Conditionally set `--cgroup-driver` to either `systemd` or `cgroupfs` for CRI other than Docker
-	if opts.nodeRegOpts.CRISocket == constants.DefaultDockerCRISocket {
-		driver, err := kubeadmutil.GetCgroupDriverDocker(opts.execer)
-		if err != nil {
-			klog.Warningf("cannot automatically assign a '--cgroup-driver' value when starting the Kubelet: %v\n", err)
-		} else {
-			kubeletFlags["cgroup-driver"] = driver
-		}
-	}
-
-	return kubeletFlags
-}

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

@@ -1,25 +0,0 @@
-// +build windows
-
-/*
-Copyright 2020 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 kubelet
-
-// buildKubeletArgMap takes a kubeletFlagsOpts object and builds based on that a string-string map with flags
-// that should be given to the local Windows kubelet daemon.
-func buildKubeletArgMap(opts kubeletFlagsOpts) map[string]string {
-	return buildKubeletArgMapCommon(opts)
-}

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

@@ -53,7 +53,7 @@ func SetInitDynamicDefaults(cfg *kubeadmapi.InitConfiguration) error {
 	if err := SetAPIEndpointDynamicDefaults(&cfg.LocalAPIEndpoint); err != nil {
 		return err
 	}
-	return SetClusterDynamicDefaults(&cfg.ClusterConfiguration, &cfg.LocalAPIEndpoint)
+	return SetClusterDynamicDefaults(&cfg.ClusterConfiguration, &cfg.LocalAPIEndpoint, &cfg.NodeRegistration)
 }
 
 // SetBootstrapTokensDynamicDefaults checks and sets configuration values for the BootstrapTokens object
@@ -141,9 +141,9 @@ func SetAPIEndpointDynamicDefaults(cfg *kubeadmapi.APIEndpoint) error {
 }
 
 // SetClusterDynamicDefaults checks and sets values for the ClusterConfiguration object
-func SetClusterDynamicDefaults(cfg *kubeadmapi.ClusterConfiguration, LocalAPIEndpoint *kubeadmapi.APIEndpoint) error {
+func SetClusterDynamicDefaults(cfg *kubeadmapi.ClusterConfiguration, localAPIEndpoint *kubeadmapi.APIEndpoint, nodeRegOpts *kubeadmapi.NodeRegistrationOptions) error {
 	// Default all the embedded ComponentConfig structs
-	componentconfigs.Default(cfg, LocalAPIEndpoint)
+	componentconfigs.Default(cfg, localAPIEndpoint, nodeRegOpts)
 
 	// Resolve possible version labels and validate version string
 	if err := NormalizeKubernetesVersion(cfg); err != nil {
@@ -159,7 +159,7 @@ func SetClusterDynamicDefaults(cfg *kubeadmapi.ClusterConfiguration, LocalAPIEnd
 			return err
 		}
 		if port == "" {
-			cfg.ControlPlaneEndpoint = net.JoinHostPort(host, strconv.FormatInt(int64(LocalAPIEndpoint.BindPort), 10))
+			cfg.ControlPlaneEndpoint = net.JoinHostPort(host, strconv.FormatInt(int64(localAPIEndpoint.BindPort), 10))
 		}
 	}