Merge pull request #2597 from corneliusweig/w/minikube-structure-test

Pass minikube docker configuration to container-structure-test

Author: nkubala

pkg/skaffold/test/structure/structure.go

@@ -19,11 +19,13 @@ package structure
 import (
 	"context"
 	"io"
+	"os"
 	"os/exec"
 
-	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
 )
 
 // Test is the entrypoint for running structure tests
@@ -38,10 +40,25 @@ func (tr *Runner) Test(ctx context.Context, out io.Writer, image string) error {
 	cmd := exec.CommandContext(ctx, "container-structure-test", args...)
 	cmd.Stdout = out
 	cmd.Stderr = out
+	cmd.Env = tr.env()
 
 	if err := util.RunCmd(cmd); err != nil {
 		return errors.Wrap(err, "running container-structure-test")
 	}
 
 	return nil
 }
+
+// env returns a merged environment of the current process environment and any extra environment.
+// 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 {
+		return nil
+	}
+
+	parentEnv := os.Environ()
+	mergedEnv := make([]string, len(parentEnv), len(parentEnv)+len(tr.extraEnv))
+	copy(mergedEnv, parentEnv)
+	return append(mergedEnv, tr.extraEnv...)
+}

pkg/skaffold/test/structure/types.go

@@ -17,12 +17,13 @@ limitations under the License.
 package structure
 
 type Runner struct {
-	testFiles []string
+	testFiles, extraEnv []string
 }
 
 // NewRunner creates a new structure.Runner.
-func NewRunner(files []string) *Runner {
+func NewRunner(files, extraEnv []string) *Runner {
 	return &Runner{
 		testFiles: files,
+		extraEnv:  extraEnv,
 	}
 }

pkg/skaffold/test/structure/types_test.go

@@ -0,0 +1,44 @@
+/*
+Copyright 2019 The Skaffold Authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+package structure
+
+import (
+	"context"
+	"io/ioutil"
+	"testing"
+
+	"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"}
+		fakeCmd := t.NewFakeCmd().
+			WithRunEnv("container-structure-test test -v warn --image "+imageName+" --config "+structureTestName, extraEnv)
+		t.Override(&util.DefaultExecCommand, fakeCmd)
+
+		testRunner := NewRunner([]string{structureTestName}, extraEnv)
+		err := testRunner.Test(context.Background(), ioutil.Discard, imageName)
+		t.CheckNoError(err)
+	})
+}

pkg/skaffold/test/test.go

@@ -20,22 +20,29 @@ import (
 	"context"
 	"io"
 
+	"github.com/pkg/errors"
+
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
+	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/test/structure"
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
-
-	"github.com/pkg/errors"
 )
 
 // 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)
+	if err != nil {
+		return nil
+	}
+
 	return FullTester{
 		testCases:  runCtx.Cfg.Test,
 		workingDir: runCtx.WorkingDir,
+		extraEnv:   client.ExtraEnv(),
 	}
 }
 
@@ -79,7 +86,7 @@ func (t FullTester) runStructureTests(ctx context.Context, out io.Writer, bRes [
 
 	fqn := resolveArtifactImageTag(testCase.ImageName, bRes)
 
-	runner := structure.NewRunner(files)
+	runner := structure.NewRunner(files, t.extraEnv)
 	return runner.Test(ctx, out, fqn)
 }
 

pkg/skaffold/test/test_test.go

@@ -87,66 +87,63 @@ func TestNoTest(t *testing.T) {
 }
 
 func TestTestSuccess(t *testing.T) {
-	tmpDir, cleanup := testutil.NewTempDir(t)
-	defer cleanup()
-
-	tmpDir.Touch("tests/test1.yaml", "tests/test2.yaml", "test3.yaml")
-
-	defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand)
-	util.DefaultExecCommand = testutil.
-		NewFakeCmd(t).
-		WithRun("container-structure-test test -v warn --image TAG --config " + tmpDir.Path("tests/test1.yaml") + " --config " + tmpDir.Path("tests/test2.yaml")).
-		WithRun("container-structure-test test -v warn --image TAG --config " + tmpDir.Path("test3.yaml"))
-
-	runCtx := &runcontext.RunContext{
-		WorkingDir: tmpDir.Root(),
-		Cfg: latest.Pipeline{
-			Test: []*latest.TestCase{
-				{
-					ImageName:      "image",
-					StructureTests: []string{"./tests/*"},
-				},
-				{},
-				{
-					ImageName:      "image",
-					StructureTests: []string{"test3.yaml"},
+	testutil.Run(t, "", func(t *testutil.T) {
+		tmpDir := t.NewTempDir()
+		tmpDir.Touch("tests/test1.yaml", "tests/test2.yaml", "test3.yaml")
+
+		fakeCmd := t.NewFakeCmd().
+			WithRun("container-structure-test test -v warn --image TAG --config " + tmpDir.Path("tests/test1.yaml") + " --config " + tmpDir.Path("tests/test2.yaml")).
+			WithRun("container-structure-test test -v warn --image TAG --config " + tmpDir.Path("test3.yaml"))
+		t.Override(&util.DefaultExecCommand, fakeCmd)
+
+		runCtx := &runcontext.RunContext{
+			WorkingDir: tmpDir.Root(),
+			Cfg: latest.Pipeline{
+				Test: []*latest.TestCase{
+					{
+						ImageName:      "image",
+						StructureTests: []string{"./tests/*"},
+					},
+					{},
+					{
+						ImageName:      "image",
+						StructureTests: []string{"test3.yaml"},
+					},
 				},
 			},
-		},
-	}
+		}
 
-	err := NewTester(runCtx).Test(context.Background(), ioutil.Discard, []build.Artifact{{
-		ImageName: "image",
-		Tag:       "TAG",
-	}})
+		err := NewTester(runCtx).Test(context.Background(), ioutil.Discard, []build.Artifact{{
+			ImageName: "image",
+			Tag:       "TAG",
+		}})
 
-	testutil.CheckError(t, false, err)
+		t.CheckError(false, err)
+	})
 }
 
 func TestTestFailure(t *testing.T) {
-	tmpDir, cleanup := testutil.NewTempDir(t)
-	defer cleanup()
-
-	tmpDir.Touch("test.yaml")
-
-	defer func(c util.Command) { util.DefaultExecCommand = c }(util.DefaultExecCommand)
-	util.DefaultExecCommand = testutil.
-		NewFakeCmd(t).
-		WithRunErr("container-structure-test test -v warn --image broken-image --config "+tmpDir.Path("test.yaml"), errors.New("FAIL"))
-
-	runCtx := &runcontext.RunContext{
-		WorkingDir: tmpDir.Root(),
-		Cfg: latest.Pipeline{
-			Test: []*latest.TestCase{
-				{
-					ImageName:      "broken-image",
-					StructureTests: []string{"test.yaml"},
+	testutil.Run(t, "", func(t *testutil.T) {
+		tmpDir := t.NewTempDir()
+		tmpDir.Touch("test.yaml")
+
+		fakeCmd := t.NewFakeCmd().
+			WithRunErr("container-structure-test test -v warn --image broken-image --config "+tmpDir.Path("test.yaml"), errors.New("FAIL"))
+		t.Override(&util.DefaultExecCommand, fakeCmd)
+
+		runCtx := &runcontext.RunContext{
+			WorkingDir: tmpDir.Root(),
+			Cfg: latest.Pipeline{
+				Test: []*latest.TestCase{
+					{
+						ImageName:      "broken-image",
+						StructureTests: []string{"test.yaml"},
+					},
 				},
 			},
-		},
-	}
-
-	err := NewTester(runCtx).Test(context.Background(), ioutil.Discard, []build.Artifact{{}})
+		}
 
-	testutil.CheckError(t, true, err)
+		err := NewTester(runCtx).Test(context.Background(), ioutil.Discard, []build.Artifact{{}})
+		t.CheckError(true, err)
+	})
 }

pkg/skaffold/test/types.go

@@ -44,6 +44,7 @@ type Tester interface {
 type FullTester struct {
 	testCases  []*latest.TestCase
 	workingDir string
+	extraEnv   []string
 }
 
 // Runner is the lowest-level test executor in Skaffold, responsible for

testutil/cmd_helper.go

@@ -34,6 +34,7 @@ type run struct {
 	command string
 	input   []byte
 	output  []byte
+	env     []string
 	err     error
 }
 
@@ -117,6 +118,14 @@ func (c *FakeCmd) WithRunOutErr(command string, output string, err error) *FakeC
 	})
 }
 
+// WithRunEnv registers a command that requires the given env variables to be set.
+func (c *FakeCmd) WithRunEnv(command string, env []string) *FakeCmd {
+	return c.addRun(run{
+		command: command,
+		env:     env,
+	})
+}
+
 func (c *FakeCmd) RunCmdOut(cmd *exec.Cmd) ([]byte, error) {
 	command := strings.Join(cmd.Args, " ")
 
@@ -129,6 +138,8 @@ func (c *FakeCmd) RunCmdOut(cmd *exec.Cmd) ([]byte, error) {
 		c.t.Errorf("expected: %s. Got: %s", r.command, command)
 	}
 
+	c.assertCmdEnv(r.env, cmd.Env)
+
 	if r.output == nil {
 		c.t.Errorf("expected RunCmd(%s) to be called. Got RunCmdOut(%s)", r.command, command)
 	}
@@ -152,6 +163,8 @@ func (c *FakeCmd) RunCmd(cmd *exec.Cmd) error {
 		c.t.Errorf("expected RunCmdOut(%s) to be called. Got RunCmd(%s)", r.command, command)
 	}
 
+	c.assertCmdEnv(r.env, cmd.Env)
+
 	if r.input != nil {
 		if cmd.Stdin == nil {
 			c.t.Error("expected to run the command with a custom stdin", command)
@@ -171,3 +184,22 @@ func (c *FakeCmd) RunCmd(cmd *exec.Cmd) error {
 
 	return r.err
 }
+
+// assertCmdEnv ensures that actualEnv contains all values from requiredEnv
+func (c *FakeCmd) assertCmdEnv(requiredEnv, actualEnv []string) {
+	if requiredEnv == nil {
+		return
+	}
+	c.t.Helper()
+
+	envs := make(map[string]struct{}, len(actualEnv))
+	for _, e := range actualEnv {
+		envs[e] = struct{}{}
+	}
+
+	for _, e := range requiredEnv {
+		if _, ok := envs[e]; !ok {
+			c.t.Errorf("expected env variable with value %q", e)
+		}
+	}
+}

testutil/util.go

@@ -35,6 +35,10 @@ type T struct {
 	teardownActions []func()
 }
 
+func (t *T) NewFakeCmd() *FakeCmd {
+	return NewFakeCmd(t.T)
+}
+
 func (t *T) FakeRun(command string) *FakeCmd {
 	return FakeRun(t.T, command)
 }