From 02bab4af636eb800d2c120e72b577f98cab516ce Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Mon, 23 Jul 2018 12:20:05 +0300 Subject: [PATCH] kubeadm: make error output more verbose Included error output of the docker/crictl into the kubeadm error output. This should help users to understand better why runtime is failing. --- cmd/kubeadm/app/cmd/config_test.go | 16 +++--- cmd/kubeadm/app/preflight/checks_test.go | 16 +++--- cmd/kubeadm/app/util/runtime/runtime.go | 52 ++++++++++------- cmd/kubeadm/app/util/runtime/runtime_test.go | 59 ++++++++++---------- 4 files changed, 77 insertions(+), 66 deletions(-) diff --git a/cmd/kubeadm/app/cmd/config_test.go b/cmd/kubeadm/app/cmd/config_test.go index e66e41939c..b7c97d0a15 100644 --- a/cmd/kubeadm/app/cmd/config_test.go +++ b/cmd/kubeadm/app/cmd/config_test.go @@ -186,12 +186,12 @@ func TestConfigImagesListRunWithoutPath(t *testing.T) { func TestImagesPull(t *testing.T) { fcmd := fakeexec.FakeCmd{ - 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 }, - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, nil }, + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return nil, nil }, }, } @@ -219,8 +219,8 @@ func TestImagesPull(t *testing.T) { t.Fatalf("expected nil but found %v", err) } - if fcmd.RunCalls != len(images) { - t.Errorf("expected %d docker calls, got %d", len(images), fcmd.RunCalls) + if fcmd.CombinedOutputCalls != len(images) { + t.Errorf("expected %d calls, got %d", len(images), fcmd.CombinedOutputCalls) } } diff --git a/cmd/kubeadm/app/preflight/checks_test.go b/cmd/kubeadm/app/preflight/checks_test.go index 609c6a8f3f..16ffdae4c4 100644 --- a/cmd/kubeadm/app/preflight/checks_test.go +++ b/cmd/kubeadm/app/preflight/checks_test.go @@ -701,14 +701,14 @@ func TestSetHasItemOrAll(t *testing.T) { func TestImagePullCheck(t *testing.T) { fcmd := fakeexec.FakeCmd{ - RunScript: []fakeexec.FakeRunAction{ - func() ([]byte, []byte, error) { return nil, nil, nil }, // Test case 1 - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, // Test case 2 - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, nil }, + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + func() ([]byte, error) { return nil, nil }, // Test case 1 + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} }, // Test case 2 + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return nil, nil }, }, } diff --git a/cmd/kubeadm/app/util/runtime/runtime.go b/cmd/kubeadm/app/util/runtime/runtime.go index 2edb57215c..da950413a5 100644 --- a/cmd/kubeadm/app/util/runtime/runtime.go +++ b/cmd/kubeadm/app/util/runtime/runtime.go @@ -86,28 +86,28 @@ func (runtime *DockerRuntime) IsDocker() bool { // IsRunning checks if runtime is running func (runtime *CRIRuntime) IsRunning() error { - if err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "info").Run(); err != nil { - return fmt.Errorf("container runtime is not running: %v", err) + if out, err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "info").CombinedOutput(); err != nil { + return fmt.Errorf("container runtime is not running: output: %s, error: %v", string(out), err) } return nil } // IsRunning checks if runtime is running func (runtime *DockerRuntime) IsRunning() error { - if err := runtime.exec.Command("docker", "info").Run(); err != nil { - return fmt.Errorf("container runtime is not running: %v", err) + if out, err := runtime.exec.Command("docker", "info").CombinedOutput(); err != nil { + return fmt.Errorf("container runtime is not running: output: %s, error: %v", string(out), err) } return nil } // ListKubeContainers lists running k8s CRI pods func (runtime *CRIRuntime) ListKubeContainers() ([]string, error) { - output, err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "pods", "-q").CombinedOutput() + out, err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "pods", "-q").CombinedOutput() if err != nil { - return nil, err + return nil, fmt.Errorf("output: %s, error: %v", string(out), err) } pods := []string{} - for _, pod := range strings.Fields(string(output)) { + for _, pod := range strings.Fields(string(out)) { if strings.HasPrefix(pod, "k8s_") { pods = append(pods, pod) } @@ -125,14 +125,14 @@ func (runtime *DockerRuntime) ListKubeContainers() ([]string, error) { func (runtime *CRIRuntime) RemoveContainers(containers []string) error { errs := []error{} for _, container := range containers { - err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "stopp", container).Run() + out, err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "stopp", container).CombinedOutput() if err != nil { // don't stop on errors, try to remove as many containers as possible - errs = append(errs, fmt.Errorf("failed to stop running pod %s: %v", container, err)) + errs = append(errs, fmt.Errorf("failed to stop running pod %s: output: %s, error: %v", container, string(out), err)) } else { - err = runtime.exec.Command("crictl", "-r", runtime.criSocket, "rmp", container).Run() + out, err = runtime.exec.Command("crictl", "-r", runtime.criSocket, "rmp", container).CombinedOutput() if err != nil { - errs = append(errs, fmt.Errorf("failed to remove running container %s: %v", container, err)) + errs = append(errs, fmt.Errorf("failed to remove running container %s: output: %s, error: %v", container, string(out), err)) } } } @@ -143,10 +143,10 @@ func (runtime *CRIRuntime) RemoveContainers(containers []string) error { func (runtime *DockerRuntime) RemoveContainers(containers []string) error { errs := []error{} for _, container := range containers { - err := runtime.exec.Command("docker", "rm", "--force", "--volumes", container).Run() + out, err := runtime.exec.Command("docker", "rm", "--force", "--volumes", container).CombinedOutput() if err != nil { // don't stop on errors, try to remove as many containers as possible - errs = append(errs, fmt.Errorf("failed to remove running container %s: %v", container, err)) + errs = append(errs, fmt.Errorf("failed to remove running container %s: output: %s, error: %v", container, string(out), err)) } } return errors.NewAggregate(errs) @@ -154,22 +154,36 @@ func (runtime *DockerRuntime) RemoveContainers(containers []string) error { // PullImage pulls the image func (runtime *CRIRuntime) PullImage(image string) error { - return runtime.exec.Command("crictl", "-r", runtime.criSocket, "pull", image).Run() + out, err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "pull", image).CombinedOutput() + if err != nil { + return fmt.Errorf("output: %s, error: %v", string(out), err) + } + return nil } // PullImage pulls the image func (runtime *DockerRuntime) PullImage(image string) error { - return runtime.exec.Command("docker", "pull", image).Run() + out, err := runtime.exec.Command("docker", "pull", image).CombinedOutput() + if err != nil { + return fmt.Errorf("output: %s, error: %v", string(out), err) + } + return nil } // ImageExists checks to see if the image exists on the system func (runtime *CRIRuntime) ImageExists(image string) (bool, error) { - err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "inspecti", image).Run() - return err == nil, err + out, err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "inspecti", image).CombinedOutput() + if err != nil { + return false, fmt.Errorf("output: %s, error: %v", string(out), err) + } + return true, nil } // ImageExists checks to see if the image exists on the system func (runtime *DockerRuntime) ImageExists(image string) (bool, error) { - err := runtime.exec.Command("docker", "inspect", image).Run() - return err == nil, err + out, err := runtime.exec.Command("docker", "inspect", image).CombinedOutput() + if err != nil { + return false, fmt.Errorf("output: %s, error: %v", string(out), err) + } + return true, nil } diff --git a/cmd/kubeadm/app/util/runtime/runtime_test.go b/cmd/kubeadm/app/util/runtime/runtime_test.go index 6ea9203488..47cc591bd9 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_test.go +++ b/cmd/kubeadm/app/util/runtime/runtime_test.go @@ -78,20 +78,21 @@ func genFakeActions(fcmd *fakeexec.FakeCmd, num int) []fakeexec.FakeCommandActio func TestIsRunning(t *testing.T) { fcmd := fakeexec.FakeCmd{ - RunScript: []fakeexec.FakeRunAction{ - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} }, + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} }, }, } + criExecer := fakeexec.FakeExec{ - CommandScript: genFakeActions(&fcmd, len(fcmd.RunScript)), + CommandScript: genFakeActions(&fcmd, len(fcmd.CombinedOutputScript)), LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/crictl", nil }, } dockerExecer := fakeexec.FakeExec{ - CommandScript: genFakeActions(&fcmd, len(fcmd.RunScript)), + CommandScript: genFakeActions(&fcmd, len(fcmd.CombinedOutputScript)), LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/docker", nil }, } @@ -100,12 +101,11 @@ func TestIsRunning(t *testing.T) { criSocket string execer fakeexec.FakeExec isError bool - runCalls int }{ - {"valid: CRI-O is running", "unix:///var/run/crio/crio.sock", criExecer, false, 1}, - {"invalid: CRI-O is not running", "unix:///var/run/crio/crio.sock", criExecer, true, 2}, - {"valid: docker is running", kubeadmapiv1alpha3.DefaultCRISocket, dockerExecer, false, 3}, - {"invalid: docker is not running", kubeadmapiv1alpha3.DefaultCRISocket, dockerExecer, true, 4}, + {"valid: CRI-O is running", "unix:///var/run/crio/crio.sock", criExecer, false}, + {"invalid: CRI-O is not running", "unix:///var/run/crio/crio.sock", criExecer, true}, + {"valid: docker is running", kubeadmapiv1alpha3.DefaultCRISocket, dockerExecer, false}, + {"invalid: docker is not running", kubeadmapiv1alpha3.DefaultCRISocket, dockerExecer, true}, } for _, tc := range cases { @@ -121,9 +121,6 @@ func TestIsRunning(t *testing.T) { if !tc.isError && isRunning != nil { t.Error("unexpected IsRunning() error") } - if fcmd.RunCalls != tc.runCalls { - t.Errorf("expected %d Run() calls, got %d", tc.runCalls, fcmd.RunCalls) - } }) } } @@ -176,10 +173,10 @@ func TestListKubeContainers(t *testing.T) { } func TestRemoveContainers(t *testing.T) { - fakeOK := func() ([]byte, []byte, error) { return nil, nil, nil } - fakeErr := func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} } + fakeOK := func() ([]byte, error) { return nil, nil } + fakeErr := func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} } fcmd := fakeexec.FakeCmd{ - RunScript: []fakeexec.FakeRunAction{ + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ fakeOK, fakeOK, fakeOK, fakeOK, fakeOK, fakeOK, // Test case 1 fakeOK, fakeOK, fakeOK, fakeErr, fakeOK, fakeOK, fakeErr, fakeOK, fakeOK, fakeErr, fakeOK, @@ -188,7 +185,7 @@ func TestRemoveContainers(t *testing.T) { }, } execer := fakeexec.FakeExec{ - CommandScript: genFakeActions(&fcmd, len(fcmd.RunScript)), + CommandScript: genFakeActions(&fcmd, len(fcmd.CombinedOutputScript)), LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/crictl", nil }, } @@ -225,15 +222,15 @@ func TestRemoveContainers(t *testing.T) { func TestPullImage(t *testing.T) { fcmd := fakeexec.FakeCmd{ - RunScript: []fakeexec.FakeRunAction{ - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} }, + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} }, }, } execer := fakeexec.FakeExec{ - CommandScript: genFakeActions(&fcmd, len(fcmd.RunScript)), + CommandScript: genFakeActions(&fcmd, len(fcmd.CombinedOutputScript)), LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/crictl", nil }, } @@ -269,15 +266,15 @@ func TestPullImage(t *testing.T) { func TestImageExists(t *testing.T) { fcmd := fakeexec.FakeCmd{ - RunScript: []fakeexec.FakeRunAction{ - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, - func() ([]byte, []byte, error) { return nil, nil, nil }, - func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, + CombinedOutputScript: []fakeexec.FakeCombinedOutputAction{ + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} }, + func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} }, }, } execer := fakeexec.FakeExec{ - CommandScript: genFakeActions(&fcmd, len(fcmd.RunScript)), + CommandScript: genFakeActions(&fcmd, len(fcmd.CombinedOutputScript)), LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/crictl", nil }, }