Skip to content

Commit

Permalink
fix skaffold test to use minikube docker-env docker context (#5815)
Browse files Browse the repository at this point in the history
What is the problem being solved?
Fixes #5696, fixing skaffold test code to use minikube docker env in test environement command context when a minikube cluster is being used.

Why is this the best approach?
This approach re-uses the minikube detection and docker context setup used by skaffold builders which is in pkg/skaffold/docker/client.go.  This way the build environment env vars will be identicla for both the build and test without having to duplicate code. The downside here is that the method used creates a docker client as well that is not used for test but this is likely ok because the docker client is a singleton (sync.Once) and as such should not require additional overhead.

What other approaches did you consider?
I considered copying over some of the minikube specific docker-env logic from pkg/skaffold/docker/client.go and removing the client creation pieces but essentially the minikube logic would be in two places and build and test would use different image env related code when the issue is mainly around them not using the same code here.  It might make sense to refactor the docker client code to smaller pieces but I didn't want to change the public API and I think what is here should work fine.

What side effects will this approach have?
There should be no side affects to this approach.  This should make it so the env vars used for build and test are aligned, most importantly for the minikube case.

What future work remains to be done?
N/A
  • Loading branch information
aaron-prindle committed May 12, 2021
1 parent 9002315 commit b889708
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 0 deletions.
11 changes: 11 additions & 0 deletions pkg/skaffold/test/custom/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ var (
const Windows string = "windows"

type Runner struct {
cfg docker.Config
customTest latestV1.CustomTest
imageName string
workspace string
Expand All @@ -50,6 +51,7 @@ type Runner struct {
// New creates a new custom.Runner.
func New(cfg docker.Config, imageName string, ws string, ct latestV1.CustomTest) (*Runner, error) {
return &Runner{
cfg: cfg,
imageName: imageName,
customTest: ct,
workspace: ws,
Expand Down Expand Up @@ -155,6 +157,7 @@ func (ct *Runner) TestDependencies() ([]string, error) {
func (ct *Runner) retrieveCmd(ctx context.Context, out io.Writer, command string, imageTag string) (*exec.Cmd, error) {
var cmd *exec.Cmd
// We evaluate the command with a shell so that it can contain env variables.

if runtime.GOOS == Windows {
cmd = exec.CommandContext(ctx, "cmd.exe", "/C", command)
} else {
Expand Down Expand Up @@ -190,6 +193,14 @@ func (ct *Runner) getEnv(imageTag string) ([]string, error) {
}

envs = append(envs, util.OSEnviron()...)

// add minikube docker env vars to command env context if minikube cluster detected
localDaemon, err := docker.NewAPIClient(ct.cfg)
if err != nil {
return nil, fmt.Errorf("getting docker client information: %w", err)
}
envs = append(envs, localDaemon.ExtraEnv()...)

return envs, nil
}

Expand Down
27 changes: 27 additions & 0 deletions pkg/skaffold/test/custom/custom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,20 +24,29 @@ import (
"runtime"
"testing"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
latestV1 "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest/v1"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/testutil"
testEvent "github.com/GoogleContainerTools/skaffold/testutil/event"
)

func fakeLocalDaemonWithExtraEnv(extraEnv []string) docker.LocalDaemon {
return docker.NewLocalDaemon(&testutil.FakeAPIClient{}, extraEnv, false, nil)
}

func TestNewCustomTestRunner(t *testing.T) {
testutil.Run(t, "Testing new custom test runner", func(t *testutil.T) {
if runtime.GOOS == Windows {
t.Override(&util.DefaultExecCommand, testutil.CmdRun("cmd.exe /C echo Running Custom Test command."))
} else {
t.Override(&util.DefaultExecCommand, testutil.CmdRun("sh -c echo Running Custom Test command."))
}
t.Override(&docker.NewAPIClient, func(cfg docker.Config) (docker.LocalDaemon, error) {
return fakeLocalDaemonWithExtraEnv([]string{}), nil
})

tmpDir := t.NewTempDir().Touch("test.yaml")

custom := latestV1.CustomTest{
Expand Down Expand Up @@ -107,6 +116,9 @@ func TestCustomCommandError(t *testing.T) {
command = test.expectedWindowsCmd
}
t.Override(&util.DefaultExecCommand, testutil.CmdRunErr(command, fmt.Errorf(test.expectedError)))
t.Override(&docker.NewAPIClient, func(cfg docker.Config) (docker.LocalDaemon, error) {
return fakeLocalDaemonWithExtraEnv([]string{}), nil
})

testCase := &latestV1.TestCase{
ImageName: "image",
Expand Down Expand Up @@ -254,6 +266,7 @@ func TestGetEnv(t *testing.T) {
testContext string
environ []string
expected []string
extraEnv []string
}{

{
Expand All @@ -269,11 +282,25 @@ func TestGetEnv(t *testing.T) {
testContext: "/some/path",
expected: []string{"IMAGE=gcr.io/image/tag:anothertag", "TEST_CONTEXT=/some/path", "PATH=/path", "HOME=/root"},
},
{
description: "make sure minikube docker env is applied when minikube profile present",
tag: "gcr.io/image/tag:anothertag",
environ: []string{"PATH=/path", "HOME=/root"},
testContext: "/some/path",
expected: []string{"IMAGE=gcr.io/image/tag:anothertag", "TEST_CONTEXT=/some/path", "PATH=/path", "HOME=/root",
"DOCKER_CERT_PATH=/path/.minikube/certs", "DOCKER_HOST=tcp://192.168.49.2:2376",
"DOCKER_TLS_VERIFY=1", "MINIKUBE_ACTIVE_DOCKERD=minikube"},
extraEnv: []string{"DOCKER_CERT_PATH=/path/.minikube/certs", "DOCKER_HOST=tcp://192.168.49.2:2376",
"DOCKER_TLS_VERIFY=1", "MINIKUBE_ACTIVE_DOCKERD=minikube"},
},
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&util.OSEnviron, func() []string { return test.environ })
t.Override(&testContext, func(string) (string, error) { return test.testContext, nil })
t.Override(&docker.NewAPIClient, func(cfg docker.Config) (docker.LocalDaemon, error) {
return fakeLocalDaemonWithExtraEnv(test.extraEnv), nil
})
tmpDir := t.NewTempDir().Touch("test.yaml")

custom := latestV1.CustomTest{
Expand Down

0 comments on commit b889708

Please sign in to comment.