Merge pull request #55717 from runcom/fix-kubead-reset

Automatic merge from submit-queue (batch tested with PRs 55682, 55444, 55456, 55717, 55131). 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: fix crictl command for reset

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



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

This PR is fixing kubeadm reset, we used the wrong command pipeline now that crictl has a new version out. This version targets kube master (1.9-dev) so this is the right fix.

**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 #

**Special notes for your reviewer**:

**Release note**:

```release-note
Fix kubeadm reset crictl command
```

@luxas PTAL

cmd/kubeadm/app/cmd/reset.go

@@ -22,6 +22,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 
 	"github.com/spf13/cobra"
 
@@ -34,7 +35,9 @@ import (
 )
 
 var (
-	crictlParamsFormat = "%s -r %s sandboxes --quiet | xargs -r %s -r %s rms"
+	crictlSandboxesParamsFormat = "%s -r %s sandboxes --quiet | xargs -r"
+	crictlStopParamsFormat      = "%s -r %s stops %s"
+	crictlRemoveParamsFormat    = "%s -r %s rms %s"
 )
 
 // NewCmdReset returns the "kubeadm reset" command
@@ -172,10 +175,27 @@ func resetWithDocker(execer utilsexec.Interface, dockerCheck preflight.Checker)
 func resetWithCrictl(execer utilsexec.Interface, dockerCheck preflight.Checker, criSocketPath, crictlPath string) {
 	if criSocketPath != "" {
 		fmt.Printf("[reset] Cleaning up running containers using crictl with socket %s\n", criSocketPath)
-		cmd := fmt.Sprintf(crictlParamsFormat, crictlPath, criSocketPath, crictlPath, criSocketPath)
+		listcmd := fmt.Sprintf(crictlSandboxesParamsFormat, crictlPath, criSocketPath)
-		if err := execer.Command("sh", "-c", cmd).Run(); err != nil {
+		output, err := execer.Command("sh", "-c", listcmd).CombinedOutput()
-			fmt.Println("[reset] Failed to stop the running containers using crictl. Trying using docker instead.")
+		if err != nil {
+			fmt.Println("[reset] Failed to list running pods using crictl. Trying using docker instead.")
 			resetWithDocker(execer, dockerCheck)
+			return
+		}
+		sandboxes := strings.Split(string(output), " ")
+		for _, s := range sandboxes {
+			stopcmd := fmt.Sprintf(crictlStopParamsFormat, crictlPath, criSocketPath, s)
+			if err := execer.Command("sh", "-c", stopcmd).Run(); err != nil {
+				fmt.Println("[reset] Failed to stop the running containers using crictl. Trying using docker instead.")
+				resetWithDocker(execer, dockerCheck)
+				return
+			}
+			removecmd := fmt.Sprintf(crictlRemoveParamsFormat, crictlPath, criSocketPath, s)
+			if err := execer.Command("sh", "-c", removecmd).Run(); err != nil {
+				fmt.Println("[reset] Failed to remove the running containers using crictl. Trying using docker instead.")
+				resetWithDocker(execer, dockerCheck)
+				return
+			}
 		}
 	} else {
 		fmt.Println("[reset] CRI socket path not provided for crictl. Trying docker instead.")

cmd/kubeadm/app/cmd/reset_test.go

@@ -230,10 +230,21 @@ func TestResetWithDocker(t *testing.T) {
 
 func TestResetWithCrictl(t *testing.T) {
 	fcmd := fakeexec.FakeCmd{
+		CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{
+			// 2: socket path provided, not runnning with crictl (1x CombinedOutput, 2x Run)
+			func() ([]byte, error) { return []byte("1"), nil },
+			// 3: socket path provided, crictl fails, reset with docker (1x CombinedOuput fail, 1x Run)
+			func() ([]byte, error) { return nil, errors.New("crictl list err") },
+		},
 		RunScript: []fakeexec.FakeRunAction{
+			// 1: socket path not provided, running with docker
+			func() ([]byte, []byte, error) { return nil, nil, nil },
+			// 2: socket path provided, now runnning with crictl (1x CombinedOutput, 2x Run)
 			func() ([]byte, []byte, error) { return nil, nil, nil },
 			func() ([]byte, []byte, error) { return nil, nil, nil },
-			func() ([]byte, []byte, error) { return nil, nil, errors.New("crictl error") },
+			// 3: socket path provided, crictl fails, reset with docker (1x CombinedOuput, 1x Run)
+			func() ([]byte, []byte, error) { return nil, nil, nil },
+			// 4: running with no socket and docker fails (1x Run)
 			func() ([]byte, []byte, error) { return nil, nil, nil },
 		},
 	}
@@ -243,9 +254,13 @@ func TestResetWithCrictl(t *testing.T) {
 			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
 			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
 			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
 		},
 	}
 
+	// 1: socket path not provided, running with docker
 	resetWithCrictl(&fexec, newFakeDockerChecker(nil, nil), "", "crictl")
 	if fcmd.RunCalls != 1 {
 		t.Errorf("expected 1 call to Run, got %d", fcmd.RunCalls)
@@ -254,25 +269,28 @@ func TestResetWithCrictl(t *testing.T) {
 		t.Errorf("expected a call to docker, got %v", fcmd.RunLog[0])
 	}
 
+	// 2: socket path provided, now runnning with crictl (1x CombinedOutput, 2x Run)
 	resetWithCrictl(&fexec, newFakeDockerChecker(nil, nil), "/test.sock", "crictl")
-	if fcmd.RunCalls != 2 {
+	if fcmd.RunCalls != 3 {
-		t.Errorf("expected 2 calls to Run, got %d", fcmd.RunCalls)
+		t.Errorf("expected 3 calls to Run, got %d", fcmd.RunCalls)
 	}
 	if !strings.Contains(fcmd.RunLog[1][2], "crictl") {
 		t.Errorf("expected a call to crictl, got %v", fcmd.RunLog[0])
 	}
+	if !strings.Contains(fcmd.RunLog[2][2], "crictl") {
+		t.Errorf("expected a call to crictl, got %v", fcmd.RunLog[0])
+	}
 
+	// 3: socket path provided, crictl fails, reset with docker
 	resetWithCrictl(&fexec, newFakeDockerChecker(nil, nil), "/test.sock", "crictl")
 	if fcmd.RunCalls != 4 {
 		t.Errorf("expected 4 calls to Run, got %d", fcmd.RunCalls)
 	}
-	if !strings.Contains(fcmd.RunLog[2][2], "crictl") {
-		t.Errorf("expected a call to crictl, got %v", fcmd.RunLog[0])
-	}
 	if !strings.Contains(fcmd.RunLog[3][2], "docker") {
 		t.Errorf("expected a call to docker, got %v", fcmd.RunLog[0])
 	}
 
+	// 4: running with no socket and docker fails (1x Run)
 	resetWithCrictl(&fexec, newFakeDockerChecker(nil, []error{errors.New("test error")}), "", "crictl")
 	if fcmd.RunCalls != 4 {
 		t.Errorf("expected 4 calls to Run, got %d", fcmd.RunCalls)
@@ -281,22 +299,32 @@ func TestResetWithCrictl(t *testing.T) {
 
 func TestReset(t *testing.T) {
 	fcmd := fakeexec.FakeCmd{
+		CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{
+			func() ([]byte, error) { return []byte("1"), nil },
+			func() ([]byte, error) { return []byte("1"), nil },
+			func() ([]byte, error) { return []byte("1"), nil },
+		},
 		RunScript: []fakeexec.FakeRunAction{
 			func() ([]byte, []byte, error) { return nil, nil, nil },
 			func() ([]byte, []byte, error) { return nil, nil, nil },
+			func() ([]byte, []byte, error) { return nil, nil, nil },
 		},
 	}
 	fexec := fakeexec.FakeExec{
 		CommandScript: []fakeexec.FakeCommandAction{
 			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
 			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
+			func(cmd string, args ...string) exec.Cmd { return fakeexec.InitFakeCmd(&fcmd, cmd, args...) },
 		},
 		LookPathFunc: func(cmd string) (string, error) { return cmd, nil },
 	}
 
 	reset(&fexec, newFakeDockerChecker(nil, nil), "/test.sock")
-	if fcmd.RunCalls != 1 {
+	if fcmd.RunCalls != 2 {
-		t.Errorf("expected 1 call to Run, got %d", fcmd.RunCalls)
+		t.Errorf("expected 2 call to Run, got %d", fcmd.RunCalls)
 	}
 	if !strings.Contains(fcmd.RunLog[0][2], "crictl") {
 		t.Errorf("expected a call to crictl, got %v", fcmd.RunLog[0])
@@ -304,10 +332,10 @@ func TestReset(t *testing.T) {
 
 	fexec.LookPathFunc = func(cmd string) (string, error) { return "", errors.New("no crictl") }
 	reset(&fexec, newFakeDockerChecker(nil, nil), "/test.sock")
-	if fcmd.RunCalls != 2 {
+	if fcmd.RunCalls != 3 {
-		t.Errorf("expected 2 calls to Run, got %d", fcmd.RunCalls)
+		t.Errorf("expected 3 calls to Run, got %d", fcmd.RunCalls)
 	}
-	if !strings.Contains(fcmd.RunLog[1][2], "docker") {
+	if !strings.Contains(fcmd.RunLog[2][2], "docker") {
 		t.Errorf("expected a call to docker, got %v", fcmd.RunLog[0])
 	}
 }