Merge pull request kubernetes#90394 from SataQiu/fix-kubeadm-20200422

kubeadm: fix the bug that kubeadm does not really respect resolvConf value set by user if systemd-resolved is active

Author: k8s-ci-robot

cmd/kubeadm/app/componentconfigs/BUILD

@@ -18,6 +18,7 @@ go_library(
         "//cmd/kubeadm/app/features:go_default_library",
         "//cmd/kubeadm/app/util:go_default_library",
         "//cmd/kubeadm/app/util/apiclient:go_default_library",
+        "//cmd/kubeadm/app/util/initsystem:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/api/errors:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/apis/meta/v1:go_default_library",
         "//staging/src/k8s.io/apimachinery/pkg/runtime:go_default_library",

cmd/kubeadm/app/componentconfigs/kubelet.go

@@ -21,7 +21,9 @@ import (
 
 	"k8s.io/apimachinery/pkg/util/version"
 	clientset "k8s.io/client-go/kubernetes"
+	"k8s.io/klog"
 	kubeletconfig "k8s.io/kubelet/config/v1beta1"
+	"k8s.io/kubernetes/cmd/kubeadm/app/util/initsystem"
 	utilpointer "k8s.io/utils/pointer"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
@@ -49,6 +51,9 @@ const (
 
 	// kubeletHealthzBindAddress specifies the default healthz bind address
 	kubeletHealthzBindAddress = "127.0.0.1"
+
+	// kubeletSystemdResolverConfig specifies the default resolver config when systemd service is active
+	kubeletSystemdResolverConfig = "/run/systemd/resolve/resolv.conf"
 )
 
 // kubeletHandler is the handler instance for the kubelet component config
@@ -173,4 +178,27 @@ func (kc *kubeletConfig) Default(cfg *kubeadmapi.ClusterConfiguration, _ *kubead
 	// We cannot show a warning for RotateCertificates==false and we must hardcode it to true.
 	// 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
+
+	ok, err := isServiceActive("systemd-resolved")
+	if err != nil {
+		klog.Warningf("cannot determine if systemd-resolved is active: %v", err)
+	}
+	if ok {
+		if kc.config.ResolverConfig == "" {
+			kc.config.ResolverConfig = kubeletSystemdResolverConfig
+		} else {
+			if kc.config.ResolverConfig != kubeletSystemdResolverConfig {
+				warnDefaultComponentConfigValue(kind, "resolvConf", kubeletSystemdResolverConfig, kc.config.ResolverConfig)
+			}
+		}
+	}
+}
+
+// isServiceActive checks whether the given service exists and is running
+func isServiceActive(name string) (bool, error) {
+	initSystem, err := initsystem.GetInitSystem()
+	if err != nil {
+		return false, err
+	}
+	return initSystem.ServiceIsActive(name), nil
 }

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

@@ -26,7 +26,6 @@ import (
 	"k8s.io/kubernetes/cmd/kubeadm/app/features"
 	"k8s.io/kubernetes/cmd/kubeadm/app/images"
 	kubeadmutil "k8s.io/kubernetes/cmd/kubeadm/app/util"
-	"k8s.io/kubernetes/cmd/kubeadm/app/util/initsystem"
 	utilsexec "k8s.io/utils/exec"
 	"os"
 	"path/filepath"
@@ -39,7 +38,6 @@ type kubeletFlagsOpts struct {
 	pauseImage               string
 	registerTaintsUsingFlags bool
 	execer                   utilsexec.Interface
-	isServiceActiveFunc      func(string) (bool, error)
 }
 
 // GetNodeNameAndHostname obtains the name for this Node using the following precedence
@@ -69,13 +67,6 @@ func WriteKubeletDynamicEnvFile(cfg *kubeadmapi.ClusterConfiguration, nodeReg *k
 		pauseImage:               images.GetPauseImage(cfg),
 		registerTaintsUsingFlags: registerTaintsUsingFlags,
 		execer:                   utilsexec.New(),
-		isServiceActiveFunc: func(name string) (bool, error) {
-			initSystem, err := initsystem.GetInitSystem()
-			if err != nil {
-				return false, err
-			}
-			return initSystem.ServiceIsActive(name), nil
-		},
 	}
 	stringMap := buildKubeletArgMap(flagOpts)
 	argList := kubeadmutil.BuildArgumentListFromMap(stringMap, nodeReg.KubeletExtraArgs)

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

@@ -89,14 +89,6 @@ var (
 	}
 )
 
-func serviceIsActiveFunc(_ string) (bool, error) {
-	return true, nil
-}
-
-func serviceIsNotActiveFunc(_ string) (bool, error) {
-	return false, nil
-}
-
 func TestBuildKubeletArgMap(t *testing.T) {
 
 	tests := []struct {
@@ -117,8 +109,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 						},
 					},
 				},
-				execer:              errCgroupExecer,
-				isServiceActiveFunc: serviceIsNotActiveFunc,
+				execer: errCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin": "cni",
@@ -131,8 +122,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 					CRISocket: "/var/run/dockershim.sock",
 					Name:      "override-name",
 				},
-				execer:              errCgroupExecer,
-				isServiceActiveFunc: serviceIsNotActiveFunc,
+				execer: errCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin":    "cni",
@@ -146,8 +136,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 					CRISocket:        "/var/run/dockershim.sock",
 					KubeletExtraArgs: map[string]string{"hostname-override": "override-name"},
 				},
-				execer:              errCgroupExecer,
-				isServiceActiveFunc: serviceIsNotActiveFunc,
+				execer: errCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin":    "cni",
@@ -160,8 +149,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/dockershim.sock",
 				},
-				execer:              systemdCgroupExecer,
-				isServiceActiveFunc: serviceIsNotActiveFunc,
+				execer: systemdCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin": "cni",
@@ -174,8 +162,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/dockershim.sock",
 				},
-				execer:              cgroupfsCgroupExecer,
-				isServiceActiveFunc: serviceIsNotActiveFunc,
+				execer: cgroupfsCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin": "cni",
@@ -188,8 +175,7 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/containerd.sock",
 				},
-				execer:              cgroupfsCgroupExecer,
-				isServiceActiveFunc: serviceIsNotActiveFunc,
+				execer: cgroupfsCgroupExecer,
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
@@ -216,38 +202,21 @@ func TestBuildKubeletArgMap(t *testing.T) {
 				},
 				registerTaintsUsingFlags: true,
 				execer:                   cgroupfsCgroupExecer,
-				isServiceActiveFunc:      serviceIsNotActiveFunc,
 			},
 			expected: map[string]string{
 				"container-runtime":          "remote",
 				"container-runtime-endpoint": "/var/run/containerd.sock",
 				"register-with-taints":       "foo=bar:baz,key=val:eff",
 			},
 		},
-		{
-			name: "systemd-resolved running",
-			opts: kubeletFlagsOpts{
-				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
-					CRISocket: "/var/run/containerd.sock",
-				},
-				execer:              cgroupfsCgroupExecer,
-				isServiceActiveFunc: serviceIsActiveFunc,
-			},
-			expected: map[string]string{
-				"container-runtime":          "remote",
-				"container-runtime-endpoint": "/var/run/containerd.sock",
-				"resolv-conf":                "/run/systemd/resolve/resolv.conf",
-			},
-		},
 		{
 			name: "pause image is set",
 			opts: kubeletFlagsOpts{
 				nodeRegOpts: &kubeadmapi.NodeRegistrationOptions{
 					CRISocket: "/var/run/dockershim.sock",
 				},
-				pauseImage:          "gcr.io/pause:3.2",
-				execer:              cgroupfsCgroupExecer,
-				isServiceActiveFunc: serviceIsNotActiveFunc,
+				pauseImage: "gcr.io/pause:3.2",
+				execer:     cgroupfsCgroupExecer,
 			},
 			expected: map[string]string{
 				"network-plugin":            "cni",

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

@@ -39,13 +39,5 @@ func buildKubeletArgMap(opts kubeletFlagsOpts) map[string]string {
 		}
 	}
 
-	ok, err := opts.isServiceActiveFunc("systemd-resolved")
-	if err != nil {
-		klog.Warningf("cannot determine if systemd-resolved is active: %v\n", err)
-	}
-	if ok {
-		kubeletFlags["resolv-conf"] = "/run/systemd/resolve/resolv.conf"
-	}
-
 	return kubeletFlags
 }