Reimplement 2 tests using fakeexec

Used fakeexec API from utils/exec/testing to test
GetKubeletVersion and KubeletVersionCheck APIs without
running kubelet.

cmd/kubeadm/app/preflight/BUILD

@@ -83,6 +83,7 @@ go_test(
         "//vendor/github.com/renstrom/dedent:go_default_library",
         "//vendor/k8s.io/apimachinery/pkg/util/sets:go_default_library",
         "//vendor/k8s.io/utils/exec:go_default_library",
+        "//vendor/k8s.io/utils/exec/testing:go_default_library",
     ],
 )
 

cmd/kubeadm/app/preflight/checks.go

@@ -647,6 +647,7 @@ func (kubever KubernetesVersionCheck) Check() (warnings, errors []error) {
 // KubeletVersionCheck validates installed kubelet version
 type KubeletVersionCheck struct {
 	KubernetesVersion string
+	exec              utilsexec.Interface
 }
 
 // Name will return KubeletVersion as name for KubeletVersionCheck
@@ -656,7 +657,7 @@ func (KubeletVersionCheck) Name() string {
 
 // Check validates kubelet version. It should be not less than minimal supported version
 func (kubever KubeletVersionCheck) Check() (warnings, errors []error) {
-	kubeletVersion, err := GetKubeletVersion()
+	kubeletVersion, err := GetKubeletVersion(kubever.exec)
 	if err != nil {
 		return nil, []error{fmt.Errorf("couldn't get kubelet version: %v", err)}
 	}
@@ -872,7 +873,7 @@ func RunInitMasterChecks(execer utilsexec.Interface, cfg *kubeadmapi.MasterConfi
 		SystemVerificationCheck{CRISocket: criSocket},
 		IsPrivilegedUserCheck{},
 		HostnameCheck{nodeName: cfg.NodeName},
-		KubeletVersionCheck{KubernetesVersion: cfg.KubernetesVersion},
+		KubeletVersionCheck{KubernetesVersion: cfg.KubernetesVersion, exec: execer},
 		ServiceCheck{Service: "kubelet", CheckIfActive: false},
 		ServiceCheck{Service: "docker", CheckIfActive: true}, // assume docker
 		FirewalldCheck{ports: []int{int(cfg.API.BindPort), 10250}},
@@ -969,7 +970,7 @@ func RunJoinNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.NodeConfigura
 		SystemVerificationCheck{CRISocket: criSocket},
 		IsPrivilegedUserCheck{},
 		HostnameCheck{cfg.NodeName},
-		KubeletVersionCheck{},
+		KubeletVersionCheck{exec: execer},
 		ServiceCheck{Service: "kubelet", CheckIfActive: false},
 		PortOpenCheck{port: 10250},
 		DirAvailableCheck{Path: filepath.Join(kubeadmconstants.KubernetesDir, kubeadmconstants.ManifestsSubDirName)},

cmd/kubeadm/app/preflight/checks_test.go

@@ -20,7 +20,6 @@ import (
 	"bytes"
 	"fmt"
 	"io/ioutil"
-	"path/filepath"
 	"strings"
 	"testing"
 
@@ -32,6 +31,7 @@ import (
 	"k8s.io/apimachinery/pkg/util/sets"
 	kubeadmapi "k8s.io/kubernetes/cmd/kubeadm/app/apis/kubeadm"
 	"k8s.io/utils/exec"
+	fakeexec "k8s.io/utils/exec/testing"
 )
 
 var (
@@ -566,14 +566,12 @@ func restoreEnv(e map[string]string) {
 }
 
 func TestKubeletVersionCheck(t *testing.T) {
-	type T struct {
+	cases := []struct {
 		kubeletVersion string
 		k8sVersion     string
 		expectErrors   bool
 		expectWarnings bool
-	}
+	}{
-
-	cases := []T{
 		{"v1.10.2", "", false, false},              // check minimally supported version when there is no information about control plane
 		{"v1.7.3", "v1.7.8", true, false},          // too old kubelet (older than kubeadmconstants.MinimumKubeletVersion), should fail.
 		{"v1.9.0", "v1.9.5", false, false},         // kubelet within same major.minor as control plane
@@ -583,27 +581,19 @@ func TestKubeletVersionCheck(t *testing.T) {
 		{"v1.10.0", "v1.9.5", true, false},         // kubelet is newer (release) than control plane, should fail.
 	}
 
-	dir, err := ioutil.TempDir("", "test-kubelet-version-check")
-	if err != nil {
-		t.Errorf("Failed to create directory for testing GetKubeletVersion: %v", err)
-	}
-	defer os.RemoveAll(dir)
-
-	// We don't want to call real kubelet or something else in $PATH
-	oldPATH := os.Getenv("PATH")
-	defer os.Setenv("PATH", oldPATH)
-
-	os.Setenv("PATH", dir)
-
-	kubeletFn := filepath.Join(dir, "kubelet")
 	for _, tc := range cases {
-
+		fcmd := fakeexec.FakeCmd{
-		content := []byte(fmt.Sprintf("#!/bin/sh\necho 'Kubernetes %s'", tc.kubeletVersion))
+			CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{
-		if err := ioutil.WriteFile(kubeletFn, content, 0755); err != nil {
+				func() ([]byte, error) { return []byte("Kubernetes " + tc.kubeletVersion), nil },
-			t.Errorf("Error creating test stub file %s: %v", kubeletFn, err)
+			},
+		}
+		fexec := &fakeexec.FakeExec{
+			CommandScript: []fakeexec.FakeCommandAction{
+				func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			},
 		}
 
-		check := KubeletVersionCheck{KubernetesVersion: tc.k8sVersion}
+		check := KubeletVersionCheck{KubernetesVersion: tc.k8sVersion, exec: fexec}
 		warnings, errors := check.Check()
 
 		switch {

cmd/kubeadm/app/preflight/utils.go

@@ -18,18 +18,19 @@ package preflight
 
 import (
 	"fmt"
-	"os/exec"
 	"regexp"
 	"strings"
 
 	"k8s.io/kubernetes/pkg/util/version"
+	utilsexec "k8s.io/utils/exec"
 )
 
 // GetKubeletVersion is helper function that returns version of kubelet available in $PATH
-func GetKubeletVersion() (*version.Version, error) {
+func GetKubeletVersion(execer utilsexec.Interface) (*version.Version, error) {
 	kubeletVersionRegex := regexp.MustCompile(`^\s*Kubernetes v((0|[1-9][0-9]*)\.(0|[1-9][0-9]*)\.(0|[1-9][0-9]*)([-0-9a-zA-Z_\.+]*)?)\s*$`)
 
-	out, err := exec.Command("kubelet", "--version").Output()
+	command := execer.Command("kubelet", "--version")
+	out, err := command.CombinedOutput()
 	if err != nil {
 		return nil, err
 	}

cmd/kubeadm/app/preflight/utils_test.go

@@ -18,52 +18,38 @@ package preflight
 
 import (
 	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
 	"testing"
+
+	utilsexec "k8s.io/utils/exec"
+	fakeexec "k8s.io/utils/exec/testing"
 )
 
 func TestGetKubeletVersion(t *testing.T) {
-	type T struct {
+	cases := []struct {
 		output   string
 		expected string
+		err      error
 		valid    bool
+	}{
+		{"Kubernetes v1.7.0", "1.7.0", nil, true},
+		{"Kubernetes v1.8.0-alpha.2.1231+afabd012389d53a", "1.8.0-alpha.2.1231+afabd012389d53a", nil, true},
+		{"something-invalid", "", nil, false},
+		{"command not found", "", fmt.Errorf("kubelet not found"), false},
+		{"", "", nil, false},
 	}
 
-	cases := []T{
-		{"v1.7.0", "1.7.0", true},
-		{"v1.8.0-alpha.2.1231+afabd012389d53a", "1.8.0-alpha.2.1231+afabd012389d53a", true},
-		{"something-invalid", "", false},
-	}
-
-	dir, err := ioutil.TempDir("", "test-kubelet-version")
-	if err != nil {
-		t.Errorf("Failed to create directory for testing GetKubeletVersion: %v", err)
-	}
-	defer os.RemoveAll(dir)
-
-	// We don't want to call real kubelet or something else in $PATH
-	oldPATH := os.Getenv("PATH")
-	defer os.Setenv("PATH", oldPATH)
-
-	os.Setenv("PATH", dir)
-
-	// First test case, kubelet not present, should be getting error
-	ver, err := GetKubeletVersion()
-	if err == nil {
-		t.Errorf("failed GetKubeletVersion: expected failure when kubelet not in PATH. Result: %v", ver)
-	}
-
-	kubeletFn := filepath.Join(dir, "kubelet")
 	for _, tc := range cases {
-
+		fcmd := fakeexec.FakeCmd{
-		content := []byte(fmt.Sprintf("#!/bin/sh\necho 'Kubernetes %s'", tc.output))
+			CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{
-		if err := ioutil.WriteFile(kubeletFn, content, 0755); err != nil {
+				func() ([]byte, error) { return []byte(tc.output), tc.err },
-			t.Errorf("Error creating test stub file %s: %v", kubeletFn, err)
+			},
 		}
-
+		fexec := &fakeexec.FakeExec{
-		ver, err := GetKubeletVersion()
+			CommandScript: []fakeexec.FakeCommandAction{
+				func(cmd string, args ...string) utilsexec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			},
+		}
+		ver, err := GetKubeletVersion(fexec)
 		switch {
 		case err != nil && tc.valid:
 			t.Errorf("GetKubeletVersion: unexpected error for %q. Error: %v", tc.output, err)