Merge pull request #55055 from runcom/check-with-crictl

Automatic merge from submit-queue (batch tested with PRs 55380, 55399, 55377, 55055). If you want to cherry-pick this change to another branch, please follow the instructions <a href="https://github.com/kubernetes/community/blob/master/contributors/devel/cherry-picks.md">here</a>.

kubeadm: use the CRI for preflights checks

Signed-off-by: Antonio Murdaca <runcom@redhat.com>



**What this PR does / why we need it**:

Add preflights checks to be performed using `crictl` and the kubernetes CRI instead of relying on docker.

**Which issue(s) this PR fixes** *(optional, in `fixes #<issue number>(, fixes #<issue_number>, ...)` format, will close the issue(s) when PR gets merged)*:
Fixes https://github.com/kubernetes/kubeadm/issues/285

**Special notes for your reviewer**:

**Release note**:

```release-note
kubeadm: use the CRI for preflights checks
```

@luxas PTAL

cmd/kubeadm/app/cmd/init.go

@@ -59,6 +59,7 @@ import (
 	"k8s.io/kubernetes/cmd/kubeadm/app/util/pubkeypin"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	"k8s.io/kubernetes/pkg/util/version"
+	utilsexec "k8s.io/utils/exec"
 )
 
 var (
@@ -110,6 +111,7 @@ func NewCmdInit(out io.Writer) *cobra.Command {
 	var skipTokenPrint bool
 	var dryRun bool
 	var featureGatesString string
+	var criSocket string
 
 	cmd := &cobra.Command{
 		Use:   "init",
@@ -124,7 +126,7 @@ func NewCmdInit(out io.Writer) *cobra.Command {
 			internalcfg := &kubeadmapi.MasterConfiguration{}
 			legacyscheme.Scheme.Convert(cfg, internalcfg, nil)
 
-			i, err := NewInit(cfgPath, internalcfg, skipPreFlight, skipTokenPrint, dryRun)
+			i, err := NewInit(cfgPath, internalcfg, skipPreFlight, skipTokenPrint, dryRun, criSocket)
 			kubeadmutil.CheckErr(err)
 			kubeadmutil.CheckErr(i.Validate(cmd))
 			kubeadmutil.CheckErr(i.Run(out))
@@ -132,7 +134,7 @@ func NewCmdInit(out io.Writer) *cobra.Command {
 	}
 
 	AddInitConfigFlags(cmd.PersistentFlags(), cfg, &featureGatesString)
-	AddInitOtherFlags(cmd.PersistentFlags(), &cfgPath, &skipPreFlight, &skipTokenPrint, &dryRun)
+	AddInitOtherFlags(cmd.PersistentFlags(), &cfgPath, &skipPreFlight, &skipTokenPrint, &dryRun, &criSocket)
 
 	return cmd
 }
@@ -188,7 +190,7 @@ func AddInitConfigFlags(flagSet *flag.FlagSet, cfg *kubeadmapiext.MasterConfigur
 }
 
 // AddInitOtherFlags adds init flags that are not bound to a configuration file to the given flagset
-func AddInitOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight, skipTokenPrint, dryRun *bool) {
+func AddInitOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight, skipTokenPrint, dryRun *bool, criSocket *string) {
 	flagSet.StringVar(
 		cfgPath, "config", *cfgPath,
 		"Path to kubeadm config file. WARNING: Usage of a configuration file is experimental.",
@@ -208,10 +210,14 @@ func AddInitOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight, sk
 		dryRun, "dry-run", *dryRun,
 		"Don't apply any changes; just output what would be done.",
 	)
+	flagSet.StringVar(
+		criSocket, "cri-socket", "/var/run/dockershim.sock",
+		`Specify the CRI socket to connect to.`,
+	)
 }
 
 // NewInit validates given arguments and instantiates Init struct with provided information.
-func NewInit(cfgPath string, cfg *kubeadmapi.MasterConfiguration, skipPreFlight, skipTokenPrint, dryRun bool) (*Init, error) {
+func NewInit(cfgPath string, cfg *kubeadmapi.MasterConfiguration, skipPreFlight, skipTokenPrint, dryRun bool, criSocket string) (*Init, error) {
 
 	fmt.Println("[kubeadm] WARNING: kubeadm is in beta, please do not use it for production clusters.")
 
@@ -247,7 +253,7 @@ func NewInit(cfgPath string, cfg *kubeadmapi.MasterConfiguration, skipPreFlight,
 	if !skipPreFlight {
 		fmt.Println("[preflight] Running pre-flight checks.")
 
-		if err := preflight.RunInitMasterChecks(cfg); err != nil {
+		if err := preflight.RunInitMasterChecks(utilsexec.New(), cfg, criSocket); err != nil {
 			return nil, err
 		}
 

cmd/kubeadm/app/cmd/join.go

@@ -38,6 +38,7 @@ import (
 	kubeconfigutil "k8s.io/kubernetes/cmd/kubeadm/app/util/kubeconfig"
 	"k8s.io/kubernetes/pkg/api/legacyscheme"
 	nodeutil "k8s.io/kubernetes/pkg/util/node"
+	utilsexec "k8s.io/utils/exec"
 )
 
 var (
@@ -100,6 +101,7 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
 
 	var skipPreFlight bool
 	var cfgPath string
+	var criSocket string
 
 	cmd := &cobra.Command{
 		Use:   "join [flags]",
@@ -112,7 +114,7 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
 			internalcfg := &kubeadmapi.NodeConfiguration{}
 			legacyscheme.Scheme.Convert(cfg, internalcfg, nil)
 
-			j, err := NewJoin(cfgPath, args, internalcfg, skipPreFlight)
+			j, err := NewJoin(cfgPath, args, internalcfg, skipPreFlight, criSocket)
 			kubeadmutil.CheckErr(err)
 			kubeadmutil.CheckErr(j.Validate(cmd))
 			kubeadmutil.CheckErr(j.Run(out))
@@ -120,7 +122,7 @@ func NewCmdJoin(out io.Writer) *cobra.Command {
 	}
 
 	AddJoinConfigFlags(cmd.PersistentFlags(), cfg)
-	AddJoinOtherFlags(cmd.PersistentFlags(), &cfgPath, &skipPreFlight)
+	AddJoinOtherFlags(cmd.PersistentFlags(), &cfgPath, &skipPreFlight, &criSocket)
 
 	return cmd
 }
@@ -151,7 +153,7 @@ func AddJoinConfigFlags(flagSet *flag.FlagSet, cfg *kubeadmapiext.NodeConfigurat
 }
 
 // AddJoinOtherFlags adds join flags that are not bound to a configuration file to the given flagset
-func AddJoinOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight *bool) {
+func AddJoinOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight *bool, criSocket *string) {
 	flagSet.StringVar(
 		cfgPath, "config", *cfgPath,
 		"Path to kubeadm config file.")
@@ -160,6 +162,10 @@ func AddJoinOtherFlags(flagSet *flag.FlagSet, cfgPath *string, skipPreFlight *bo
 		skipPreFlight, "skip-preflight-checks", false,
 		"Skip preflight checks normally run before modifying the system.",
 	)
+	flagSet.StringVar(
+		criSocket, "cri-socket", "/var/run/dockershim.sock",
+		`Specify the CRI socket to connect to.`,
+	)
 }
 
 // Join defines struct used by kubeadm join command
@@ -168,7 +174,7 @@ type Join struct {
 }
 
 // NewJoin instantiates Join struct with given arguments
-func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, skipPreFlight bool) (*Join, error) {
+func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, skipPreFlight bool, criSocket string) (*Join, error) {
 	fmt.Println("[kubeadm] WARNING: kubeadm is in beta, please do not use it for production clusters.")
 
 	if cfg.NodeName == "" {
@@ -189,7 +195,7 @@ func NewJoin(cfgPath string, args []string, cfg *kubeadmapi.NodeConfiguration, s
 		fmt.Println("[preflight] Running pre-flight checks.")
 
 		// Then continue with the others...
-		if err := preflight.RunJoinNodeChecks(cfg); err != nil {
+		if err := preflight.RunJoinNodeChecks(utilsexec.New(), cfg, criSocket); err != nil {
 			return nil, err
 		}
 

cmd/kubeadm/app/cmd/phases/BUILD

@@ -50,6 +50,7 @@ go_library(
         "//pkg/util/version:go_default_library",
         "//vendor/github.com/spf13/cobra:go_default_library",
         "//vendor/k8s.io/client-go/kubernetes:go_default_library",
+        "//vendor/k8s.io/utils/exec:go_default_library",
     ],
 )
 

cmd/kubeadm/app/cmd/phases/preflight.go

@@ -22,6 +22,7 @@ import (
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	cmdutil "k8s.io/kubernetes/cmd/kubeadm/app/cmd/util"
 	"k8s.io/kubernetes/cmd/kubeadm/app/preflight"
+	utilsexec "k8s.io/utils/exec"
 )
 
 // NewCmdPreFlight calls cobra.Command for preflight checks
@@ -44,7 +45,8 @@ func NewCmdPreFlightMaster() *cobra.Command {
 		Short: "Run master pre-flight checks",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			cfg := &kubeadmapi.MasterConfiguration{}
-			return preflight.RunInitMasterChecks(cfg)
+			criSocket := ""
+			return preflight.RunInitMasterChecks(utilsexec.New(), cfg, criSocket)
 		},
 	}
 
@@ -58,7 +60,8 @@ func NewCmdPreFlightNode() *cobra.Command {
 		Short: "Run node pre-flight checks",
 		RunE: func(cmd *cobra.Command, args []string) error {
 			cfg := &kubeadmapi.NodeConfiguration{}
-			return preflight.RunJoinNodeChecks(cfg)
+			criSocket := ""
+			return preflight.RunJoinNodeChecks(utilsexec.New(), cfg, criSocket)
 		},
 	}
 

cmd/kubeadm/app/preflight/BUILD

@@ -36,6 +36,7 @@ go_library(
         "//vendor/github.com/blang/semver:go_default_library",
         "//vendor/github.com/spf13/pflag:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/net:go_default_library",
+        "//vendor/k8s.io/utils/exec:go_default_library",
     ],
 )
 
@@ -50,6 +51,7 @@ go_test(
     deps = [
         "//cmd/kubeadm/app/apis/kubeadm:go_default_library",
         "//vendor/github.com/renstrom/dedent:go_default_library",
+        "//vendor/k8s.io/utils/exec:go_default_library",
     ],
 )
 

cmd/kubeadm/app/preflight/checks.go

@@ -26,7 +26,6 @@ import (
 	"net"
 	"net/http"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"runtime"
 	"strings"
@@ -54,6 +53,7 @@ import (
 	kubeadmversion "k8s.io/kubernetes/pkg/version"
 	schedulerapp "k8s.io/kubernetes/plugin/cmd/kube-scheduler/app"
 	"k8s.io/kubernetes/test/e2e_node/system"
+	utilsexec "k8s.io/utils/exec"
 )
 
 const (
@@ -83,6 +83,21 @@ type Checker interface {
 	Check() (warnings, errors []error)
 }
 
+// CRICheck verifies the container runtime through the CRI.
+type CRICheck struct {
+	socket string
+	exec   utilsexec.Interface
+}
+
+// Check validates the container runtime through the CRI.
+func (criCheck CRICheck) Check() (warnings, errors []error) {
+	if err := criCheck.exec.Command("sh", "-c", fmt.Sprintf("crictl -r %s info", criCheck.socket)).Run(); err != nil {
+		errors = append(errors, fmt.Errorf("unable to check if the container runtime at %q is running: %s", criCheck.socket, err))
+		return warnings, errors
+	}
+	return warnings, errors
+}
+
 // ServiceCheck verifies that the given service is enabled and active. If we do not
 // detect a supported init system however, all checks are skipped and a warning is
 // returned.
@@ -259,11 +274,12 @@ func (fcc FileContentCheck) Check() (warnings, errors []error) {
 type InPathCheck struct {
 	executable string
 	mandatory  bool
+	exec       utilsexec.Interface
 }
 
 // Check validates if the given executable is present in the path.
 func (ipc InPathCheck) Check() (warnings, errors []error) {
-	_, err := exec.LookPath(ipc.executable)
+	_, err := ipc.exec.LookPath(ipc.executable)
 	if err != nil {
 		if ipc.mandatory {
 			// Return as an error:
@@ -418,7 +434,9 @@ func (eac ExtraArgsCheck) Check() (warnings, errors []error) {
 }
 
 // SystemVerificationCheck defines struct used for for running the system verification node check in test/e2e_node/system
-type SystemVerificationCheck struct{}
+type SystemVerificationCheck struct {
+	CRISocket string
+}
 
 // Check runs all individual checks
 func (sysver SystemVerificationCheck) Check() (warnings, errors []error) {
@@ -431,8 +449,13 @@ func (sysver SystemVerificationCheck) Check() (warnings, errors []error) {
 	var warns []error
 	// All the common validators we'd like to run:
 	var validators = []system.Validator{
-		&system.KernelValidator{Reporter: reporter},
+		&system.KernelValidator{Reporter: reporter}}
-		&system.DockerValidator{Reporter: reporter}}
+
+	// run the docker validator only with dockershim
+	if sysver.CRISocket == "/var/run/dockershim.sock" {
+		// https://github.com/kubernetes/kubeadm/issues/533
+		validators = append(validators, &system.DockerValidator{Reporter: reporter})
+	}
 
 	if runtime.GOOS == "linux" {
 		//add linux validators
@@ -689,20 +712,24 @@ func getEtcdVersionResponse(client *http.Client, url string, target interface{})
 }
 
 // RunInitMasterChecks executes all individual, applicable to Master node checks.
-func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
+func RunInitMasterChecks(execer utilsexec.Interface, cfg *kubeadmapi.MasterConfiguration, criSocket string) error {
 	// First, check if we're root separately from the other preflight checks and fail fast
 	if err := RunRootCheckOnly(); err != nil {
 		return err
 	}
 
+	// check if we can use crictl to perform checks via the CRI
+	criCtlChecker := InPathCheck{executable: "crictl", mandatory: false, exec: execer}
+	warns, _ := criCtlChecker.Check()
+	useCRI := len(warns) == 0
+
 	checks := []Checker{
 		KubernetesVersionCheck{KubernetesVersion: cfg.KubernetesVersion, KubeadmVersion: kubeadmversion.Get().GitVersion},
-		SystemVerificationCheck{},
+		SystemVerificationCheck{CRISocket: criSocket},
 		IsPrivilegedUserCheck{},
 		HostnameCheck{nodeName: cfg.NodeName},
 		KubeletVersionCheck{KubernetesVersion: cfg.KubernetesVersion},
 		ServiceCheck{Service: "kubelet", CheckIfActive: false},
-		ServiceCheck{Service: "docker", CheckIfActive: true},
 		FirewalldCheck{ports: []int{int(cfg.API.BindPort), 10250}},
 		PortOpenCheck{port: int(cfg.API.BindPort)},
 		PortOpenCheck{port: 10250},
@@ -711,15 +738,16 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
 		DirAvailableCheck{Path: filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ManifestsSubDirName)},
 		FileContentCheck{Path: bridgenf, Content: []byte{'1'}},
 		SwapCheck{},
-		InPathCheck{executable: "ip", mandatory: true},
+		InPathCheck{executable: "ip", mandatory: true, exec: execer},
-		InPathCheck{executable: "iptables", mandatory: true},
+		InPathCheck{executable: "iptables", mandatory: true, exec: execer},
-		InPathCheck{executable: "mount", mandatory: true},
+		InPathCheck{executable: "mount", mandatory: true, exec: execer},
-		InPathCheck{executable: "nsenter", mandatory: true},
+		InPathCheck{executable: "nsenter", mandatory: true, exec: execer},
-		InPathCheck{executable: "ebtables", mandatory: false},
+		InPathCheck{executable: "ebtables", mandatory: false, exec: execer},
-		InPathCheck{executable: "ethtool", mandatory: false},
+		InPathCheck{executable: "ethtool", mandatory: false, exec: execer},
-		InPathCheck{executable: "socat", mandatory: false},
+		InPathCheck{executable: "socat", mandatory: false, exec: execer},
-		InPathCheck{executable: "tc", mandatory: false},
+		InPathCheck{executable: "tc", mandatory: false, exec: execer},
-		InPathCheck{executable: "touch", mandatory: false},
+		InPathCheck{executable: "touch", mandatory: false, exec: execer},
+		criCtlChecker,
 		ExtraArgsCheck{
 			APIServerExtraArgs:         cfg.APIServerExtraArgs,
 			ControllerManagerExtraArgs: cfg.ControllerManagerExtraArgs,
@@ -730,6 +758,13 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
 		HTTPProxyCIDRCheck{Proto: "https", CIDR: cfg.Networking.PodSubnet},
 	}
 
+	if useCRI {
+		checks = append(checks, CRICheck{socket: criSocket, exec: execer})
+	} else {
+		// assume docker
+		checks = append(checks, ServiceCheck{Service: "docker", CheckIfActive: true})
+	}
+
 	if len(cfg.Etcd.Endpoints) == 0 {
 		// Only do etcd related checks when no external endpoints were specified
 		checks = append(checks,
@@ -773,38 +808,49 @@ func RunInitMasterChecks(cfg *kubeadmapi.MasterConfiguration) error {
 }
 
 // RunJoinNodeChecks executes all individual, applicable to node checks.
-func RunJoinNodeChecks(cfg *kubeadmapi.NodeConfiguration) error {
+func RunJoinNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.NodeConfiguration, criSocket string) error {
 	// First, check if we're root separately from the other preflight checks and fail fast
 	if err := RunRootCheckOnly(); err != nil {
 		return err
 	}
 
+	// check if we can use crictl to perform checks via the CRI
+	criCtlChecker := InPathCheck{executable: "crictl", mandatory: false, exec: execer}
+	warns, _ := criCtlChecker.Check()
+	useCRI := len(warns) == 0
+
 	checks := []Checker{
-		SystemVerificationCheck{},
+		SystemVerificationCheck{CRISocket: criSocket},
 		IsPrivilegedUserCheck{},
 		HostnameCheck{cfg.NodeName},
 		KubeletVersionCheck{},
 		ServiceCheck{Service: "kubelet", CheckIfActive: false},
-		ServiceCheck{Service: "docker", CheckIfActive: true},
 		PortOpenCheck{port: 10250},
 		DirAvailableCheck{Path: filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ManifestsSubDirName)},
 		FileAvailableCheck{Path: cfg.CACertPath},
 		FileAvailableCheck{Path: filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.KubeletKubeConfigFileName)},
 	}
+	if useCRI {
+		checks = append(checks, CRICheck{socket: criSocket, exec: execer})
+	} else {
+		// assume docker
+		checks = append(checks, ServiceCheck{Service: "docker", CheckIfActive: true})
+	}
 	//non-windows checks
 	if runtime.GOOS == "linux" {
 		checks = append(checks,
 			FileContentCheck{Path: bridgenf, Content: []byte{'1'}},
 			SwapCheck{},
-			InPathCheck{executable: "ip", mandatory: true},
+			InPathCheck{executable: "ip", mandatory: true, exec: execer},
-			InPathCheck{executable: "iptables", mandatory: true},
+			InPathCheck{executable: "iptables", mandatory: true, exec: execer},
-			InPathCheck{executable: "mount", mandatory: true},
+			InPathCheck{executable: "mount", mandatory: true, exec: execer},
-			InPathCheck{executable: "nsenter", mandatory: true},
+			InPathCheck{executable: "nsenter", mandatory: true, exec: execer},
-			InPathCheck{executable: "ebtables", mandatory: false},
+			InPathCheck{executable: "ebtables", mandatory: false, exec: execer},
-			InPathCheck{executable: "ethtool", mandatory: false},
+			InPathCheck{executable: "ethtool", mandatory: false, exec: execer},
-			InPathCheck{executable: "socat", mandatory: false},
+			InPathCheck{executable: "socat", mandatory: false, exec: execer},
-			InPathCheck{executable: "tc", mandatory: false},
+			InPathCheck{executable: "tc", mandatory: false, exec: execer},
-			InPathCheck{executable: "touch", mandatory: false})
+			InPathCheck{executable: "touch", mandatory: false, exec: execer},
+			criCtlChecker)
 	}
 
 	if len(cfg.DiscoveryTokenAPIServers) > 0 {

cmd/kubeadm/app/preflight/checks_test.go

@@ -30,6 +30,7 @@ import (
 	"os"
 
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
+	"k8s.io/utils/exec"
 )
 
 var (
@@ -217,7 +218,7 @@ func TestRunInitMasterChecks(t *testing.T) {
 	}
 
 	for _, rt := range tests {
-		actual := RunInitMasterChecks(rt.cfg)
+		actual := RunInitMasterChecks(exec.New(), rt.cfg, "")
 		if (actual == nil) != rt.expected {
 			t.Errorf(
 				"failed RunInitMasterChecks:\n\texpected: %t\n\t  actual: %t\n\t error: %v",
@@ -253,7 +254,7 @@ func TestRunJoinNodeChecks(t *testing.T) {
 	}
 
 	for _, rt := range tests {
-		actual := RunJoinNodeChecks(rt.cfg)
+		actual := RunJoinNodeChecks(exec.New(), rt.cfg, "")
 		if (actual == nil) != rt.expected {
 			t.Errorf(
 				"failed RunJoinNodeChecks:\n\texpected: %t\n\t  actual: %t",
@@ -280,8 +281,8 @@ func TestRunChecks(t *testing.T) {
 		{[]Checker{FileContentCheck{Path: "/does/not/exist"}}, false, ""},
 		{[]Checker{FileContentCheck{Path: "/"}}, true, ""},
 		{[]Checker{FileContentCheck{Path: "/", Content: []byte("does not exist")}}, false, ""},
-		{[]Checker{InPathCheck{executable: "foobarbaz"}}, true, "[preflight] WARNING: foobarbaz not found in system path\n"},
+		{[]Checker{InPathCheck{executable: "foobarbaz", exec: exec.New()}}, true, "[preflight] WARNING: foobarbaz not found in system path\n"},
-		{[]Checker{InPathCheck{executable: "foobarbaz", mandatory: true}}, false, ""},
+		{[]Checker{InPathCheck{executable: "foobarbaz", mandatory: true, exec: exec.New()}}, false, ""},
 		{[]Checker{ExtraArgsCheck{
 			APIServerExtraArgs:         map[string]string{"secure-port": "1234"},
 			ControllerManagerExtraArgs: map[string]string{"use-service-account-credentials": "true"},