From c086c235f25b3c4ccd9a18e1a252751ba57afeea Mon Sep 17 00:00:00 2001 From: Ed Bartosh Date: Tue, 31 Jul 2018 13:43:59 +0300 Subject: [PATCH] kubeadm: fix runtime.ImageExists API This API return error when underlying 'inspect' command fails. This makes ImagePullCheck to fail as it doesn't expect runtime.ImageExists to return an error even if image doesn't exist. Fixed this by returning error nil even when inspect command fails. --- cmd/kubeadm/app/preflight/checks_test.go | 27 +++++++++++++++----- cmd/kubeadm/app/util/runtime/runtime.go | 14 +++------- cmd/kubeadm/app/util/runtime/runtime_test.go | 24 +++++++---------- 3 files changed, 33 insertions(+), 32 deletions(-) diff --git a/cmd/kubeadm/app/preflight/checks_test.go b/cmd/kubeadm/app/preflight/checks_test.go index 16ffdae4c4..c66979d021 100644 --- a/cmd/kubeadm/app/preflight/checks_test.go +++ b/cmd/kubeadm/app/preflight/checks_test.go @@ -701,14 +701,24 @@ func TestSetHasItemOrAll(t *testing.T) { func TestImagePullCheck(t *testing.T) { fcmd := fakeexec.FakeCmd{ + RunScript: []fakeexec.FakeRunAction{ + // Test case 1: img1 and img2 exist, img3 doesn't exist + 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: images don't exist + func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, + func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, + func() ([]byte, []byte, error) { return nil, nil, &fakeexec.FakeExitError{Status: 1} }, + }, 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 }, + // Test case1: pull only img3 func() ([]byte, error) { return nil, nil }, + // Test case 2: fail to pull image2 and image3 func() ([]byte, error) { return nil, nil }, + func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} }, + func() ([]byte, error) { return []byte("error"), &fakeexec.FakeExitError{Status: 1} }, }, } @@ -721,6 +731,9 @@ func TestImagePullCheck(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...) }, }, LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/docker", nil }, } @@ -746,7 +759,7 @@ func TestImagePullCheck(t *testing.T) { if len(warnings) != 0 { t.Fatalf("did not expect any warnings but got %q", warnings) } - if len(errors) != 1 { - t.Fatalf("expected 1 errors but got %d: %q", len(errors), errors) + if len(errors) != 2 { + t.Fatalf("expected 2 errors but got %d: %q", len(errors), errors) } } diff --git a/cmd/kubeadm/app/util/runtime/runtime.go b/cmd/kubeadm/app/util/runtime/runtime.go index da950413a5..1526895216 100644 --- a/cmd/kubeadm/app/util/runtime/runtime.go +++ b/cmd/kubeadm/app/util/runtime/runtime.go @@ -172,18 +172,12 @@ func (runtime *DockerRuntime) PullImage(image string) error { // ImageExists checks to see if the image exists on the system func (runtime *CRIRuntime) ImageExists(image string) (bool, error) { - 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 + err := runtime.exec.Command("crictl", "-r", runtime.criSocket, "inspecti", image).Run() + return err == nil, nil } // ImageExists checks to see if the image exists on the system func (runtime *DockerRuntime) ImageExists(image string) (bool, error) { - 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 + err := runtime.exec.Command("docker", "inspect", image).Run() + return err == nil, nil } diff --git a/cmd/kubeadm/app/util/runtime/runtime_test.go b/cmd/kubeadm/app/util/runtime/runtime_test.go index 47cc591bd9..d4702d1120 100644 --- a/cmd/kubeadm/app/util/runtime/runtime_test.go +++ b/cmd/kubeadm/app/util/runtime/runtime_test.go @@ -266,15 +266,15 @@ func TestPullImage(t *testing.T) { func TestImageExists(t *testing.T) { fcmd := fakeexec.FakeCmd{ - 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} }, + 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} }, }, } execer := fakeexec.FakeExec{ - CommandScript: genFakeActions(&fcmd, len(fcmd.CombinedOutputScript)), + CommandScript: genFakeActions(&fcmd, len(fcmd.RunScript)), LookPathFunc: func(cmd string) (string, error) { return "/usr/bin/crictl", nil }, } @@ -282,7 +282,7 @@ func TestImageExists(t *testing.T) { name string criSocket string image string - isError bool + result bool }{ {"valid: test if image exists using CRI", "unix:///var/run/crio/crio.sock", "image1", false}, {"invalid: CRI inspecti failure", "unix:///var/run/crio/crio.sock", "image2", true}, @@ -298,14 +298,8 @@ func TestImageExists(t *testing.T) { } result, err := runtime.ImageExists(tc.image) - if result && err != nil { - t.Errorf("unexpected ImageExists result %v and error: %v", result, err) - } - if !tc.isError && err != nil { - t.Errorf("unexpected ImageExists error: %v, criSocket: %s, image: %s", err, tc.criSocket, tc.image) - } - if tc.isError && err == nil { - t.Errorf("unexpected ImageExists success, criSocket: %s, image: %s", tc.criSocket, tc.image) + if !tc.result != result { + t.Errorf("unexpected ImageExists result: %t, criSocket: %s, image: %s, expected result: %t", err, tc.criSocket, tc.image, tc.result) } }) }