Run container-structure-test on remote images

Fixes GoogleContainerTools#3907 Fixes GoogleContainerTools#1082 Signed-off-by: David Gageot <[email protected]>
dgageot · Apr 20, 2020 · 728493e · 728493e
1 parent bd28019
commit 728493e
Show file tree

Hide file tree

Showing 7 changed files with 92 additions and 41 deletions.
diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go
@@ -66,7 +66,7 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) {
 		return nil, fmt.Errorf("initializing cache: %w", err)
 	}
 
-	tester := getTester(runCtx)
+	tester := getTester(runCtx, imagesAreLocal)
 	syncer := getSyncer(runCtx)
 
 	deployer, err := getDeployer(runCtx)
@@ -199,8 +199,8 @@ func getBuilder(runCtx *runcontext.RunContext) (build.Builder, error) {
 	}
 }
 
-func getTester(runCtx *runcontext.RunContext) test.Tester {
-	return test.NewTester(runCtx)
+func getTester(runCtx *runcontext.RunContext, imagesAreLocal bool) test.Tester {
+	return test.NewTester(runCtx, imagesAreLocal)
 }
 
 func getSyncer(runCtx *runcontext.RunContext) sync.Syncer {

diff --git a/pkg/skaffold/test/structure/structure.go b/pkg/skaffold/test/structure/structure.go
@@ -32,6 +32,15 @@ import (
 func (tr *Runner) Test(ctx context.Context, out io.Writer, image string) error {
 	logrus.Infof("Running structure tests for files %v", tr.testFiles)
 
+	if !tr.imagesAreLocal {
+		// First we have to pull the image.
+		// `container-structure-test` should be able to do it by itself
+		// but, in reality, it'll fail to pull images that are not public.
+		if err := tr.localDaemon.Pull(ctx, out, image); err != nil {
+			return fmt.Errorf("unable to docker pull image %q: %w", image, err)
+		}
+	}
+
 	args := []string{"test", "-v", "warn", "--image", image}
 	for _, f := range tr.testFiles {
 		args = append(args, "--config", f)
@@ -53,12 +62,13 @@ func (tr *Runner) Test(ctx context.Context, out io.Writer, image string) error {
 // This ensures that the correct docker environment configuration is passed to container-structure-test,
 // for example when running on minikube.
 func (tr *Runner) env() []string {
-	if tr.extraEnv == nil {
+	extraEnv := tr.localDaemon.ExtraEnv()
+	if extraEnv == nil {
 		return nil
 	}
 
 	parentEnv := os.Environ()
-	mergedEnv := make([]string, len(parentEnv), len(parentEnv)+len(tr.extraEnv))
+	mergedEnv := make([]string, len(parentEnv), len(parentEnv)+len(extraEnv))
 	copy(mergedEnv, parentEnv)
-	return append(mergedEnv, tr.extraEnv...)
+	return append(mergedEnv, extraEnv...)
 }
diff --git a/pkg/skaffold/test/structure/types.go b/pkg/skaffold/test/structure/types.go
@@ -16,14 +16,19 @@ limitations under the License.
 
 package structure
 
+import "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
+
 type Runner struct {
-	testFiles, extraEnv []string
+	testFiles      []string
+	localDaemon    docker.LocalDaemon
+	imagesAreLocal bool
 }
 
 // NewRunner creates a new structure.Runner.
-func NewRunner(files, extraEnv []string) *Runner {
+func NewRunner(files []string, localDaemon docker.LocalDaemon, imagesAreLocal bool) *Runner {
 	return &Runner{
-		testFiles: files,
-		extraEnv:  extraEnv,
+		testFiles:      files,
+		localDaemon:    localDaemon,
+		imagesAreLocal: imagesAreLocal,
 	}
 }
diff --git a/pkg/skaffold/test/structure/types_test.go b/pkg/skaffold/test/structure/types_test.go
@@ -21,25 +21,57 @@ import (
 	"io/ioutil"
 	"testing"
 
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/GoogleContainerTools/skaffold/testutil"
 )
 
 func TestNewRunner(t *testing.T) {
-	const (
-		imageName         = "foo.io/baz"
-		structureTestName = "foo.testcase"
-	)
-
-	testutil.Run(t, "", func(t *testutil.T) {
-		extraEnv := []string{"SOME=env_var", "OTHER=env_value"}
-		t.Override(&util.DefaultExecCommand, testutil.CmdRunEnv(
-			"container-structure-test test -v warn --image "+imageName+" --config "+structureTestName,
-			extraEnv,
-		))
-
-		testRunner := NewRunner([]string{structureTestName}, extraEnv)
-		err := testRunner.Test(context.Background(), ioutil.Discard, imageName)
-		t.CheckNoError(err)
-	})
+	var tests = []struct {
+		description    string
+		api            *testutil.FakeAPIClient
+		imagesAreLocal bool
+		shouldErr      bool
+	}{
+		{
+			description:    "success - local image",
+			api:            &testutil.FakeAPIClient{},
+			imagesAreLocal: true,
+			shouldErr:      false,
+		},
+		{
+			description:    "success - remote image",
+			api:            &testutil.FakeAPIClient{},
+			imagesAreLocal: false,
+			shouldErr:      false,
+		},
+		{
+			description: "failure - remote image not found",
+			api: &testutil.FakeAPIClient{
+				ErrImagePull: true,
+			},
+			imagesAreLocal: false,
+			shouldErr:      true,
+		},
+	}
+
+	for _, test := range tests {
+		testutil.Run(t, test.description, func(t *testutil.T) {
+			extraEnv := []string{"SOME=env_var", "OTHER=env_value"}
+			localDaemon := docker.NewLocalDaemon(test.api, extraEnv, false, nil)
+			t.Override(&util.DefaultExecCommand, testutil.CmdRunEnv(
+				"container-structure-test test -v warn --image foo.io/baz --config foo.testcase",
+				extraEnv,
+			))
+
+			testRunner := NewRunner([]string{"foo.testcase"}, localDaemon, test.imagesAreLocal)
+			err := testRunner.Test(context.Background(), ioutil.Discard, "foo.io/baz")
+
+			if test.shouldErr {
+				t.CheckErrorContains(`unable to docker pull image "foo.io/baz"`, err)
+			} else {
+				t.CheckNoError(err)
+			}
+		})
+	}
 }
diff --git a/pkg/skaffold/test/test.go b/pkg/skaffold/test/test.go
@@ -32,16 +32,17 @@ import (
 // NewTester parses the provided test cases from the Skaffold config,
 // and returns a Tester instance with all the necessary test runners
 // to run all specified tests.
-func NewTester(runCtx *runcontext.RunContext) Tester {
-	client, err := docker.NewAPIClient(runCtx)
+func NewTester(runCtx *runcontext.RunContext, imagesAreLocal bool) Tester {
+	localDaemon, err := docker.NewAPIClient(runCtx)
 	if err != nil {
 		return nil
 	}
 
 	return FullTester{
-		testCases:  runCtx.Cfg.Test,
-		workingDir: runCtx.WorkingDir,
-		extraEnv:   client.ExtraEnv(),
+		testCases:      runCtx.Cfg.Test,
+		workingDir:     runCtx.WorkingDir,
+		localDaemon:    localDaemon,
+		imagesAreLocal: imagesAreLocal,
 	}
 }
 
@@ -85,7 +86,7 @@ func (t FullTester) runStructureTests(ctx context.Context, out io.Writer, bRes [
 
 	fqn := resolveArtifactImageTag(testCase.ImageName, bRes)
 
-	runner := structure.NewRunner(files, t.extraEnv)
+	runner := structure.NewRunner(files, t.localDaemon, t.imagesAreLocal)
 	return runner.Test(ctx, out, fqn)
 }
 

diff --git a/pkg/skaffold/test/test_test.go b/pkg/skaffold/test/test_test.go
@@ -32,7 +32,7 @@ import (
 func TestNoTestDependencies(t *testing.T) {
 	runCtx := &runcontext.RunContext{}
 
-	deps, err := NewTester(runCtx).TestDependencies()
+	deps, err := NewTester(runCtx, true).TestDependencies()
 
 	testutil.CheckErrorAndDeepEqual(t, false, err, 0, len(deps))
 }
@@ -53,7 +53,7 @@ func TestTestDependencies(t *testing.T) {
 		},
 	}
 
-	deps, err := NewTester(runCtx).TestDependencies()
+	deps, err := NewTester(runCtx, true).TestDependencies()
 
 	expectedDeps := tmpDir.Paths("tests/test1.yaml", "tests/test2.yaml", "test3.yaml")
 	testutil.CheckErrorAndDeepEqual(t, false, err, expectedDeps, deps)
@@ -68,7 +68,7 @@ func TestWrongPattern(t *testing.T) {
 		},
 	}
 
-	tester := NewTester(runCtx)
+	tester := NewTester(runCtx, true)
 
 	_, err := tester.TestDependencies()
 	testutil.CheckError(t, true, err)
@@ -80,7 +80,7 @@ func TestWrongPattern(t *testing.T) {
 func TestNoTest(t *testing.T) {
 	runCtx := &runcontext.RunContext{}
 
-	err := NewTester(runCtx).Test(context.Background(), ioutil.Discard, nil)
+	err := NewTester(runCtx, true).Test(context.Background(), ioutil.Discard, nil)
 
 	testutil.CheckError(t, false, err)
 }
@@ -111,7 +111,8 @@ func TestTestSuccess(t *testing.T) {
 			},
 		}
 
-		err := NewTester(runCtx).Test(context.Background(), ioutil.Discard, []build.Artifact{{
+		imagesAreLocal := true
+		err := NewTester(runCtx, imagesAreLocal).Test(context.Background(), ioutil.Discard, []build.Artifact{{
 			ImageName: "image",
 			Tag:       "TAG",
 		}})
@@ -141,7 +142,7 @@ func TestTestFailure(t *testing.T) {
 			},
 		}
 
-		err := NewTester(runCtx).Test(context.Background(), ioutil.Discard, []build.Artifact{{}})
+		err := NewTester(runCtx, true).Test(context.Background(), ioutil.Discard, []build.Artifact{{}})
 		t.CheckError(true, err)
 	})
 }
diff --git a/pkg/skaffold/test/types.go b/pkg/skaffold/test/types.go
@@ -21,6 +21,7 @@ import (
 	"io"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 )
 
@@ -42,9 +43,10 @@ type Tester interface {
 // FullTester should always be the ONLY implementation of the Tester interface;
 // newly added testing implementations should implement the Runner interface.
 type FullTester struct {
-	testCases  []*latest.TestCase
-	workingDir string
-	extraEnv   []string
+	testCases      []*latest.TestCase
+	localDaemon    docker.LocalDaemon
+	workingDir     string
+	imagesAreLocal bool
 }
 
 // Runner is the lowest-level test executor in Skaffold, responsible for