Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Introduce Config interfaces #4598

Merged
merged 1 commit into from
Aug 31, 2020
Merged

Introduce Config interfaces #4598

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions cmd/skaffold/app/cmd/runner_test.go
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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