Skip to content

Commit

Permalink
Create a Docker daemon from a RunContext
Browse files Browse the repository at this point in the history
Signed-off-by: David Gageot <[email protected]>
  • Loading branch information
dgageot committed Jul 23, 2019
1 parent f142948 commit 774b787
Show file tree
Hide file tree
Showing 12 changed files with 40 additions and 34 deletions.
5 changes: 3 additions & 2 deletions integration/build_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import (
"4d63.com/tz"
"github.com/GoogleContainerTools/skaffold/integration/skaffold"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/docker/docker/api/types"
)
Expand Down Expand Up @@ -114,7 +115,7 @@ func removeImage(t *testing.T, image string) {
return
}

client, err := docker.NewAPIClient(false, nil)
client, err := docker.NewAPIClient(&runcontext.RunContext{})
failNowIfError(t, err)

ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(5*time.Second))
Expand All @@ -133,7 +134,7 @@ func checkImageExists(t *testing.T, image string) {
return
}

client, err := docker.NewAPIClient(false, nil)
client, err := docker.NewAPIClient(&runcontext.RunContext{})
failNowIfError(t, err)

ctx, cancel := context.WithDeadline(context.Background(), time.Now().Add(5*time.Second))
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ func NewCache(runCtx *runcontext.RunContext, imagesAreLocal bool, dependencies D
return &noCache{}, nil
}

client, err := docker.NewAPIClient(runCtx.Opts.Prune(), runCtx.InsecureRegistries)
client, err := docker.NewAPIClient(runCtx)
if imagesAreLocal && err != nil {
return nil, errors.Wrap(err, "getting local Docker client")
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/skaffold/build/cache/retrieve_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func TestCacheBuildLocal(t *testing.T) {

// Mock Docker
dockerDaemon := docker.NewLocalDaemon(&testutil.FakeAPIClient{}, nil, false, nil)
t.Override(&docker.NewAPIClient, func(bool, map[string]bool) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return dockerDaemon, nil
})

Expand Down Expand Up @@ -180,7 +180,7 @@ func TestCacheBuildRemote(t *testing.T) {

// Mock Docker
dockerDaemon := docker.NewLocalDaemon(&testutil.FakeAPIClient{}, nil, false, nil)
t.Override(&docker.NewAPIClient, func(bool, map[string]bool) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return dockerDaemon, nil
})

Expand Down
3 changes: 2 additions & 1 deletion pkg/skaffold/build/local/custom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,15 @@ import (
"testing"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/testutil"
)

func TestRetrieveEnv(t *testing.T) {
testutil.Run(t, "", func(t *testutil.T) {
extraEnv := []string{"EXTRA_ENV=additional"}
t.Override(&docker.NewAPIClient, func(bool, map[string]bool) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return docker.NewLocalDaemon(&testutil.FakeAPIClient{}, extraEnv, false, nil), nil
})

Expand Down
5 changes: 3 additions & 2 deletions pkg/skaffold/build/local/jib_gradle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/testutil"
Expand Down Expand Up @@ -67,7 +68,7 @@ func TestBuildJibGradleToDocker(t *testing.T) {
}

t.Override(&util.DefaultExecCommand, test.cmd)
t.Override(&docker.NewAPIClient, func(bool, map[string]bool) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return docker.NewLocalDaemon(api, nil, false, nil), nil
})

Expand Down Expand Up @@ -129,7 +130,7 @@ func TestBuildJibGradleToRegistry(t *testing.T) {
}
return "", errors.New("unknown remote tag")
})
t.Override(&docker.NewAPIClient, func(bool, map[string]bool) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return docker.NewLocalDaemon(&testutil.FakeAPIClient{}, nil, false, nil), nil
})

Expand Down
5 changes: 3 additions & 2 deletions pkg/skaffold/build/local/jib_maven_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"testing"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/testutil"
Expand Down Expand Up @@ -83,7 +84,7 @@ func TestBuildJibMavenToDocker(t *testing.T) {
api := &testutil.FakeAPIClient{
TagToImageID: map[string]string{"img:tag": "imageID"},
}
t.Override(&docker.NewAPIClient, func(bool, map[string]bool) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return docker.NewLocalDaemon(api, nil, false, nil), nil
})

Expand Down Expand Up @@ -161,7 +162,7 @@ func TestBuildJibMavenToRegistry(t *testing.T) {
}
return "", errors.New("unknown remote tag")
})
t.Override(&docker.NewAPIClient, func(bool, map[string]bool) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return docker.NewLocalDaemon(&testutil.FakeAPIClient{}, nil, false, nil), nil
})

Expand Down
10 changes: 5 additions & 5 deletions pkg/skaffold/build/local/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -219,7 +219,7 @@ func TestLocalRun(t *testing.T) {
t.Override(&docker.DefaultAuthHelper, testAuthHelper{})
fakeWarner := &warnings.Collect{}
t.Override(&warnings.Printf, fakeWarner.Warnf)
t.Override(&docker.NewAPIClient, func(bool, map[string]bool) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return docker.NewLocalDaemon(&test.api, nil, false, nil), nil
})

Expand Down Expand Up @@ -256,18 +256,18 @@ func TestNewBuilder(t *testing.T) {
localBuild latest.LocalBuild
expectedBuilder *Builder
localClusterFn func() (bool, error)
localDockerFn func(bool, map[string]bool) (docker.LocalDaemon, error)
localDockerFn func(*runcontext.RunContext) (docker.LocalDaemon, error)
}{
{
description: "failed to get docker client",
localDockerFn: func(bool, map[string]bool) (docker.LocalDaemon, error) {
localDockerFn: func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return nil, errors.New("dummy docker error")
},
shouldErr: true,
},
{
description: "pushImages becomes !localCluster when local:push is not defined",
localDockerFn: func(bool, map[string]bool) (docker.LocalDaemon, error) {
localDockerFn: func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return dummyDaemon, nil
},
localClusterFn: func() (b bool, e error) {
Expand All @@ -289,7 +289,7 @@ func TestNewBuilder(t *testing.T) {
},
{
description: "pushImages defined in config (local:push)",
localDockerFn: func(bool, map[string]bool) (docker.LocalDaemon, error) {
localDockerFn: func(*runcontext.RunContext) (docker.LocalDaemon, error) {
return dummyDaemon, nil
},
localClusterFn: func() (b bool, e error) {
Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/local/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ var getLocalCluster = configutil.GetLocalCluster

// NewBuilder returns an new instance of a local Builder.
func NewBuilder(runCtx *runcontext.RunContext) (*Builder, error) {
localDocker, err := docker.NewAPIClient(runCtx.Opts.Prune(), runCtx.InsecureRegistries)
localDocker, err := docker.NewAPIClient(runCtx)
if err != nil {
return nil, errors.Wrap(err, "getting docker client")
}
Expand Down
14 changes: 8 additions & 6 deletions pkg/skaffold/debug/debug.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,16 +22,15 @@ import (
"context"
"strings"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"

"k8s.io/apimachinery/pkg/runtime"
serializer "k8s.io/apimachinery/pkg/runtime/serializer/json"
"k8s.io/client-go/kubernetes/scheme"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
)

var (
Expand Down Expand Up @@ -99,7 +98,10 @@ func findArtifact(image string, builds []build.Artifact) *build.Artifact {
// retrieveImageConfiguration retrieves the image container configuration for
// the given build artifact
func retrieveImageConfiguration(ctx context.Context, artifact *build.Artifact, insecureRegistries map[string]bool) (imageConfiguration, error) {
apiClient, err := docker.NewAPIClient(false, insecureRegistries)
// TODO: use the proper RunContext
apiClient, err := docker.NewAPIClient(&runcontext.RunContext{
InsecureRegistries: insecureRegistries,
})
if err != nil {
return imageConfiguration{}, errors.Wrap(err, "could not connect to local docker daemon")
}
Expand Down
14 changes: 4 additions & 10 deletions pkg/skaffold/docker/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import (
"sync"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context"
runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/version"
"github.com/docker/docker/api"
Expand All @@ -52,16 +52,10 @@ var (
)

// NewAPIClientImpl guesses the docker client to use based on current kubernetes context.
func NewAPIClientImpl(forceRemove bool, insecureRegistries map[string]bool) (LocalDaemon, error) {
func NewAPIClientImpl(runCtx *runcontext.RunContext) (LocalDaemon, error) {
dockerAPIClientOnce.Do(func() {
kubeConfig, err := kubectx.CurrentConfig()
if err != nil {
dockerAPIClientErr = errors.Wrap(err, "getting current cluster context")
return
}

env, apiClient, err := newAPIClient(kubeConfig.CurrentContext)
dockerAPIClient = NewLocalDaemon(apiClient, env, forceRemove, insecureRegistries)
env, apiClient, err := newAPIClient(runCtx.KubeContext)
dockerAPIClient = NewLocalDaemon(apiClient, env, runCtx.Opts.Prune(), runCtx.InsecureRegistries)
dockerAPIClientErr = err
})

Expand Down
4 changes: 3 additions & 1 deletion pkg/skaffold/docker/image_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"context"
"strings"

runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/pkg/errors"
"github.com/sirupsen/logrus"
Expand All @@ -33,7 +34,8 @@ func RetrieveWorkingDir(tagged string, insecureRegistries map[string]bool) (stri
return "/", nil
}

localDocker, err := NewAPIClient(false, nil)
// TODO: use the proper RunContext
localDocker, err := NewAPIClient(&runcontext.RunContext{})
if err == nil {
cf, err = localDocker.ConfigFile(context.Background(), tagged)
}
Expand Down
6 changes: 5 additions & 1 deletion pkg/skaffold/docker/parse.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"path/filepath"
"strings"

runcontext "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/context"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
v1 "github.com/google/go-containerregistry/pkg/v1"
"github.com/moby/buildkit/frontend/dockerfile/command"
Expand Down Expand Up @@ -347,7 +348,10 @@ func fromInstruction(node *parser.Node) from {
}

func retrieveImage(image string, insecureRegistries map[string]bool) (*v1.ConfigFile, error) {
localDaemon, err := NewAPIClient(false, insecureRegistries) // Cached after first call
// TODO: use the proper RunContext
localDaemon, err := NewAPIClient(&runcontext.RunContext{
InsecureRegistries: insecureRegistries,
})
if err != nil {
return nil, errors.Wrap(err, "getting docker client")
}
Expand Down

0 comments on commit 774b787

Please sign in to comment.