Merge pull request #58883 from chuckha/crictl

Automatic merge from submit-queue (batch tested with PRs 57500, 58840, 58883). 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>.

Adds breadcrumb to crictl warning

Signed-off-by: Chuck Ha <ha.chuck@gmail.com>



**What this PR does / why we need it**:
This PR adds a breadcrumb to a `kubeadm` preflight check.

**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 kubernetes/kubeadm#613

**Special notes for your reviewer**:

**Release note**:

```release-note
NONE
```

cmd/kubeadm/app/constants/constants.go

@@ -200,6 +200,9 @@ const (
 	CoreDNS = "coredns"
 	// KubeDNS defines a variable used internally when referring to the kube-dns addon for a cluster
 	KubeDNS = "kube-dns"
+
+	// CRICtlPackage defines the go package that installs crictl
+	CRICtlPackage = "github.com/kubernetes-incubator/cri-tools/cmd/crictl"
 )
 
 var (

cmd/kubeadm/app/preflight/checks.go

@@ -20,6 +20,7 @@ import (
 	"bufio"
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -347,6 +348,7 @@ type InPathCheck struct {
 	mandatory  bool
 	exec       utilsexec.Interface
 	label      string
+	suggestion string
 }
 
 // Name returns label for individual InPathCheck. If not known, will return based on path.
@@ -358,7 +360,7 @@ func (ipc InPathCheck) Name() string {
 }
 
 // Check validates if the given executable is present in the path.
-func (ipc InPathCheck) Check() (warnings, errors []error) {
+func (ipc InPathCheck) Check() (warnings, errs []error) {
 	_, err := ipc.exec.LookPath(ipc.executable)
 	if err != nil {
 		if ipc.mandatory {
@@ -366,7 +368,11 @@ func (ipc InPathCheck) Check() (warnings, errors []error) {
 			return nil, []error{fmt.Errorf("%s not found in system path", ipc.executable)}
 		}
 		// Return as a warning:
-		return []error{fmt.Errorf("%s not found in system path", ipc.executable)}, nil
+		warningMessage := fmt.Sprintf("%s not found in system path", ipc.executable)
+		if ipc.suggestion != "" {
+			warningMessage += fmt.Sprintf("\nSuggestion: %s", ipc.suggestion)
+		}
+		return []error{errors.New(warningMessage)}, nil
 	}
 	return nil, nil
 }
@@ -847,7 +853,12 @@ func RunInitMasterChecks(execer utilsexec.Interface, cfg *kubeadmapi.MasterConfi
 	}
 
 	// check if we can use crictl to perform checks via the CRI
-	criCtlChecker := InPathCheck{executable: "crictl", mandatory: false, exec: execer}
+	criCtlChecker := InPathCheck{
+		executable: "crictl",
+		mandatory:  false,
+		exec:       execer,
+		suggestion: fmt.Sprintf("go get %v", kubeadmconstants.CRICtlPackage),
+	}
 	warns, _ := criCtlChecker.Check()
 	useCRI := len(warns) == 0
 
@@ -948,7 +959,12 @@ func RunJoinNodeChecks(execer utilsexec.Interface, cfg *kubeadmapi.NodeConfigura
 	}
 
 	// check if we can use crictl to perform checks via the CRI
-	criCtlChecker := InPathCheck{executable: "crictl", mandatory: false, exec: execer}
+	criCtlChecker := InPathCheck{
+		executable: "crictl",
+		mandatory:  false,
+		exec:       execer,
+		suggestion: fmt.Sprintf("go get %v", kubeadmconstants.CRICtlPackage),
+	}
 	warns, _ := criCtlChecker.Check()
 	useCRI := len(warns) == 0
 

cmd/kubeadm/app/preflight/checks_test.go

@@ -299,6 +299,7 @@ func TestRunChecks(t *testing.T) {
 		{[]Checker{ExtraArgsCheck{
 			APIServerExtraArgs: map[string]string{"invalid-argument": "foo"},
 		}}, true, "\t[WARNING ExtraArgs]: kube-apiserver: failed to parse extra argument --invalid-argument=foo\n"},
+		{[]Checker{InPathCheck{executable: "foobar", mandatory: false, exec: exec.New(), suggestion: "install foobar"}}, true, "\t[WARNING FileExisting-foobar]: foobar not found in system path\nSuggestion: install foobar\n"},
 	}
 	for _, rt := range tokenTest {
 		buf := new(bytes.Buffer)