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 16, 2020
1 parent 283e9c4 commit c43fb29
Show file tree
Hide file tree
Showing 27 changed files with 249 additions and 154 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
21 changes: 14 additions & 7 deletions pkg/skaffold/build/cache/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (

"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 +54,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
DevMode() bool
}

// 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 +80,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.DevMode())
return getHashForArtifact(ctx, dependencies, a, cfg.DevMode())
},
}, 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 @@ -200,7 +200,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
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 @@ -23,7 +23,6 @@ import (
"testing"

"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 @@ -77,7 +76,7 @@ func TestDockerCLIBuild(t *testing.T) {
test.expectedEnv,
))
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
})

Expand Down Expand Up @@ -272,18 +272,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 @@ -306,7 +306,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
DevMode() bool
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.SkipTests(),
devMode: runCtx.DevMode(),
prune: runCtx.Prune(),
pruneChildren: !runCtx.NoPruneChildren(),
insecureRegistries: runCtx.GetInsecureRegistries(),
muted: runCtx.Muted(),
skipTests: cfg.SkipTests(),
devMode: cfg.DevMode(),
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
29 changes: 20 additions & 9 deletions pkg/skaffold/deploy/kubectl.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ import (
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/color"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/config"
deploy "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/event"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest"
"github.com/GoogleContainerTools/skaffold/pkg/skaffold/util"
)
Expand All @@ -54,17 +54,28 @@ type KubectlDeployer struct {
skipRender bool
}

type Config interface {
deploy.Config
docker.Config

Pipeline() latest.Pipeline
GetWorkingDir() string
GlobalConfig() string
DefaultRepo() *string
SkipRender() bool
}

// NewKubectlDeployer returns a new KubectlDeployer for a DeployConfig filled
// with the needed configuration for `kubectl apply`
func NewKubectlDeployer(runCtx *runcontext.RunContext, labels map[string]string) *KubectlDeployer {
func NewKubectlDeployer(cfg Config, labels map[string]string) *KubectlDeployer {
return &KubectlDeployer{
KubectlDeploy: runCtx.Pipeline().Deploy.KubectlDeploy,
workingDir: runCtx.GetWorkingDir(),
globalConfig: runCtx.GlobalConfig(),
defaultRepo: runCtx.DefaultRepo(),
kubectl: deploy.NewCLI(runCtx, runCtx.Pipeline().Deploy.KubectlDeploy.Flags),
insecureRegistries: runCtx.GetInsecureRegistries(),
skipRender: runCtx.SkipRender(),
KubectlDeploy: cfg.Pipeline().Deploy.KubectlDeploy,
workingDir: cfg.GetWorkingDir(),
globalConfig: cfg.GlobalConfig(),
defaultRepo: cfg.DefaultRepo(),
kubectl: deploy.NewCLI(cfg, cfg.Pipeline().Deploy.KubectlDeploy.Flags),
insecureRegistries: cfg.GetInsecureRegistries(),
skipRender: cfg.SkipRender(),
labels: labels,
}
}
Expand Down

0 comments on commit c43fb29

Please sign in to comment.