Skip to content

Commit

Permalink
Introduce Config interfaces
Browse files Browse the repository at this point in the history
Signed-off-by: David Gageot <david@gageot.net>
  • Loading branch information
dgageot committed Aug 31, 2020
1 parent 13efa7e commit 1f577f9
Show file tree
Hide file tree
Showing 30 changed files with 258 additions and 161 deletions.
3 changes: 1 addition & 2 deletions cmd/skaffold/app/cmd/runner_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import (

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"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/update"
"github.com/GoogleContainerTools/skaffold/testutil"
Expand Down Expand Up @@ -86,7 +85,7 @@ func TestCreateNewRunner(t *testing.T) {
}
for _, test := range tests {
testutil.Run(t, test.description, func(t *testutil.T) {
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) {
return nil, nil
})
t.Override(&update.GetLatestAndCurrentVersion, func() (semver.Version, semver.Version, error) {
Expand Down
22 changes: 15 additions & 7 deletions pkg/skaffold/build/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@ import (
homedir "github.com/mitchellh/go-homedir"
"github.com/sirupsen/logrus"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"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/util"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/yaml"
Expand Down Expand Up @@ -55,13 +55,21 @@ type cache struct {
// DependencyLister fetches a list of dependencies for an artifact
type DependencyLister func(ctx context.Context, artifact *latest.Artifact) ([]string, error)

type Config interface {
docker.Config

CacheArtifacts() bool
CacheFile() string
Mode() config.RunMode
}

// NewCache returns the current state of the cache
func NewCache(runCtx *runcontext.RunContext, imagesAreLocal bool, dependencies DependencyLister) (Cache, error) {
if !runCtx.CacheArtifacts() {
func NewCache(cfg Config, imagesAreLocal bool, dependencies DependencyLister) (Cache, error) {
if !cfg.CacheArtifacts() {
return &noCache{}, nil
}

cacheFile, err := resolveCacheFile(runCtx.CacheFile())
cacheFile, err := resolveCacheFile(cfg.CacheFile())
if err != nil {
logrus.Warnf("Error resolving cache file, not using skaffold cache: %v", err)
return &noCache{}, nil
Expand All @@ -73,19 +81,19 @@ func NewCache(runCtx *runcontext.RunContext, imagesAreLocal bool, dependencies D
return &noCache{}, nil
}

client, err := docker.NewAPIClient(runCtx)
client, err := docker.NewAPIClient(cfg)
if imagesAreLocal && err != nil {
return nil, fmt.Errorf("getting local Docker client: %w", err)
}

return &cache{
artifactCache: artifactCache,
client: client,
insecureRegistries: runCtx.GetInsecureRegistries(),
insecureRegistries: cfg.GetInsecureRegistries(),
cacheFile: cacheFile,
imagesAreLocal: imagesAreLocal,
hashForArtifact: func(ctx context.Context, a *latest.Artifact) (string, error) {
return getHashForArtifact(ctx, dependencies, a, runCtx.Mode())
return getHashForArtifact(ctx, dependencies, a, cfg.Mode())
},
}, nil
}
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 @@ -118,7 +118,7 @@ func TestCacheBuildLocal(t *testing.T) {
// Mock Docker
t.Override(&docker.DefaultAuthHelper, stubAuth{})
dockerDaemon := docker.NewLocalDaemon(&testutil.FakeAPIClient{}, nil, false, nil)
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) {
return dockerDaemon, nil
})

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

// Mock Docker
dockerDaemon := docker.NewLocalDaemon(&testutil.FakeAPIClient{}, nil, false, nil)
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) {
return dockerDaemon, nil
})
t.Override(&docker.DefaultAuthHelper, stubAuth{})
Expand Down
26 changes: 18 additions & 8 deletions pkg/skaffold/build/cluster/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ import (
"time"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
)

Expand All @@ -39,20 +40,29 @@ type Builder struct {
muted build.Muted
}

type Config interface {
kubectl.Config
docker.Config

Pipeline() latest.Pipeline
GetKubeContext() string
Muted() config.Muted
}

// NewBuilder creates a new Builder that builds artifacts on cluster.
func NewBuilder(runCtx *runcontext.RunContext) (*Builder, error) {
timeout, err := time.ParseDuration(runCtx.Pipeline().Build.Cluster.Timeout)
func NewBuilder(cfg Config) (*Builder, error) {
timeout, err := time.ParseDuration(cfg.Pipeline().Build.Cluster.Timeout)
if err != nil {
return nil, fmt.Errorf("parsing timeout: %w", err)
}

return &Builder{
ClusterDetails: runCtx.Pipeline().Build.Cluster,
kubectlcli: kubectl.NewFromRunContext(runCtx),
ClusterDetails: cfg.Pipeline().Build.Cluster,
kubectlcli: kubectl.NewFromRunContext(cfg),
timeout: timeout,
kubeContext: runCtx.GetKubeContext(),
insecureRegistries: runCtx.GetInsecureRegistries(),
muted: runCtx.Muted(),
kubeContext: cfg.GetKubeContext(),
insecureRegistries: cfg.GetInsecureRegistries(),
muted: cfg.Muted(),
}, nil
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/skaffold/build/cluster/types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestNewBuilder(t *testing.T) {
tests := []struct {
description string
shouldErr bool
runCtx *runcontext.RunContext
runCtx Config
expectedBuilder *Builder
}{
{
Expand Down
21 changes: 15 additions & 6 deletions pkg/skaffold/build/gcb/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ import (
"k8s.io/apimachinery/pkg/util/wait"

"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
)

Expand Down Expand Up @@ -83,13 +84,21 @@ type Builder struct {
muted build.Muted
}

type Config interface {
docker.Config

Pipeline() latest.Pipeline
SkipTests() bool
Muted() config.Muted
}

// NewBuilder creates a new Builder that builds artifacts with Google Cloud Build.
func NewBuilder(runCtx *runcontext.RunContext) *Builder {
func NewBuilder(cfg Config) *Builder {
return &Builder{
GoogleCloudBuild: runCtx.Pipeline().Build.GoogleCloudBuild,
skipTests: runCtx.SkipTests(),
insecureRegistries: runCtx.GetInsecureRegistries(),
muted: runCtx.Muted(),
GoogleCloudBuild: cfg.Pipeline().Build.GoogleCloudBuild,
skipTests: cfg.SkipTests(),
insecureRegistries: cfg.GetInsecureRegistries(),
muted: cfg.Muted(),
}
}

Expand Down
6 changes: 6 additions & 0 deletions pkg/skaffold/build/jib/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@ type Builder struct {
skipTests bool
}

type Config interface {
docker.Config

SkipTests() bool
}

// NewArtifactBuilder returns a new customjib artifact builder
func NewArtifactBuilder(localDocker docker.LocalDaemon, insecureRegistries map[string]bool, pushImages, skipTests bool) *Builder {
return &Builder{
Expand Down
3 changes: 1 addition & 2 deletions pkg/skaffold/build/local/docker_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/cluster"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"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/util"
"github.com/GoogleContainerTools/skaffold/testutil"
Expand Down Expand Up @@ -86,7 +85,7 @@ func TestDockerCLIBuild(t *testing.T) {
))
t.Override(&cluster.GetClient, func() cluster.Client { return fakeMinikubeClient{} })
t.Override(&util.OSEnviron, func() []string { return []string{"KEY=VALUE"} })
t.Override(&docker.NewAPIClient, func(*runcontext.RunContext) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) {
return docker.NewLocalDaemon(&testutil.FakeAPIClient{}, test.extraEnv, 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 @@ -232,7 +232,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(*runcontext.RunContext) (docker.LocalDaemon, error) {
t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) {
return docker.NewLocalDaemon(test.api, nil, false, nil), nil
})
t.Override(&docker.EvalBuildArgs, func(mode config.RunMode, workspace string, a *latest.DockerArtifact) (map[string]*string, error) {
Expand Down Expand Up @@ -274,18 +274,18 @@ func TestNewBuilder(t *testing.T) {
localBuild latest.LocalBuild
expectedBuilder *Builder
localClusterFn func(string, string) (bool, error)
localDockerFn func(*runcontext.RunContext) (docker.LocalDaemon, error)
localDockerFn func(docker.Config) (docker.LocalDaemon, error)
}{
{
description: "failed to get docker client",
localDockerFn: func(*runcontext.RunContext) (docker.LocalDaemon, error) {
localDockerFn: func(docker.Config) (docker.LocalDaemon, error) {
return nil, errors.New("dummy docker error")
},
shouldErr: true,
},
{
description: "pushImages becomes !localCluster when local:push is not defined",
localDockerFn: func(*runcontext.RunContext) (docker.LocalDaemon, error) {
localDockerFn: func(docker.Config) (docker.LocalDaemon, error) {
return dummyDaemon, nil
},
localClusterFn: func(string, string) (b bool, e error) {
Expand All @@ -308,7 +308,7 @@ func TestNewBuilder(t *testing.T) {
},
{
description: "pushImages defined in config (local:push)",
localDockerFn: func(*runcontext.RunContext) (docker.LocalDaemon, error) {
localDockerFn: func(docker.Config) (docker.LocalDaemon, error) {
return dummyDaemon, nil
},
localClusterFn: func(string, string) (b bool, e error) {
Expand Down
39 changes: 25 additions & 14 deletions pkg/skaffold/build/local/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
)

Expand All @@ -52,9 +51,21 @@ type Builder struct {

var getLocalCluster = config.GetLocalCluster

type Config interface {
docker.Config

Pipeline() latest.Pipeline
GlobalConfig() string
GetKubeContext() string
SkipTests() bool
Mode() config.RunMode
NoPruneChildren() bool
Muted() config.Muted
}

// NewBuilder returns an new instance of a local Builder.
func NewBuilder(runCtx *runcontext.RunContext) (*Builder, error) {
localDocker, err := docker.NewAPIClient(runCtx)
func NewBuilder(cfg Config) (*Builder, error) {
localDocker, err := docker.NewAPIClient(cfg)
if err != nil {
return nil, fmt.Errorf("getting docker client: %w", err)
}
Expand All @@ -63,31 +74,31 @@ func NewBuilder(runCtx *runcontext.RunContext) (*Builder, error) {
// remove minikubeProfile from here and instead detect it by matching the
// kubecontext API Server to minikube profiles

localCluster, err := getLocalCluster(runCtx.GlobalConfig(), runCtx.MinikubeProfile())
localCluster, err := getLocalCluster(cfg.GlobalConfig(), cfg.MinikubeProfile())
if err != nil {
return nil, fmt.Errorf("getting localCluster: %w", err)
}

var pushImages bool
if runCtx.Pipeline().Build.LocalBuild.Push == nil {
if cfg.Pipeline().Build.LocalBuild.Push == nil {
pushImages = !localCluster
logrus.Debugf("push value not present, defaulting to %t because localCluster is %t", pushImages, localCluster)
} else {
pushImages = *runCtx.Pipeline().Build.LocalBuild.Push
pushImages = *cfg.Pipeline().Build.LocalBuild.Push
}

return &Builder{
cfg: *runCtx.Pipeline().Build.LocalBuild,
kubeContext: runCtx.GetKubeContext(),
cfg: *cfg.Pipeline().Build.LocalBuild,
kubeContext: cfg.GetKubeContext(),
localDocker: localDocker,
localCluster: localCluster,
pushImages: pushImages,
skipTests: runCtx.Opts.SkipTests,
mode: runCtx.Mode(),
prune: runCtx.Opts.Prune(),
pruneChildren: !runCtx.Opts.NoPruneChildren,
insecureRegistries: runCtx.InsecureRegistries,
muted: runCtx.Muted(),
skipTests: cfg.SkipTests(),
mode: cfg.Mode(),
prune: cfg.Prune(),
pruneChildren: !cfg.NoPruneChildren(),
insecureRegistries: cfg.GetInsecureRegistries(),
muted: cfg.Muted(),
}, nil
}

Expand Down
13 changes: 6 additions & 7 deletions pkg/skaffold/deploy/helm.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants"
"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/util"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/walk"
Expand Down Expand Up @@ -80,13 +79,13 @@ type HelmDeployer struct {
}

// NewHelmDeployer returns a configured HelmDeployer
func NewHelmDeployer(runCtx *runcontext.RunContext, labels map[string]string) *HelmDeployer {
func NewHelmDeployer(cfg Config, labels map[string]string) *HelmDeployer {
return &HelmDeployer{
HelmDeploy: runCtx.Pipeline().Deploy.HelmDeploy,
kubeContext: runCtx.GetKubeContext(),
kubeConfig: runCtx.GetKubeConfig(),
namespace: runCtx.GetKubeNamespace(),
forceDeploy: runCtx.ForceDeploy(),
HelmDeploy: cfg.Pipeline().Deploy.HelmDeploy,
kubeContext: cfg.GetKubeContext(),
kubeConfig: cfg.GetKubeConfig(),
namespace: cfg.GetKubeNamespace(),
forceDeploy: cfg.ForceDeploy(),
labels: labels,
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/skaffold/deploy/helm_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -403,7 +403,7 @@ func TestHelmDeploy(t *testing.T) {
tests := []struct {
description string
commands util.Command
runContext *runcontext.RunContext
runContext Config
builds []build.Artifact
shouldErr bool
expectedWarnings []string
Expand Down Expand Up @@ -833,7 +833,7 @@ func TestHelmCleanup(t *testing.T) {
tests := []struct {
description string
commands util.Command
runContext *runcontext.RunContext
runContext Config
builds []build.Artifact
shouldErr bool
expectedWarnings []string
Expand Down Expand Up @@ -1081,7 +1081,7 @@ func TestHelmRender(t *testing.T) {
description string
shouldErr bool
commands util.Command
runContext *runcontext.RunContext
runContext Config
outputFile string
expected string
builds []build.Artifact
Expand Down

0 comments on commit 1f577f9

Please sign in to comment.