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

SataQiu · SataQiu · commit afb1ea44b966 · 2020-04-27T15:32:10.000+08:00
diff --git a/cmd/kubeadm/app/apis/kubeadm/types.go b/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
diff --git a/cmd/kubeadm/app/componentconfigs/BUILD b/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",
     ],
 )
diff --git a/cmd/kubeadm/app/componentconfigs/configset.go b/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
 		}
 	}
diff --git a/cmd/kubeadm/app/componentconfigs/configset_test.go b/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))
diff --git a/cmd/kubeadm/app/componentconfigs/kubelet.go b/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)
diff --git a/cmd/kubeadm/app/componentconfigs/kubelet_test.go b/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)
 			}
diff --git a/cmd/kubeadm/app/componentconfigs/kubeproxy.go b/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
diff --git a/cmd/kubeadm/app/componentconfigs/kubeproxy_test.go b/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)
 			}
diff --git a/cmd/kubeadm/app/phases/kubelet/BUILD b/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",
     ],
 )
 
diff --git a/cmd/kubeadm/app/phases/kubelet/flags.go b/cmd/kubeadm/app/phases/kubelet/flags.go
@@ -18,26 +18,26 @@ 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 {
 	nodeRegOpts              *kubeadmapi.NodeRegistrationOptions
 	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)
+}
diff --git a/cmd/kubeadm/app/phases/kubelet/flags_test.go b/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",
 			},
 		},
diff --git a/cmd/kubeadm/app/phases/kubelet/flags_unix.go b/cmd/kubeadm/app/phases/kubelet/flags_unix.go
diff --git a/cmd/kubeadm/app/phases/kubelet/flags_windows.go b/cmd/kubeadm/app/phases/kubelet/flags_windows.go
diff --git a/cmd/kubeadm/app/util/config/initconfiguration.go b/cmd/kubeadm/app/util/config/initconfiguration.go

Original file line number	Diff line number	Diff line change
`@@ -444,7 +444,7 @@ type ComponentConfig interface {`
`444`	`444`	`Unmarshal(docmap DocumentMap) error`
`445`	`445`
`446`	`446`	`// Default patches the component config with kubeadm preferred defaults`
`447`		`- Default(cfg ClusterConfiguration, localAPIEndpoint APIEndpoint)`
	`447`	`+ Default(cfg ClusterConfiguration, localAPIEndpoint APIEndpoint, nodeRegOpts *NodeRegistrationOptions)`
`448`	`448`	`}`
`449`	`449`
`450`	`450`	`// ComponentConfigMap is a map between a group name (as in GVK group) and a ComponentConfig`
Original file line number	Diff line number	Diff line change
`@@ -32,6 +32,7 @@ go_library(`
`32`	`32`	`"//staging/src/k8s.io/kubelet/config/v1beta1:go_default_library",`
`33`	`33`	`"//vendor/github.com/pkg/errors:go_default_library",`
`34`	`34`	`"//vendor/k8s.io/klog:go_default_library",`
	`35`	`+ "//vendor/k8s.io/utils/exec:go_default_library",`
`35`	`36`	`"//vendor/k8s.io/utils/pointer:go_default_library",`
`36`	`37`	`],`
`37`	`38`	`)`
Original file line number	Diff line number	Diff line change
`@@ -139,17 +139,17 @@ func ensureInitializedComponentConfigs(clusterCfg *kubeadmapi.ClusterConfigurati`
`139`	`139`	`}`
`140`	`140`
`141`	`141`	`// Default sets up defaulted component configs in the supplied ClusterConfiguration`
`142`		`-func Default(clusterCfg kubeadmapi.ClusterConfiguration, localAPIEndpoint kubeadmapi.APIEndpoint) {`
	`142`	`+func Default(clusterCfg kubeadmapi.ClusterConfiguration, localAPIEndpoint kubeadmapi.APIEndpoint, nodeRegOpts *kubeadmapi.NodeRegistrationOptions) {`
`143`	`143`	`ensureInitializedComponentConfigs(clusterCfg)`
`144`	`144`
`145`	`145`	`for _, handler := range known {`
`146`	`146`	`// If the component config exists, simply default it. Otherwise, create it before defaulting.`
`147`	`147`	`group := handler.GroupVersion.Group`
`148`	`148`	`if componentCfg, ok := clusterCfg.ComponentConfigs[group]; ok {`
`149`		`- componentCfg.Default(clusterCfg, localAPIEndpoint)`
	`149`	`+ componentCfg.Default(clusterCfg, localAPIEndpoint, nodeRegOpts)`
`150`	`150`	`} else {`
`151`	`151`	`componentCfg := handler.CreateEmpty()`
`152`		`- componentCfg.Default(clusterCfg, localAPIEndpoint)`
	`152`	`+ componentCfg.Default(clusterCfg, localAPIEndpoint, nodeRegOpts)`
`153`	`153`	`clusterCfg.ComponentConfigs[group] = componentCfg`
`154`	`154`	`}`
`155`	`155`	`}`
Original file line number	Diff line number	Diff line change
`@@ -323,7 +323,7 @@ func TestKubeletDefault(t *testing.T) {`
`323`	`323`	`for _, test := range tests {`
`324`	`324`	`t.Run(test.name, func(t *testing.T) {`
`325`	`325`	`got := &kubeletConfig{}`
`326`		`- got.Default(&test.clusterCfg, &kubeadmapi.APIEndpoint{})`
	`326`	`+ got.Default(&test.clusterCfg, &kubeadmapi.APIEndpoint{}, &kubeadmapi.NodeRegistrationOptions{})`
`327`	`327`	`if !reflect.DeepEqual(got, &test.expected) {`
`328`	`328`	`t.Fatalf("Missmatch between expected and got:\nExpected:\n%v\n---\nGot:\n%v", test.expected, got)`
`329`	`329`	`}`
Original file line number	Diff line number	Diff line change
`@@ -77,7 +77,7 @@ func kubeProxyDefaultBindAddress(localAdvertiseAddress string) string {`
`77`	`77`	`return kubeadmapiv1beta2.DefaultProxyBindAddressv6`
`78`	`78`	`}`
`79`	`79`
`80`		`-func (kp kubeProxyConfig) Default(cfg kubeadmapi.ClusterConfiguration, localAPIEndpoint *kubeadmapi.APIEndpoint) {`
	`80`	`+func (kp kubeProxyConfig) Default(cfg kubeadmapi.ClusterConfiguration, localAPIEndpoint kubeadmapi.APIEndpoint, _ kubeadmapi.NodeRegistrationOptions) {`
`81`	`81`	`const kind = "KubeProxyConfiguration"`
`82`	`82`
`83`	`83`	`// The below code is necessary because while KubeProxy may be defined, the user may not`
Original file line number	Diff line number	Diff line change
`@@ -297,7 +297,7 @@ func TestKubeProxyDefault(t *testing.T) {`
`297`	`297`	`for _, test := range tests {`
`298`	`298`	`t.Run(test.name, func(t *testing.T) {`
`299`	`299`	`got := &kubeProxyConfig{}`
`300`		`- got.Default(&test.clusterCfg, &test.endpoint)`
	`300`	`+ got.Default(&test.clusterCfg, &test.endpoint, &kubeadmapi.NodeRegistrationOptions{})`
`301`	`301`	`if !reflect.DeepEqual(got, &test.expected) {`
`302`	`302`	`t.Fatalf("Missmatch between expected and got:\nExpected:\n%v\n---\nGot:\n%v", test.expected, got)`
`303`	`303`	`}`