From ab85fe69c36c5638bfeeb09f214a3704cff62b02 Mon Sep 17 00:00:00 2001 From: gsquared94 Date: Wed, 16 Dec 2020 22:35:15 -0800 Subject: [PATCH 1/9] wip --- cmd/skaffold/app/cmd/build.go | 16 +- cmd/skaffold/app/cmd/delete.go | 2 +- cmd/skaffold/app/cmd/deploy.go | 9 +- cmd/skaffold/app/cmd/dev.go | 8 +- cmd/skaffold/app/cmd/diagnose.go | 23 +- cmd/skaffold/app/cmd/filter.go | 13 +- cmd/skaffold/app/cmd/find_configs.go | 4 +- cmd/skaffold/app/cmd/fix.go | 26 ++- cmd/skaffold/app/cmd/generate_pipeline.go | 4 +- cmd/skaffold/app/cmd/render.go | 4 +- cmd/skaffold/app/cmd/run.go | 4 +- cmd/skaffold/app/cmd/runner.go | 59 +++-- cmd/skaffold/app/cmd/util.go | 4 +- .../examples/microservices/skaffold.yaml | 42 +++- integration/render_test.go | 6 +- pkg/skaffold/build/build.go | 20 ++ pkg/skaffold/build/builder_mux.go | 103 +++++++++ pkg/skaffold/build/cache/cache.go | 56 +++-- pkg/skaffold/build/cache/lookup.go | 12 +- pkg/skaffold/build/cache/lookup_test.go | 16 +- pkg/skaffold/build/cache/retrieve.go | 12 +- pkg/skaffold/build/cluster/cluster.go | 29 ++- pkg/skaffold/build/cluster/cluster_test.go | 4 +- pkg/skaffold/build/cluster/secret_test.go | 8 +- pkg/skaffold/build/cluster/types.go | 8 +- pkg/skaffold/build/cluster/types_test.go | 2 +- pkg/skaffold/build/gcb/buildpacks_test.go | 2 +- pkg/skaffold/build/gcb/cloud_build.go | 17 +- pkg/skaffold/build/gcb/docker_test.go | 4 +- pkg/skaffold/build/gcb/jib_test.go | 4 +- pkg/skaffold/build/gcb/kaniko_test.go | 2 +- pkg/skaffold/build/gcb/spec_test.go | 2 +- pkg/skaffold/build/gcb/types.go | 5 +- pkg/skaffold/build/local/local.go | 32 +-- pkg/skaffold/build/local/local_test.go | 6 +- pkg/skaffold/build/local/prune.go | 19 +- pkg/skaffold/build/local/types.go | 21 +- pkg/skaffold/build/tag/tagger_mux.go | 122 ++++++++++ pkg/skaffold/deploy/helm/deploy.go | 4 +- pkg/skaffold/deploy/kpt/kpt.go | 4 +- pkg/skaffold/deploy/kubectl/kubectl.go | 10 +- pkg/skaffold/deploy/kustomize/kustomize.go | 10 +- pkg/skaffold/deploy/status/status_check.go | 7 +- pkg/skaffold/deploy/types/types.go | 2 +- pkg/skaffold/diagnose/diagnose.go | 83 +++---- pkg/skaffold/event/event.go | 12 +- pkg/skaffold/event/util.go | 45 ++-- pkg/skaffold/instrumentation/meter.go | 18 +- pkg/skaffold/kubernetes/log.go | 23 +- pkg/skaffold/kubernetes/manifest/images.go | 2 +- pkg/skaffold/runner/deploy.go | 15 +- pkg/skaffold/runner/generate_pipeline.go | 31 +-- pkg/skaffold/runner/logger.go | 2 +- pkg/skaffold/runner/new.go | 211 +++++++----------- pkg/skaffold/runner/new_test.go | 6 +- pkg/skaffold/runner/portforwarder.go | 2 +- pkg/skaffold/runner/runcontext/context.go | 112 ++++++++-- pkg/skaffold/runner/runner.go | 12 +- pkg/skaffold/runner/runner_test.go | 8 +- pkg/skaffold/runner/util/util.go | 18 +- pkg/skaffold/schema/defaults/defaults.go | 8 +- pkg/skaffold/schema/validation/validation.go | 40 ++-- pkg/skaffold/schema/versions.go | 126 ++++++----- pkg/skaffold/test/test.go | 10 +- pkg/skaffold/test/types.go | 2 +- pkg/skaffold/trigger/triggers.go | 4 +- 66 files changed, 988 insertions(+), 569 deletions(-) create mode 100644 pkg/skaffold/build/builder_mux.go create mode 100644 pkg/skaffold/build/tag/tagger_mux.go diff --git a/cmd/skaffold/app/cmd/build.go b/cmd/skaffold/app/cmd/build.go index f4fcb836e7a..4c89e5cc803 100644 --- a/cmd/skaffold/app/cmd/build.go +++ b/cmd/skaffold/app/cmd/build.go @@ -65,8 +65,8 @@ func doBuild(ctx context.Context, out io.Writer) error { buildOut = ioutil.Discard } - return withRunner(ctx, func(r runner.Runner, config *latest.SkaffoldConfig) error { - bRes, err := r.BuildAndTest(ctx, buildOut, targetArtifacts(opts, config)) + return withRunner(ctx, func(r runner.Runner, configs []*latest.SkaffoldConfig) error { + bRes, err := r.BuildAndTest(ctx, buildOut, targetArtifacts(opts, configs)) if quietFlag || buildOutputFlag != "" { cmdOut := flags.BuildOutput{Builds: bRes} @@ -92,14 +92,14 @@ func doBuild(ctx context.Context, out io.Writer) error { }) } -func targetArtifacts(opts config.SkaffoldOptions, cfg *latest.SkaffoldConfig) []*latest.Artifact { +func targetArtifacts(opts config.SkaffoldOptions, configs []*latest.SkaffoldConfig) []*latest.Artifact { var targetArtifacts []*latest.Artifact - - for _, artifact := range cfg.Build.Artifacts { - if opts.IsTargetImage(artifact) { - targetArtifacts = append(targetArtifacts, artifact) + for _, cfg := range configs { + for _, artifact := range cfg.Build.Artifacts { + if opts.IsTargetImage(artifact) { + targetArtifacts = append(targetArtifacts, artifact) + } } } - return targetArtifacts } diff --git a/cmd/skaffold/app/cmd/delete.go b/cmd/skaffold/app/cmd/delete.go index 979f5b5aebb..05f82f8ed98 100644 --- a/cmd/skaffold/app/cmd/delete.go +++ b/cmd/skaffold/app/cmd/delete.go @@ -35,7 +35,7 @@ func NewCmdDelete() *cobra.Command { } func doDelete(ctx context.Context, out io.Writer) error { - return withRunner(ctx, func(r runner.Runner, _ *latest.SkaffoldConfig) error { + return withRunner(ctx, func(r runner.Runner, _ []*latest.SkaffoldConfig) error { return r.Cleanup(ctx, out) }) } diff --git a/cmd/skaffold/app/cmd/deploy.go b/cmd/skaffold/app/cmd/deploy.go index eef03dc0a42..b48eea4af15 100644 --- a/cmd/skaffold/app/cmd/deploy.go +++ b/cmd/skaffold/app/cmd/deploy.go @@ -51,12 +51,15 @@ func NewCmdDeploy() *cobra.Command { } func doDeploy(ctx context.Context, out io.Writer) error { - return withRunner(ctx, func(r runner.Runner, config *latest.SkaffoldConfig) error { + return withRunner(ctx, func(r runner.Runner, configs []*latest.SkaffoldConfig) error { if opts.SkipRender { return r.DeployAndLog(ctx, out, []build.Artifact{}) } - - buildArtifacts, err := getBuildArtifactsAndSetTags(r, config) + var artifacts []*latest.Artifact + for _, cfg := range configs { + artifacts = append(artifacts, cfg.Build.Artifacts...) + } + buildArtifacts, err := getBuildArtifactsAndSetTags(r, artifacts) if err != nil { tips.PrintUseRunVsDeploy(out) return err diff --git a/cmd/skaffold/app/cmd/dev.go b/cmd/skaffold/app/cmd/dev.go index 524a17ce4b0..09f398df091 100644 --- a/cmd/skaffold/app/cmd/dev.go +++ b/cmd/skaffold/app/cmd/dev.go @@ -60,8 +60,12 @@ func runDev(ctx context.Context, out io.Writer) error { case <-ctx.Done(): return nil default: - err := withRunner(ctx, func(r runner.Runner, config *latest.SkaffoldConfig) error { - err := r.Dev(ctx, out, config.Build.Artifacts) + err := withRunner(ctx, func(r runner.Runner, configs []*latest.SkaffoldConfig) error { + var artifacts []*latest.Artifact + for _, cfg := range configs { + artifacts = append(artifacts, cfg.Build.Artifacts...) + } + err := r.Dev(ctx, out, artifacts) if r.HasDeployed() { cleanup = func() { diff --git a/cmd/skaffold/app/cmd/diagnose.go b/cmd/skaffold/app/cmd/diagnose.go index b52463ef7cc..2789e28b3ad 100644 --- a/cmd/skaffold/app/cmd/diagnose.go +++ b/cmd/skaffold/app/cmd/diagnose.go @@ -46,24 +46,25 @@ func NewCmdDiagnose() *cobra.Command { } func doDiagnose(ctx context.Context, out io.Writer) error { - runCtx, config, err := runContext(opts) + runCtx, configs, err := runContext(opts) if err != nil { return err } - if !yamlOnly { - fmt.Fprintln(out, "Skaffold version:", version.Get().GitCommit) - fmt.Fprintln(out, "Configuration version:", config.APIVersion) - fmt.Fprintln(out, "Number of artifacts:", len(config.Build.Artifacts)) + for _, config := range configs { + if !yamlOnly { + fmt.Fprintln(out, "Skaffold version:", version.Get().GitCommit) + fmt.Fprintln(out, "Configuration version:", config.APIVersion) + fmt.Fprintln(out, "Number of artifacts:", len(config.Build.Artifacts)) - if err := diagnose.CheckArtifacts(ctx, runCtx, out); err != nil { - return fmt.Errorf("running diagnostic on artifacts: %w", err) - } + if err := diagnose.CheckArtifacts(ctx, runCtx, out); err != nil { + return fmt.Errorf("running diagnostic on artifacts: %w", err) + } - color.Blue.Fprintln(out, "\nConfiguration") + color.Blue.Fprintln(out, "\nConfiguration") + } } - - buf, err := yaml.Marshal(config) + buf, err := yaml.Marshal(configs) if err != nil { return fmt.Errorf("marshalling configuration: %w", err) } diff --git a/cmd/skaffold/app/cmd/filter.go b/cmd/skaffold/app/cmd/filter.go index f877f98976b..d73499bdb31 100644 --- a/cmd/skaffold/app/cmd/filter.go +++ b/cmd/skaffold/app/cmd/filter.go @@ -58,7 +58,7 @@ func NewCmdFilter() *cobra.Command { // runFilter loads the Kubernetes manifests from stdin and applies the debug transformations. // Unlike `skaffold debug`, this filtering affects all images and not just the built artifacts. func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildArtifacts []build.Artifact) error { - return withRunner(ctx, func(r runner.Runner, cfg *latest.SkaffoldConfig) error { + return withRunner(ctx, func(r runner.Runner, configs []*latest.SkaffoldConfig) error { manifestList, err := manifest.Load(os.Stdin) if err != nil { return fmt.Errorf("loading manifests: %w", err) @@ -69,7 +69,7 @@ func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildA if err != nil { return fmt.Errorf("resolving debug helpers: %w", err) } - insecureRegistries, err := getInsecureRegistries(opts, cfg) + insecureRegistries, err := getInsecureRegistries(opts, configs) if err != nil { return fmt.Errorf("retrieving insecure registries: %w", err) } @@ -87,12 +87,17 @@ func runFilter(ctx context.Context, out io.Writer, debuggingFilters bool, buildA }) } -func getInsecureRegistries(opts config.SkaffoldOptions, cfg *latest.SkaffoldConfig) (map[string]bool, error) { +func getInsecureRegistries(opts config.SkaffoldOptions, configs []*latest.SkaffoldConfig) (map[string]bool, error) { cfgRegistries, err := config.GetInsecureRegistries(opts.GlobalConfig) if err != nil { return nil, err } - regList := append(opts.InsecureRegistries, cfg.Build.InsecureRegistries...) + var regList []string + + regList = append(regList, opts.InsecureRegistries...) + for _, cfg := range configs { + regList = append(regList, cfg.Build.InsecureRegistries...) + } regList = append(regList, cfgRegistries...) insecureRegistries := make(map[string]bool, len(regList)) for _, r := range regList { diff --git a/cmd/skaffold/app/cmd/find_configs.go b/cmd/skaffold/app/cmd/find_configs.go index 8af43f5c7e7..b49c3af971b 100644 --- a/cmd/skaffold/app/cmd/find_configs.go +++ b/cmd/skaffold/app/cmd/find_configs.go @@ -88,8 +88,8 @@ func findConfigs(directory string) (map[string]string, error) { } err := walk.From(directory).When(isYaml).Do(func(path string, _ walk.Dirent) error { - if cfg, err := schema.ParseConfig(path); err == nil { - pathToVersion[path] = cfg.GetVersion() + if cfgs, err := schema.ParseConfig(path); err == nil && len(cfgs) > 0 { + pathToVersion[path] = cfgs[0].GetVersion() } return nil }) diff --git a/cmd/skaffold/app/cmd/fix.go b/cmd/skaffold/app/cmd/fix.go index 5f6b592db36..d072ea80bc9 100644 --- a/cmd/skaffold/app/cmd/fix.go +++ b/cmd/skaffold/app/cmd/fix.go @@ -51,39 +51,47 @@ func doFix(_ context.Context, out io.Writer) error { } func fix(out io.Writer, configFile string, toVersion string, overwrite bool) error { - cfg, err := schema.ParseConfig(configFile) + parsedCfgs, err := schema.ParseConfig(configFile) if err != nil { return err } - - if cfg.GetVersion() == toVersion { + needsUpdate := false + for _, cfg := range parsedCfgs { + if cfg.GetVersion() != toVersion { + needsUpdate = true + break + } + } + if !needsUpdate { color.Default.Fprintln(out, "config is already version", toVersion) return nil } - cfg, err = schema.ParseConfigAndUpgrade(configFile, toVersion) + versionedCfgs, err := schema.ParseConfigAndUpgrade(configFile, toVersion) if err != nil { return err } + var cfgs []*latest.SkaffoldConfig + for _, cfg := range versionedCfgs { + cfgs = append(cfgs, cfg.(*latest.SkaffoldConfig)) + } // TODO(dgageot): We should be able run validations on any schema version // but that's not the case. They can only run on the latest version for now. if toVersion == latest.Version { - if err := validation.Process(cfg.(*latest.SkaffoldConfig)); err != nil { + if err := validation.Process(cfgs); err != nil { return fmt.Errorf("validating upgraded config: %w", err) } } - - newCfg, err := yaml.Marshal(cfg) + newCfg, err := yaml.Marshal(cfgs) if err != nil { return fmt.Errorf("marshaling new config: %w", err) } - if overwrite { if err := ioutil.WriteFile(configFile, newCfg, 0644); err != nil { return fmt.Errorf("writing config file: %w", err) } - color.Default.Fprintf(out, "New config at version %s generated and written to %s\n", cfg.GetVersion(), opts.ConfigurationFile) + color.Default.Fprintf(out, "New config at version %s generated and written to %s\n", latest.Version, opts.ConfigurationFile) } else { out.Write(newCfg) } diff --git a/cmd/skaffold/app/cmd/generate_pipeline.go b/cmd/skaffold/app/cmd/generate_pipeline.go index db38e97241a..259d3718b0d 100644 --- a/cmd/skaffold/app/cmd/generate_pipeline.go +++ b/cmd/skaffold/app/cmd/generate_pipeline.go @@ -44,8 +44,8 @@ func NewCmdGeneratePipeline() *cobra.Command { } func doGeneratePipeline(ctx context.Context, out io.Writer) error { - return withRunner(ctx, func(r runner.Runner, config *latest.SkaffoldConfig) error { - if err := r.GeneratePipeline(ctx, out, config, configFiles, "pipeline.yaml"); err != nil { + return withRunner(ctx, func(r runner.Runner, configs []*latest.SkaffoldConfig) error { + if err := r.GeneratePipeline(ctx, out, configs, configFiles, "pipeline.yaml"); err != nil { return fmt.Errorf("generating : %w", err) } color.Default.Fprintln(out, "Pipeline config written to pipeline.yaml!") diff --git a/cmd/skaffold/app/cmd/render.go b/cmd/skaffold/app/cmd/render.go index e251294122d..400ec41f67a 100644 --- a/cmd/skaffold/app/cmd/render.go +++ b/cmd/skaffold/app/cmd/render.go @@ -60,14 +60,14 @@ func doRender(ctx context.Context, out io.Writer) error { buildOut = out } - return withRunner(ctx, func(r runner.Runner, config *latest.SkaffoldConfig) error { + return withRunner(ctx, func(r runner.Runner, configs []*latest.SkaffoldConfig) error { var bRes []build.Artifact if renderFromBuildOutputFile.String() != "" { bRes = renderFromBuildOutputFile.BuildArtifacts() } else { var err error - bRes, err = r.BuildAndTest(ctx, buildOut, targetArtifacts(opts, config)) + bRes, err = r.BuildAndTest(ctx, buildOut, targetArtifacts(opts, configs)) if err != nil { return fmt.Errorf("executing build: %w", err) } diff --git a/cmd/skaffold/app/cmd/run.go b/cmd/skaffold/app/cmd/run.go index 201a3c3f5ce..9961846decf 100644 --- a/cmd/skaffold/app/cmd/run.go +++ b/cmd/skaffold/app/cmd/run.go @@ -41,8 +41,8 @@ func NewCmdRun() *cobra.Command { } func doRun(ctx context.Context, out io.Writer) error { - return withRunner(ctx, func(r runner.Runner, config *latest.SkaffoldConfig) error { - bRes, err := r.BuildAndTest(ctx, out, targetArtifacts(opts, config)) + return withRunner(ctx, func(r runner.Runner, configs []*latest.SkaffoldConfig) error { + bRes, err := r.BuildAndTest(ctx, out, targetArtifacts(opts, configs)) if err != nil { return fmt.Errorf("failed to build: %w", err) } diff --git a/cmd/skaffold/app/cmd/runner.go b/cmd/skaffold/app/cmd/runner.go index 201ddb80b2e..269a5e66c50 100644 --- a/cmd/skaffold/app/cmd/runner.go +++ b/cmd/skaffold/app/cmd/runner.go @@ -22,17 +22,18 @@ import ( "fmt" "os" + kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation" - kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/validation" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/update" @@ -41,7 +42,7 @@ import ( // For tests var createRunner = createNewRunner -func withRunner(ctx context.Context, action func(runner.Runner, *latest.SkaffoldConfig) error) error { +func withRunner(ctx context.Context, action func(runner.Runner, []*latest.SkaffoldConfig) error) error { runner, config, err := createRunner(opts) sErrors.SetSkaffoldOptions(opts) if err != nil { @@ -54,23 +55,23 @@ func withRunner(ctx context.Context, action func(runner.Runner, *latest.Skaffold } // createNewRunner creates a Runner and returns the SkaffoldConfig associated with it. -func createNewRunner(opts config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { - runCtx, config, err := runContext(opts) +func createNewRunner(opts config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { + runCtx, configs, err := runContext(opts) if err != nil { return nil, nil, err } - instrumentation.InitMeter(runCtx, config) + instrumentation.InitMeter(runCtx, configs) runner, err := runner.NewForConfig(runCtx) if err != nil { event.InititializationFailed(err) return nil, nil, fmt.Errorf("creating runner: %w", err) } - return runner, config, nil + return runner, configs, nil } -func runContext(opts config.SkaffoldOptions) (*runcontext.RunContext, *latest.SkaffoldConfig, error) { +func runContext(opts config.SkaffoldOptions) (*runcontext.RunContext, []*latest.SkaffoldConfig, error) { parsed, err := schema.ParseConfigAndUpgrade(opts.ConfigurationFile, latest.Version) if err != nil { if os.IsNotExist(errors.Unwrap(err)) { @@ -84,32 +85,52 @@ func runContext(opts config.SkaffoldOptions) (*runcontext.RunContext, *latest.Sk return nil, nil, fmt.Errorf("parsing skaffold config: %w", err) } - config := parsed.(*latest.SkaffoldConfig) - - if err = schema.ApplyProfiles(config, opts); err != nil { - return nil, nil, fmt.Errorf("applying profiles: %w", err) + if len(parsed) == 0 { + return nil, nil, fmt.Errorf("skaffold config file %s is empty", opts.ConfigurationFile) } - kubectx.ConfigureKubeConfig(opts.KubeConfig, opts.KubeContext, config.Deploy.KubeContext) + setDefaultDeployer := setDefaultDeployer(parsed) + var pipelines []latest.Pipeline + var configs []*latest.SkaffoldConfig + for _, cfg := range parsed { + config := cfg.(*latest.SkaffoldConfig) - if err := defaults.Set(config); err != nil { - return nil, nil, fmt.Errorf("setting default values: %w", err) + if err = schema.ApplyProfiles(config, opts); err != nil { + return nil, nil, fmt.Errorf("applying profiles: %w", err) + } + if err := defaults.Set(config, setDefaultDeployer); err != nil { + return nil, nil, fmt.Errorf("setting default values: %w", err) + } + pipelines = append(pipelines, config.Pipeline) + configs = append(configs, config) } - if err := validation.Process(config); err != nil { + kubectx.ConfigureKubeConfig(opts.KubeConfig, opts.KubeContext, configs[0].Deploy.KubeContext) + + if err := validation.Process(configs); err != nil { return nil, nil, fmt.Errorf("invalid skaffold config: %w", err) } - runCtx, err := runcontext.GetRunContext(opts, config.Pipeline) + runCtx, err := runcontext.GetRunContext(opts, pipelines) if err != nil { return nil, nil, fmt.Errorf("getting run context: %w", err) } - if err := validation.ProcessWithRunContext(config, runCtx); err != nil { + if err := validation.ProcessWithRunContext(runCtx); err != nil { return nil, nil, fmt.Errorf("invalid skaffold config: %w", err) } - return runCtx, config, nil + return runCtx, configs, nil +} + +func setDefaultDeployer(configs []util.VersionedConfig) bool { + // set the default deployer only if no deployer is explicitly specified in any config + for _, cfg := range configs { + if cfg.(*latest.SkaffoldConfig).Deploy.DeployType != (latest.DeployType{}) { + return false + } + } + return true } func warnIfUpdateIsAvailable() { diff --git a/cmd/skaffold/app/cmd/util.go b/cmd/skaffold/app/cmd/util.go index 47adb392a74..0381aa5891d 100644 --- a/cmd/skaffold/app/cmd/util.go +++ b/cmd/skaffold/app/cmd/util.go @@ -25,8 +25,8 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" ) -func getBuildArtifactsAndSetTags(r runner.Runner, config *latest.SkaffoldConfig) ([]build.Artifact, error) { - buildArtifacts, err := getArtifacts(fromBuildOutputFile.BuildArtifacts(), preBuiltImages.Artifacts(), config.Build.Artifacts) +func getBuildArtifactsAndSetTags(r runner.Runner, artifacts []*latest.Artifact) ([]build.Artifact, error) { + buildArtifacts, err := getArtifacts(fromBuildOutputFile.BuildArtifacts(), preBuiltImages.Artifacts(), artifacts) if err != nil { return nil, err } diff --git a/integration/examples/microservices/skaffold.yaml b/integration/examples/microservices/skaffold.yaml index 87a0c297d3d..60b39d8ba75 100644 --- a/integration/examples/microservices/skaffold.yaml +++ b/integration/examples/microservices/skaffold.yaml @@ -7,24 +7,42 @@ build: requires: - image: base alias: BASE - - image: leeroy-app - context: leeroy-app - requires: - - image: base - alias: BASE - - image: base - context: base deploy: kubectl: manifests: - leeroy-web/kubernetes/* - - leeroy-app/kubernetes/* portForward: - resourceType: deployment resourceName: leeroy-web port: 8080 localPort: 9000 - - resourceType: deployment - resourceName: leeroy-app - port: http - localPort: 9001 \ No newline at end of file + +--- + +apiVersion: skaffold/v2beta11 +kind: Config +build: + artifacts: + - image: leeroy-app + context: leeroy-app + requires: + - image: base + alias: BASE +deploy: + kubectl: + manifests: + - leeroy-app/kubernetes/* +portForward: +- resourceType: deployment + resourceName: leeroy-app + port: http + localPort: 9001 + +--- + +apiVersion: skaffold/v2beta11 +kind: Config +build: + artifacts: + - image: base + context: base \ No newline at end of file diff --git a/integration/render_test.go b/integration/render_test.go index 74a595a1d1d..e8a7356564f 100644 --- a/integration/render_test.go +++ b/integration/render_test.go @@ -79,7 +79,7 @@ spec: Chdir() deployer, err := kubectl.NewDeployer(&runcontext.RunContext{ WorkingDir: ".", - Cfg: latest.Pipeline{ + Pipelines: latest.Pipeline{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ KubectlDeploy: &latest.KubectlDeploy{ @@ -234,7 +234,7 @@ spec: deployer, err := kubectl.NewDeployer(&runcontext.RunContext{ WorkingDir: ".", - Cfg: latest.Pipeline{ + Pipelines: latest.Pipeline{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ KubectlDeploy: &latest.KubectlDeploy{ @@ -421,7 +421,7 @@ spec: for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { deployer, err := helm.NewDeployer(&runcontext.RunContext{ - Cfg: latest.Pipeline{ + Pipelines: latest.Pipeline{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ HelmDeploy: &latest.HelmDeploy{ diff --git a/pkg/skaffold/build/build.go b/pkg/skaffold/build/build.go index 3081629a7ca..822941a4444 100644 --- a/pkg/skaffold/build/build.go +++ b/pkg/skaffold/build/build.go @@ -62,6 +62,26 @@ type Builder interface { Prune(context.Context, io.Writer) error } +// PipelineBuilder is an interface for a specific Skaffold config pipeline build type. +// Current implementations are the `local`, `cluster` and `gcb` +type PipelineBuilder interface { + + // PreBuild executes any one-time setup required prior to starting any build on this builder + PreBuild(ctx context.Context, out io.Writer) error + + // Build returns the `ArtifactBuilder` based on this build pipeline type + Build(ctx context.Context, out io.Writer, artifact *latest.Artifact) ArtifactBuilder + + // PostBuild executes any one-time teardown required after all builds on this builder are complete + PostBuild(ctx context.Context, out io.Writer) error + + // Concurrency specifies the max number of builds that can run at any one time. If concurrency is 0, then all builds can run in parallel. + Concurrency() int + + // Prune removes images built in this pipeline + Prune(context.Context, io.Writer) error +} + type ErrSyncMapNotSupported struct{} func (ErrSyncMapNotSupported) Error() string { diff --git a/pkg/skaffold/build/builder_mux.go b/pkg/skaffold/build/builder_mux.go new file mode 100644 index 00000000000..86068b93b6f --- /dev/null +++ b/pkg/skaffold/build/builder_mux.go @@ -0,0 +1,103 @@ +/* +Copyright 2020 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package build + +import ( + "context" + "fmt" + "io" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" +) + +type BuilderMux struct { + builders []PipelineBuilder + byImageName map[string]PipelineBuilder + store ArtifactStore + concurrency int +} + +func NewBuilderMux(runCtx *runcontext.RunContext, store ArtifactStore, builder func(p latest.Pipeline) (PipelineBuilder, error)) (Builder, error) { + pipelines := runCtx.GetPipelines() + m := make(map[string]PipelineBuilder) + var sl []PipelineBuilder + minConcurrency := -1 + for _, p := range pipelines { + b, err := builder(p) + if err != nil { + return nil, fmt.Errorf("creating builder: %w", err) + } + sl = append(sl, b) + for _, a := range p.Build.Artifacts { + m[a.ImageName] = b + } + concurrency := b.Concurrency() + if minConcurrency < 0 { + minConcurrency = concurrency + } else if concurrency > 0 && concurrency < minConcurrency { + // set mux concurrency to be the minimum of all builders' concurrency. (concurrency = 0 means unlimited) + minConcurrency = concurrency + } + } + if minConcurrency > len(m) { + minConcurrency = 0 // if specified concurrency is greater than maximum number of parallel jobs, then just set it to unlimited + } + + return &BuilderMux{builders: sl, byImageName: m, store: store, concurrency: minConcurrency}, nil +} + +func (b *BuilderMux) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]Artifact, error) { + m := make(map[PipelineBuilder]bool) + for _, a := range artifacts { + m[b.byImageName[a.ImageName]] = true + } + + for builder := range m { + if err := builder.PreBuild(ctx, out); err != nil { + return nil, err + } + } + + builder := func(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) { + p := b.byImageName[artifact.ImageName] + artifactBuilder := p.Build(ctx, out, artifact) + return artifactBuilder(ctx, out, artifact, tag) + } + ar, err := InOrder(ctx, out, tags, artifacts, builder, b.concurrency, b.store) + if err != nil { + return nil, err + } + + for builder := range m { + if err := builder.PostBuild(ctx, out); err != nil { + return nil, err + } + } + + return ar, nil +} + +func (b *BuilderMux) Prune(ctx context.Context, writer io.Writer) error { + for _, builder := range b.builders { + if err := builder.Prune(ctx, writer); err != nil { + return err + } + } + return nil +} diff --git a/pkg/skaffold/build/cache/cache.go b/pkg/skaffold/build/cache/cache.go index 9adc6f70c78..a451b562d30 100644 --- a/pkg/skaffold/build/cache/cache.go +++ b/pkg/skaffold/build/cache/cache.go @@ -46,16 +46,16 @@ type ArtifactCache map[string]ImageDetails // cache holds any data necessary for accessing the cache type cache struct { - artifactCache ArtifactCache - artifactGraph build.ArtifactGraph - artifactStore build.ArtifactStore - cacheMutex sync.RWMutex - client docker.LocalDaemon - cfg Config - cacheFile string - imagesAreLocal bool - tryImportMissing bool - lister DependencyLister + artifactCache ArtifactCache + artifactGraph build.ArtifactGraph + artifactStore build.ArtifactStore + cacheMutex sync.RWMutex + client docker.LocalDaemon + cfg Config + cacheFile string + isLocalImage func(imageName string) (bool, error) + importMissingImage func(imageName string) (bool, error) + lister DependencyLister } // DependencyLister fetches a list of dependencies for an artifact @@ -63,14 +63,15 @@ type DependencyLister func(ctx context.Context, artifact *latest.Artifact) ([]st type Config interface { docker.Config - + Pipeline(imageName string) (*latest.Pipeline, bool) + GetCluster() config.Cluster CacheArtifacts() bool CacheFile() string Mode() config.RunMode } // NewCache returns the current state of the cache -func NewCache(cfg Config, imagesAreLocal bool, tryImportMissing bool, dependencies DependencyLister, graph build.ArtifactGraph, store build.ArtifactStore) (Cache, error) { +func NewCache(cfg Config, isLocalImage func(imageName string) (bool, error), dependencies DependencyLister, graph build.ArtifactGraph, store build.ArtifactStore) (Cache, error) { if !cfg.CacheArtifacts() { return &noCache{}, nil } @@ -88,20 +89,29 @@ func NewCache(cfg Config, imagesAreLocal bool, tryImportMissing bool, dependenci } client, err := docker.NewAPIClient(cfg) - if imagesAreLocal && err != nil { - return nil, fmt.Errorf("getting local Docker client: %w", err) + + importMissingImage := func(imageName string) (bool, error) { + pipeline, found := cfg.Pipeline(imageName) + if !found { + return false, fmt.Errorf("unknown pipeline for image %q", imageName) + } + + if pipeline.Build.GoogleCloudBuild != nil || pipeline.Build.Cluster != nil { + return false, nil + } + return pipeline.Build.LocalBuild.TryImportMissing, nil } return &cache{ - artifactCache: artifactCache, - artifactGraph: graph, - artifactStore: store, - client: client, - cfg: cfg, - cacheFile: cacheFile, - imagesAreLocal: imagesAreLocal, - tryImportMissing: tryImportMissing, - lister: dependencies, + artifactCache: artifactCache, + artifactGraph: graph, + artifactStore: store, + client: client, + cfg: cfg, + cacheFile: cacheFile, + isLocalImage: isLocalImage, + importMissingImage: importMissingImage, + lister: dependencies, }, nil } diff --git a/pkg/skaffold/build/cache/lookup.go b/pkg/skaffold/build/cache/lookup.go index 9374e8e5f08..412dcfbe330 100644 --- a/pkg/skaffold/build/cache/lookup.go +++ b/pkg/skaffold/build/cache/lookup.go @@ -65,7 +65,9 @@ func (c *cache) lookup(ctx context.Context, a *latest.Artifact, tag string, h ar } } - if c.imagesAreLocal { + if isLocal, err := c.isLocalImage(a.ImageName); err != nil { + return failed{err} + } else if isLocal { return c.lookupLocal(ctx, hash, tag, entry) } return c.lookupRemote(ctx, hash, tag, entry) @@ -121,12 +123,14 @@ func (c *cache) lookupRemote(ctx context.Context, hash, tag string, entry ImageD } func (c *cache) tryImport(ctx context.Context, a *latest.Artifact, tag string, hash string) (ImageDetails, error) { - if !c.tryImportMissing { + entry := ImageDetails{} + + if importMissing, err := c.importMissingImage(a.ImageName); err != nil { + return entry, err + } else if !importMissing { return ImageDetails{}, fmt.Errorf("import of missing images disabled") } - entry := ImageDetails{} - if !c.client.ImageExists(ctx, tag) { logrus.Debugf("Importing artifact %s from docker registry", tag) err := c.client.Pull(ctx, ioutil.Discard, tag) diff --git a/pkg/skaffold/build/cache/lookup_test.go b/pkg/skaffold/build/cache/lookup_test.go index 2767771d01c..b4f359fb4a9 100644 --- a/pkg/skaffold/build/cache/lookup_test.go +++ b/pkg/skaffold/build/cache/lookup_test.go @@ -118,10 +118,10 @@ func TestLookupLocal(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { cache := &cache{ - imagesAreLocal: true, - artifactCache: test.cache, - client: fakeLocalDaemon(test.api), - cfg: &mockConfig{mode: config.RunModes.Build}, + isLocalImage: true, + artifactCache: test.cache, + client: fakeLocalDaemon(test.api), + cfg: &mockConfig{mode: config.RunModes.Build}, } t.Override(&newArtifactHasherFunc, func(_ build.ArtifactGraph, _ DependencyLister, _ config.RunMode) artifactHasher { return test.hasher }) @@ -206,10 +206,10 @@ func TestLookupRemote(t *testing.T) { }) cache := &cache{ - imagesAreLocal: false, - artifactCache: test.cache, - client: fakeLocalDaemon(test.api), - cfg: &mockConfig{mode: config.RunModes.Build}, + isLocalImage: false, + artifactCache: test.cache, + client: fakeLocalDaemon(test.api), + cfg: &mockConfig{mode: config.RunModes.Build}, } t.Override(&newArtifactHasherFunc, func(_ build.ArtifactGraph, _ DependencyLister, _ config.RunMode) artifactHasher { return test.hasher }) details := cache.lookupArtifacts(context.Background(), map[string]string{"artifact": "tag"}, []*latest.Artifact{{ diff --git a/pkg/skaffold/build/cache/retrieve.go b/pkg/skaffold/build/cache/retrieve.go index 5db4975e47e..404251f6789 100644 --- a/pkg/skaffold/build/cache/retrieve.go +++ b/pkg/skaffold/build/cache/retrieve.go @@ -81,7 +81,9 @@ func (c *cache) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, ar } default: - if c.imagesAreLocal { + if isLocal, err := c.isLocalImage(artifact.ImageName); err != nil { + return nil, err + } else if isLocal { color.Green.Fprintln(out, "Found Locally") } else { color.Green.Fprintln(out, "Found Remotely") @@ -95,7 +97,9 @@ func (c *cache) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, ar tag := tags[artifact.ImageName] var uniqueTag string - if c.imagesAreLocal { + if isLocal, err := c.isLocalImage(artifact.ImageName); err != nil { + return nil, err + } else if isLocal { var err error uniqueTag, err = build.TagWithImageID(ctx, tag, entry.ID, c.client) if err != nil { @@ -149,7 +153,9 @@ func maintainArtifactOrder(built []build.Artifact, artifacts []*latest.Artifact) func (c *cache) addArtifacts(ctx context.Context, bRes []build.Artifact, hashByName map[string]string) error { for _, a := range bRes { entry := ImageDetails{} - if c.imagesAreLocal { + if isLocal, err := c.isLocalImage(a.ImageName); err != nil { + return err + } else if isLocal { imageID, err := c.client.ImageID(ctx, a.Tag) if err != nil { return err diff --git a/pkg/skaffold/build/cluster/cluster.go b/pkg/skaffold/build/cluster/cluster.go index 6e5f3d836ee..18d1e75461a 100644 --- a/pkg/skaffold/build/cluster/cluster.go +++ b/pkg/skaffold/build/cluster/cluster.go @@ -24,7 +24,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/custom" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/misc" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" @@ -32,23 +31,33 @@ import ( ) // Build builds a list of artifacts with Kaniko. -func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) { +func (b *Builder) Build(ctx context.Context, out io.Writer, artifact *latest.Artifact) build.ArtifactBuilder { + builder := build.WithLogFile(b.buildArtifact, b.cfg.Muted()) + return builder +} + +func (b *Builder) PreBuild(ctx context.Context, out io.Writer) error { teardownPullSecret, err := b.setupPullSecret(ctx, out) if err != nil { - return nil, fmt.Errorf("setting up pull secret: %w", err) + return fmt.Errorf("setting up pull secret: %w", err) } - defer teardownPullSecret() + b.teardownFunc = append(b.teardownFunc, teardownPullSecret) if b.DockerConfig != nil { teardownDockerConfigSecret, err := b.setupDockerConfigSecret(ctx, out) if err != nil { - return nil, fmt.Errorf("setting up docker config secret: %w", err) + return fmt.Errorf("setting up docker config secret: %w", err) } - defer teardownDockerConfigSecret() + b.teardownFunc = append(b.teardownFunc, teardownDockerConfigSecret) } + return nil +} - builder := build.WithLogFile(b.buildArtifact, b.cfg.Muted()) - return build.InOrder(ctx, out, tags, artifacts, builder, b.ClusterDetails.Concurrency, b.artifactStore) +func (b *Builder) PostBuild(_ context.Context, _ io.Writer) error { + for _, f := range b.teardownFunc { + f() + } + return nil } func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) { @@ -61,6 +70,10 @@ func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, artifact *la return build.TagWithDigest(tag, digest), nil } +func (b *Builder) Concurrency() int { + return b.ClusterDetails.Concurrency +} + func (b *Builder) runBuildForArtifact(ctx context.Context, out io.Writer, a *latest.Artifact, tag string) (string, error) { // required artifacts as build-args requiredImages := docker.ResolveDependencyImages(a.Dependencies, b.artifactStore, true) diff --git a/pkg/skaffold/build/cluster/cluster_test.go b/pkg/skaffold/build/cluster/cluster_test.go index 998ea209ba8..6cc1302d1b2 100644 --- a/pkg/skaffold/build/cluster/cluster_test.go +++ b/pkg/skaffold/build/cluster/cluster_test.go @@ -35,7 +35,7 @@ func TestRetrieveEnv(t *testing.T) { }, Timeout: "2m", }, - }) + }, nil) testutil.CheckError(t, false, err) actual := builder.retrieveExtraEnv() @@ -48,7 +48,7 @@ func TestRetrieveEnvMinimal(t *testing.T) { cluster: latest.ClusterDetails{ Timeout: "20m", }, - }) + }, nil) testutil.CheckError(t, false, err) actual := builder.retrieveExtraEnv() diff --git a/pkg/skaffold/build/cluster/secret_test.go b/pkg/skaffold/build/cluster/secret_test.go index 41444081105..d224ff37d64 100644 --- a/pkg/skaffold/build/cluster/secret_test.go +++ b/pkg/skaffold/build/cluster/secret_test.go @@ -46,7 +46,7 @@ func TestCreateSecret(t *testing.T) { PullSecretPath: tmpDir.Path("secret.json"), Namespace: "ns", }, - }) + }, nil) t.CheckNoError(err) // Should create a secret @@ -77,7 +77,7 @@ func TestExistingSecretNotFound(t *testing.T) { Timeout: "20m", PullSecretName: "kaniko-secret", }, - }) + }, nil) t.CheckNoError(err) // should fail to retrieve an existing secret @@ -102,7 +102,7 @@ func TestExistingSecret(t *testing.T) { Timeout: "20m", PullSecretName: "kaniko-secret", }, - }) + }, nil) t.CheckNoError(err) // should retrieve an existing secret @@ -123,7 +123,7 @@ func TestSkipSecretCreation(t *testing.T) { cluster: latest.ClusterDetails{ Timeout: "20m", }, - }) + }, nil) t.CheckNoError(err) // should retrieve an existing secret diff --git a/pkg/skaffold/build/cluster/types.go b/pkg/skaffold/build/cluster/types.go index 5cd5fa563df..48287d3a3fe 100644 --- a/pkg/skaffold/build/cluster/types.go +++ b/pkg/skaffold/build/cluster/types.go @@ -38,27 +38,27 @@ type Builder struct { mode config.RunMode timeout time.Duration artifactStore build.ArtifactStore + teardownFunc []func() } type Config interface { kubectl.Config docker.Config - Pipeline() latest.Pipeline GetKubeContext() string Muted() config.Muted Mode() config.RunMode } // NewBuilder creates a new Builder that builds artifacts on cluster. -func NewBuilder(cfg Config) (*Builder, error) { - timeout, err := time.ParseDuration(cfg.Pipeline().Build.Cluster.Timeout) +func NewBuilder(cfg Config, buildCfg *latest.ClusterDetails) (*Builder, error) { + timeout, err := time.ParseDuration(buildCfg.Timeout) if err != nil { return nil, fmt.Errorf("parsing timeout: %w", err) } return &Builder{ - ClusterDetails: cfg.Pipeline().Build.Cluster, + ClusterDetails: buildCfg, cfg: cfg, kubectlcli: kubectl.NewCLI(cfg, ""), mode: cfg.Mode(), diff --git a/pkg/skaffold/build/cluster/types_test.go b/pkg/skaffold/build/cluster/types_test.go index 9c040a0e423..8e04e6e10fe 100644 --- a/pkg/skaffold/build/cluster/types_test.go +++ b/pkg/skaffold/build/cluster/types_test.go @@ -72,7 +72,7 @@ func TestNewBuilder(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - _, err := NewBuilder(test.cfg) + _, err := NewBuilder(test.cfg, nil) t.CheckError(test.shouldErr, err) }) diff --git a/pkg/skaffold/build/gcb/buildpacks_test.go b/pkg/skaffold/build/gcb/buildpacks_test.go index b3316d2fab8..4b0bab1579c 100644 --- a/pkg/skaffold/build/gcb/buildpacks_test.go +++ b/pkg/skaffold/build/gcb/buildpacks_test.go @@ -194,7 +194,7 @@ func TestBuildpackBuildSpec(t *testing.T) { gcb: latest.GoogleCloudBuild{ PackImage: "pack/image", }, - }) + }, nil) builder.ArtifactStore(store) buildSpec, err := builder.buildSpec(artifact, "img", "bucket", "object") t.CheckError(test.shouldErr, err) diff --git a/pkg/skaffold/build/gcb/cloud_build.go b/pkg/skaffold/build/gcb/cloud_build.go index 704b9efb6ef..00485207dc7 100644 --- a/pkg/skaffold/build/gcb/cloud_build.go +++ b/pkg/skaffold/build/gcb/cloud_build.go @@ -34,7 +34,6 @@ import ( "k8s.io/apimachinery/pkg/util/wait" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/color" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" @@ -45,9 +44,21 @@ import ( ) // Build builds a list of artifacts with Google Cloud Build. -func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) { +func (b *Builder) Build(ctx context.Context, out io.Writer, artifact *latest.Artifact) build.ArtifactBuilder { builder := build.WithLogFile(b.buildArtifactWithCloudBuild, b.muted) - return build.InOrder(ctx, out, tags, artifacts, builder, b.GoogleCloudBuild.Concurrency, b.artifactStore) + return builder +} + +func (b *Builder) PreBuild(_ context.Context, _ io.Writer) error { + return nil +} + +func (b *Builder) PostBuild(_ context.Context, _ io.Writer) error { + return nil +} + +func (b *Builder) Concurrency() int { + return b.GoogleCloudBuild.Concurrency } func (b *Builder) buildArtifactWithCloudBuild(ctx context.Context, out io.Writer, artifact *latest.Artifact, tag string) (string, error) { diff --git a/pkg/skaffold/build/gcb/docker_test.go b/pkg/skaffold/build/gcb/docker_test.go index f8aaf0f16f1..dddce0f23fd 100644 --- a/pkg/skaffold/build/gcb/docker_test.go +++ b/pkg/skaffold/build/gcb/docker_test.go @@ -150,7 +150,7 @@ func TestDockerBuildSpec(t *testing.T) { MachineType: "n1-standard-1", Timeout: "10m", }, - }) + }, nil) store := mockArtifactStore{ "img2": "img2:tag", "img3": "img3:tag", @@ -179,7 +179,7 @@ func TestPullCacheFrom(t *testing.T) { gcb: latest.GoogleCloudBuild{ DockerImage: "docker/docker", }, - }) + }, nil) desc, err := builder.dockerBuildSpec(artifact, "nginx2") expected := []*cloudbuild.BuildStep{{ diff --git a/pkg/skaffold/build/gcb/jib_test.go b/pkg/skaffold/build/gcb/jib_test.go index d624257a022..a01d312485e 100644 --- a/pkg/skaffold/build/gcb/jib_test.go +++ b/pkg/skaffold/build/gcb/jib_test.go @@ -73,7 +73,7 @@ func TestJibMavenBuildSpec(t *testing.T) { gcb: latest.GoogleCloudBuild{ MavenImage: "maven:3.6.0", }, - }) + }, nil) builder.skipTests = test.skipTests store := mockArtifactStore{ "img2": "img2:tag", @@ -124,7 +124,7 @@ func TestJibGradleBuildSpec(t *testing.T) { gcb: latest.GoogleCloudBuild{ GradleImage: "gradle:5.1.1", }, - }) + }, nil) builder.skipTests = test.skipTests buildSpec, err := builder.buildSpec(artifact, "img", "bucket", "object") diff --git a/pkg/skaffold/build/gcb/kaniko_test.go b/pkg/skaffold/build/gcb/kaniko_test.go index cb5f0fd227d..5133582c390 100644 --- a/pkg/skaffold/build/gcb/kaniko_test.go +++ b/pkg/skaffold/build/gcb/kaniko_test.go @@ -363,7 +363,7 @@ func TestKanikoBuildSpec(t *testing.T) { MachineType: "n1-standard-1", Timeout: "10m", }, - }) + }, nil) defaultExpectedArgs := []string{ "--destination", "gcr.io/nginx", diff --git a/pkg/skaffold/build/gcb/spec_test.go b/pkg/skaffold/build/gcb/spec_test.go index 845269b1d04..4a3922f6436 100644 --- a/pkg/skaffold/build/gcb/spec_test.go +++ b/pkg/skaffold/build/gcb/spec_test.go @@ -46,7 +46,7 @@ func TestBuildSpecFail(t *testing.T) { testutil.Run(t, test.description, func(t *testutil.T) { builder := NewBuilder(&mockConfig{ gcb: latest.GoogleCloudBuild{}, - }) + }, nil) _, err := builder.buildSpec(test.artifact, "tag", "bucket", "object") diff --git a/pkg/skaffold/build/gcb/types.go b/pkg/skaffold/build/gcb/types.go index 0e8f1b29e8d..856a2a788fb 100644 --- a/pkg/skaffold/build/gcb/types.go +++ b/pkg/skaffold/build/gcb/types.go @@ -89,15 +89,14 @@ type Builder struct { 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(cfg Config) *Builder { +func NewBuilder(cfg Config, buildCfg *latest.GoogleCloudBuild) *Builder { return &Builder{ - GoogleCloudBuild: cfg.Pipeline().Build.GoogleCloudBuild, + GoogleCloudBuild: buildCfg, cfg: cfg, skipTests: cfg.SkipTests(), muted: cfg.Muted(), diff --git a/pkg/skaffold/build/local/local.go b/pkg/skaffold/build/local/local.go index 09751075f13..1df06c44c9e 100644 --- a/pkg/skaffold/build/local/local.go +++ b/pkg/skaffold/build/local/local.go @@ -23,7 +23,6 @@ import ( "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/color" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" @@ -31,28 +30,35 @@ import ( // Build runs a docker build on the host and tags the resulting image with // its checksum. It streams build progress to the writer argument. -func (b *Builder) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]build.Artifact, error) { - if b.localCluster { - color.Default.Fprintf(out, "Found [%s] context, using local docker daemon.\n", b.kubeContext) - } - defer b.localDocker.Close() - +func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact) build.ArtifactBuilder { if b.prune { - b.localPruner.asynchronousCleanupOldImages(ctx, artifacts) + b.localPruner.asynchronousCleanupOldImages(ctx, []string{a.ImageName}) } - builder := build.WithLogFile(b.buildArtifact, b.muted) - rt, err := build.InOrder(ctx, out, tags, artifacts, builder, *b.local.Concurrency, b.artifactStore) + return builder +} +func (b *Builder) PreBuild(_ context.Context, out io.Writer) error { + if b.localCluster { + color.Default.Fprintf(out, "Found [%s] context, using local docker daemon.\n", b.kubeContext) + } + return nil +} + +func (b *Builder) PostBuild(ctx context.Context, _ io.Writer) error { + defer b.localDocker.Close() if b.prune { if b.mode == config.RunModes.Build { - b.localPruner.synchronousCleanupOldImages(ctx, artifacts) + b.localPruner.synchronousCleanupOldImages(ctx, b.builtImages) } else { - b.localPruner.asynchronousCleanupOldImages(ctx, artifacts) + b.localPruner.asynchronousCleanupOldImages(ctx, b.builtImages) } } + return nil +} - return rt, err +func (b *Builder) Concurrency() int { + return *b.local.Concurrency } func (b *Builder) buildArtifact(ctx context.Context, out io.Writer, a *latest.Artifact, tag string) (string, error) { diff --git a/pkg/skaffold/build/local/local_test.go b/pkg/skaffold/build/local/local_test.go index 205a3949bdc..4bdd1f2d228 100644 --- a/pkg/skaffold/build/local/local_test.go +++ b/pkg/skaffold/build/local/local_test.go @@ -258,7 +258,7 @@ func TestLocalRun(t *testing.T) { Push: util.BoolPtr(test.pushImages), Concurrency: &constants.DefaultLocalConcurrency, }, - }) + }, nil) t.CheckNoError(err) builder.ArtifactStore(build.NewArtifactStore()) res, err := builder.Build(context.Background(), ioutil.Discard, test.tags, test.artifacts) @@ -322,7 +322,7 @@ func TestNewBuilder(t *testing.T) { builder, err := NewBuilder(&mockConfig{ local: test.localBuild, cluster: test.cluster, - }) + }, nil) t.CheckError(test.shouldErr, err) if !test.shouldErr { @@ -403,7 +403,7 @@ func TestGetArtifactBuilder(t *testing.T) { local: latest.LocalBuild{ Concurrency: &constants.DefaultLocalConcurrency, }, - }) + }, nil) t.CheckNoError(err) b.ArtifactStore(build.NewArtifactStore()) diff --git a/pkg/skaffold/build/local/prune.go b/pkg/skaffold/build/local/prune.go index 35b3224d984..fdff6c6e496 100644 --- a/pkg/skaffold/build/local/prune.go +++ b/pkg/skaffold/build/local/prune.go @@ -27,7 +27,6 @@ import ( "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" ) const ( @@ -68,7 +67,7 @@ func (p *pruner) listImages(ctx context.Context, name string) ([]types.ImageSumm return imgs, nil } -func (p *pruner) cleanup(ctx context.Context, sync bool, artifacts []*latest.Artifact) { +func (p *pruner) cleanup(ctx context.Context, sync bool, artifacts []string) { toPrune := p.collectImagesToPrune(ctx, artifacts) if len(toPrune) == 0 { return @@ -89,11 +88,11 @@ func (p *pruner) cleanup(ctx context.Context, sync bool, artifacts []*latest.Art } } -func (p *pruner) asynchronousCleanupOldImages(ctx context.Context, artifacts []*latest.Artifact) { +func (p *pruner) asynchronousCleanupOldImages(ctx context.Context, artifacts []string) { p.cleanup(ctx, false /*async*/, artifacts) } -func (p *pruner) synchronousCleanupOldImages(ctx context.Context, artifacts []*latest.Artifact) { +func (p *pruner) synchronousCleanupOldImages(ctx context.Context, artifacts []string) { p.cleanup(ctx, true /*sync*/, artifacts) } @@ -148,21 +147,21 @@ func (p *pruner) runPrune(ctx context.Context, ids []string) error { return nil } -func (p *pruner) collectImagesToPrune(ctx context.Context, artifacts []*latest.Artifact) []string { +func (p *pruner) collectImagesToPrune(ctx context.Context, artifacts []string) []string { // in case we're trying to build multiple images with the same ref in the same pipeline imgNameCount := make(map[string]int) for _, a := range artifacts { - imgNameCount[a.ImageName]++ + imgNameCount[a]++ } imgProcessed := make(map[string]struct{}) var rt []string for _, a := range artifacts { - if _, ok := imgProcessed[a.ImageName]; ok { + if _, ok := imgProcessed[a]; ok { continue } - imgProcessed[a.ImageName] = struct{}{} + imgProcessed[a] = struct{}{} - imgs, err := p.listImages(ctx, a.ImageName) + imgs, err := p.listImages(ctx, a) if err != nil { switch err { case context.Canceled, context.DeadlineExceeded: @@ -171,7 +170,7 @@ func (p *pruner) collectImagesToPrune(ctx context.Context, artifacts []*latest.A logrus.Warnf("failed to list images: %v", err) continue } - for i := imgNameCount[a.ImageName]; i < len(imgs); i++ { + for i := imgNameCount[a]; i < len(imgs); i++ { rt = append(rt, imgs[i].ID) } } diff --git a/pkg/skaffold/build/local/types.go b/pkg/skaffold/build/local/types.go index e55973254c2..4c331571c98 100644 --- a/pkg/skaffold/build/local/types.go +++ b/pkg/skaffold/build/local/types.go @@ -60,7 +60,6 @@ type Builder struct { type Config interface { docker.Config - Pipeline() latest.Pipeline GlobalConfig() string GetKubeContext() string GetCluster() config.Cluster @@ -71,7 +70,7 @@ type Config interface { } // NewBuilder returns an new instance of a local Builder. -func NewBuilder(cfg Config) (*Builder, error) { +func NewBuilder(cfg Config, buildCfg *latest.LocalBuild) (*Builder, error) { localDocker, err := docker.NewAPIClient(cfg) if err != nil { return nil, fmt.Errorf("getting docker client: %w", err) @@ -80,17 +79,17 @@ func NewBuilder(cfg Config) (*Builder, error) { cluster := cfg.GetCluster() var pushImages bool - if cfg.Pipeline().Build.LocalBuild.Push == nil { + if buildCfg.Push == nil { pushImages = cluster.PushImages logrus.Debugf("push value not present, defaulting to %t because cluster.PushImages is %t", pushImages, cluster.PushImages) } else { - pushImages = *cfg.Pipeline().Build.LocalBuild.Push + pushImages = *buildCfg.Push } - tryImportMissing := cfg.Pipeline().Build.LocalBuild.TryImportMissing + tryImportMissing := buildCfg.TryImportMissing return &Builder{ - local: *cfg.Pipeline().Build.LocalBuild, + local: *buildCfg, cfg: cfg, kubeContext: cfg.GetKubeContext(), localDocker: localDocker, @@ -111,16 +110,8 @@ func (b *Builder) ArtifactStore(store build.ArtifactStore) { b.artifactStore = store } -func (b *Builder) PushImages() bool { - return b.pushImages -} - -func (b *Builder) TryImportMissing() bool { - return b.tryImportMissing -} - // Prune uses the docker API client to remove all images built with Skaffold -func (b *Builder) Prune(ctx context.Context, out io.Writer) error { +func (b *Builder) Prune(ctx context.Context, _ io.Writer) error { var toPrune []string seen := make(map[string]bool) diff --git a/pkg/skaffold/build/tag/tagger_mux.go b/pkg/skaffold/build/tag/tagger_mux.go new file mode 100644 index 00000000000..49dbe810971 --- /dev/null +++ b/pkg/skaffold/build/tag/tagger_mux.go @@ -0,0 +1,122 @@ +/* +Copyright 2020 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tag + +import ( + "fmt" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" +) + +type TaggerMux struct { + taggers []Tagger + byImageName map[string]Tagger +} + +func (t *TaggerMux) GenerateTag(workingDir, imageName string) (string, error) { + tagger, found := t.byImageName[imageName] + if !found { + return "", fmt.Errorf("no valid tagger found for artifact: %q", imageName) + } + return tagger.GenerateTag(workingDir, imageName) +} + +func NewTaggerMux(runCtx *runcontext.RunContext) (Tagger, error) { + pipelines := runCtx.GetPipelines() + m := make(map[string]Tagger) + sl := make([]Tagger, len(pipelines)) + for _, p := range pipelines { + t, err := getTagger(runCtx, &p.Build.TagPolicy) + if err != nil { + return nil, fmt.Errorf("creating tagger: %w", err) + } + sl = append(sl, t) + for _, a := range p.Build.Artifacts { + m[a.ImageName] = t + } + } + return &TaggerMux{taggers: sl, byImageName: m}, nil +} + +func getTagger(runCtx *runcontext.RunContext, t *latest.TagPolicy) (Tagger, error) { + switch { + case runCtx.CustomTag() != "": + return &CustomTag{ + Tag: runCtx.CustomTag(), + }, nil + + case t.EnvTemplateTagger != nil: + return NewEnvTemplateTagger(t.EnvTemplateTagger.Template) + + case t.ShaTagger != nil: + return &ChecksumTagger{}, nil + + case t.GitTagger != nil: + return NewGitCommit(t.GitTagger.Prefix, t.GitTagger.Variant, t.GitTagger.IgnoreChanges) + + case t.DateTimeTagger != nil: + return NewDateTimeTagger(t.DateTimeTagger.Format, t.DateTimeTagger.TimeZone), nil + + case t.CustomTemplateTagger != nil: + components, err := CreateComponents(t.CustomTemplateTagger) + + if err != nil { + return nil, fmt.Errorf("creating components: %w", err) + } + + return NewCustomTemplateTagger(t.CustomTemplateTagger.Template, components) + + default: + return nil, fmt.Errorf("unknown tagger for strategy %+v", t) + } +} + +// CreateComponents creates a map of taggers for CustomTemplateTagger +func CreateComponents(t *latest.CustomTemplateTagger) (map[string]Tagger, error) { + components := map[string]Tagger{} + + for _, taggerComponent := range t.Components { + name, c := taggerComponent.Name, taggerComponent.Component + + if _, ok := components[name]; ok { + return nil, fmt.Errorf("multiple components with name %s", name) + } + + switch { + case c.EnvTemplateTagger != nil: + components[name], _ = NewEnvTemplateTagger(c.EnvTemplateTagger.Template) + + case c.ShaTagger != nil: + components[name] = &ChecksumTagger{} + + case c.GitTagger != nil: + components[name], _ = NewGitCommit(c.GitTagger.Prefix, c.GitTagger.Variant, c.GitTagger.IgnoreChanges) + + case c.DateTimeTagger != nil: + components[name] = NewDateTimeTagger(c.DateTimeTagger.Format, c.DateTimeTagger.TimeZone) + + case c.CustomTemplateTagger != nil: + return nil, fmt.Errorf("nested customTemplate components are not supported in skaffold (%s)", name) + + default: + return nil, fmt.Errorf("unknown component for custom template: %s %+v", name, c) + } + } + + return components, nil +} diff --git a/pkg/skaffold/deploy/helm/deploy.go b/pkg/skaffold/deploy/helm/deploy.go index e246a50af43..0e69896c809 100644 --- a/pkg/skaffold/deploy/helm/deploy.go +++ b/pkg/skaffold/deploy/helm/deploy.go @@ -91,7 +91,7 @@ type Deployer struct { } // NewDeployer returns a configured Deployer. Returns an error if current version of helm is less than 3.0.0. -func NewDeployer(cfg kubectl.Config, labels map[string]string) (*Deployer, error) { +func NewDeployer(cfg kubectl.Config, labels map[string]string, h *latest.HelmDeploy) (*Deployer, error) { hv, err := binVer() if err != nil { return nil, versionGetErr(err) @@ -102,7 +102,7 @@ func NewDeployer(cfg kubectl.Config, labels map[string]string) (*Deployer, error } return &Deployer{ - HelmDeploy: cfg.Pipeline().Deploy.HelmDeploy, + HelmDeploy: h, kubeContext: cfg.GetKubeContext(), kubeConfig: cfg.GetKubeConfig(), namespace: cfg.GetKubeNamespace(), diff --git a/pkg/skaffold/deploy/kpt/kpt.go b/pkg/skaffold/deploy/kpt/kpt.go index 2f89c4a4dd3..c48a00d329e 100644 --- a/pkg/skaffold/deploy/kpt/kpt.go +++ b/pkg/skaffold/deploy/kpt/kpt.go @@ -69,9 +69,9 @@ type Deployer struct { } // NewDeployer generates a new Deployer object contains the kptDeploy schema. -func NewDeployer(cfg types.Config, labels map[string]string) *Deployer { +func NewDeployer(cfg types.Config, labels map[string]string, d *latest.KptDeploy) *Deployer { return &Deployer{ - KptDeploy: cfg.Pipeline().Deploy.KptDeploy, + KptDeploy: d, insecureRegistries: cfg.GetInsecureRegistries(), labels: labels, globalConfig: cfg.GlobalConfig(), diff --git a/pkg/skaffold/deploy/kubectl/kubectl.go b/pkg/skaffold/deploy/kubectl/kubectl.go index bd4e390c68b..8600d37d3c1 100644 --- a/pkg/skaffold/deploy/kubectl/kubectl.go +++ b/pkg/skaffold/deploy/kubectl/kubectl.go @@ -57,22 +57,22 @@ type Deployer struct { // NewDeployer returns a new Deployer for a DeployConfig filled // with the needed configuration for `kubectl apply` -func NewDeployer(cfg Config, labels map[string]string) (*Deployer, error) { +func NewDeployer(cfg Config, labels map[string]string, d *latest.KubectlDeploy) (*Deployer, error) { defaultNamespace := "" - if cfg.Pipeline().Deploy.KubectlDeploy.DefaultNamespace != nil { + if d.DefaultNamespace != nil { var err error - defaultNamespace, err = util.ExpandEnvTemplate(*cfg.Pipeline().Deploy.KubectlDeploy.DefaultNamespace, nil) + defaultNamespace, err = util.ExpandEnvTemplate(*d.DefaultNamespace, nil) if err != nil { return nil, err } } return &Deployer{ - KubectlDeploy: cfg.Pipeline().Deploy.KubectlDeploy, + KubectlDeploy: d, workingDir: cfg.GetWorkingDir(), globalConfig: cfg.GlobalConfig(), defaultRepo: cfg.DefaultRepo(), - kubectl: NewCLI(cfg, cfg.Pipeline().Deploy.KubectlDeploy.Flags, defaultNamespace), + kubectl: NewCLI(cfg, d.Flags, defaultNamespace), insecureRegistries: cfg.GetInsecureRegistries(), skipRender: cfg.SkipRender(), labels: labels, diff --git a/pkg/skaffold/deploy/kustomize/kustomize.go b/pkg/skaffold/deploy/kustomize/kustomize.go index df4ecf202fc..2dbda412a76 100644 --- a/pkg/skaffold/deploy/kustomize/kustomize.go +++ b/pkg/skaffold/deploy/kustomize/kustomize.go @@ -100,22 +100,22 @@ type Deployer struct { useKubectlKustomize bool } -func NewDeployer(cfg kubectl.Config, labels map[string]string) (*Deployer, error) { +func NewDeployer(cfg kubectl.Config, labels map[string]string, d *latest.KustomizeDeploy) (*Deployer, error) { defaultNamespace := "" - if cfg.Pipeline().Deploy.KustomizeDeploy.DefaultNamespace != nil { + if d.DefaultNamespace != nil { var err error - defaultNamespace, err = util.ExpandEnvTemplate(*cfg.Pipeline().Deploy.KustomizeDeploy.DefaultNamespace, nil) + defaultNamespace, err = util.ExpandEnvTemplate(*d.DefaultNamespace, nil) if err != nil { return nil, err } } - kubectl := kubectl.NewCLI(cfg, cfg.Pipeline().Deploy.KustomizeDeploy.Flags, defaultNamespace) + kubectl := kubectl.NewCLI(cfg, d.Flags, defaultNamespace) // if user has kustomize binary, prioritize that over kubectl kustomize useKubectlKustomize := !kustomizeBinaryCheck() && kubectlVersionCheck(kubectl) return &Deployer{ - KustomizeDeploy: cfg.Pipeline().Deploy.KustomizeDeploy, + KustomizeDeploy: d, kubectl: kubectl, insecureRegistries: cfg.GetInsecureRegistries(), globalConfig: cfg.GlobalConfig(), diff --git a/pkg/skaffold/deploy/status/status_check.go b/pkg/skaffold/deploy/status/status_check.go index 6e6612739b3..b7840fddb6b 100644 --- a/pkg/skaffold/deploy/status/status_check.go +++ b/pkg/skaffold/deploy/status/status_check.go @@ -38,7 +38,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event" pkgkubectl "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl" kubernetesclient "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/client" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/proto" ) @@ -67,7 +66,7 @@ type Config interface { kubectl.Config GetNamespaces() []string - Pipeline() latest.Pipeline + StatusCheckDeadlineSeconds() int Muted() config.Muted } @@ -90,7 +89,7 @@ func NewStatusChecker(cfg Config, labeller *label.DefaultLabeller) Checker { muteLogs: cfg.Muted().MuteStatusCheck(), cfg: cfg, labeller: labeller, - deadlineSeconds: cfg.Pipeline().Deploy.StatusCheckDeadlineSeconds, + deadlineSeconds: cfg.StatusCheckDeadlineSeconds(), } } @@ -111,7 +110,7 @@ func (s statusChecker) statusCheck(ctx context.Context, out io.Writer) (proto.St deployments := make([]*resource.Deployment, 0) for _, n := range s.cfg.GetNamespaces() { newDeployments, err := getDeployments(ctx, client, n, s.labeller, - getDeadline(s.cfg.Pipeline().Deploy.StatusCheckDeadlineSeconds)) + getDeadline(s.deadlineSeconds)) if err != nil { return proto.StatusCode_STATUSCHECK_DEPLOYMENT_FETCH_ERR, fmt.Errorf("could not fetch deployments: %w", err) } diff --git a/pkg/skaffold/deploy/types/types.go b/pkg/skaffold/deploy/types/types.go index 6bd0c4696cf..5f9440f3342 100644 --- a/pkg/skaffold/deploy/types/types.go +++ b/pkg/skaffold/deploy/types/types.go @@ -26,7 +26,7 @@ import ( type Config interface { docker.Config - Pipeline() latest.Pipeline + GetPipelines() []latest.Pipeline GetWorkingDir() string GlobalConfig() string ConfigurationFile() string diff --git a/pkg/skaffold/diagnose/diagnose.go b/pkg/skaffold/diagnose/diagnose.go index 01a758692a3..d467dc0238a 100644 --- a/pkg/skaffold/diagnose/diagnose.go +++ b/pkg/skaffold/diagnose/diagnose.go @@ -34,61 +34,62 @@ import ( type Config interface { docker.Config - Pipeline() latest.Pipeline + GetPipelines() []latest.Pipeline } func CheckArtifacts(ctx context.Context, cfg Config, out io.Writer) error { - for _, artifact := range cfg.Pipeline().Build.Artifacts { - color.Default.Fprintf(out, "\n%s: %s\n", typeOfArtifact(artifact), artifact.ImageName) + for _, p := range cfg.GetPipelines() { + for _, artifact := range p.Build.Artifacts { + color.Default.Fprintf(out, "\n%s: %s\n", typeOfArtifact(artifact), artifact.ImageName) - if artifact.DockerArtifact != nil { - size, err := sizeOfDockerContext(ctx, artifact, cfg) + if artifact.DockerArtifact != nil { + size, err := sizeOfDockerContext(ctx, artifact, cfg) + if err != nil { + return fmt.Errorf("computing the size of the Docker context: %w", err) + } + + fmt.Fprintf(out, " - Size of the context: %vbytes\n", size) + } + + timeDeps1, deps, err := timeToListDependencies(ctx, artifact, cfg) if err != nil { - return fmt.Errorf("computing the size of the Docker context: %w", err) + return fmt.Errorf("listing artifact dependencies: %w", err) + } + timeDeps2, _, err := timeToListDependencies(ctx, artifact, cfg) + if err != nil { + return fmt.Errorf("listing artifact dependencies: %w", err) } - fmt.Fprintf(out, " - Size of the context: %vbytes\n", size) - } + fmt.Fprintln(out, " - Dependencies:", len(deps), "files") + fmt.Fprintf(out, " - Time to list dependencies: %v (2nd time: %v)\n", timeDeps1, timeDeps2) - timeDeps1, deps, err := timeToListDependencies(ctx, artifact, cfg) - if err != nil { - return fmt.Errorf("listing artifact dependencies: %w", err) - } - timeDeps2, _, err := timeToListDependencies(ctx, artifact, cfg) - if err != nil { - return fmt.Errorf("listing artifact dependencies: %w", err) - } - - fmt.Fprintln(out, " - Dependencies:", len(deps), "files") - fmt.Fprintf(out, " - Time to list dependencies: %v (2nd time: %v)\n", timeDeps1, timeDeps2) + timeSyncMap1, err := timeToConstructSyncMap(artifact, cfg) + if err != nil { + if _, isNotSupported := err.(build.ErrSyncMapNotSupported); !isNotSupported { + return fmt.Errorf("construct artifact dependencies: %w", err) + } + } + timeSyncMap2, err := timeToConstructSyncMap(artifact, cfg) + if err != nil { + if _, isNotSupported := err.(build.ErrSyncMapNotSupported); !isNotSupported { + return fmt.Errorf("construct artifact dependencies: %w", err) + } + } else { + fmt.Fprintf(out, " - Time to construct sync-map: %v (2nd time: %v)\n", timeSyncMap1, timeSyncMap2) + } - timeSyncMap1, err := timeToConstructSyncMap(artifact, cfg) - if err != nil { - if _, isNotSupported := err.(build.ErrSyncMapNotSupported); !isNotSupported { - return fmt.Errorf("construct artifact dependencies: %w", err) + timeMTimes1, err := timeToComputeMTimes(deps) + if err != nil { + return fmt.Errorf("computing modTimes: %w", err) } - } - timeSyncMap2, err := timeToConstructSyncMap(artifact, cfg) - if err != nil { - if _, isNotSupported := err.(build.ErrSyncMapNotSupported); !isNotSupported { - return fmt.Errorf("construct artifact dependencies: %w", err) + timeMTimes2, err := timeToComputeMTimes(deps) + if err != nil { + return fmt.Errorf("computing modTimes: %w", err) } - } else { - fmt.Fprintf(out, " - Time to construct sync-map: %v (2nd time: %v)\n", timeSyncMap1, timeSyncMap2) - } - timeMTimes1, err := timeToComputeMTimes(deps) - if err != nil { - return fmt.Errorf("computing modTimes: %w", err) - } - timeMTimes2, err := timeToComputeMTimes(deps) - if err != nil { - return fmt.Errorf("computing modTimes: %w", err) + fmt.Fprintf(out, " - Time to compute mTimes on dependencies: %v (2nd time: %v)\n", timeMTimes1, timeMTimes2) } - - fmt.Fprintf(out, " - Time to compute mTimes on dependencies: %v (2nd time: %v)\n", timeMTimes1, timeMTimes2) } - return nil } diff --git a/pkg/skaffold/event/event.go b/pkg/skaffold/event/event.go index db03deff0fb..0368be9d333 100644 --- a/pkg/skaffold/event/event.go +++ b/pkg/skaffold/event/event.go @@ -154,12 +154,14 @@ func (ev *eventHandler) forEachEvent(callback func(*proto.LogEntry) error) error return <-listener.errors } -func emptyState(p latest.Pipeline, kubeContext string, autoBuild, autoDeploy, autoSync bool) proto.State { +func emptyState(pipelines []latest.Pipeline, kubeContext string, autoBuild, autoDeploy, autoSync bool) proto.State { builds := map[string]string{} - for _, a := range p.Build.Artifacts { - builds[a.ImageName] = NotStarted + for _, p := range pipelines { + for _, a := range p.Build.Artifacts { + builds[a.ImageName] = NotStarted + } } - metadata := initializeMetadata(p, kubeContext) + metadata := initializeMetadata(pipelines, kubeContext) return emptyStateWithArtifacts(builds, metadata, autoBuild, autoDeploy, autoSync) } @@ -186,7 +188,7 @@ func emptyStateWithArtifacts(builds map[string]string, metadata *proto.Metadata, } // InitializeState instantiates the global state of the skaffold runner, as well as the event log. -func InitializeState(c latest.Pipeline, kc string, autoBuild, autoDeploy, autoSync bool) { +func InitializeState(c []latest.Pipeline, kc string, autoBuild, autoDeploy, autoSync bool) { handler.setState(emptyState(c, kc, autoBuild, autoDeploy, autoSync)) } diff --git a/pkg/skaffold/event/util.go b/pkg/skaffold/event/util.go index 9ce048cabfb..da0bb6a9c13 100644 --- a/pkg/skaffold/event/util.go +++ b/pkg/skaffold/event/util.go @@ -23,27 +23,47 @@ import ( "github.com/GoogleContainerTools/skaffold/proto" ) -func initializeMetadata(p latest.Pipeline, kubeContext string) *proto.Metadata { +func initializeMetadata(pipelines []latest.Pipeline, kubeContext string) *proto.Metadata { + + artifactCount := 0 + for _, p := range pipelines { + artifactCount += len(p.Build.Artifacts) + } m := &proto.Metadata{ Build: &proto.BuildMetadata{ - NumberOfArtifacts: int32(len(p.Build.Artifacts)), + NumberOfArtifacts: int32(artifactCount), }, Deploy: &proto.DeployMetadata{}, } + // all pipelines are constrained to have the same build type. switch { - case p.Build.LocalBuild != nil: + case pipelines[0].Build.LocalBuild != nil: m.Build.Type = proto.BuildType_LOCAL - case p.Build.GoogleCloudBuild != nil: + case pipelines[0].Build.GoogleCloudBuild != nil: m.Build.Type = proto.BuildType_GCB - case p.Build.Cluster != nil: + case pipelines[0].Build.Cluster != nil: m.Build.Type = proto.BuildType_CLUSTER default: m.Build.Type = proto.BuildType_UNKNOWN_BUILD_TYPE } - m.Build.Builders = getBuilders(p.Build) - m.Deploy = getDeploy(p.Deploy, kubeContext) + var builders []*proto.BuildMetadata_ImageBuilder + var deployers []*proto.DeployMetadata_Deployer + for _, p := range pipelines { + builders = append(builders, getBuilders(p.Build)...) + deployers = append(deployers, getDeploy(p.Deploy)...) + } + m.Build.Builders = builders + + if len(deployers) == 0 { + m.Deploy = &proto.DeployMetadata{} + } else { + m.Deploy = &proto.DeployMetadata{ + Deployers: deployers, + Cluster: getClusterType(kubeContext), + } + } return m } @@ -76,7 +96,7 @@ func getBuilders(b latest.BuildConfig) []*proto.BuildMetadata_ImageBuilder { return builders } -func getDeploy(d latest.DeployConfig, c string) *proto.DeployMetadata { +func getDeploy(d latest.DeployConfig) []*proto.DeployMetadata_Deployer { var deployers []*proto.DeployMetadata_Deployer if d.HelmDeploy != nil { @@ -88,14 +108,7 @@ func getDeploy(d latest.DeployConfig, c string) *proto.DeployMetadata { if d.KustomizeDeploy != nil { deployers = append(deployers, &proto.DeployMetadata_Deployer{Type: proto.DeployerType_KUSTOMIZE, Count: 1}) } - if len(deployers) == 0 { - return &proto.DeployMetadata{} - } - - return &proto.DeployMetadata{ - Deployers: deployers, - Cluster: getClusterType(c), - } + return deployers } func updateOrAddKey(m map[proto.BuilderType]int, k proto.BuilderType) { diff --git a/pkg/skaffold/instrumentation/meter.go b/pkg/skaffold/instrumentation/meter.go index eb131c529cb..0d53f011aec 100644 --- a/pkg/skaffold/instrumentation/meter.go +++ b/pkg/skaffold/instrumentation/meter.go @@ -75,17 +75,19 @@ var ( skipExport = os.Getenv("SKAFFOLD_EXPORT_METRICS") ) -func InitMeter(runCtx *runcontext.RunContext, config *latest.SkaffoldConfig) { +func InitMeter(runCtx *runcontext.RunContext, configs []*latest.SkaffoldConfig) { meter.Command = runCtx.Opts.Command - meter.PlatformType = yamltags.GetYamlTag(config.Build.BuildType) - for _, artifact := range config.Pipeline.Build.Artifacts { - meter.Builders[yamltags.GetYamlTag(artifact.ArtifactType)] = true - if artifact.Sync != nil { - meter.SyncType[yamltags.GetYamlTag(artifact.Sync)] = true + meter.PlatformType = yamltags.GetYamlTag(configs[0].Build.BuildType) + for _, config := range configs { + for _, artifact := range config.Pipeline.Build.Artifacts { + meter.Builders[yamltags.GetYamlTag(artifact.ArtifactType)] = true + if artifact.Sync != nil { + meter.SyncType[yamltags.GetYamlTag(artifact.Sync)] = true + } } + meter.Deployers = append(meter.Deployers, yamltags.GetYamlTags(config.Deploy.DeployType)...) + meter.BuildArtifacts += len(config.Pipeline.Build.Artifacts) } - meter.Deployers = yamltags.GetYamlTags(config.Deploy.DeployType) - meter.BuildArtifacts = len(config.Pipeline.Build.Artifacts) } func SetErrorCode(errorCode proto.StatusCode) { diff --git a/pkg/skaffold/kubernetes/log.go b/pkg/skaffold/kubernetes/log.go index 645786d0ee6..98a53cfa228 100644 --- a/pkg/skaffold/kubernetes/log.go +++ b/pkg/skaffold/kubernetes/log.go @@ -37,7 +37,7 @@ import ( type LogAggregator struct { output io.Writer kubectlcli *kubectl.CLI - config latest.LogsConfig + config Config podWatcher PodWatcher colorPicker ColorPicker @@ -48,8 +48,13 @@ type LogAggregator struct { outputLock sync.Mutex } +type Config interface { + Pipeline(imageName string) (*latest.Pipeline, bool) + DefaultPipeline() *latest.Pipeline +} + // NewLogAggregator creates a new LogAggregator for a given output. -func NewLogAggregator(out io.Writer, cli *kubectl.CLI, imageNames []string, podSelector PodSelector, namespaces []string, config latest.LogsConfig) *LogAggregator { +func NewLogAggregator(out io.Writer, cli *kubectl.CLI, imageNames []string, podSelector PodSelector, namespaces []string, config Config) *LogAggregator { return &LogAggregator{ output: out, kubectlcli: cli, @@ -175,7 +180,17 @@ func (a *LogAggregator) printLogLine(headerColor color.Color, prefix, text strin } func (a *LogAggregator) prefix(pod *v1.Pod, container v1.ContainerStatus) string { - switch a.config.Prefix { + var c *latest.Pipeline + var present bool + for _, container := range pod.Spec.Containers { + if c, present = a.config.Pipeline(stripTag(container.Image)); present { + break + } + } + if c == nil { + c = a.config.DefaultPipeline() + } + switch c.Deploy.Logs.Prefix { case "auto": if pod.Name != container.Name { return podAndContainerPrefix(pod, container) @@ -188,7 +203,7 @@ func (a *LogAggregator) prefix(pod *v1.Pod, container v1.ContainerStatus) string case "none": return "" default: - panic("unsupported prefix: " + a.config.Prefix) + panic("unsupported prefix: " + c.Deploy.Logs.Prefix) } } diff --git a/pkg/skaffold/kubernetes/manifest/images.go b/pkg/skaffold/kubernetes/manifest/images.go index 981c5b59154..b8fb2a7d934 100644 --- a/pkg/skaffold/kubernetes/manifest/images.go +++ b/pkg/skaffold/kubernetes/manifest/images.go @@ -119,7 +119,7 @@ func (r *imageReplacer) Visit(o map[string]interface{}, k string, v interface{}) func (r *imageReplacer) Check() { for imageName := range r.tagsByImageName { if !r.found[imageName] { - warnings.Printf("image [%s] is not used by the deployment", imageName) + logrus.Debugf("image [%s] is not used by the current deployment", imageName) } } } diff --git a/pkg/skaffold/runner/deploy.go b/pkg/skaffold/runner/deploy.go index f3d7f117f2d..32593e421d4 100644 --- a/pkg/skaffold/runner/deploy.go +++ b/pkg/skaffold/runner/deploy.go @@ -46,7 +46,16 @@ func (r *SkaffoldRunner) Deploy(ctx context.Context, out io.Writer, artifacts [] fmt.Fprintln(out, artifact.Tag) } - if r.imagesAreLocal && len(artifacts) > 0 { + var localImages []build.Artifact + for _, a := range artifacts { + if isLocal, err := r.isLocalImage(a.ImageName); err != nil { + return err + } else if isLocal { + localImages = append(localImages, a) + } + } + + if len(localImages) > 0 { logrus.Debugln(`Local images can't be referenced by digest. They are tagged and referenced by a unique, local only, tag instead. See https://skaffold.dev/docs/pipeline-stages/taggers/#how-tagging-works`) @@ -59,8 +68,8 @@ See https://skaffold.dev/docs/pipeline-stages/taggers/#how-tagging-works`) return fmt.Errorf("unable to connect to Kubernetes: %w", err) } - if r.imagesAreLocal && r.runCtx.Cluster.LoadImages { - err := r.loadImagesIntoCluster(ctx, out, artifacts) + if len(localImages) > 0 && r.runCtx.Cluster.LoadImages { + err := r.loadImagesIntoCluster(ctx, out, localImages) if err != nil { return err } diff --git a/pkg/skaffold/runner/generate_pipeline.go b/pkg/skaffold/runner/generate_pipeline.go index 38597701757..3bd00cd6a90 100644 --- a/pkg/skaffold/runner/generate_pipeline.go +++ b/pkg/skaffold/runner/generate_pipeline.go @@ -29,16 +29,19 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" ) -func (r *SkaffoldRunner) GeneratePipeline(ctx context.Context, out io.Writer, config *latest.SkaffoldConfig, configPaths []string, fileOut string) error { +func (r *SkaffoldRunner) GeneratePipeline(ctx context.Context, out io.Writer, configs []*latest.SkaffoldConfig, configPaths []string, fileOut string) error { // Keep track of files, configs, and profiles. This will be used to know which files to write // profiles to and what flags to add to task commands - baseConfig := []*pipeline.ConfigFile{ - { + var baseConfig []*pipeline.ConfigFile + for _, config := range configs { + cfgFile := &pipeline.ConfigFile{ Path: r.runCtx.ConfigurationFile(), Config: config, Profile: nil, - }, + } + baseConfig = append(baseConfig, cfgFile) } + configFiles, err := setupConfigFiles(configPaths) if err != nil { return fmt.Errorf("setting up ConfigFiles: %w", err) @@ -71,21 +74,23 @@ func setupConfigFiles(configPaths []string) ([]*pipeline.ConfigFile, error) { // Read all given config files to read contents into SkaffoldConfig var configFiles []*pipeline.ConfigFile for _, path := range configPaths { - parsed, err := schema.ParseConfigAndUpgrade(path, latest.Version) + parsedCfgs, err := schema.ParseConfigAndUpgrade(path, latest.Version) if err != nil { return nil, fmt.Errorf("parsing config %q: %w", path, err) } - config := parsed.(*latest.SkaffoldConfig) + for _, parsedCfg := range parsedCfgs { + config := parsedCfg.(*latest.SkaffoldConfig) - if err := defaults.Set(config); err != nil { - return nil, fmt.Errorf("setting default values for extra configs: %w", err) - } + if err := defaults.Set(config, true); err != nil { + return nil, fmt.Errorf("setting default values for extra configs: %w", err) + } - configFile := &pipeline.ConfigFile{ - Path: path, - Config: config, + configFile := &pipeline.ConfigFile{ + Path: path, + Config: config, + } + configFiles = append(configFiles, configFile) } - configFiles = append(configFiles, configFile) } return configFiles, nil diff --git a/pkg/skaffold/runner/logger.go b/pkg/skaffold/runner/logger.go index 716a661a120..7bb32f050db 100644 --- a/pkg/skaffold/runner/logger.go +++ b/pkg/skaffold/runner/logger.go @@ -33,5 +33,5 @@ func (r *SkaffoldRunner) createLogger(out io.Writer, artifacts []build.Artifact) imageNames = append(imageNames, artifact.Tag) } - return kubernetes.NewLogAggregator(out, r.kubectlCLI, imageNames, r.podSelector, r.runCtx.GetNamespaces(), r.runCtx.Pipeline().Deploy.Logs) + return kubernetes.NewLogAggregator(out, r.kubectlCLI, imageNames, r.podSelector, r.runCtx.GetNamespaces(), r.runCtx) } diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index 97cee7a3a36..2fce4dfc638 100644 --- a/pkg/skaffold/runner/new.go +++ b/pkg/skaffold/runner/new.go @@ -20,6 +20,7 @@ import ( "context" "fmt" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" @@ -27,7 +28,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/cluster" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/gcb" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/local" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kpt" @@ -48,28 +48,27 @@ import ( // NewForConfig returns a new SkaffoldRunner for a SkaffoldConfig func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) { - event.InitializeState(runCtx.Pipeline(), runCtx.GetKubeContext(), runCtx.AutoBuild(), runCtx.AutoDeploy(), runCtx.AutoSync()) + event.InitializeState(runCtx.GetPipelines(), runCtx.GetKubeContext(), runCtx.AutoBuild(), runCtx.AutoDeploy(), runCtx.AutoSync()) event.LogMetaEvent() kubectlCLI := pkgkubectl.NewCLI(runCtx, "") - tagger, err := getTagger(runCtx) + tagger, err := tag.NewTaggerMux(runCtx) if err != nil { return nil, fmt.Errorf("creating tagger: %w", err) } store := build.NewArtifactStore() - builder, imagesAreLocal, err := getBuilder(runCtx, store) + builder, err := build.NewBuilderMux(runCtx, store, func(p latest.Pipeline) (build.PipelineBuilder, error) { + return getBuilder(runCtx, store, p) + }) if err != nil { return nil, fmt.Errorf("creating builder: %w", err) } - - tryImportMissing := false - if localBuilder, ok := builder.(*local.Builder); ok { - tryImportMissing = localBuilder.TryImportMissing() + isLocalImage := func(imageName string) (bool, error) { + return isImageLocal(runCtx, imageName) } - labeller := label.NewLabeller(runCtx.AddSkaffoldLabels(), runCtx.CustomLabels()) - tester := getTester(runCtx, imagesAreLocal) + tester := getTester(runCtx, isLocalImage) syncer := getSyncer(runCtx) var deployer deploy.Deployer deployer, err = getDeployer(runCtx, labeller.Labels()) @@ -90,8 +89,8 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) { return append(buildDependencies, testDependencies...), nil } - graph := build.ToArtifactGraph(runCtx.Pipeline().Build.Artifacts) - artifactCache, err := cache.NewCache(runCtx, imagesAreLocal, tryImportMissing, depLister, graph, store) + graph := build.ToArtifactGraph(runCtx.Artifacts()) + artifactCache, err := cache.NewCache(runCtx, isLocalImage, depLister, graph, store) if err != nil { return nil, fmt.Errorf("initializing cache: %w", err) } @@ -120,14 +119,14 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) { Trigger: trigger, intentChan: intentChan, }, - artifactStore: store, - kubectlCLI: kubectlCLI, - labeller: labeller, - podSelector: kubernetes.NewImageList(), - cache: artifactCache, - runCtx: runCtx, - intents: intents, - imagesAreLocal: imagesAreLocal, + artifactStore: store, + kubectlCLI: kubectlCLI, + labeller: labeller, + podSelector: kubernetes.NewImageList(), + cache: artifactCache, + runCtx: runCtx, + intents: intents, + isLocalImage: isLocalImage, }, nil } @@ -165,83 +164,99 @@ func setupTrigger(triggerName string, setIntent func(bool), setAutoTrigger func( }) } -// getBuilder creates a builder from a given RunContext. -// Returns that builder, a bool to indicate that images are local -// (ie don't need to be pushed) and an error. -func getBuilder(runCtx *runcontext.RunContext, store build.ArtifactStore) (build.Builder, bool, error) { - b := runCtx.Pipeline().Build +func isImageLocal(runCtx *runcontext.RunContext, imageName string) (bool, error) { + pipeline, found := runCtx.Pipeline(imageName) + if !found { + return false, fmt.Errorf("unknown pipeline for image %q", imageName) + } + if pipeline.Build.GoogleCloudBuild != nil || pipeline.Build.Cluster != nil { + return false, nil + } + + cl := runCtx.GetCluster() + var pushImages bool + if pipeline.Build.LocalBuild.Push == nil { + pushImages = cl.PushImages + logrus.Debugf("push value not present, defaulting to %t because cluster.PushImages is %t", pushImages, cl.PushImages) + } else { + pushImages = *pipeline.Build.LocalBuild.Push + } + return !pushImages, nil +} +// getBuilder creates a builder from a given RunContext and build pipeline type. +func getBuilder(runCtx *runcontext.RunContext, store build.ArtifactStore, p latest.Pipeline) (build.PipelineBuilder, error) { switch { - case b.LocalBuild != nil: + case p.Build.LocalBuild != nil: logrus.Debugln("Using builder: local") - builder, err := local.NewBuilder(runCtx) + builder, err := local.NewBuilder(runCtx, p.Build.LocalBuild) if err != nil { - return nil, false, err + return nil, err } builder.ArtifactStore(store) - return builder, !builder.PushImages(), nil + return builder, nil - case b.GoogleCloudBuild != nil: + case p.Build.GoogleCloudBuild != nil: logrus.Debugln("Using builder: google cloud") - builder := gcb.NewBuilder(runCtx) + builder := gcb.NewBuilder(runCtx, p.Build.GoogleCloudBuild) builder.ArtifactStore(store) - return builder, false, nil + return builder, nil - case b.Cluster != nil: + case p.Build.Cluster != nil: logrus.Debugln("Using builder: cluster") - builder, err := cluster.NewBuilder(runCtx) + builder, err := cluster.NewBuilder(runCtx, p.Build.Cluster) if err != nil { - return nil, false, err + return nil, err } builder.ArtifactStore(store) - return builder, false, err + return builder, err default: - return nil, false, fmt.Errorf("unknown builder for config %+v", b) + return nil, fmt.Errorf("unknown builder for config %+v", p.Build) } } -func getTester(cfg test.Config, imagesAreLocal bool) test.Tester { - return test.NewTester(cfg, imagesAreLocal) +func getTester(cfg test.Config, isLocalImage func(imageName string) (bool, error)) test.Tester { + return test.NewTester(cfg, isLocalImage) } func getSyncer(cfg sync.Config) sync.Syncer { return sync.NewSyncer(cfg) } -func getDeployer(cfg kubectl.Config, labels map[string]string) (deploy.Deployer, error) { - d := cfg.Pipeline().Deploy +func getDeployer(runCtx *runcontext.RunContext, labels map[string]string) (deploy.Deployer, error) { + deployerCfg := runCtx.Deployers() var deployers deploy.DeployerMux - - if d.HelmDeploy != nil { - h, err := helm.NewDeployer(cfg, labels) - if err != nil { - return nil, err + for _, d := range deployerCfg { + if d.HelmDeploy != nil { + h, err := helm.NewDeployer(runCtx, labels, d.HelmDeploy) + if err != nil { + return nil, err + } + deployers = append(deployers, h) } - deployers = append(deployers, h) - } - if d.KptDeploy != nil { - deployers = append(deployers, kpt.NewDeployer(cfg, labels)) - } + if d.KptDeploy != nil { + deployers = append(deployers, kpt.NewDeployer(runCtx, labels, d.KptDeploy)) + } - if d.KubectlDeploy != nil { - deployer, err := kubectl.NewDeployer(cfg, labels) - if err != nil { - return nil, err + if d.KubectlDeploy != nil { + deployer, err := kubectl.NewDeployer(runCtx, labels, d.KubectlDeploy) + if err != nil { + return nil, err + } + deployers = append(deployers, deployer) } - deployers = append(deployers, deployer) - } - if d.KustomizeDeploy != nil { - deployer, err := kustomize.NewDeployer(cfg, labels) - if err != nil { - return nil, err + if d.KustomizeDeploy != nil { + deployer, err := kustomize.NewDeployer(runCtx, labels, d.KustomizeDeploy) + if err != nil { + return nil, err + } + deployers = append(deployers, deployer) } - deployers = append(deployers, deployer) } - // avoid muxing overhead when only a single deployer is configured if len(deployers) == 1 { return deployers[0], nil @@ -249,73 +264,3 @@ func getDeployer(cfg kubectl.Config, labels map[string]string) (deploy.Deployer, return deployers, nil } - -func getTagger(runCtx *runcontext.RunContext) (tag.Tagger, error) { - t := runCtx.Pipeline().Build.TagPolicy - - switch { - case runCtx.CustomTag() != "": - return &tag.CustomTag{ - Tag: runCtx.CustomTag(), - }, nil - - case t.EnvTemplateTagger != nil: - return tag.NewEnvTemplateTagger(t.EnvTemplateTagger.Template) - - case t.ShaTagger != nil: - return &tag.ChecksumTagger{}, nil - - case t.GitTagger != nil: - return tag.NewGitCommit(t.GitTagger.Prefix, t.GitTagger.Variant, t.GitTagger.IgnoreChanges) - - case t.DateTimeTagger != nil: - return tag.NewDateTimeTagger(t.DateTimeTagger.Format, t.DateTimeTagger.TimeZone), nil - - case t.CustomTemplateTagger != nil: - components, err := CreateComponents(t.CustomTemplateTagger) - - if err != nil { - return nil, fmt.Errorf("creating components: %w", err) - } - - return tag.NewCustomTemplateTagger(t.CustomTemplateTagger.Template, components) - - default: - return nil, fmt.Errorf("unknown tagger for strategy %+v", t) - } -} - -// CreateComponents creates a map of taggers for CustomTemplateTagger -func CreateComponents(t *latest.CustomTemplateTagger) (map[string]tag.Tagger, error) { - components := map[string]tag.Tagger{} - - for _, taggerComponent := range t.Components { - name, c := taggerComponent.Name, taggerComponent.Component - - if _, ok := components[name]; ok { - return nil, fmt.Errorf("multiple components with name %s", name) - } - - switch { - case c.EnvTemplateTagger != nil: - components[name], _ = tag.NewEnvTemplateTagger(c.EnvTemplateTagger.Template) - - case c.ShaTagger != nil: - components[name] = &tag.ChecksumTagger{} - - case c.GitTagger != nil: - components[name], _ = tag.NewGitCommit(c.GitTagger.Prefix, c.GitTagger.Variant, c.GitTagger.IgnoreChanges) - - case c.DateTimeTagger != nil: - components[name] = tag.NewDateTimeTagger(c.DateTimeTagger.Format, c.DateTimeTagger.TimeZone) - - case c.CustomTemplateTagger != nil: - return nil, fmt.Errorf("nested customTemplate components are not supported in skaffold (%s)", name) - - default: - return nil, fmt.Errorf("unknown component for custom template: %s %+v", name, c) - } - } - - return components, nil -} diff --git a/pkg/skaffold/runner/new_test.go b/pkg/skaffold/runner/new_test.go index 92eee83cdea..8111bb27b37 100644 --- a/pkg/skaffold/runner/new_test.go +++ b/pkg/skaffold/runner/new_test.go @@ -61,7 +61,7 @@ func TestGetDeployer(tOuter *testing.T) { description: "kubectl deployer", cfg: latest.DeployType{KubectlDeploy: &latest.KubectlDeploy{}}, expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ - Cfg: latest.Pipeline{ + Pipelines: latest.Pipeline{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ KubectlDeploy: &latest.KubectlDeploy{ @@ -76,7 +76,7 @@ func TestGetDeployer(tOuter *testing.T) { description: "kustomize deployer", cfg: latest.DeployType{KustomizeDeploy: &latest.KustomizeDeploy{}}, expected: t.RequireNonNilResult(kustomize.NewDeployer(&runcontext.RunContext{ - Cfg: latest.Pipeline{ + Pipelines: latest.Pipeline{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ KustomizeDeploy: &latest.KustomizeDeploy{ @@ -115,7 +115,7 @@ func TestGetDeployer(tOuter *testing.T) { } deployer, err := getDeployer(&runcontext.RunContext{ - Cfg: latest.Pipeline{ + Pipelines: latest.Pipeline{ Deploy: latest.DeployConfig{ DeployType: test.cfg, }, diff --git a/pkg/skaffold/runner/portforwarder.go b/pkg/skaffold/runner/portforwarder.go index c6fd637b389..f2b82b1abe1 100644 --- a/pkg/skaffold/runner/portforwarder.go +++ b/pkg/skaffold/runner/portforwarder.go @@ -33,5 +33,5 @@ func (r *SkaffoldRunner) createForwarder(out io.Writer) *portforward.ForwarderMa r.runCtx.GetNamespaces(), r.labeller.RunIDSelector(), r.runCtx.Opts.PortForward, - r.runCtx.Pipeline().PortForward) + r.runCtx.PortForwardResources()) } diff --git a/pkg/skaffold/runner/runcontext/context.go b/pkg/skaffold/runner/runcontext/context.go index ad81a165aa9..06a4df75011 100644 --- a/pkg/skaffold/runner/runcontext/context.go +++ b/pkg/skaffold/runner/runcontext/context.go @@ -34,23 +34,76 @@ const ( ) type RunContext struct { - Opts config.SkaffoldOptions - Cfg latest.Pipeline - - KubeContext string - Namespaces []string - WorkingDir string - InsecureRegistries map[string]bool - Cluster config.Cluster + Opts config.SkaffoldOptions + Pipelines []latest.Pipeline + KubeContext string + Namespaces []string + WorkingDir string + InsecureRegistries map[string]bool + Cluster config.Cluster + pipelinesByImageName map[string]*latest.Pipeline } -func (rc *RunContext) GetKubeContext() string { return rc.KubeContext } -func (rc *RunContext) GetNamespaces() []string { return rc.Namespaces } -func (rc *RunContext) Pipeline() latest.Pipeline { return rc.Cfg } -func (rc *RunContext) GetInsecureRegistries() map[string]bool { return rc.InsecureRegistries } -func (rc *RunContext) GetWorkingDir() string { return rc.WorkingDir } -func (rc *RunContext) GetCluster() config.Cluster { return rc.Cluster } +// Pipeline returns the first `latest.Pipeline` that matches the given artifact `imageName`. +func (rc *RunContext) Pipeline(imageName string) (*latest.Pipeline, bool) { + p, found := rc.pipelinesByImageName[imageName] + return p, found +} + +func (rc *RunContext) PortForwardResources() []*latest.PortForwardResource { + var pf []*latest.PortForwardResource + for _, p := range rc.Pipelines { + pf = append(pf, p.PortForward...) + } + return pf +} + +func (rc *RunContext) Artifacts() []*latest.Artifact { + var artifacts []*latest.Artifact + for _, p := range rc.Pipelines { + for _, a := range p.Build.Artifacts { + artifacts = append(artifacts, a) + } + } + return artifacts +} + +func (rc *RunContext) Deployers() []latest.DeployType { + var deployers []latest.DeployType + for _, p := range rc.Pipelines { + deployers = append(deployers, p.Deploy.DeployType) + } + return deployers +} +func (rc *RunContext) TestCases() []*latest.TestCase { + var tests []*latest.TestCase + for _, p := range rc.Pipelines { + tests = append(tests, p.Test...) + } + return tests +} + +func (rc *RunContext) StatusCheckDeadlineSeconds() int { + c := 0 + // set the group status check deadline to maximum of any individually specified value + for _, p := range rc.Pipelines { + if p.Deploy.StatusCheckDeadlineSeconds > c { + c = p.Deploy.StatusCheckDeadlineSeconds + } + } + return c +} + +// DefaultPipeline returns the first `latest.Pipeline` it finds. +func (rc *RunContext) DefaultPipeline() *latest.Pipeline { return &rc.Pipelines[0] } + +func (rc *RunContext) GetKubeContext() string { return rc.KubeContext } +func (rc *RunContext) GetNamespaces() []string { return rc.Namespaces } +func (rc *RunContext) GetPipelines() []latest.Pipeline { return rc.Pipelines } +func (rc *RunContext) GetInsecureRegistries() map[string]bool { return rc.InsecureRegistries } +func (rc *RunContext) GetWorkingDir() string { return rc.WorkingDir } +func (rc *RunContext) GetCluster() config.Cluster { return rc.Cluster } func (rc *RunContext) AddSkaffoldLabels() bool { return rc.Opts.AddSkaffoldLabels } func (rc *RunContext) AutoBuild() bool { return rc.Opts.AutoBuild } func (rc *RunContext) AutoDeploy() bool { return rc.Opts.AutoDeploy } @@ -84,7 +137,7 @@ func (rc *RunContext) Trigger() string { return rc.Opt func (rc *RunContext) WaitForDeletions() config.WaitForDeletions { return rc.Opts.WaitForDeletions } func (rc *RunContext) WatchPollInterval() int { return rc.Opts.WatchPollInterval } -func GetRunContext(opts config.SkaffoldOptions, cfg latest.Pipeline) (*RunContext, error) { +func GetRunContext(opts config.SkaffoldOptions, pipelines []latest.Pipeline) (*RunContext, error) { kubeConfig, err := kubectx.CurrentConfig() if err != nil { return nil, fmt.Errorf("getting current cluster context: %w", err) @@ -98,7 +151,7 @@ func GetRunContext(opts config.SkaffoldOptions, cfg latest.Pipeline) (*RunContex return nil, fmt.Errorf("finding current directory: %w", err) } - namespaces, err := runnerutil.GetAllPodNamespaces(opts.Namespace, cfg) + namespaces, err := runnerutil.GetAllPodNamespaces(opts.Namespace, pipelines) if err != nil { return nil, fmt.Errorf("getting namespace list: %w", err) } @@ -108,12 +161,22 @@ func GetRunContext(opts config.SkaffoldOptions, cfg latest.Pipeline) (*RunContex if err != nil { logrus.Warnf("error retrieving insecure registries from global config: push/pull issues may exist...") } - regList := append(opts.InsecureRegistries, cfg.Build.InsecureRegistries...) + var regList []string + regList = append(regList, opts.InsecureRegistries...) + for _, cfg := range pipelines { + regList = append(regList, cfg.Build.InsecureRegistries...) + } regList = append(regList, cfgRegistries...) insecureRegistries := make(map[string]bool, len(regList)) for _, r := range regList { insecureRegistries[r] = true } + m := make(map[string]*latest.Pipeline) + for _, p := range pipelines { + for _, a := range p.Build.Artifacts { + m[a.ImageName] = &p + } + } // TODO(https://github.com/GoogleContainerTools/skaffold/issues/3668): // remove minikubeProfile from here and instead detect it by matching the @@ -124,13 +187,14 @@ func GetRunContext(opts config.SkaffoldOptions, cfg latest.Pipeline) (*RunContex } return &RunContext{ - Opts: opts, - Cfg: cfg, - WorkingDir: cwd, - KubeContext: kubeContext, - Namespaces: namespaces, - InsecureRegistries: insecureRegistries, - Cluster: cluster, + Opts: opts, + Pipelines: pipelines, + WorkingDir: cwd, + KubeContext: kubeContext, + Namespaces: namespaces, + InsecureRegistries: insecureRegistries, + Cluster: cluster, + pipelinesByImageName: m, }, nil } diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index 3d1bc633da1..f89fd93b476 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -46,7 +46,7 @@ type Runner interface { ApplyDefaultRepo(tag string) (string, error) BuildAndTest(context.Context, io.Writer, []*latest.Artifact) ([]build.Artifact, error) DeployAndLog(context.Context, io.Writer, []build.Artifact) error - GeneratePipeline(context.Context, io.Writer, *latest.SkaffoldConfig, []string, string) error + GeneratePipeline(context.Context, io.Writer, []*latest.SkaffoldConfig, []string, string) error Render(context.Context, io.Writer, []build.Artifact, bool, string) error Cleanup(context.Context, io.Writer) error Prune(context.Context, io.Writer) error @@ -74,11 +74,11 @@ type SkaffoldRunner struct { // podSelector is used to determine relevant pods for logging and portForwarding podSelector *kubernetes.ImageList - imagesAreLocal bool - hasBuilt bool - hasDeployed bool - intents *intents - devIteration int + isLocalImage func(imageName string) (bool, error) + hasBuilt bool + hasDeployed bool + intents *intents + devIteration int } // for testing diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index b2d9702dd4d..fbeafaa3397 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -221,7 +221,7 @@ func createRunner(t *testutil.T, testBench *TestBench, monitor filemon.Monitor) defaults.Set(cfg) runCtx := &runcontext.RunContext{ - Cfg: cfg.Pipeline, + Pipelines: cfg.Pipeline, Opts: config.SkaffoldOptions{ Trigger: "polling", WatchPollInterval: 100, @@ -401,7 +401,7 @@ func TestNewForConfig(t *testing.T) { AndRunWithOutput("kubectl version --client -ojson", "v1.5.6")) runCtx := &runcontext.RunContext{ - Cfg: test.pipeline, + Pipelines: test.pipeline, Opts: config.SkaffoldOptions{ Trigger: "polling", }, @@ -497,8 +497,8 @@ func TestTriggerCallbackAndIntents(t *testing.T) { }, } r, _ := NewForConfig(&runcontext.RunContext{ - Opts: opts, - Cfg: pipeline, + Opts: opts, + Pipelines: pipeline, }) r.intents.resetBuild() diff --git a/pkg/skaffold/runner/util/util.go b/pkg/skaffold/runner/util/util.go index a238be397d5..af359ff2c73 100644 --- a/pkg/skaffold/runner/util/util.go +++ b/pkg/skaffold/runner/util/util.go @@ -28,7 +28,7 @@ import ( // + The namespace passed on the command line // + Current kube context's namespace // + Namespaces referenced in Helm releases -func GetAllPodNamespaces(configNamespace string, cfg latest.Pipeline) ([]string, error) { +func GetAllPodNamespaces(configNamespace string, pipelines []latest.Pipeline) ([]string, error) { nsMap := make(map[string]bool) if configNamespace == "" { @@ -49,7 +49,7 @@ func GetAllPodNamespaces(configNamespace string, cfg latest.Pipeline) ([]string, } // Set additional namespaces each helm release referenced - for _, namespace := range collectHelmReleasesNamespaces(cfg) { + for _, namespace := range collectHelmReleasesNamespaces(pipelines) { nsMap[namespace] = true } @@ -63,16 +63,16 @@ func GetAllPodNamespaces(configNamespace string, cfg latest.Pipeline) ([]string, return namespaces, nil } -func collectHelmReleasesNamespaces(cfg latest.Pipeline) []string { +func collectHelmReleasesNamespaces(pipelines []latest.Pipeline) []string { var namespaces []string - - if cfg.Deploy.HelmDeploy != nil { - for _, release := range cfg.Deploy.HelmDeploy.Releases { - if release.Namespace != "" { - namespaces = append(namespaces, release.Namespace) + for _, cfg := range pipelines { + if cfg.Deploy.HelmDeploy != nil { + for _, release := range cfg.Deploy.HelmDeploy.Releases { + if release.Namespace != "" { + namespaces = append(namespaces, release.Namespace) + } } } } - return namespaces } diff --git a/pkg/skaffold/schema/defaults/defaults.go b/pkg/skaffold/schema/defaults/defaults.go index ffc2667fd99..d4ad83eceba 100644 --- a/pkg/skaffold/schema/defaults/defaults.go +++ b/pkg/skaffold/schema/defaults/defaults.go @@ -40,12 +40,14 @@ const ( ) // Set makes sure default values are set on a SkaffoldConfig. -func Set(c *latest.SkaffoldConfig) error { +func Set(c *latest.SkaffoldConfig, setDefaultDeployer bool) error { defaultToLocalBuild(c) - defaultToKubectlDeploy(c) + if setDefaultDeployer { + defaultToKubectlDeploy(c) + setDefaultKubectlManifests(c) + } setDefaultTagger(c) setDefaultKustomizePath(c) - setDefaultKubectlManifests(c) setDefaultLogsConfig(c) for _, a := range c.Build.Artifacts { diff --git a/pkg/skaffold/schema/validation/validation.go b/pkg/skaffold/schema/validation/validation.go index 2113cc8b6ce..cd2095b72e7 100644 --- a/pkg/skaffold/schema/validation/validation.go +++ b/pkg/skaffold/schema/validation/validation.go @@ -24,8 +24,6 @@ import ( "strings" "time" - "github.com/docker/docker/api/types" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/misc" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors" @@ -34,6 +32,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/yamltags" "github.com/GoogleContainerTools/skaffold/proto" + "github.com/docker/docker/api/types" ) var ( @@ -43,18 +42,21 @@ var ( ) // Process checks if the Skaffold pipeline is valid and returns all encountered errors as a concatenated string -func Process(config *latest.SkaffoldConfig) error { - errs := visitStructs(config, validateYamltags) - errs = append(errs, validateImageNames(config.Build.Artifacts)...) - errs = append(errs, validateArtifactDependencies(config.Build.Artifacts)...) - errs = append(errs, validateDockerNetworkMode(config.Build.Artifacts)...) - errs = append(errs, validateCustomDependencies(config.Build.Artifacts)...) - errs = append(errs, validateSyncRules(config.Build.Artifacts)...) - errs = append(errs, validatePortForwardResources(config.PortForward)...) - errs = append(errs, validateJibPluginTypes(config.Build.Artifacts)...) - errs = append(errs, validateLogPrefix(config.Deploy.Logs)...) - errs = append(errs, validateArtifactTypes(config.Build)...) - errs = append(errs, validateTaggingPolicy(config.Build)...) +func Process(configs []*latest.SkaffoldConfig) error { + var errs []error + for _, config := range configs { + errs = visitStructs(config, validateYamltags) + errs = append(errs, validateImageNames(config.Build.Artifacts)...) + errs = append(errs, validateDockerNetworkMode(config.Build.Artifacts)...) + errs = append(errs, validateCustomDependencies(config.Build.Artifacts)...) + errs = append(errs, validateSyncRules(config.Build.Artifacts)...) + errs = append(errs, validatePortForwardResources(config.PortForward)...) + errs = append(errs, validateJibPluginTypes(config.Build.Artifacts)...) + errs = append(errs, validateLogPrefix(config.Deploy.Logs)...) + errs = append(errs, validateArtifactTypes(config.Build)...) + errs = append(errs, validateTaggingPolicy(config.Build)...) + } + errs = append(errs, validateArtifactDependencies(configs)...) if len(errs) == 0 { return nil @@ -69,9 +71,9 @@ func Process(config *latest.SkaffoldConfig) error { // ProcessWithRunContext checks if the Skaffold pipeline is valid when a RunContext is required. // It returns all encountered errors as a concatenated string. -func ProcessWithRunContext(config *latest.SkaffoldConfig, runCtx *runcontext.RunContext) error { +func ProcessWithRunContext(runCtx *runcontext.RunContext) error { var errs []error - errs = append(errs, validateDockerNetworkContainerExists(config.Build.Artifacts, runCtx)...) + errs = append(errs, validateDockerNetworkContainerExists(runCtx.Artifacts(), runCtx)...) if len(errs) == 0 { return nil @@ -115,7 +117,11 @@ func validateImageNames(artifacts []*latest.Artifact) (errs []error) { return } -func validateArtifactDependencies(artifacts []*latest.Artifact) (errs []error) { +func validateArtifactDependencies(configs []*latest.SkaffoldConfig) (errs []error) { + var artifacts []*latest.Artifact + for _, c := range configs { + artifacts = append(artifacts, c.Build.Artifacts...) + } errs = append(errs, validateUniqueDependencyAliases(artifacts)...) errs = append(errs, validateAcyclicDependencies(artifacts)...) errs = append(errs, validateValidDependencyAliases(artifacts)...) diff --git a/pkg/skaffold/schema/versions.go b/pkg/skaffold/schema/versions.go index fd1e40c3de6..9b3e696f93f 100644 --- a/pkg/skaffold/schema/versions.go +++ b/pkg/skaffold/schema/versions.go @@ -145,54 +145,57 @@ func IsSkaffoldConfig(file string) bool { } // ParseConfig reads a configuration file. -func ParseConfig(filename string) (util.VersionedConfig, error) { - buf, err := misc.ReadConfiguration(filename) +func ParseConfig(filename string) ([]util.VersionedConfig, error) { + var sl []util.VersionedConfig + buffer, err := misc.ReadConfiguration(filename) if err != nil { return nil, fmt.Errorf("read skaffold config: %w", err) } + arr := bytes.Split(buffer, []byte("---")) + for _, buf := range arr { + // This is to quickly check that it's possibly a skaffold.yaml, + // without parsing the whole file. + if !bytes.Contains(buf, []byte("apiVersion")) { + return nil, errors.New("missing apiVersion") + } - // This is to quickly check that it's possibly a skaffold.yaml, - // without parsing the whole file. - if !bytes.Contains(buf, []byte("apiVersion")) { - return nil, errors.New("missing apiVersion") - } - - apiVersion := &APIVersion{} - if err := yaml.Unmarshal(buf, apiVersion); err != nil { - return nil, fmt.Errorf("parsing api version: %w", err) - } + apiVersion := &APIVersion{} + if err := yaml.Unmarshal(buf, apiVersion); err != nil { + return nil, fmt.Errorf("parsing api version: %w", err) + } - factory, present := SchemaVersions.Find(apiVersion.Version) - if !present { - return nil, fmt.Errorf("unknown api version: %q", apiVersion.Version) - } + factory, present := SchemaVersions.Find(apiVersion.Version) + if !present { + return nil, fmt.Errorf("unknown api version: %q", apiVersion.Version) + } - // Remove all top-level keys starting with `.` so they can be used as YAML anchors - parsed := make(map[string]interface{}) - if err := yaml.UnmarshalStrict(buf, parsed); err != nil { - return nil, fmt.Errorf("unable to parse YAML: %w", err) - } - for field := range parsed { - if strings.HasPrefix(field, ".") { - delete(parsed, field) + // Remove all top-level keys starting with `.` so they can be used as YAML anchors + parsed := make(map[string]interface{}) + if err := yaml.UnmarshalStrict(buf, parsed); err != nil { + return nil, fmt.Errorf("unable to parse YAML: %w", err) + } + for field := range parsed { + if strings.HasPrefix(field, ".") { + delete(parsed, field) + } + } + buf, err = yaml.Marshal(parsed) + if err != nil { + return nil, fmt.Errorf("unable to re-marshal YAML without dotted keys: %w", err) } - } - buf, err = yaml.Marshal(parsed) - if err != nil { - return nil, fmt.Errorf("unable to re-marshal YAML without dotted keys: %w", err) - } - cfg := factory() - if err := yaml.UnmarshalStrict(buf, cfg); err != nil { - return nil, fmt.Errorf("unable to parse config: %w", err) + cfg := factory() + if err := yaml.UnmarshalStrict(buf, cfg); err != nil { + return nil, fmt.Errorf("unable to parse config: %w", err) + } + sl = append(sl, cfg) } - - return cfg, nil + return sl, nil } // ParseConfigAndUpgrade reads a configuration file and upgrades it to a given version. -func ParseConfigAndUpgrade(filename, toVersion string) (util.VersionedConfig, error) { - cfg, err := ParseConfig(filename) +func ParseConfigAndUpgrade(filename, toVersion string) ([]util.VersionedConfig, error) { + configs, err := ParseConfig(filename) if err != nil { return nil, err } @@ -201,32 +204,39 @@ func ParseConfigAndUpgrade(filename, toVersion string) (util.VersionedConfig, er if _, present := SchemaVersions.Find(toVersion); !present { return nil, fmt.Errorf("unknown api version: %q", toVersion) } + upgradeNeeded := false + for _, cfg := range configs { + // Check that the config's version is not newer than the target version + currentVersion, err := apiversion.Parse(cfg.GetVersion()) + if err != nil { + return nil, err + } + targetVersion, err := apiversion.Parse(toVersion) + if err != nil { + return nil, err + } - // Check that the config's version is not newer than the target version - currentVersion, err := apiversion.Parse(cfg.GetVersion()) - if err != nil { - return nil, err - } - targetVersion, err := apiversion.Parse(toVersion) - if err != nil { - return nil, err - } - - if currentVersion.EQ(targetVersion) { - return cfg, nil + if currentVersion.NE(targetVersion) { + upgradeNeeded = true + } + if currentVersion.GT(targetVersion) { + return nil, fmt.Errorf("config version %q is more recent than target version %q: upgrade Skaffold", cfg.GetVersion(), toVersion) + } } - if currentVersion.GT(targetVersion) { - return nil, fmt.Errorf("config version %q is more recent than target version %q: upgrade Skaffold", cfg.GetVersion(), toVersion) + if !upgradeNeeded { + return configs, nil } + logrus.Debugf("config version out of date: upgrading to latest %q", toVersion) - logrus.Debugf("config version %q out of date: upgrading to latest %q", cfg.GetVersion(), toVersion) - - for cfg.GetVersion() != toVersion { - cfg, err = cfg.Upgrade() - if err != nil { - return nil, fmt.Errorf("transforming skaffold config: %w", err) + var upgraded []util.VersionedConfig + for _, cfg := range configs { + for cfg.GetVersion() != toVersion { + cfg, err = cfg.Upgrade() + if err != nil { + return nil, fmt.Errorf("transforming skaffold config: %w", err) + } } + upgraded = append(upgraded, cfg) } - - return cfg, nil + return upgraded, nil } diff --git a/pkg/skaffold/test/test.go b/pkg/skaffold/test/test.go index f2308bb9a31..73ffa70f8be 100644 --- a/pkg/skaffold/test/test.go +++ b/pkg/skaffold/test/test.go @@ -37,7 +37,7 @@ import ( type Config interface { docker.Config - Pipeline() latest.Pipeline + TestCases() []*latest.TestCase GetWorkingDir() string Muted() config.Muted } @@ -45,14 +45,14 @@ type Config interface { // NewTester parses the provided test cases from the Skaffold config, // and returns a Tester instance with all the necessary test runners // to run all specified tests. -func NewTester(cfg Config, imagesAreLocal bool) Tester { +func NewTester(cfg Config, imagesAreLocal func(imageName string) (bool, error)) Tester { localDaemon, err := docker.NewAPIClient(cfg) if err != nil { return nil } return FullTester{ - testCases: cfg.Pipeline().Test, + testCases: cfg.TestCases(), workingDir: cfg.GetWorkingDir(), muted: cfg.Muted(), localDaemon: localDaemon, @@ -132,7 +132,9 @@ func (t FullTester) runStructureTests(ctx context.Context, out io.Writer, bRes [ return nil } - if !t.imagesAreLocal { + if imageIsLocal, err := t.imagesAreLocal(tc.ImageName); err != nil { + return err + } else if !imageIsLocal { // The image is remote so we have to pull it locally. // `container-structure-test` currently can't do it: // https://github.com/GoogleContainerTools/container-structure-test/issues/253. diff --git a/pkg/skaffold/test/types.go b/pkg/skaffold/test/types.go index a1903ced2ec..ca64b6d129d 100644 --- a/pkg/skaffold/test/types.go +++ b/pkg/skaffold/test/types.go @@ -51,7 +51,7 @@ type FullTester struct { localDaemon docker.LocalDaemon muted Muted workingDir string - imagesAreLocal bool + imagesAreLocal func(imageName string) (bool, error) } // Runner is the lowest-level test executor in Skaffold, responsible for diff --git a/pkg/skaffold/trigger/triggers.go b/pkg/skaffold/trigger/triggers.go index f937e71205c..a5180b18dca 100644 --- a/pkg/skaffold/trigger/triggers.go +++ b/pkg/skaffold/trigger/triggers.go @@ -42,8 +42,8 @@ type Trigger interface { } type Config interface { - Pipeline() latest.Pipeline Trigger() string + Artifacts() []*latest.Artifact WatchPollInterval() int } @@ -68,7 +68,7 @@ func NewTrigger(cfg Config, isActive func() bool) (Trigger, error) { func newFSNotifyTrigger(cfg Config, isActive func() bool) *fsNotifyTrigger { workspaces := map[string]struct{}{} - for _, a := range cfg.Pipeline().Build.Artifacts { + for _, a := range cfg.Artifacts() { workspaces[a.Workspace] = struct{}{} } return &fsNotifyTrigger{ From 49859145e5349e0be242df3f544ad9911e2fd072 Mon Sep 17 00:00:00 2001 From: gsquared94 Date: Mon, 21 Dec 2020 18:05:38 -0800 Subject: [PATCH 2/9] fix tests --- cmd/skaffold/app/cmd/build_test.go | 20 +-- cmd/skaffold/app/cmd/debug_test.go | 4 +- cmd/skaffold/app/cmd/dev_test.go | 8 +- cmd/skaffold/app/cmd/diagnose.go | 2 +- cmd/skaffold/app/cmd/fix.go | 10 +- cmd/skaffold/app/cmd/run_test.go | 6 +- cmd/skaffold/app/cmd/runner.go | 6 +- cmd/skaffold/app/cmd/test.go | 8 +- integration/render_test.go | 24 ++- pkg/skaffold/build/builder_mux.go | 14 +- pkg/skaffold/build/cache/cache.go | 13 +- pkg/skaffold/build/cache/lookup_test.go | 18 +- pkg/skaffold/build/cache/retrieve.go | 18 +- pkg/skaffold/build/cache/retrieve_test.go | 17 +- pkg/skaffold/build/cluster/cluster_test.go | 23 ++- pkg/skaffold/build/cluster/secret_test.go | 42 ++--- pkg/skaffold/build/cluster/types_test.go | 32 ++-- pkg/skaffold/build/gcb/buildpacks_test.go | 8 +- pkg/skaffold/build/gcb/docker_test.go | 22 +-- pkg/skaffold/build/gcb/jib_test.go | 16 +- pkg/skaffold/build/gcb/kaniko_test.go | 14 +- pkg/skaffold/build/gcb/spec_test.go | 11 +- pkg/skaffold/build/local/local.go | 3 + pkg/skaffold/build/local/local_test.go | 127 ++++++-------- pkg/skaffold/build/local/prune_test.go | 19 +- pkg/skaffold/build/scheduler_test.go | 6 +- pkg/skaffold/build/tag/tagger_mux_test.go | 91 ++++++++++ pkg/skaffold/deploy/helm/helm_test.go | 48 ++---- pkg/skaffold/deploy/kpt/kpt_test.go | 37 ++-- pkg/skaffold/deploy/kubectl/kubectl_test.go | 45 ++--- .../deploy/kustomize/kustomize_test.go | 25 +-- .../deploy/status/status_check_test.go | 8 +- pkg/skaffold/event/event_test.go | 44 ++--- pkg/skaffold/event/util.go | 1 - pkg/skaffold/event/util_test.go | 6 +- pkg/skaffold/kubernetes/log.go | 8 +- pkg/skaffold/kubernetes/log_test.go | 20 ++- .../kubernetes/manifest/images_test.go | 1 - .../portforward/entry_manager_test.go | 2 +- .../portforward/pod_forwarder_test.go | 4 +- .../portforward/resource_forwarder_test.go | 4 +- pkg/skaffold/runner/build_deploy_test.go | 29 ++-- pkg/skaffold/runner/deploy_test.go | 5 +- pkg/skaffold/runner/dev_test.go | 66 +++---- pkg/skaffold/runner/new.go | 2 +- pkg/skaffold/runner/new_test.go | 104 ++--------- pkg/skaffold/runner/runcontext/context.go | 118 ++++++++----- pkg/skaffold/runner/runner_test.go | 27 ++- pkg/skaffold/runner/util/util_test.go | 2 +- pkg/skaffold/schema/defaults/defaults_test.go | 26 +-- pkg/skaffold/schema/profiles_test.go | 9 +- pkg/skaffold/schema/samples_test.go | 15 +- pkg/skaffold/schema/validation/validation.go | 3 +- .../schema/validation/validation_test.go | 163 ++++++++++-------- pkg/skaffold/schema/versions.go | 69 ++++---- pkg/skaffold/schema/versions_test.go | 11 +- pkg/skaffold/test/test_test.go | 30 ++-- pkg/skaffold/trigger/triggers_test.go | 10 +- pkg/skaffold/yaml/yaml.go | 26 +++ 59 files changed, 772 insertions(+), 778 deletions(-) create mode 100644 pkg/skaffold/build/tag/tagger_mux_test.go diff --git a/cmd/skaffold/app/cmd/build_test.go b/cmd/skaffold/app/cmd/build_test.go index 747bd9a67f6..4794af63763 100644 --- a/cmd/skaffold/app/cmd/build_test.go +++ b/cmd/skaffold/app/cmd/build_test.go @@ -49,8 +49,8 @@ func (r *mockRunner) Stop() error { } func TestTagFlag(t *testing.T) { - mockCreateRunner := func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { - return &mockRunner{}, &latest.SkaffoldConfig{}, nil + mockCreateRunner := func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { + return &mockRunner{}, []*latest.SkaffoldConfig{{}}, nil } testutil.Run(t, "override tag with argument", func(t *testutil.T) { @@ -68,8 +68,8 @@ func TestTagFlag(t *testing.T) { } func TestQuietFlag(t *testing.T) { - mockCreateRunner := func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { - return &mockRunner{}, &latest.SkaffoldConfig{}, nil + mockCreateRunner := func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { + return &mockRunner{}, []*latest.SkaffoldConfig{{}}, nil } tests := []struct { @@ -114,8 +114,8 @@ func TestQuietFlag(t *testing.T) { } func TestFileOutputFlag(t *testing.T) { - mockCreateRunner := func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { - return &mockRunner{}, &latest.SkaffoldConfig{}, nil + mockCreateRunner := func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { + return &mockRunner{}, []*latest.SkaffoldConfig{{}}, nil } tests := []struct { @@ -177,16 +177,16 @@ func TestFileOutputFlag(t *testing.T) { } func TestRunBuild(t *testing.T) { - errRunner := func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { + errRunner := func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { return nil, nil, errors.New("some error") } - mockCreateRunner := func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { - return &mockRunner{}, &latest.SkaffoldConfig{}, nil + mockCreateRunner := func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { + return &mockRunner{}, []*latest.SkaffoldConfig{{}}, nil } tests := []struct { description string - mock func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) + mock func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) shouldErr bool }{ { diff --git a/cmd/skaffold/app/cmd/debug_test.go b/cmd/skaffold/app/cmd/debug_test.go index e48d3820459..5530f4bf0be 100644 --- a/cmd/skaffold/app/cmd/debug_test.go +++ b/cmd/skaffold/app/cmd/debug_test.go @@ -48,8 +48,8 @@ func TestNewCmdDebug(t *testing.T) { func TestDebugIndependentFromDev(t *testing.T) { mockRunner := &mockDevRunner{} testutil.Run(t, "DevDebug", func(t *testutil.T) { - t.Override(&createRunner, func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { - return mockRunner, &latest.SkaffoldConfig{}, nil + t.Override(&createRunner, func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { + return mockRunner, []*latest.SkaffoldConfig{{}}, nil }) t.Override(&opts, config.SkaffoldOptions{}) t.Override(&doDev, func(context.Context, io.Writer) error { diff --git a/cmd/skaffold/app/cmd/dev_test.go b/cmd/skaffold/app/cmd/dev_test.go index b163702561a..6e603631294 100644 --- a/cmd/skaffold/app/cmd/dev_test.go +++ b/cmd/skaffold/app/cmd/dev_test.go @@ -95,8 +95,8 @@ func TestDoDev(t *testing.T) { hasDeployed: test.hasDeployed, errDev: context.Canceled, } - t.Override(&createRunner, func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { - return mockRunner, &latest.SkaffoldConfig{}, nil + t.Override(&createRunner, func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { + return mockRunner, []*latest.SkaffoldConfig{{}}, nil }) t.Override(&opts, config.SkaffoldOptions{ Cleanup: true, @@ -145,8 +145,8 @@ func TestDevConfigChange(t *testing.T) { testutil.Run(t, "test config change", func(t *testutil.T) { mockRunner := &mockConfigChangeRunner{} - t.Override(&createRunner, func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { - return mockRunner, &latest.SkaffoldConfig{}, nil + t.Override(&createRunner, func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { + return mockRunner, []*latest.SkaffoldConfig{{}}, nil }) t.Override(&opts, config.SkaffoldOptions{ Cleanup: true, diff --git a/cmd/skaffold/app/cmd/diagnose.go b/cmd/skaffold/app/cmd/diagnose.go index 2789e28b3ad..fbb3f884b18 100644 --- a/cmd/skaffold/app/cmd/diagnose.go +++ b/cmd/skaffold/app/cmd/diagnose.go @@ -64,7 +64,7 @@ func doDiagnose(ctx context.Context, out io.Writer) error { color.Blue.Fprintln(out, "\nConfiguration") } } - buf, err := yaml.Marshal(configs) + buf, err := yaml.MarshalWithSeparator(configs) if err != nil { return fmt.Errorf("marshalling configuration: %w", err) } diff --git a/cmd/skaffold/app/cmd/fix.go b/cmd/skaffold/app/cmd/fix.go index d072ea80bc9..a705d088d5d 100644 --- a/cmd/skaffold/app/cmd/fix.go +++ b/cmd/skaffold/app/cmd/fix.go @@ -72,18 +72,18 @@ func fix(out io.Writer, configFile string, toVersion string, overwrite bool) err return err } - var cfgs []*latest.SkaffoldConfig - for _, cfg := range versionedCfgs { - cfgs = append(cfgs, cfg.(*latest.SkaffoldConfig)) - } // TODO(dgageot): We should be able run validations on any schema version // but that's not the case. They can only run on the latest version for now. if toVersion == latest.Version { + var cfgs []*latest.SkaffoldConfig + for _, cfg := range versionedCfgs { + cfgs = append(cfgs, cfg.(*latest.SkaffoldConfig)) + } if err := validation.Process(cfgs); err != nil { return fmt.Errorf("validating upgraded config: %w", err) } } - newCfg, err := yaml.Marshal(cfgs) + newCfg, err := yaml.MarshalWithSeparator(versionedCfgs) if err != nil { return fmt.Errorf("marshaling new config: %w", err) } diff --git a/cmd/skaffold/app/cmd/run_test.go b/cmd/skaffold/app/cmd/run_test.go index e512f15d804..19a025ea3df 100644 --- a/cmd/skaffold/app/cmd/run_test.go +++ b/cmd/skaffold/app/cmd/run_test.go @@ -69,8 +69,8 @@ func (r *mockRunRunner) DeployAndLog(context.Context, io.Writer, []build.Artifac func TestBuildImageFlag(t *testing.T) { testutil.Run(t, "", func(t *testutil.T) { mockRunner := &mockRunRunner{} - t.Override(&createRunner, func(config.SkaffoldOptions) (runner.Runner, *latest.SkaffoldConfig, error) { - return mockRunner, &latest.SkaffoldConfig{ + t.Override(&createRunner, func(config.SkaffoldOptions) (runner.Runner, []*latest.SkaffoldConfig, error) { + return mockRunner, []*latest.SkaffoldConfig{{ Pipeline: latest.Pipeline{ Build: latest.BuildConfig{ Artifacts: []*latest.Artifact{ @@ -81,7 +81,7 @@ func TestBuildImageFlag(t *testing.T) { }, }, }, - }, nil + }}, nil }) t.Override(&opts, config.SkaffoldOptions{ TargetImages: []string{"test"}, diff --git a/cmd/skaffold/app/cmd/runner.go b/cmd/skaffold/app/cmd/runner.go index 269a5e66c50..172cce49427 100644 --- a/cmd/skaffold/app/cmd/runner.go +++ b/cmd/skaffold/app/cmd/runner.go @@ -22,19 +22,19 @@ import ( "fmt" "os" - kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/event" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/instrumentation" + kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/defaults" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/util" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/validation" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/update" ) diff --git a/cmd/skaffold/app/cmd/test.go b/cmd/skaffold/app/cmd/test.go index e117c336cef..3538c94da85 100644 --- a/cmd/skaffold/app/cmd/test.go +++ b/cmd/skaffold/app/cmd/test.go @@ -39,8 +39,12 @@ func NewCmdTest() *cobra.Command { } func doTest(ctx context.Context, out io.Writer) error { - return withRunner(ctx, func(r runner.Runner, config *latest.SkaffoldConfig) error { - buildArtifacts, err := getBuildArtifactsAndSetTags(r, config) + return withRunner(ctx, func(r runner.Runner, configs []*latest.SkaffoldConfig) error { + var artifacts []*latest.Artifact + for _, c := range configs { + artifacts = append(artifacts, c.Build.Artifacts...) + } + buildArtifacts, err := getBuildArtifactsAndSetTags(r, artifacts) if err != nil { tips.PrintForTest(out) return err diff --git a/integration/render_test.go b/integration/render_test.go index e8a7356564f..baa7905fff7 100644 --- a/integration/render_test.go +++ b/integration/render_test.go @@ -79,7 +79,7 @@ spec: Chdir() deployer, err := kubectl.NewDeployer(&runcontext.RunContext{ WorkingDir: ".", - Pipelines: latest.Pipeline{ + Pipelines: runcontext.NewPipelines([]latest.Pipeline{{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ KubectlDeploy: &latest.KubectlDeploy{ @@ -87,8 +87,10 @@ spec: }, }, }, - }, - }, nil) + }}), + }, nil, &latest.KubectlDeploy{ + Manifests: []string{"deployment.yaml"}, + }) t.RequireNoError(err) var b bytes.Buffer err = deployer.Render(context.Background(), &b, test.builds, false, test.renderPath) @@ -234,7 +236,7 @@ spec: deployer, err := kubectl.NewDeployer(&runcontext.RunContext{ WorkingDir: ".", - Pipelines: latest.Pipeline{ + Pipelines: runcontext.NewPipelines([]latest.Pipeline{{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ KubectlDeploy: &latest.KubectlDeploy{ @@ -242,11 +244,13 @@ spec: }, }, }, - }, + }}), Opts: config.SkaffoldOptions{ AddSkaffoldLabels: true, }, - }, nil) + }, nil, &latest.KubectlDeploy{ + Manifests: []string{"deployment.yaml"}, + }) t.RequireNoError(err) var b bytes.Buffer err = deployer.Render(context.Background(), &b, test.builds, false, "") @@ -421,7 +425,7 @@ spec: for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { deployer, err := helm.NewDeployer(&runcontext.RunContext{ - Pipelines: latest.Pipeline{ + Pipelines: runcontext.NewPipelines([]latest.Pipeline{{ Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ HelmDeploy: &latest.HelmDeploy{ @@ -429,8 +433,10 @@ spec: }, }, }, - }, - }, nil) + }}), + }, nil, &latest.HelmDeploy{ + Releases: test.helmReleases, + }) t.RequireNoError(err) var b bytes.Buffer err = deployer.Render(context.Background(), &b, test.builds, true, "") diff --git a/pkg/skaffold/build/builder_mux.go b/pkg/skaffold/build/builder_mux.go index 86068b93b6f..668203e3e06 100644 --- a/pkg/skaffold/build/builder_mux.go +++ b/pkg/skaffold/build/builder_mux.go @@ -22,10 +22,10 @@ import ( "io" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" ) +// BuilderMux encapsulates multiple build configs. type BuilderMux struct { builders []PipelineBuilder byImageName map[string]PipelineBuilder @@ -33,8 +33,14 @@ type BuilderMux struct { concurrency int } -func NewBuilderMux(runCtx *runcontext.RunContext, store ArtifactStore, builder func(p latest.Pipeline) (PipelineBuilder, error)) (Builder, error) { - pipelines := runCtx.GetPipelines() +// Config represents an interface for getting all config pipelines. +type Config interface { + GetPipelines() []latest.Pipeline +} + +// NewBuilderMux returns an implementation of `build.BuilderMux`. +func NewBuilderMux(cfg Config, store ArtifactStore, builder func(p latest.Pipeline) (PipelineBuilder, error)) (Builder, error) { + pipelines := cfg.GetPipelines() m := make(map[string]PipelineBuilder) var sl []PipelineBuilder minConcurrency := -1 @@ -62,6 +68,7 @@ func NewBuilderMux(runCtx *runcontext.RunContext, store ArtifactStore, builder f return &BuilderMux{builders: sl, byImageName: m, store: store, concurrency: minConcurrency}, nil } +// Build executes the specific image builder for each artifact in the given artifact slice. func (b *BuilderMux) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]Artifact, error) { m := make(map[PipelineBuilder]bool) for _, a := range artifacts { @@ -93,6 +100,7 @@ func (b *BuilderMux) Build(ctx context.Context, out io.Writer, tags tag.ImageTag return ar, nil } +// Prune removes built images. func (b *BuilderMux) Prune(ctx context.Context, writer io.Writer) error { for _, builder := range b.builders { if err := builder.Prune(ctx, writer); err != nil { diff --git a/pkg/skaffold/build/cache/cache.go b/pkg/skaffold/build/cache/cache.go index a451b562d30..511bdecc7b5 100644 --- a/pkg/skaffold/build/cache/cache.go +++ b/pkg/skaffold/build/cache/cache.go @@ -63,7 +63,8 @@ type DependencyLister func(ctx context.Context, artifact *latest.Artifact) ([]st type Config interface { docker.Config - Pipeline(imageName string) (*latest.Pipeline, bool) + Pipeline(imageName string) (latest.Pipeline, bool) + GetPipelines() []latest.Pipeline GetCluster() config.Cluster CacheArtifacts() bool CacheFile() string @@ -89,6 +90,16 @@ func NewCache(cfg Config, isLocalImage func(imageName string) (bool, error), dep } client, err := docker.NewAPIClient(cfg) + if err != nil { + // error only if any pipeline is local. + for _, p := range cfg.GetPipelines() { + for _, a := range p.Build.Artifacts { + if local, _ := isLocalImage(a.ImageName); local { + return nil, fmt.Errorf("getting local Docker client: %w", err) + } + } + } + } importMissingImage := func(imageName string) (bool, error) { pipeline, found := cfg.Pipeline(imageName) diff --git a/pkg/skaffold/build/cache/lookup_test.go b/pkg/skaffold/build/cache/lookup_test.go index b4f359fb4a9..bc6fb98d123 100644 --- a/pkg/skaffold/build/cache/lookup_test.go +++ b/pkg/skaffold/build/cache/lookup_test.go @@ -118,10 +118,11 @@ func TestLookupLocal(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { cache := &cache{ - isLocalImage: true, - artifactCache: test.cache, - client: fakeLocalDaemon(test.api), - cfg: &mockConfig{mode: config.RunModes.Build}, + isLocalImage: func(string) (bool, error) { return true, nil }, + importMissingImage: func(imageName string) (bool, error) { return false, nil }, + artifactCache: test.cache, + client: fakeLocalDaemon(test.api), + cfg: &mockConfig{mode: config.RunModes.Build}, } t.Override(&newArtifactHasherFunc, func(_ build.ArtifactGraph, _ DependencyLister, _ config.RunMode) artifactHasher { return test.hasher }) @@ -206,10 +207,11 @@ func TestLookupRemote(t *testing.T) { }) cache := &cache{ - isLocalImage: false, - artifactCache: test.cache, - client: fakeLocalDaemon(test.api), - cfg: &mockConfig{mode: config.RunModes.Build}, + isLocalImage: func(string) (bool, error) { return false, nil }, + importMissingImage: func(imageName string) (bool, error) { return false, nil }, + artifactCache: test.cache, + client: fakeLocalDaemon(test.api), + cfg: &mockConfig{mode: config.RunModes.Build}, } t.Override(&newArtifactHasherFunc, func(_ build.ArtifactGraph, _ DependencyLister, _ config.RunMode) artifactHasher { return test.hasher }) details := cache.lookupArtifacts(context.Background(), map[string]string{"artifact": "tag"}, []*latest.Artifact{{ diff --git a/pkg/skaffold/build/cache/retrieve.go b/pkg/skaffold/build/cache/retrieve.go index 404251f6789..130438a3760 100644 --- a/pkg/skaffold/build/cache/retrieve.go +++ b/pkg/skaffold/build/cache/retrieve.go @@ -81,9 +81,11 @@ func (c *cache) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, ar } default: - if isLocal, err := c.isLocalImage(artifact.ImageName); err != nil { + isLocal, err := c.isLocalImage(artifact.ImageName) + if err != nil { return nil, err - } else if isLocal { + } + if isLocal { color.Green.Fprintln(out, "Found Locally") } else { color.Green.Fprintln(out, "Found Remotely") @@ -97,9 +99,11 @@ func (c *cache) Build(ctx context.Context, out io.Writer, tags tag.ImageTags, ar tag := tags[artifact.ImageName] var uniqueTag string - if isLocal, err := c.isLocalImage(artifact.ImageName); err != nil { + isLocal, err := c.isLocalImage(artifact.ImageName) + if err != nil { return nil, err - } else if isLocal { + } + if isLocal { var err error uniqueTag, err = build.TagWithImageID(ctx, tag, entry.ID, c.client) if err != nil { @@ -153,9 +157,11 @@ func maintainArtifactOrder(built []build.Artifact, artifacts []*latest.Artifact) func (c *cache) addArtifacts(ctx context.Context, bRes []build.Artifact, hashByName map[string]string) error { for _, a := range bRes { entry := ImageDetails{} - if isLocal, err := c.isLocalImage(a.ImageName); err != nil { + isLocal, err := c.isLocalImage(a.ImageName) + if err != nil { return err - } else if isLocal { + } + if isLocal { imageID, err := c.client.ImageID(ctx, a.Tag) if err != nil { return err diff --git a/pkg/skaffold/build/cache/retrieve_test.go b/pkg/skaffold/build/cache/retrieve_test.go index 08d3563a6d4..6f021b5dcc7 100644 --- a/pkg/skaffold/build/cache/retrieve_test.go +++ b/pkg/skaffold/build/cache/retrieve_test.go @@ -137,10 +137,11 @@ func TestCacheBuildLocal(t *testing.T) { // Create cache cfg := &mockConfig{ + pipeline: latest.Pipeline{Build: latest.BuildConfig{BuildType: latest.BuildType{LocalBuild: &latest.LocalBuild{TryImportMissing: false}}}}, cacheFile: tmpDir.Path("cache"), } store := make(mockArtifactStore) - artifactCache, err := NewCache(cfg, true, false, deps, build.ToArtifactGraph(artifacts), store) + artifactCache, err := NewCache(cfg, func(imageName string) (bool, error) { return true, nil }, deps, build.ToArtifactGraph(artifacts), store) t.CheckNoError(err) // First build: Need to build both artifacts @@ -233,9 +234,10 @@ func TestCacheBuildRemote(t *testing.T) { // Create cache cfg := &mockConfig{ + pipeline: latest.Pipeline{Build: latest.BuildConfig{BuildType: latest.BuildType{LocalBuild: &latest.LocalBuild{TryImportMissing: false}}}}, cacheFile: tmpDir.Path("cache"), } - artifactCache, err := NewCache(cfg, false, false, deps, build.ToArtifactGraph(artifacts), make(mockArtifactStore)) + artifactCache, err := NewCache(cfg, func(imageName string) (bool, error) { return false, nil }, deps, build.ToArtifactGraph(artifacts), make(mockArtifactStore)) t.CheckNoError(err) // First build: Need to build both artifacts @@ -317,9 +319,10 @@ func TestCacheFindMissing(t *testing.T) { // Create cache cfg := &mockConfig{ + pipeline: latest.Pipeline{Build: latest.BuildConfig{BuildType: latest.BuildType{LocalBuild: &latest.LocalBuild{TryImportMissing: true}}}}, cacheFile: tmpDir.Path("cache"), } - artifactCache, err := NewCache(cfg, false, true, deps, build.ToArtifactGraph(artifacts), make(mockArtifactStore)) + artifactCache, err := NewCache(cfg, func(imageName string) (bool, error) { return false, nil }, deps, build.ToArtifactGraph(artifacts), make(mockArtifactStore)) t.CheckNoError(err) // Because the artifacts are in the docker registry, we expect them to be imported correctly. @@ -339,8 +342,10 @@ type mockConfig struct { runcontext.RunContext // Embedded to provide the default values. cacheFile string mode config.RunMode + pipeline latest.Pipeline } -func (c *mockConfig) CacheArtifacts() bool { return true } -func (c *mockConfig) CacheFile() string { return c.cacheFile } -func (c *mockConfig) Mode() config.RunMode { return c.mode } +func (c *mockConfig) CacheArtifacts() bool { return true } +func (c *mockConfig) CacheFile() string { return c.cacheFile } +func (c *mockConfig) Mode() config.RunMode { return c.mode } +func (c *mockConfig) Pipeline(string) (latest.Pipeline, bool) { return c.pipeline, true } diff --git a/pkg/skaffold/build/cluster/cluster_test.go b/pkg/skaffold/build/cluster/cluster_test.go index 6cc1302d1b2..7d0d6bf4ea9 100644 --- a/pkg/skaffold/build/cluster/cluster_test.go +++ b/pkg/skaffold/build/cluster/cluster_test.go @@ -27,15 +27,14 @@ func TestRetrieveEnv(t *testing.T) { builder, err := NewBuilder(&mockConfig{ kubeContext: "kubecontext", namespace: "test-namespace", - cluster: latest.ClusterDetails{ - Namespace: "namespace", - PullSecretName: "pullSecret", - DockerConfig: &latest.DockerConfig{ - SecretName: "dockerconfig", - }, - Timeout: "2m", + }, &latest.ClusterDetails{ + Namespace: "namespace", + PullSecretName: "pullSecret", + DockerConfig: &latest.DockerConfig{ + SecretName: "dockerconfig", }, - }, nil) + Timeout: "2m", + }) testutil.CheckError(t, false, err) actual := builder.retrieveExtraEnv() @@ -44,11 +43,9 @@ func TestRetrieveEnv(t *testing.T) { } func TestRetrieveEnvMinimal(t *testing.T) { - builder, err := NewBuilder(&mockConfig{ - cluster: latest.ClusterDetails{ - Timeout: "20m", - }, - }, nil) + builder, err := NewBuilder(&mockConfig{}, &latest.ClusterDetails{ + Timeout: "20m", + }) testutil.CheckError(t, false, err) actual := builder.retrieveExtraEnv() diff --git a/pkg/skaffold/build/cluster/secret_test.go b/pkg/skaffold/build/cluster/secret_test.go index d224ff37d64..5b0bfa55f46 100644 --- a/pkg/skaffold/build/cluster/secret_test.go +++ b/pkg/skaffold/build/cluster/secret_test.go @@ -39,14 +39,12 @@ func TestCreateSecret(t *testing.T) { return fakeKubernetesclient, nil }) - builder, err := NewBuilder(&mockConfig{ - cluster: latest.ClusterDetails{ - Timeout: "20m", - PullSecretName: "kaniko-secret", - PullSecretPath: tmpDir.Path("secret.json"), - Namespace: "ns", - }, - }, nil) + builder, err := NewBuilder(&mockConfig{}, &latest.ClusterDetails{ + Timeout: "20m", + PullSecretName: "kaniko-secret", + PullSecretPath: tmpDir.Path("secret.json"), + Namespace: "ns", + }) t.CheckNoError(err) // Should create a secret @@ -72,12 +70,10 @@ func TestExistingSecretNotFound(t *testing.T) { return fake.NewSimpleClientset(), nil }) - builder, err := NewBuilder(&mockConfig{ - cluster: latest.ClusterDetails{ - Timeout: "20m", - PullSecretName: "kaniko-secret", - }, - }, nil) + builder, err := NewBuilder(&mockConfig{}, &latest.ClusterDetails{ + Timeout: "20m", + PullSecretName: "kaniko-secret", + }) t.CheckNoError(err) // should fail to retrieve an existing secret @@ -97,12 +93,10 @@ func TestExistingSecret(t *testing.T) { }), nil }) - builder, err := NewBuilder(&mockConfig{ - cluster: latest.ClusterDetails{ - Timeout: "20m", - PullSecretName: "kaniko-secret", - }, - }, nil) + builder, err := NewBuilder(&mockConfig{}, &latest.ClusterDetails{ + Timeout: "20m", + PullSecretName: "kaniko-secret", + }) t.CheckNoError(err) // should retrieve an existing secret @@ -119,11 +113,9 @@ func TestSkipSecretCreation(t *testing.T) { return nil, nil }) - builder, err := NewBuilder(&mockConfig{ - cluster: latest.ClusterDetails{ - Timeout: "20m", - }, - }, nil) + builder, err := NewBuilder(&mockConfig{}, &latest.ClusterDetails{ + Timeout: "20m", + }) t.CheckNoError(err) // should retrieve an existing secret diff --git a/pkg/skaffold/build/cluster/types_test.go b/pkg/skaffold/build/cluster/types_test.go index 8e04e6e10fe..cffe5b6d832 100644 --- a/pkg/skaffold/build/cluster/types_test.go +++ b/pkg/skaffold/build/cluster/types_test.go @@ -36,13 +36,13 @@ func TestNewBuilder(t *testing.T) { description string shouldErr bool cfg Config + cluster *latest.ClusterDetails }{ { description: "failed to parse cluster build timeout", - cfg: &mockConfig{ - cluster: latest.ClusterDetails{ - Timeout: "illegal", - }, + cfg: &mockConfig{}, + cluster: &latest.ClusterDetails{ + Timeout: "illegal", }, shouldErr: true, }, @@ -51,10 +51,10 @@ func TestNewBuilder(t *testing.T) { cfg: &mockConfig{ kubeContext: kubeContext, namespace: namespace, - cluster: latest.ClusterDetails{ - Timeout: "100s", - Namespace: "test-ns", - }, + }, + cluster: &latest.ClusterDetails{ + Timeout: "100s", + Namespace: "test-ns", }, }, { @@ -63,16 +63,16 @@ func TestNewBuilder(t *testing.T) { kubeContext: kubeContext, namespace: namespace, insecureRegistries: map[string]bool{"insecure-reg1": true}, - cluster: latest.ClusterDetails{ - Timeout: "100s", - Namespace: "test-ns", - }, + }, + cluster: &latest.ClusterDetails{ + Timeout: "100s", + Namespace: "test-ns", }, }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - _, err := NewBuilder(test.cfg, nil) + _, err := NewBuilder(test.cfg, test.cluster) t.CheckError(test.shouldErr, err) }) @@ -90,15 +90,9 @@ type mockConfig struct { namespace string insecureRegistries map[string]bool runMode config.RunMode - cluster latest.ClusterDetails } func (c *mockConfig) GetKubeContext() string { return c.kubeContext } func (c *mockConfig) GetKubeNamespace() string { return c.namespace } func (c *mockConfig) GetInsecureRegistries() map[string]bool { return c.insecureRegistries } func (c *mockConfig) Mode() config.RunMode { return c.runMode } -func (c *mockConfig) Pipeline() latest.Pipeline { - var pipeline latest.Pipeline - pipeline.Build.BuildType.Cluster = &c.cluster - return pipeline -} diff --git a/pkg/skaffold/build/gcb/buildpacks_test.go b/pkg/skaffold/build/gcb/buildpacks_test.go index 4b0bab1579c..3e5d080fc1b 100644 --- a/pkg/skaffold/build/gcb/buildpacks_test.go +++ b/pkg/skaffold/build/gcb/buildpacks_test.go @@ -190,11 +190,9 @@ func TestBuildpackBuildSpec(t *testing.T) { "img2": "img2:tag", "img3": "img3:tag", } - builder := NewBuilder(&mockConfig{ - gcb: latest.GoogleCloudBuild{ - PackImage: "pack/image", - }, - }, nil) + builder := NewBuilder(&mockConfig{}, &latest.GoogleCloudBuild{ + PackImage: "pack/image", + }) builder.ArtifactStore(store) buildSpec, err := builder.buildSpec(artifact, "img", "bucket", "object") t.CheckError(test.shouldErr, err) diff --git a/pkg/skaffold/build/gcb/docker_test.go b/pkg/skaffold/build/gcb/docker_test.go index dddce0f23fd..e41f11711c4 100644 --- a/pkg/skaffold/build/gcb/docker_test.go +++ b/pkg/skaffold/build/gcb/docker_test.go @@ -143,14 +143,12 @@ func TestDockerBuildSpec(t *testing.T) { } return m, nil }) - builder := NewBuilder(&mockConfig{ - gcb: latest.GoogleCloudBuild{ - DockerImage: "docker/docker", - DiskSizeGb: 100, - MachineType: "n1-standard-1", - Timeout: "10m", - }, - }, nil) + builder := NewBuilder(&mockConfig{}, &latest.GoogleCloudBuild{ + DockerImage: "docker/docker", + DiskSizeGb: 100, + MachineType: "n1-standard-1", + Timeout: "10m", + }) store := mockArtifactStore{ "img2": "img2:tag", "img3": "img3:tag", @@ -175,11 +173,9 @@ func TestPullCacheFrom(t *testing.T) { }, }, } - builder := NewBuilder(&mockConfig{ - gcb: latest.GoogleCloudBuild{ - DockerImage: "docker/docker", - }, - }, nil) + builder := NewBuilder(&mockConfig{}, &latest.GoogleCloudBuild{ + DockerImage: "docker/docker", + }) desc, err := builder.dockerBuildSpec(artifact, "nginx2") expected := []*cloudbuild.BuildStep{{ diff --git a/pkg/skaffold/build/gcb/jib_test.go b/pkg/skaffold/build/gcb/jib_test.go index a01d312485e..806197a2572 100644 --- a/pkg/skaffold/build/gcb/jib_test.go +++ b/pkg/skaffold/build/gcb/jib_test.go @@ -69,11 +69,9 @@ func TestJibMavenBuildSpec(t *testing.T) { }, } - builder := NewBuilder(&mockConfig{ - gcb: latest.GoogleCloudBuild{ - MavenImage: "maven:3.6.0", - }, - }, nil) + builder := NewBuilder(&mockConfig{}, &latest.GoogleCloudBuild{ + MavenImage: "maven:3.6.0", + }) builder.skipTests = test.skipTests store := mockArtifactStore{ "img2": "img2:tag", @@ -120,11 +118,9 @@ func TestJibGradleBuildSpec(t *testing.T) { }, } - builder := NewBuilder(&mockConfig{ - gcb: latest.GoogleCloudBuild{ - GradleImage: "gradle:5.1.1", - }, - }, nil) + builder := NewBuilder(&mockConfig{}, &latest.GoogleCloudBuild{ + GradleImage: "gradle:5.1.1", + }) builder.skipTests = test.skipTests buildSpec, err := builder.buildSpec(artifact, "img", "bucket", "object") diff --git a/pkg/skaffold/build/gcb/kaniko_test.go b/pkg/skaffold/build/gcb/kaniko_test.go index 5133582c390..6db0f1af2d2 100644 --- a/pkg/skaffold/build/gcb/kaniko_test.go +++ b/pkg/skaffold/build/gcb/kaniko_test.go @@ -356,14 +356,12 @@ func TestKanikoBuildSpec(t *testing.T) { }, } - builder := NewBuilder(&mockConfig{ - gcb: latest.GoogleCloudBuild{ - KanikoImage: "gcr.io/kaniko-project/executor", - DiskSizeGb: 100, - MachineType: "n1-standard-1", - Timeout: "10m", - }, - }, nil) + builder := NewBuilder(&mockConfig{}, &latest.GoogleCloudBuild{ + KanikoImage: "gcr.io/kaniko-project/executor", + DiskSizeGb: 100, + MachineType: "n1-standard-1", + Timeout: "10m", + }) defaultExpectedArgs := []string{ "--destination", "gcr.io/nginx", diff --git a/pkg/skaffold/build/gcb/spec_test.go b/pkg/skaffold/build/gcb/spec_test.go index 4a3922f6436..cc38828ce8b 100644 --- a/pkg/skaffold/build/gcb/spec_test.go +++ b/pkg/skaffold/build/gcb/spec_test.go @@ -44,9 +44,7 @@ func TestBuildSpecFail(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - builder := NewBuilder(&mockConfig{ - gcb: latest.GoogleCloudBuild{}, - }, nil) + builder := NewBuilder(&mockConfig{}, &latest.GoogleCloudBuild{}) _, err := builder.buildSpec(test.artifact, "tag", "bucket", "object") @@ -57,11 +55,4 @@ func TestBuildSpecFail(t *testing.T) { type mockConfig struct { runcontext.RunContext // Embedded to provide the default values. - gcb latest.GoogleCloudBuild -} - -func (c *mockConfig) Pipeline() latest.Pipeline { - var pipeline latest.Pipeline - pipeline.Build.BuildType.GoogleCloudBuild = &c.gcb - return pipeline } diff --git a/pkg/skaffold/build/local/local.go b/pkg/skaffold/build/local/local.go index 1df06c44c9e..40365f6dcf8 100644 --- a/pkg/skaffold/build/local/local.go +++ b/pkg/skaffold/build/local/local.go @@ -58,6 +58,9 @@ func (b *Builder) PostBuild(ctx context.Context, _ io.Writer) error { } func (b *Builder) Concurrency() int { + if b.local.Concurrency == nil { + return 0 + } return *b.local.Concurrency } diff --git a/pkg/skaffold/build/local/local_test.go b/pkg/skaffold/build/local/local_test.go index 4bdd1f2d228..c3c017bcce0 100644 --- a/pkg/skaffold/build/local/local_test.go +++ b/pkg/skaffold/build/local/local_test.go @@ -31,7 +31,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/custom" dockerbuilder "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/docker" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/jib" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" @@ -56,9 +55,9 @@ func TestLocalRun(t *testing.T) { tests := []struct { description string api *testutil.FakeAPIClient - tags tag.ImageTags - artifacts []*latest.Artifact - expected []build.Artifact + tag string + artifact *latest.Artifact + expected string expectedWarnings []string expectedPushed map[string]string pushImages bool @@ -66,29 +65,26 @@ func TestLocalRun(t *testing.T) { }{ { description: "single build (local)", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{}, - }}, + }, }, - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), + tag: "gcr.io/test/image:tag", api: &testutil.FakeAPIClient{}, pushImages: false, - expected: []build.Artifact{{ - ImageName: "gcr.io/test/image", - Tag: "gcr.io/test/image:1", - }}, + expected: "gcr.io/test/image:1", }, { description: "error getting image digest", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{}, - }}, + }, }, - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), + tag: "gcr.io/test/image:tag", api: &testutil.FakeAPIClient{ ErrImageInspect: true, }, @@ -96,32 +92,29 @@ func TestLocalRun(t *testing.T) { }, { description: "single build (remote)", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{}, - }}, + }, }, - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), + tag: "gcr.io/test/image:tag", api: &testutil.FakeAPIClient{}, pushImages: true, - expected: []build.Artifact{{ - ImageName: "gcr.io/test/image", - Tag: "gcr.io/test/image:tag@sha256:51ae7fa00c92525c319404a3a6d400e52ff9372c5a39cb415e0486fe425f3165", - }}, + expected: "gcr.io/test/image:tag@sha256:51ae7fa00c92525c319404a3a6d400e52ff9372c5a39cb415e0486fe425f3165", expectedPushed: map[string]string{ "gcr.io/test/image:tag": "sha256:51ae7fa00c92525c319404a3a6d400e52ff9372c5a39cb415e0486fe425f3165", }, }, { description: "error build", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{}, - }}, + }, }, - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), + tag: "gcr.io/test/image:tag", api: &testutil.FakeAPIClient{ ErrImageBuild: true, }, @@ -129,13 +122,13 @@ func TestLocalRun(t *testing.T) { }, { description: "Don't push on build error", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{}, - }}, + }, }, - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), + tag: "gcr.io/test/image:tag", pushImages: true, api: &testutil.FakeAPIClient{ ErrImageBuild: true, @@ -144,89 +137,80 @@ func TestLocalRun(t *testing.T) { }, { description: "unknown artifact type", - artifacts: []*latest.Artifact{{}}, + artifact: &latest.Artifact{}, api: &testutil.FakeAPIClient{}, shouldErr: true, }, { description: "cache-from images already pulled", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{ CacheFrom: []string{"pull1", "pull2"}, }, - }}, + }, }, - api: (&testutil.FakeAPIClient{}).Add("pull1", "imageID1").Add("pull2", "imageID2"), - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), - expected: []build.Artifact{{ - ImageName: "gcr.io/test/image", - Tag: "gcr.io/test/image:1", - }}, + api: (&testutil.FakeAPIClient{}).Add("pull1", "imageID1").Add("pull2", "imageID2"), + tag: "gcr.io/test/image:tag", + expected: "gcr.io/test/image:1", }, { description: "pull cache-from images", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{ CacheFrom: []string{"pull1", "pull2"}, }, - }}, + }, }, - api: (&testutil.FakeAPIClient{}).Add("pull1", "imageid").Add("pull2", "anotherimageid"), - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), - expected: []build.Artifact{{ - ImageName: "gcr.io/test/image", - Tag: "gcr.io/test/image:1", - }}, + api: (&testutil.FakeAPIClient{}).Add("pull1", "imageid").Add("pull2", "anotherimageid"), + tag: "gcr.io/test/image:tag", + expected: "gcr.io/test/image:1", }, { description: "ignore cache-from pull error", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{ CacheFrom: []string{"pull1"}, }, - }}, + }, }, api: (&testutil.FakeAPIClient{ ErrImagePull: true, }).Add("pull1", ""), - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), - expected: []build.Artifact{{ - ImageName: "gcr.io/test/image", - Tag: "gcr.io/test/image:1", - }}, + tag: "gcr.io/test/image:tag", + expected: "gcr.io/test/image:1", expectedWarnings: []string{"cacheFrom image couldn't be pulled: pull1\n"}, }, { description: "error checking cache-from image", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{ CacheFrom: []string{"pull"}, }, - }}, + }, }, api: &testutil.FakeAPIClient{ ErrImageInspect: true, }, - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), + tag: "gcr.io/test/image:tag", shouldErr: true, }, { description: "fail fast docker not found", - artifacts: []*latest.Artifact{{ + artifact: &latest.Artifact{ ImageName: "gcr.io/test/image", ArtifactType: latest.ArtifactType{ DockerArtifact: &latest.DockerArtifact{}, - }}, + }, }, - tags: tag.ImageTags(map[string]string{"gcr.io/test/image": "gcr.io/test/image:tag"}), + tag: "gcr.io/test/image:tag", api: &testutil.FakeAPIClient{ ErrVersion: true, }, @@ -245,24 +229,23 @@ func TestLocalRun(t *testing.T) { t.Override(&docker.EvalBuildArgs, func(_ config.RunMode, _ string, _ string, args map[string]*string, _ map[string]*string) (map[string]*string, error) { return args, nil }) - event.InitializeState(latest.Pipeline{ + event.InitializeState([]latest.Pipeline{{ Deploy: latest.DeployConfig{}, Build: latest.BuildConfig{ BuildType: latest.BuildType{ LocalBuild: &latest.LocalBuild{}, }, - }}, "", true, true, true) + }}}, "", true, true, true) - builder, err := NewBuilder(&mockConfig{ - local: latest.LocalBuild{ + builder, err := NewBuilder(&mockConfig{}, + &latest.LocalBuild{ Push: util.BoolPtr(test.pushImages), Concurrency: &constants.DefaultLocalConcurrency, - }, - }, nil) + }) t.CheckNoError(err) builder.ArtifactStore(build.NewArtifactStore()) - res, err := builder.Build(context.Background(), ioutil.Discard, test.tags, test.artifacts) - + ab := builder.Build(context.Background(), ioutil.Discard, test.artifact) + res, err := ab(context.Background(), ioutil.Discard, test.artifact, test.tag) t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, res) t.CheckDeepEqual(test.expectedWarnings, fakeWarner.Warnings) t.CheckDeepEqual(test.expectedPushed, test.api.Pushed()) @@ -322,7 +305,7 @@ func TestNewBuilder(t *testing.T) { builder, err := NewBuilder(&mockConfig{ local: test.localBuild, cluster: test.cluster, - }, nil) + }, &test.localBuild) t.CheckError(test.shouldErr, err) if !test.shouldErr { @@ -399,11 +382,7 @@ func TestGetArtifactBuilder(t *testing.T) { return args, nil }) - b, err := NewBuilder(&mockConfig{ - local: latest.LocalBuild{ - Concurrency: &constants.DefaultLocalConcurrency, - }, - }, nil) + b, err := NewBuilder(&mockConfig{}, &latest.LocalBuild{Concurrency: &constants.DefaultLocalConcurrency}) t.CheckNoError(err) b.ArtifactStore(build.NewArtifactStore()) @@ -437,12 +416,6 @@ type mockConfig struct { cluster config.Cluster } -func (c *mockConfig) Pipeline() latest.Pipeline { - var pipeline latest.Pipeline - pipeline.Build.BuildType.LocalBuild = &c.local - return pipeline -} - func (c *mockConfig) Mode() config.RunMode { return c.mode } diff --git a/pkg/skaffold/build/local/prune_test.go b/pkg/skaffold/build/local/prune_test.go index c54dc2c8033..657053295fd 100644 --- a/pkg/skaffold/build/local/prune_test.go +++ b/pkg/skaffold/build/local/prune_test.go @@ -21,7 +21,6 @@ import ( "sort" "testing" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/testutil" ) @@ -183,26 +182,10 @@ func TestCollectPruneImages(t *testing.T) { }), true) res := pruner.collectImagesToPrune( - context.Background(), artifacts(test.imagesToBuild...)) + context.Background(), test.imagesToBuild) sort.Strings(test.expectedToPrune) sort.Strings(res) t.CheckDeepEqual(res, test.expectedToPrune) }) } } -func artifacts(images ...string) []*latest.Artifact { - rt := make([]*latest.Artifact, 0) - for _, image := range images { - rt = append(rt, a(image)) - } - return rt -} - -func a(name string) *latest.Artifact { - return &latest.Artifact{ - ImageName: name, - ArtifactType: latest.ArtifactType{ - DockerArtifact: &latest.DockerArtifact{}, - }, - } -} diff --git a/pkg/skaffold/build/scheduler_test.go b/pkg/skaffold/build/scheduler_test.go index 791ac0077ac..f93e637ab73 100644 --- a/pkg/skaffold/build/scheduler_test.go +++ b/pkg/skaffold/build/scheduler_test.go @@ -385,15 +385,15 @@ func setDependencies(a []*latest.Artifact, d map[int][]int) { } func initializeEvents() { - pipe := latest.Pipeline{ + pipes := []latest.Pipeline{{ Deploy: latest.DeployConfig{}, Build: latest.BuildConfig{ BuildType: latest.BuildType{ LocalBuild: &latest.LocalBuild{}, }, }, - } - event.InitializeState(pipe, "temp", true, true, true) + }} + event.InitializeState(pipes, "temp", true, true, true) } func errorsComparer(a, b error) bool { diff --git a/pkg/skaffold/build/tag/tagger_mux_test.go b/pkg/skaffold/build/tag/tagger_mux_test.go new file mode 100644 index 00000000000..5f5d72fec14 --- /dev/null +++ b/pkg/skaffold/build/tag/tagger_mux_test.go @@ -0,0 +1,91 @@ +/* +Copyright 2020 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tag + +import ( + "testing" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestCreateComponents(t *testing.T) { + gitExample, _ := NewGitCommit("", "", false) + envExample, _ := NewEnvTemplateTagger("test") + + tests := []struct { + description string + customTemplateTagger *latest.CustomTemplateTagger + expected map[string]Tagger + shouldErr bool + }{ + { + description: "correct component types", + customTemplateTagger: &latest.CustomTemplateTagger{ + Components: []latest.TaggerComponent{ + {Name: "FOO", Component: latest.TagPolicy{GitTagger: &latest.GitTagger{}}}, + {Name: "FOE", Component: latest.TagPolicy{ShaTagger: &latest.ShaTagger{}}}, + {Name: "BAR", Component: latest.TagPolicy{EnvTemplateTagger: &latest.EnvTemplateTagger{Template: "test"}}}, + {Name: "BAT", Component: latest.TagPolicy{DateTimeTagger: &latest.DateTimeTagger{}}}, + }, + }, + expected: map[string]Tagger{ + "FOO": gitExample, + "FOE": &ChecksumTagger{}, + "BAR": envExample, + "BAT": NewDateTimeTagger("", ""), + }, + }, + { + description: "customTemplate is an invalid component", + customTemplateTagger: &latest.CustomTemplateTagger{ + Components: []latest.TaggerComponent{ + {Name: "FOO", Component: latest.TagPolicy{CustomTemplateTagger: &latest.CustomTemplateTagger{Template: "test"}}}, + }, + }, + shouldErr: true, + }, + { + description: "recurring names", + customTemplateTagger: &latest.CustomTemplateTagger{ + Components: []latest.TaggerComponent{ + {Name: "FOO", Component: latest.TagPolicy{GitTagger: &latest.GitTagger{}}}, + {Name: "FOO", Component: latest.TagPolicy{GitTagger: &latest.GitTagger{}}}, + }, + }, + shouldErr: true, + }, + { + description: "unknown component", + customTemplateTagger: &latest.CustomTemplateTagger{ + Components: []latest.TaggerComponent{ + {Name: "FOO", Component: latest.TagPolicy{}}, + }, + }, + shouldErr: true, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + components, err := CreateComponents(test.customTemplateTagger) + t.CheckErrorAndDeepEqual(test.shouldErr, err, len(test.expected), len(components)) + for k, v := range test.expected { + t.CheckTypeEquality(v, components[k]) + } + }) + } +} diff --git a/pkg/skaffold/deploy/helm/helm_test.go b/pkg/skaffold/deploy/helm/helm_test.go index f2340a4bdb9..2400fdb84b9 100644 --- a/pkg/skaffold/deploy/helm/helm_test.go +++ b/pkg/skaffold/deploy/helm/helm_test.go @@ -445,9 +445,7 @@ func TestNewDeployer(t *testing.T) { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, testutil.CmdRunWithOutput("helm version --client", test.helmVersion)) - _, err := NewDeployer(&helmConfig{ - helm: testDeployConfig, - }, nil) + _, err := NewDeployer(&helmConfig{}, nil, &testDeployConfig) t.CheckError(test.shouldErr, err) }) } @@ -939,11 +937,10 @@ func TestHelmDeploy(t *testing.T) { t.Override(&osExecutable, func() (string, error) { return "SKAFFOLD-BINARY", nil }) deployer, err := NewDeployer(&helmConfig{ - helm: test.helm, namespace: test.namespace, force: test.force, configFile: "test.yaml", - }, nil) + }, nil, &test.helm) t.RequireNoError(err) if test.configure != nil { @@ -1018,9 +1015,8 @@ func TestHelmCleanup(t *testing.T) { t.Override(&util.DefaultExecCommand, test.commands) deployer, err := NewDeployer(&helmConfig{ - helm: test.helm, namespace: test.namespace, - }, nil) + }, nil, &test.helm) t.RequireNoError(err) deployer.Cleanup(context.Background(), ioutil.Discard) @@ -1119,19 +1115,18 @@ func TestHelmDependencies(t *testing.T) { tmpDir := t.NewTempDir(). Touch(test.files...) - deployer, err := NewDeployer(&helmConfig{ - helm: latest.HelmDeploy{ - Releases: []latest.HelmRelease{{ - Name: "skaffold-helm", - ChartPath: tmpDir.Root(), - ValuesFiles: test.valuesFiles, - ArtifactOverrides: map[string]string{"image": "skaffold-helm"}, - Overrides: schemautil.HelmOverrides{Values: map[string]interface{}{"foo": "bar"}}, - SetValues: map[string]string{"some.key": "somevalue"}, - SkipBuildDependencies: test.skipBuildDependencies, - Remote: test.remote, - }}, - }}, nil) + deployer, err := NewDeployer(&helmConfig{}, nil, &latest.HelmDeploy{ + Releases: []latest.HelmRelease{{ + Name: "skaffold-helm", + ChartPath: tmpDir.Root(), + ValuesFiles: test.valuesFiles, + ArtifactOverrides: map[string]string{"image": "skaffold-helm"}, + Overrides: schemautil.HelmOverrides{Values: map[string]interface{}{"foo": "bar"}}, + SetValues: map[string]string{"some.key": "somevalue"}, + SkipBuildDependencies: test.skipBuildDependencies, + Remote: test.remote, + }}, + }) t.RequireNoError(err) deps, err := deployer.Dependencies() @@ -1329,9 +1324,8 @@ func TestHelmRender(t *testing.T) { t.Override(&util.OSEnviron, func() []string { return []string{"FOO=FOOBAR"} }) t.Override(&util.DefaultExecCommand, test.commands) deployer, err := NewDeployer(&helmConfig{ - helm: test.helm, namespace: test.namespace, - }, nil) + }, nil, &test.helm) t.RequireNoError(err) err = deployer.Render(context.Background(), ioutil.Discard, test.builds, true, file) t.CheckError(test.shouldErr, err) @@ -1400,9 +1394,7 @@ func TestGenerateSkaffoldDebugFilter(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, testutil.CmdRunWithOutput("helm version --client", version31)) - h, err := NewDeployer(&helmConfig{ - helm: testDeployConfig, - }, nil) + h, err := NewDeployer(&helmConfig{}, nil, &testDeployConfig) t.RequireNoError(err) result := h.generateSkaffoldDebugFilter(test.buildFile) t.CheckDeepEqual(test.result, result) @@ -1414,7 +1406,6 @@ type helmConfig struct { runcontext.RunContext // Embedded to provide the default values. namespace string force bool - helm latest.HelmDeploy configFile string } @@ -1423,8 +1414,3 @@ func (c *helmConfig) GetKubeConfig() string { return kubectl.TestKubeConfig func (c *helmConfig) GetKubeContext() string { return kubectl.TestKubeContext } func (c *helmConfig) GetKubeNamespace() string { return c.namespace } func (c *helmConfig) ConfigurationFile() string { return c.configFile } -func (c *helmConfig) Pipeline() latest.Pipeline { - var pipeline latest.Pipeline - pipeline.Deploy.DeployType.HelmDeploy = &c.helm - return pipeline -} diff --git a/pkg/skaffold/deploy/kpt/kpt_test.go b/pkg/skaffold/deploy/kpt/kpt_test.go index e13e4167f46..773984ae659 100644 --- a/pkg/skaffold/deploy/kpt/kpt_test.go +++ b/pkg/skaffold/deploy/kpt/kpt_test.go @@ -220,9 +220,7 @@ func TestKpt_Deploy(t *testing.T) { tmpDir.WriteFiles(test.kustomizations) - k := NewDeployer(&kptConfig{ - kpt: test.kpt, - }, nil) + k := NewDeployer(&kptConfig{}, nil, &test.kpt) if k.Live.Apply.Dir == "valid_path" { // 0755 is a permission setting where the owner can read, write, and execute. @@ -362,9 +360,7 @@ func TestKpt_Dependencies(t *testing.T) { tmpDir.WriteFiles(test.createFiles) tmpDir.WriteFiles(test.kustomizations) - k := NewDeployer(&kptConfig{ - kpt: test.kpt, - }, nil) + k := NewDeployer(&kptConfig{}, nil, &test.kpt) res, err := k.Dependencies() @@ -417,14 +413,13 @@ func TestKpt_Cleanup(t *testing.T) { k := NewDeployer(&kptConfig{ workingDir: ".", - kpt: latest.KptDeploy{ - Live: latest.KptLive{ - Apply: latest.KptApplyInventory{ - Dir: test.applyDir, - }, + }, nil, &latest.KptDeploy{ + Live: latest.KptLive{ + Apply: latest.KptApplyInventory{ + Dir: test.applyDir, }, }, - }, nil) + }) err := k.Cleanup(context.Background(), ioutil.Discard) @@ -774,8 +769,7 @@ spec: k := NewDeployer(&kptConfig{ workingDir: ".", - kpt: test.kpt, - }, test.labels) + }, test.labels, &test.kpt) var b bytes.Buffer err := k.Render(context.Background(), &b, test.builds, true, "") @@ -849,10 +843,9 @@ func TestKpt_GetApplyDir(t *testing.T) { k := NewDeployer(&kptConfig{ workingDir: ".", - kpt: latest.KptDeploy{ - Live: test.live, - }, - }, nil) + }, nil, &latest.KptDeploy{ + Live: test.live, + }) applyDir, err := k.getApplyDir(context.Background()) @@ -1009,7 +1002,7 @@ spec: } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - k := NewDeployer(&kptConfig{}, nil) + k := NewDeployer(&kptConfig{}, nil, nil) actualManifest, err := k.excludeKptFn(test.manifests) t.CheckErrorAndDeepEqual(false, err, test.expected.String(), actualManifest.String()) }) @@ -1136,14 +1129,8 @@ func TestVersionCheck(t *testing.T) { type kptConfig struct { runcontext.RunContext // Embedded to provide the default values. workingDir string - kpt latest.KptDeploy } func (c *kptConfig) WorkingDir() string { return c.workingDir } func (c *kptConfig) GetKubeContext() string { return kubectl.TestKubeContext } func (c *kptConfig) GetKubeNamespace() string { return kubectl.TestNamespace } -func (c *kptConfig) Pipeline() latest.Pipeline { - var pipeline latest.Pipeline - pipeline.Deploy.DeployType.KptDeploy = &c.kpt - return pipeline -} diff --git a/pkg/skaffold/deploy/kubectl/kubectl_test.go b/pkg/skaffold/deploy/kubectl/kubectl_test.go index aaa8889f92a..bb53dc7756e 100644 --- a/pkg/skaffold/deploy/kubectl/kubectl_test.go +++ b/pkg/skaffold/deploy/kubectl/kubectl_test.go @@ -233,7 +233,6 @@ func TestKubectlDeploy(t *testing.T) { k, err := NewDeployer(&kubectlConfig{ workingDir: ".", - kubectl: test.kubectl, force: test.forceDeploy, waitForDeletions: config.WaitForDeletions{ Enabled: test.waitForDeletions, @@ -242,7 +241,7 @@ func TestKubectlDeploy(t *testing.T) { }, RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{ Namespace: skaffoldNamespaceOption}}, - }, nil) + }, nil, &test.kubectl) t.RequireNoError(err) _, err = k.Deploy(context.Background(), ioutil.Discard, test.builds) @@ -315,9 +314,8 @@ func TestKubectlCleanup(t *testing.T) { k, err := NewDeployer(&kubectlConfig{ workingDir: ".", - kubectl: test.kubectl, RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{Namespace: TestNamespace}}, - }, nil) + }, nil, &test.kubectl) t.RequireNoError(err) err = k.Cleanup(context.Background(), ioutil.Discard) @@ -363,9 +361,8 @@ func TestKubectlDeployerRemoteCleanup(t *testing.T) { k, err := NewDeployer(&kubectlConfig{ workingDir: ".", - kubectl: test.kubectl, RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{Namespace: TestNamespace}}, - }, nil) + }, nil, &test.kubectl) t.RequireNoError(err) err = k.Cleanup(context.Background(), ioutil.Discard) @@ -395,13 +392,11 @@ func TestKubectlRedeploy(t *testing.T) { deployer, err := NewDeployer(&kubectlConfig{ workingDir: ".", - kubectl: latest.KubectlDeploy{ - Manifests: []string{tmpDir.Path("deployment-app.yaml"), tmpDir.Path("deployment-web.yaml")}}, waitForDeletions: config.WaitForDeletions{ Enabled: true, Delay: 0 * time.Millisecond, Max: 10 * time.Second}, - }, nil) + }, nil, &latest.KubectlDeploy{Manifests: []string{tmpDir.Path("deployment-app.yaml"), tmpDir.Path("deployment-web.yaml")}}) t.RequireNoError(err) // Deploy one manifest @@ -460,15 +455,12 @@ func TestKubectlWaitForDeletions(t *testing.T) { deployer, err := NewDeployer(&kubectlConfig{ workingDir: tmpDir.Root(), - kubectl: latest.KubectlDeploy{ - Manifests: []string{tmpDir.Path("deployment-web.yaml")}, - }, waitForDeletions: config.WaitForDeletions{ Enabled: true, Delay: 0 * time.Millisecond, Max: 10 * time.Second, }, - }, nil) + }, nil, &latest.KubectlDeploy{Manifests: []string{tmpDir.Path("deployment-web.yaml")}}) t.RequireNoError(err) var out bytes.Buffer @@ -500,15 +492,12 @@ func TestKubectlWaitForDeletionsFails(t *testing.T) { deployer, err := NewDeployer(&kubectlConfig{ workingDir: tmpDir.Root(), - kubectl: latest.KubectlDeploy{ - Manifests: []string{tmpDir.Path("deployment-web.yaml")}, - }, waitForDeletions: config.WaitForDeletions{ Enabled: true, Delay: 10 * time.Second, Max: 100 * time.Millisecond, }, - }, nil) + }, nil, &latest.KubectlDeploy{Manifests: []string{tmpDir.Path("deployment-web.yaml")}}) t.RequireNoError(err) _, err = deployer.Deploy(context.Background(), ioutil.Discard, []build.Artifact{ @@ -569,11 +558,7 @@ func TestDependencies(t *testing.T) { Touch("00/b.yaml", "00/a.yaml"). Chdir() - k, err := NewDeployer(&kubectlConfig{ - kubectl: latest.KubectlDeploy{ - Manifests: test.manifests, - }, - }, nil) + k, err := NewDeployer(&kubectlConfig{}, nil, &latest.KubectlDeploy{Manifests: test.manifests}) t.RequireNoError(err) dependencies, err := k.Dependencies() @@ -689,10 +674,9 @@ spec: deployer, err := NewDeployer(&kubectlConfig{ workingDir: ".", defaultRepo: "gcr.io/project", - kubectl: latest.KubectlDeploy{ - Manifests: []string{tmpDir.Path("deployment.yaml")}, - }, - }, nil) + }, nil, &latest.KubectlDeploy{ + Manifests: []string{tmpDir.Path("deployment.yaml")}, + }) t.RequireNoError(err) var b bytes.Buffer err = deployer.Render(context.Background(), &b, test.builds, true, "") @@ -733,10 +717,9 @@ func TestGCSManifests(t *testing.T) { } k, err := NewDeployer(&kubectlConfig{ workingDir: ".", - kubectl: test.kubectl, skipRender: test.skipRender, RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{Namespace: TestNamespace}}, - }, nil) + }, nil, &test.kubectl) t.RequireNoError(err) _, err = k.Deploy(context.Background(), ioutil.Discard, nil) @@ -753,7 +736,6 @@ type kubectlConfig struct { skipRender bool force bool waitForDeletions config.WaitForDeletions - kubectl latest.KubectlDeploy } func (c *kubectlConfig) GetKubeContext() string { return "kubecontext" } @@ -763,8 +745,3 @@ func (c *kubectlConfig) SkipRender() bool { return c.sk func (c *kubectlConfig) ForceDeploy() bool { return c.force } func (c *kubectlConfig) DefaultRepo() *string { return &c.defaultRepo } func (c *kubectlConfig) WaitForDeletions() config.WaitForDeletions { return c.waitForDeletions } -func (c *kubectlConfig) Pipeline() latest.Pipeline { - var pipeline latest.Pipeline - pipeline.Deploy.DeployType.KubectlDeploy = &c.kubectl - return pipeline -} diff --git a/pkg/skaffold/deploy/kustomize/kustomize_test.go b/pkg/skaffold/deploy/kustomize/kustomize_test.go index cbe9db7c191..0104393421f 100644 --- a/pkg/skaffold/deploy/kustomize/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize/kustomize_test.go @@ -184,10 +184,9 @@ func TestKustomizeDeploy(t *testing.T) { Delay: 0 * time.Second, Max: 10 * time.Second, }, - kustomize: test.kustomize, RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{ Namespace: skaffoldNamespaceOption, - }}}, nil) + }}}, nil, &test.kustomize) t.RequireNoError(err) _, err = k.Deploy(context.Background(), ioutil.Discard, test.builds) @@ -251,10 +250,9 @@ func TestKustomizeCleanup(t *testing.T) { k, err := NewDeployer(&kustomizeConfig{ workingDir: tmpDir.Root(), - kustomize: test.kustomize, RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{ Namespace: kubectl.TestNamespace}}, - }, nil) + }, nil, &test.kustomize) t.RequireNoError(err) err = k.Cleanup(context.Background(), ioutil.Discard) @@ -456,11 +454,7 @@ func TestDependenciesForKustomization(t *testing.T) { tmpDir.Write(path, contents) } - k, err := NewDeployer(&kustomizeConfig{ - kustomize: latest.KustomizeDeploy{ - KustomizePaths: kustomizePaths, - }, - }, nil) + k, err := NewDeployer(&kustomizeConfig{}, nil, &latest.KustomizeDeploy{KustomizePaths: kustomizePaths}) t.RequireNoError(err) deps, err := k.Dependencies() @@ -703,11 +697,10 @@ spec: k, err := NewDeployer(&kustomizeConfig{ workingDir: ".", - kustomize: latest.KustomizeDeploy{ - KustomizePaths: kustomizationPaths, - }, RunContext: runcontext.RunContext{Opts: config.SkaffoldOptions{Namespace: kubectl.TestNamespace}}, - }, test.labels) + }, test.labels, &latest.KustomizeDeploy{ + KustomizePaths: kustomizationPaths, + }) t.RequireNoError(err) var b bytes.Buffer @@ -723,7 +716,6 @@ type kustomizeConfig struct { force bool workingDir string waitForDeletions config.WaitForDeletions - kustomize latest.KustomizeDeploy } func (c *kustomizeConfig) ForceDeploy() bool { return c.force } @@ -731,8 +723,3 @@ func (c *kustomizeConfig) WaitForDeletions() config.WaitForDeletions { return c. func (c *kustomizeConfig) WorkingDir() string { return c.workingDir } func (c *kustomizeConfig) GetKubeContext() string { return kubectl.TestKubeContext } func (c *kustomizeConfig) GetKubeNamespace() string { return c.Opts.Namespace } -func (c *kustomizeConfig) Pipeline() latest.Pipeline { - var pipeline latest.Pipeline - pipeline.Deploy.DeployType.KustomizeDeploy = &c.kustomize - return pipeline -} diff --git a/pkg/skaffold/deploy/status/status_check_test.go b/pkg/skaffold/deploy/status/status_check_test.go index 77761a9c84f..5cd8d0c674b 100644 --- a/pkg/skaffold/deploy/status/status_check_test.go +++ b/pkg/skaffold/deploy/status/status_check_test.go @@ -293,7 +293,7 @@ func TestGetDeployStatus(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - event.InitializeState(latest.Pipeline{}, "test", true, true, true) + event.InitializeState([]latest.Pipeline{{}}, "test", true, true, true) errCode, err := getSkaffoldDeployStatus(test.counter, test.deployments) t.CheckError(test.shouldErr, err) if test.shouldErr { @@ -370,7 +370,7 @@ func TestPrintSummaryStatus(t *testing.T) { out := new(bytes.Buffer) rc := newCounter(10) rc.pending = test.pending - event.InitializeState(latest.Pipeline{}, "test", true, true, true) + event.InitializeState([]latest.Pipeline{{}}, "test", true, true, true) r := withStatus(resource.NewDeployment(test.deployment, test.namespace, 0), test.ae) // report status once and set it changed to false. r.ReportSinceLastUpdated(false) @@ -460,7 +460,7 @@ func TestPrintStatus(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { out := new(bytes.Buffer) - event.InitializeState(latest.Pipeline{}, "test", true, true, true) + event.InitializeState([]latest.Pipeline{{}}, "test", true, true, true) checker := statusChecker{labeller: labeller} actual := checker.printStatus(test.rs, out) t.CheckDeepEqual(test.expectedOut, out.String()) @@ -591,7 +591,7 @@ func TestPollDeployment(t *testing.T) { testutil.Run(t, test.description, func(t *testutil.T) { t.Override(&util.DefaultExecCommand, test.command) t.Override(&defaultPollPeriodInMilliseconds, 100) - event.InitializeState(latest.Pipeline{}, "test", true, true, true) + event.InitializeState([]latest.Pipeline{{}}, "test", true, true, true) mockVal := mockValidator{runs: test.runs} dep := test.dep.WithValidator(mockVal) diff --git a/pkg/skaffold/event/event_test.go b/pkg/skaffold/event/event_test.go index 9c548ab0336..c4f29684ad7 100644 --- a/pkg/skaffold/event/event_test.go +++ b/pkg/skaffold/event/event_test.go @@ -67,7 +67,7 @@ func TestGetLogEvents(t *testing.T) { func TestGetState(t *testing.T) { ev := newHandler() - ev.state = emptyState(latest.Pipeline{}, "test", true, true, true) + ev.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) ev.stateLock.Lock() ev.state.BuildState.Artifacts["img"] = Complete @@ -82,7 +82,7 @@ func TestDeployInProgress(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().DeployState.Status == NotStarted }) DeployInProgress() @@ -93,7 +93,7 @@ func TestDeployFailed(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().DeployState.Status == NotStarted }) DeployFailed(errors.New("BUG")) @@ -107,7 +107,7 @@ func TestDeployComplete(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().DeployState.Status == NotStarted }) DeployComplete() @@ -121,11 +121,11 @@ func TestBuildInProgress(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{Build: latest.BuildConfig{ + handler.state = emptyState([]latest.Pipeline{{Build: latest.BuildConfig{ Artifacts: []*latest.Artifact{{ ImageName: "img", }}, - }}, "test", true, true, true) + }}}, "test", true, true, true) wait(t, func() bool { return handler.getState().BuildState.Artifacts["img"] == NotStarted }) BuildInProgress("img") @@ -136,11 +136,11 @@ func TestBuildFailed(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{Build: latest.BuildConfig{ + handler.state = emptyState([]latest.Pipeline{{Build: latest.BuildConfig{ Artifacts: []*latest.Artifact{{ ImageName: "img", }}, - }}, "test", true, true, true) + }}}, "test", true, true, true) wait(t, func() bool { return handler.getState().BuildState.Artifacts["img"] == NotStarted }) BuildFailed("img", errors.New("BUG")) @@ -154,11 +154,11 @@ func TestBuildComplete(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{Build: latest.BuildConfig{ + handler.state = emptyState([]latest.Pipeline{{Build: latest.BuildConfig{ Artifacts: []*latest.Artifact{{ ImageName: "img", }}, - }}, "test", true, true, true) + }}}, "test", true, true, true) wait(t, func() bool { return handler.getState().BuildState.Artifacts["img"] == NotStarted }) BuildComplete("img") @@ -169,7 +169,7 @@ func TestPortForwarded(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().ForwardedPorts[8080] == nil }) PortForwarded(8080, schemautil.FromInt(8888), "pod", "container", "ns", "portname", "resourceType", "resourceName", "127.0.0.1") @@ -186,7 +186,7 @@ func TestStatusCheckEventStarted(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().StatusCheckState.Status == NotStarted }) StatusCheckEventStarted() @@ -197,7 +197,7 @@ func TestStatusCheckEventInProgress(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().StatusCheckState.Status == NotStarted }) StatusCheckEventInProgress("[2/5 deployment(s) are still pending]") @@ -208,7 +208,7 @@ func TestStatusCheckEventSucceeded(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().StatusCheckState.Status == NotStarted }) statusCheckEventSucceeded() @@ -219,7 +219,7 @@ func TestStatusCheckEventFailed(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().StatusCheckState.Status == NotStarted }) StatusCheckEventEnded(proto.StatusCode_STATUSCHECK_FAILED_SCHEDULING, errors.New("one or more deployments failed")) @@ -233,7 +233,7 @@ func TestResourceStatusCheckEventUpdated(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().StatusCheckState.Status == NotStarted }) ResourceStatusCheckEventUpdated("ns:pod/foo", proto.ActionableErr{ @@ -247,7 +247,7 @@ func TestResourceStatusCheckEventSucceeded(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().StatusCheckState.Status == NotStarted }) resourceStatusCheckEventSucceeded("ns:pod/foo") @@ -258,7 +258,7 @@ func TestResourceStatusCheckEventFailed(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().StatusCheckState.Status == NotStarted }) resourceStatusCheckEventFailed("ns:pod/foo", proto.ActionableErr{ @@ -272,7 +272,7 @@ func TestFileSyncInProgress(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().FileSyncState.Status == NotStarted }) FileSyncInProgress(5, "image") @@ -283,7 +283,7 @@ func TestFileSyncFailed(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().FileSyncState.Status == NotStarted }) FileSyncFailed(5, "image", errors.New("BUG")) @@ -294,7 +294,7 @@ func TestFileSyncSucceeded(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) wait(t, func() bool { return handler.getState().FileSyncState.Status == NotStarted }) FileSyncSucceeded(5, "image") @@ -305,7 +305,7 @@ func TestDebuggingContainer(t *testing.T) { defer func() { handler = newHandler() }() handler = newHandler() - handler.state = emptyState(latest.Pipeline{}, "test", true, true, true) + handler.state = emptyState([]latest.Pipeline{{}}, "test", true, true, true) found := func() bool { for _, dc := range handler.getState().DebuggingContainers { diff --git a/pkg/skaffold/event/util.go b/pkg/skaffold/event/util.go index da0bb6a9c13..4b0912eb46a 100644 --- a/pkg/skaffold/event/util.go +++ b/pkg/skaffold/event/util.go @@ -24,7 +24,6 @@ import ( ) func initializeMetadata(pipelines []latest.Pipeline, kubeContext string) *proto.Metadata { - artifactCount := 0 for _, p := range pipelines { artifactCount += len(p.Build.Artifacts) diff --git a/pkg/skaffold/event/util_test.go b/pkg/skaffold/event/util_test.go index b8b89afa598..37fba466724 100644 --- a/pkg/skaffold/event/util_test.go +++ b/pkg/skaffold/event/util_test.go @@ -124,9 +124,7 @@ func TestEmptyState(t *testing.T) { }, cluster: "some-private", expected: &proto.Metadata{ - Build: &proto.BuildMetadata{ - Builders: []*proto.BuildMetadata_ImageBuilder{}, - }, + Build: &proto.BuildMetadata{}, Deploy: &proto.DeployMetadata{ Cluster: proto.ClusterType_OTHER, Deployers: []*proto.DeployMetadata_Deployer{{Type: proto.DeployerType_KUSTOMIZE, Count: 1}}, @@ -137,7 +135,7 @@ func TestEmptyState(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { handler = &eventHandler{ - state: emptyState(test.cfg, test.cluster, true, true, true), + state: emptyState([]latest.Pipeline{test.cfg}, test.cluster, true, true, true), } metadata := handler.state.Metadata builders := metadata.Build.Builders diff --git a/pkg/skaffold/kubernetes/log.go b/pkg/skaffold/kubernetes/log.go index 98a53cfa228..361c4a45e09 100644 --- a/pkg/skaffold/kubernetes/log.go +++ b/pkg/skaffold/kubernetes/log.go @@ -49,8 +49,8 @@ type LogAggregator struct { } type Config interface { - Pipeline(imageName string) (*latest.Pipeline, bool) - DefaultPipeline() *latest.Pipeline + Pipeline(imageName string) (latest.Pipeline, bool) + DefaultPipeline() latest.Pipeline } // NewLogAggregator creates a new LogAggregator for a given output. @@ -180,14 +180,14 @@ func (a *LogAggregator) printLogLine(headerColor color.Color, prefix, text strin } func (a *LogAggregator) prefix(pod *v1.Pod, container v1.ContainerStatus) string { - var c *latest.Pipeline + var c latest.Pipeline var present bool for _, container := range pod.Spec.Containers { if c, present = a.config.Pipeline(stripTag(container.Image)); present { break } } - if c == nil { + if !present { c = a.config.DefaultPipeline() } switch c.Deploy.Logs.Prefix { diff --git a/pkg/skaffold/kubernetes/log_test.go b/pkg/skaffold/kubernetes/log_test.go index f7e97677e15..7df7f0ce9cb 100644 --- a/pkg/skaffold/kubernetes/log_test.go +++ b/pkg/skaffold/kubernetes/log_test.go @@ -187,9 +187,9 @@ func TestPrefix(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - logger := NewLogAggregator(nil, nil, nil, nil, nil, latest.LogsConfig{ + logger := NewLogAggregator(nil, nil, nil, nil, nil, &mockConfig{log: latest.LogsConfig{ Prefix: test.prefix, - }) + }}) p := logger.prefix(&test.pod, test.container) @@ -211,3 +211,19 @@ func containerWithName(n string) v1.ContainerStatus { Name: n, } } + +type mockConfig struct { + log latest.LogsConfig +} + +func (c *mockConfig) Pipeline(string) (latest.Pipeline, bool) { + var pipeline latest.Pipeline + pipeline.Deploy.Logs = c.log + return pipeline, true +} + +func (c *mockConfig) DefaultPipeline() latest.Pipeline { + var pipeline latest.Pipeline + pipeline.Deploy.Logs = c.log + return pipeline +} diff --git a/pkg/skaffold/kubernetes/manifest/images_test.go b/pkg/skaffold/kubernetes/manifest/images_test.go index c4dbfc38547..240007b6e8d 100644 --- a/pkg/skaffold/kubernetes/manifest/images_test.go +++ b/pkg/skaffold/kubernetes/manifest/images_test.go @@ -132,7 +132,6 @@ spec: t.CheckDeepEqual(expected.String(), resultManifest.String()) t.CheckDeepEqual([]string{ "Couldn't parse image [not valid]: invalid reference format", - "image [skaffold/unused] is not used by the deployment", }, fakeWarner.Warnings) }) } diff --git a/pkg/skaffold/kubernetes/portforward/entry_manager_test.go b/pkg/skaffold/kubernetes/portforward/entry_manager_test.go index 8ea05790a93..9e1a08211a4 100644 --- a/pkg/skaffold/kubernetes/portforward/entry_manager_test.go +++ b/pkg/skaffold/kubernetes/portforward/entry_manager_test.go @@ -28,7 +28,7 @@ import ( ) func TestStop(t *testing.T) { - event.InitializeState(latest.Pipeline{}, "test", true, true, true) + event.InitializeState([]latest.Pipeline{{}}, "test", true, true, true) pfe1 := newPortForwardEntry(0, latest.PortForwardResource{ Type: constants.Pod, diff --git a/pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go b/pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go index 278735d71fd..3a29e3a9d91 100644 --- a/pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go +++ b/pkg/skaffold/kubernetes/portforward/pod_forwarder_test.go @@ -401,7 +401,7 @@ func TestAutomaticPortForwardPod(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - event.InitializeState(latest.Pipeline{}, "test", true, true, true) + event.InitializeState([]latest.Pipeline{{}}, "test", true, true, true) taken := map[int]struct{}{} t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort("127.0.0.1", taken, test.availablePorts)) t.Override(&topLevelOwnerKey, func(context.Context, metav1.Object, string) string { return "owner" }) @@ -474,7 +474,7 @@ func TestStartPodForwarder(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - event.InitializeState(latest.Pipeline{}, "", true, true, true) + event.InitializeState([]latest.Pipeline{{}}, "", true, true, true) t.Override(&topLevelOwnerKey, func(context.Context, metav1.Object, string) string { return "owner" }) t.Override(&newPodWatcher, func(kubernetes.PodSelector, []string) kubernetes.PodWatcher { return &fakePodWatcher{ diff --git a/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go b/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go index 8bf8c9bf880..d11e55c1e24 100644 --- a/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go +++ b/pkg/skaffold/kubernetes/portforward/resource_forwarder_test.go @@ -121,7 +121,7 @@ func TestStart(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - event.InitializeState(latest.Pipeline{}, "", true, true, true) + event.InitializeState([]latest.Pipeline{{}}, "", true, true, true) t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort("127.0.0.1", map[int]struct{}{}, test.availablePorts)) t.Override(&retrieveServices, func(context.Context, string, []string) ([]*latest.PortForwardResource, error) { return test.resources, nil @@ -263,7 +263,7 @@ func TestUserDefinedResources(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - event.InitializeState(latest.Pipeline{}, "", true, true, true) + event.InitializeState([]latest.Pipeline{{}}, "", true, true, true) t.Override(&retrieveAvailablePort, mockRetrieveAvailablePort("127.0.0.1", map[int]struct{}{}, []int{8080, 9000})) t.Override(&retrieveServices, func(context.Context, string, []string) ([]*latest.PortForwardResource, error) { return []*latest.PortForwardResource{svc}, nil diff --git a/pkg/skaffold/runner/build_deploy_test.go b/pkg/skaffold/runner/build_deploy_test.go index bcf09b21e00..4049465a3c8 100644 --- a/pkg/skaffold/runner/build_deploy_test.go +++ b/pkg/skaffold/runner/build_deploy_test.go @@ -34,6 +34,7 @@ func TestTest(t *testing.T) { tests := []struct { description string testBench *TestBench + cfg []*latest.Artifact artifacts []build.Artifact expectedActions []Actions shouldErr bool @@ -41,6 +42,7 @@ func TestTest(t *testing.T) { { description: "test no error", testBench: &TestBench{}, + cfg: []*latest.Artifact{{ImageName: "img1"}, {ImageName: "img2"}}, artifacts: []build.Artifact{ {ImageName: "img1", Tag: "img1:tag1"}, {ImageName: "img2", Tag: "img2:tag2"}, @@ -58,6 +60,7 @@ func TestTest(t *testing.T) { { description: "missing tag", testBench: &TestBench{}, + cfg: []*latest.Artifact{{ImageName: "image1"}}, artifacts: []build.Artifact{{ImageName: "image1"}}, expectedActions: []Actions{{ Tested: []string{""}, @@ -72,7 +75,7 @@ func TestTest(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - runner := createRunner(t, test.testBench, nil) + runner := createRunner(t, test.testBench, nil, test.cfg) err := runner.Test(context.Background(), ioutil.Discard, test.artifacts) @@ -133,7 +136,7 @@ func TestBuildTestDeploy(t *testing.T) { ImageName: "img", }} - runner := createRunner(t, test.testBench, nil) + runner := createRunner(t, test.testBench, nil, artifacts) bRes, err := runner.BuildAndTest(ctx, ioutil.Discard, artifacts) if err == nil { err = runner.DeployAndLog(ctx, ioutil.Discard, bRes) @@ -147,13 +150,14 @@ func TestBuildTestDeploy(t *testing.T) { func TestBuildAndTestDryRun(t *testing.T) { testutil.Run(t, "", func(t *testutil.T) { testBench := &TestBench{} - runner := createRunner(t, testBench, nil) - runner.runCtx.Opts.DryRun = true - - bRes, err := runner.BuildAndTest(context.Background(), ioutil.Discard, []*latest.Artifact{ + artifacts := []*latest.Artifact{ {ImageName: "img1"}, {ImageName: "img2"}, - }) + } + runner := createRunner(t, testBench, nil, artifacts) + runner.runCtx.Opts.DryRun = true + + bRes, err := runner.BuildAndTest(context.Background(), ioutil.Discard, artifacts) t.CheckNoError(err) t.CheckDeepEqual([]build.Artifact{ @@ -167,13 +171,14 @@ func TestBuildAndTestDryRun(t *testing.T) { func TestBuildAndTestSkipBuild(t *testing.T) { testutil.Run(t, "", func(t *testutil.T) { testBench := &TestBench{} - runner := createRunner(t, testBench, nil) - runner.runCtx.Opts.DigestSource = "none" - - bRes, err := runner.BuildAndTest(context.Background(), ioutil.Discard, []*latest.Artifact{ + artifacts := []*latest.Artifact{ {ImageName: "img1"}, {ImageName: "img2"}, - }) + } + runner := createRunner(t, testBench, nil, artifacts) + runner.runCtx.Opts.DigestSource = "none" + + bRes, err := runner.BuildAndTest(context.Background(), ioutil.Discard, artifacts) t.CheckNoError(err) t.CheckDeepEqual([]build.Artifact{}, bRes) diff --git a/pkg/skaffold/runner/deploy_test.go b/pkg/skaffold/runner/deploy_test.go index 0a0bfa9aa53..91148ab43bf 100644 --- a/pkg/skaffold/runner/deploy_test.go +++ b/pkg/skaffold/runner/deploy_test.go @@ -34,6 +34,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubectl" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/client" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/runcontext" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" "github.com/GoogleContainerTools/skaffold/testutil" ) @@ -72,7 +73,7 @@ func TestDeploy(t *testing.T) { return dummyStatusChecker{} }) - runner := createRunner(t, test.testBench, nil) + runner := createRunner(t, test.testBench, nil, []*latest.Artifact{{ImageName: "img1"}, {ImageName: "img2"}}) runner.runCtx.Opts.StatusCheck = test.statusCheck out := new(bytes.Buffer) @@ -122,7 +123,7 @@ func TestDeployNamespace(t *testing.T) { return dummyStatusChecker{} }) - runner := createRunner(t, test.testBench, nil) + runner := createRunner(t, test.testBench, nil, []*latest.Artifact{{ImageName: "img1"}, {ImageName: "img2"}}) runner.runCtx.Namespaces = test.Namespaces runner.Deploy(context.Background(), ioutil.Discard, []build.Artifact{ diff --git a/pkg/skaffold/runner/dev_test.go b/pkg/skaffold/runner/dev_test.go index 5cd31737b56..fe2968b0585 100644 --- a/pkg/skaffold/runner/dev_test.go +++ b/pkg/skaffold/runner/dev_test.go @@ -137,14 +137,14 @@ func TestDevFailFirstCycle(t *testing.T) { testutil.Run(t, test.description, func(t *testutil.T) { t.SetupFakeKubernetesContext(api.Config{CurrentContext: "cluster1"}) t.Override(&client.Client, mockK8sClient) - + artifacts := []*latest.Artifact{{ + ImageName: "img", + }} // runner := createRunner(t, test.testBench).WithMonitor(test.monitor) - runner := createRunner(t, test.testBench, test.monitor) + runner := createRunner(t, test.testBench, test.monitor, artifacts) test.testBench.firstMonitor = test.monitor.Run - err := runner.Dev(context.Background(), ioutil.Discard, []*latest.Artifact{{ - ImageName: "img", - }}) + err := runner.Dev(context.Background(), ioutil.Discard, artifacts) t.CheckErrorAndDeepEqual(true, err, test.expectedActions, test.testBench.Actions()) }) @@ -269,16 +269,16 @@ func TestDev(t *testing.T) { t.SetupFakeKubernetesContext(api.Config{CurrentContext: "cluster1"}) t.Override(&client.Client, mockK8sClient) test.testBench.cycles = len(test.watchEvents) - + artifacts := []*latest.Artifact{ + {ImageName: "img1"}, + {ImageName: "img2"}, + } runner := createRunner(t, test.testBench, &TestMonitor{ events: test.watchEvents, testBench: test.testBench, - }) + }, artifacts) - err := runner.Dev(context.Background(), ioutil.Discard, []*latest.Artifact{ - {ImageName: "img1"}, - {ImageName: "img2"}, - }) + err := runner.Dev(context.Background(), ioutil.Discard, artifacts) t.CheckNoError(err) t.CheckDeepEqual(test.expectedActions, test.testBench.Actions()) @@ -404,16 +404,16 @@ func TestDev_WithDependencies(t *testing.T) { t.SetupFakeKubernetesContext(api.Config{CurrentContext: "cluster1"}) t.Override(&client.Client, mockK8sClient) test.testBench.cycles = len(test.watchEvents) - + artifacts := []*latest.Artifact{ + {ImageName: "img1"}, + {ImageName: "img2", Dependencies: []*latest.ArtifactDependency{{ImageName: "img1"}}}, + } runner := createRunner(t, test.testBench, &TestMonitor{ events: test.watchEvents, testBench: test.testBench, - }) + }, artifacts) - err := runner.Dev(context.Background(), ioutil.Discard, []*latest.Artifact{ - {ImageName: "img1"}, - {ImageName: "img2", Dependencies: []*latest.ArtifactDependency{{ImageName: "img1"}}}, - }) + err := runner.Dev(context.Background(), ioutil.Discard, artifacts) t.CheckNoError(err) t.CheckDeepEqual(test.expectedActions, test.testBench.Actions()) @@ -494,13 +494,7 @@ func TestDevSync(t *testing.T) { t.Override(&fileSyncSucceeded, func(int, string) { actualFileSyncEventCalls.Succeeded++ }) t.Override(&sync.WorkingDir, func(string, docker.Config) (string, error) { return "/", nil }) test.testBench.cycles = len(test.watchEvents) - - runner := createRunner(t, test.testBench, &TestMonitor{ - events: test.watchEvents, - testBench: test.testBench, - }) - - err := runner.Dev(context.Background(), ioutil.Discard, []*latest.Artifact{ + artifacts := []*latest.Artifact{ { ImageName: "img1", Sync: &latest.Sync{ @@ -510,7 +504,13 @@ func TestDevSync(t *testing.T) { { ImageName: "img2", }, - }) + } + runner := createRunner(t, test.testBench, &TestMonitor{ + events: test.watchEvents, + testBench: test.testBench, + }, artifacts) + + err := runner.Dev(context.Background(), ioutil.Discard, artifacts) t.CheckNoError(err) t.CheckDeepEqual(test.expectedActions, test.testBench.Actions()) @@ -585,13 +585,7 @@ func TestDevSync_WithDependencies(t *testing.T) { t.Override(&fileSyncSucceeded, func(int, string) { actualFileSyncEventCalls.Succeeded++ }) t.Override(&sync.WorkingDir, func(string, docker.Config) (string, error) { return "/", nil }) test.testBench.cycles = len(test.watchEvents) - - runner := createRunner(t, test.testBench, &TestMonitor{ - events: test.watchEvents, - testBench: test.testBench, - }) - - err := runner.Dev(context.Background(), ioutil.Discard, []*latest.Artifact{ + artifacts := []*latest.Artifact{ { ImageName: "img1", Sync: &latest.Sync{ @@ -602,7 +596,13 @@ func TestDevSync_WithDependencies(t *testing.T) { { ImageName: "img2", }, - }) + } + runner := createRunner(t, test.testBench, &TestMonitor{ + events: test.watchEvents, + testBench: test.testBench, + }, artifacts) + + err := runner.Dev(context.Background(), ioutil.Discard, artifacts) t.CheckNoError(err) t.CheckDeepEqual(test.expectedActions, test.testBench.Actions()) diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index 2fce4dfc638..d99a4b4dd94 100644 --- a/pkg/skaffold/runner/new.go +++ b/pkg/skaffold/runner/new.go @@ -20,7 +20,6 @@ import ( "context" "fmt" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/sirupsen/logrus" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" @@ -28,6 +27,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/cluster" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/gcb" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/local" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kpt" diff --git a/pkg/skaffold/runner/new_test.go b/pkg/skaffold/runner/new_test.go index 8111bb27b37..7f66dc5844d 100644 --- a/pkg/skaffold/runner/new_test.go +++ b/pkg/skaffold/runner/new_test.go @@ -20,7 +20,6 @@ import ( "reflect" "testing" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/helm" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kpt" @@ -61,36 +60,24 @@ func TestGetDeployer(tOuter *testing.T) { description: "kubectl deployer", cfg: latest.DeployType{KubectlDeploy: &latest.KubectlDeploy{}}, expected: t.RequireNonNilResult(kubectl.NewDeployer(&runcontext.RunContext{ - Pipelines: latest.Pipeline{ - Deploy: latest.DeployConfig{ - DeployType: latest.DeployType{ - KubectlDeploy: &latest.KubectlDeploy{ - Flags: latest.KubectlFlags{}, - }, - }, - }, - }, - }, nil)).(deploy.Deployer), + Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}), + }, nil, &latest.KubectlDeploy{ + Flags: latest.KubectlFlags{}, + })).(deploy.Deployer), }, { description: "kustomize deployer", cfg: latest.DeployType{KustomizeDeploy: &latest.KustomizeDeploy{}}, expected: t.RequireNonNilResult(kustomize.NewDeployer(&runcontext.RunContext{ - Pipelines: latest.Pipeline{ - Deploy: latest.DeployConfig{ - DeployType: latest.DeployType{ - KustomizeDeploy: &latest.KustomizeDeploy{ - Flags: latest.KubectlFlags{}, - }, - }, - }, - }, - }, nil)).(deploy.Deployer), + Pipelines: runcontext.NewPipelines([]latest.Pipeline{{}}), + }, nil, &latest.KustomizeDeploy{ + Flags: latest.KubectlFlags{}, + })).(deploy.Deployer), }, { description: "kpt deployer", cfg: latest.DeployType{KptDeploy: &latest.KptDeploy{}}, - expected: kpt.NewDeployer(&runcontext.RunContext{}, nil), + expected: kpt.NewDeployer(&runcontext.RunContext{}, nil, &latest.KptDeploy{}), }, { description: "multiple deployers", @@ -101,7 +88,7 @@ func TestGetDeployer(tOuter *testing.T) { helmVersion: `version.BuildInfo{Version:"v3.0.0"}`, expected: deploy.DeployerMux{ &helm.Deployer{}, - kpt.NewDeployer(&runcontext.RunContext{}, nil), + kpt.NewDeployer(&runcontext.RunContext{}, nil, &latest.KptDeploy{}), }, }, } @@ -115,11 +102,11 @@ func TestGetDeployer(tOuter *testing.T) { } deployer, err := getDeployer(&runcontext.RunContext{ - Pipelines: latest.Pipeline{ + Pipelines: runcontext.NewPipelines([]latest.Pipeline{{ Deploy: latest.DeployConfig{ DeployType: test.cfg, }, - }, + }}), }, nil) t.CheckError(test.shouldErr, err) @@ -137,70 +124,3 @@ func TestGetDeployer(tOuter *testing.T) { } }) } - -func TestCreateComponents(t *testing.T) { - gitExample, _ := tag.NewGitCommit("", "", false) - envExample, _ := tag.NewEnvTemplateTagger("test") - - tests := []struct { - description string - customTemplateTagger *latest.CustomTemplateTagger - expected map[string]tag.Tagger - shouldErr bool - }{ - { - description: "correct component types", - customTemplateTagger: &latest.CustomTemplateTagger{ - Components: []latest.TaggerComponent{ - {Name: "FOO", Component: latest.TagPolicy{GitTagger: &latest.GitTagger{}}}, - {Name: "FOE", Component: latest.TagPolicy{ShaTagger: &latest.ShaTagger{}}}, - {Name: "BAR", Component: latest.TagPolicy{EnvTemplateTagger: &latest.EnvTemplateTagger{Template: "test"}}}, - {Name: "BAT", Component: latest.TagPolicy{DateTimeTagger: &latest.DateTimeTagger{}}}, - }, - }, - expected: map[string]tag.Tagger{ - "FOO": gitExample, - "FOE": &tag.ChecksumTagger{}, - "BAR": envExample, - "BAT": tag.NewDateTimeTagger("", ""), - }, - }, - { - description: "customTemplate is an invalid component", - customTemplateTagger: &latest.CustomTemplateTagger{ - Components: []latest.TaggerComponent{ - {Name: "FOO", Component: latest.TagPolicy{CustomTemplateTagger: &latest.CustomTemplateTagger{Template: "test"}}}, - }, - }, - shouldErr: true, - }, - { - description: "recurring names", - customTemplateTagger: &latest.CustomTemplateTagger{ - Components: []latest.TaggerComponent{ - {Name: "FOO", Component: latest.TagPolicy{GitTagger: &latest.GitTagger{}}}, - {Name: "FOO", Component: latest.TagPolicy{GitTagger: &latest.GitTagger{}}}, - }, - }, - shouldErr: true, - }, - { - description: "unknown component", - customTemplateTagger: &latest.CustomTemplateTagger{ - Components: []latest.TaggerComponent{ - {Name: "FOO", Component: latest.TagPolicy{}}, - }, - }, - shouldErr: true, - }, - } - for _, test := range tests { - testutil.Run(t, test.description, func(t *testutil.T) { - components, err := CreateComponents(test.customTemplateTagger) - t.CheckErrorAndDeepEqual(test.shouldErr, err, len(test.expected), len(components)) - for k, v := range test.expected { - t.CheckTypeEquality(v, components[k]) - } - }) - } -} diff --git a/pkg/skaffold/runner/runcontext/context.go b/pkg/skaffold/runner/runcontext/context.go index 06a4df75011..abed331fcc6 100644 --- a/pkg/skaffold/runner/runcontext/context.go +++ b/pkg/skaffold/runner/runcontext/context.go @@ -34,73 +34,111 @@ const ( ) type RunContext struct { - Opts config.SkaffoldOptions - Pipelines []latest.Pipeline - KubeContext string - Namespaces []string - WorkingDir string - InsecureRegistries map[string]bool - Cluster config.Cluster - pipelinesByImageName map[string]*latest.Pipeline -} - -// Pipeline returns the first `latest.Pipeline` that matches the given artifact `imageName`. -func (rc *RunContext) Pipeline(imageName string) (*latest.Pipeline, bool) { - p, found := rc.pipelinesByImageName[imageName] + Opts config.SkaffoldOptions + Pipelines Pipelines + KubeContext string + Namespaces []string + WorkingDir string + InsecureRegistries map[string]bool + Cluster config.Cluster +} + +// Pipelines encapsulates multiple config pipelines +type Pipelines struct { + pipelines []latest.Pipeline + pipelinesByImageName map[string]latest.Pipeline +} + +// All returns all config pipelines. +func (ps Pipelines) All() []latest.Pipeline { + return ps.pipelines +} + +// Default returns the first `latest.Pipeline`. +func (ps Pipelines) Head() latest.Pipeline { + return ps.pipelines[0] //there always exists atleast one pipeline. +} + +// Select returns the first `latest.Pipeline` that matches the given artifact `imageName`. +func (ps Pipelines) Select(imageName string) (latest.Pipeline, bool) { + p, found := ps.pipelinesByImageName[imageName] return p, found } -func (rc *RunContext) PortForwardResources() []*latest.PortForwardResource { +func (ps Pipelines) PortForwardResources() []*latest.PortForwardResource { var pf []*latest.PortForwardResource - for _, p := range rc.Pipelines { + for _, p := range ps.pipelines { pf = append(pf, p.PortForward...) } return pf } -func (rc *RunContext) Artifacts() []*latest.Artifact { +func (ps Pipelines) Artifacts() []*latest.Artifact { var artifacts []*latest.Artifact - for _, p := range rc.Pipelines { - for _, a := range p.Build.Artifacts { - artifacts = append(artifacts, a) - } + for _, p := range ps.pipelines { + artifacts = append(artifacts, p.Build.Artifacts...) } return artifacts } -func (rc *RunContext) Deployers() []latest.DeployType { +func (ps Pipelines) Deployers() []latest.DeployType { var deployers []latest.DeployType - for _, p := range rc.Pipelines { + for _, p := range ps.pipelines { deployers = append(deployers, p.Deploy.DeployType) } return deployers } -func (rc *RunContext) TestCases() []*latest.TestCase { +func (ps Pipelines) TestCases() []*latest.TestCase { var tests []*latest.TestCase - for _, p := range rc.Pipelines { + for _, p := range ps.pipelines { tests = append(tests, p.Test...) } return tests } -func (rc *RunContext) StatusCheckDeadlineSeconds() int { +func (ps Pipelines) StatusCheckDeadlineSeconds() int { c := 0 // set the group status check deadline to maximum of any individually specified value - for _, p := range rc.Pipelines { + for _, p := range ps.pipelines { if p.Deploy.StatusCheckDeadlineSeconds > c { c = p.Deploy.StatusCheckDeadlineSeconds } } return c } +func NewPipelines(pipelines []latest.Pipeline) Pipelines { + m := make(map[string]latest.Pipeline) + for _, p := range pipelines { + for _, a := range p.Build.Artifacts { + m[a.ImageName] = p + } + } + return Pipelines{pipelines: pipelines, pipelinesByImageName: m} +} + +func (rc *RunContext) Pipeline(imageName string) (latest.Pipeline, bool) { + return rc.Pipelines.Select(imageName) +} + +func (rc *RunContext) PortForwardResources() []*latest.PortForwardResource { + return rc.Pipelines.PortForwardResources() +} + +func (rc *RunContext) Artifacts() []*latest.Artifact { return rc.Pipelines.Artifacts() } + +func (rc *RunContext) Deployers() []latest.DeployType { return rc.Pipelines.Deployers() } -// DefaultPipeline returns the first `latest.Pipeline` it finds. -func (rc *RunContext) DefaultPipeline() *latest.Pipeline { return &rc.Pipelines[0] } +func (rc *RunContext) TestCases() []*latest.TestCase { return rc.Pipelines.TestCases() } +func (rc *RunContext) StatusCheckDeadlineSeconds() int { + return rc.Pipelines.StatusCheckDeadlineSeconds() +} + +func (rc *RunContext) DefaultPipeline() latest.Pipeline { return rc.Pipelines.Head() } func (rc *RunContext) GetKubeContext() string { return rc.KubeContext } func (rc *RunContext) GetNamespaces() []string { return rc.Namespaces } -func (rc *RunContext) GetPipelines() []latest.Pipeline { return rc.Pipelines } +func (rc *RunContext) GetPipelines() []latest.Pipeline { return rc.Pipelines.All() } func (rc *RunContext) GetInsecureRegistries() map[string]bool { return rc.InsecureRegistries } func (rc *RunContext) GetWorkingDir() string { return rc.WorkingDir } func (rc *RunContext) GetCluster() config.Cluster { return rc.Cluster } @@ -171,12 +209,7 @@ func GetRunContext(opts config.SkaffoldOptions, pipelines []latest.Pipeline) (*R for _, r := range regList { insecureRegistries[r] = true } - m := make(map[string]*latest.Pipeline) - for _, p := range pipelines { - for _, a := range p.Build.Artifacts { - m[a.ImageName] = &p - } - } + ps := NewPipelines(pipelines) // TODO(https://github.com/GoogleContainerTools/skaffold/issues/3668): // remove minikubeProfile from here and instead detect it by matching the @@ -187,14 +220,13 @@ func GetRunContext(opts config.SkaffoldOptions, pipelines []latest.Pipeline) (*R } return &RunContext{ - Opts: opts, - Pipelines: pipelines, - WorkingDir: cwd, - KubeContext: kubeContext, - Namespaces: namespaces, - InsecureRegistries: insecureRegistries, - Cluster: cluster, - pipelinesByImageName: m, + Opts: opts, + Pipelines: ps, + WorkingDir: cwd, + KubeContext: kubeContext, + Namespaces: namespaces, + InsecureRegistries: insecureRegistries, + Cluster: cluster, }, nil } diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index fbeafaa3397..32de8ec5a01 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -25,9 +25,6 @@ import ( "k8s.io/client-go/tools/clientcmd/api" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/cluster" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/gcb" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/local" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/tag" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/config" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" @@ -206,7 +203,7 @@ func (r *SkaffoldRunner) WithMonitor(m filemon.Monitor) *SkaffoldRunner { return r } -func createRunner(t *testutil.T, testBench *TestBench, monitor filemon.Monitor) *SkaffoldRunner { +func createRunner(t *testutil.T, testBench *TestBench, monitor filemon.Monitor, artifacts []*latest.Artifact) *SkaffoldRunner { cfg := &latest.SkaffoldConfig{ Pipeline: latest.Pipeline{ Build: latest.BuildConfig{ @@ -214,14 +211,15 @@ func createRunner(t *testutil.T, testBench *TestBench, monitor filemon.Monitor) // Use the fastest tagger ShaTagger: &latest.ShaTagger{}, }, + Artifacts: artifacts, }, Deploy: latest.DeployConfig{StatusCheckDeadlineSeconds: 60}, }, } - defaults.Set(cfg) + defaults.Set(cfg, true) runCtx := &runcontext.RunContext{ - Pipelines: cfg.Pipeline, + Pipelines: runcontext.NewPipelines([]latest.Pipeline{cfg.Pipeline}), Opts: config.SkaffoldOptions{ Trigger: "polling", WatchPollInterval: 100, @@ -261,7 +259,7 @@ func TestNewForConfig(t *testing.T) { pipeline latest.Pipeline shouldErr bool cacheArtifacts bool - expectedBuilder build.Builder + expectedBuilder build.BuilderMux expectedTester test.Tester expectedDeployer deploy.Deployer }{ @@ -280,7 +278,6 @@ func TestNewForConfig(t *testing.T) { }, }, }, - expectedBuilder: &local.Builder{}, expectedTester: &test.FullTester{}, expectedDeployer: &kubectl.Deployer{}, }, @@ -299,7 +296,6 @@ func TestNewForConfig(t *testing.T) { }, }, }, - expectedBuilder: &gcb.Builder{}, expectedTester: &test.FullTester{}, expectedDeployer: &kubectl.Deployer{}, }, @@ -318,7 +314,6 @@ func TestNewForConfig(t *testing.T) { }, }, }, - expectedBuilder: &cluster.Builder{}, expectedTester: &test.FullTester{}, expectedDeployer: &kubectl.Deployer{}, }, @@ -343,7 +338,6 @@ func TestNewForConfig(t *testing.T) { description: "unknown builder and tagger", pipeline: latest.Pipeline{}, shouldErr: true, - expectedBuilder: &local.Builder{}, expectedTester: &test.FullTester{}, expectedDeployer: &kubectl.Deployer{}, }, @@ -362,7 +356,6 @@ func TestNewForConfig(t *testing.T) { }, }, }, - expectedBuilder: &local.Builder{}, expectedTester: &test.FullTester{}, expectedDeployer: &kubectl.Deployer{}, cacheArtifacts: true, @@ -384,8 +377,7 @@ func TestNewForConfig(t *testing.T) { }, }, }, - expectedBuilder: &local.Builder{}, - expectedTester: &test.FullTester{}, + expectedTester: &test.FullTester{}, expectedDeployer: deploy.DeployerMux([]deploy.Deployer{ &helm.Deployer{}, &kubectl.Deployer{}, @@ -401,17 +393,18 @@ func TestNewForConfig(t *testing.T) { AndRunWithOutput("kubectl version --client -ojson", "v1.5.6")) runCtx := &runcontext.RunContext{ - Pipelines: test.pipeline, + Pipelines: runcontext.NewPipelines([]latest.Pipeline{test.pipeline}), Opts: config.SkaffoldOptions{ Trigger: "polling", }, } cfg, err := NewForConfig(runCtx) + t.CheckError(test.shouldErr, err) t.CheckError(test.shouldErr, err) if cfg != nil { - b, _t, d := WithTimings(test.expectedBuilder, test.expectedTester, test.expectedDeployer, test.cacheArtifacts) + b, _t, d := WithTimings(&test.expectedBuilder, test.expectedTester, test.expectedDeployer, test.cacheArtifacts) if test.shouldErr { t.CheckError(true, err) @@ -498,7 +491,7 @@ func TestTriggerCallbackAndIntents(t *testing.T) { } r, _ := NewForConfig(&runcontext.RunContext{ Opts: opts, - Pipelines: pipeline, + Pipelines: runcontext.NewPipelines([]latest.Pipeline{pipeline}), }) r.intents.resetBuild() diff --git a/pkg/skaffold/runner/util/util_test.go b/pkg/skaffold/runner/util/util_test.go index 00fed94be4c..28096d262cd 100644 --- a/pkg/skaffold/runner/util/util_test.go +++ b/pkg/skaffold/runner/util/util_test.go @@ -80,7 +80,7 @@ func TestGetAllPodNamespaces(t *testing.T) { }, nil }) - namespaces, err := GetAllPodNamespaces(test.argNamespace, test.cfg) + namespaces, err := GetAllPodNamespaces(test.argNamespace, []latest.Pipeline{test.cfg}) t.CheckNoError(err) t.CheckDeepEqual(test.expected, namespaces) diff --git a/pkg/skaffold/schema/defaults/defaults_test.go b/pkg/skaffold/schema/defaults/defaults_test.go index cf9ee807f2b..60082cb8b01 100644 --- a/pkg/skaffold/schema/defaults/defaults_test.go +++ b/pkg/skaffold/schema/defaults/defaults_test.go @@ -88,7 +88,7 @@ func TestSetDefaults(t *testing.T) { }, } - err := Set(cfg) + err := Set(cfg, true) testutil.CheckError(t, false, err) @@ -173,7 +173,7 @@ func TestSetDefaultsOnCluster(t *testing.T) { }, }, } - err := Set(cfg) + err := Set(cfg, true) t.CheckNoError(err) t.CheckDeepEqual("ns", cfg.Build.Cluster.Namespace) @@ -197,7 +197,7 @@ func TestSetDefaultsOnCluster(t *testing.T) { }, }, } - err = Set(cfg) + err = Set(cfg, true) t.CheckNoError(err) @@ -218,13 +218,13 @@ func TestSetDefaultsOnCluster(t *testing.T) { }, } - err = Set(cfg) + err = Set(cfg, true) t.CheckNoError(err) t.CheckDeepEqual(path, cfg.Build.Cluster.PullSecretMountPath) // default docker config cfg.Pipeline.Build.BuildType.Cluster.DockerConfig = &latest.DockerConfig{} - err = Set(cfg) + err = Set(cfg, true) t.CheckNoError(err) @@ -232,7 +232,7 @@ func TestSetDefaultsOnCluster(t *testing.T) { cfg.Pipeline.Build.BuildType.Cluster.DockerConfig = &latest.DockerConfig{ Path: "/path", } - err = Set(cfg) + err = Set(cfg, true) t.CheckNoError(err) t.CheckDeepEqual("/path", cfg.Build.Cluster.DockerConfig.Path) @@ -241,7 +241,7 @@ func TestSetDefaultsOnCluster(t *testing.T) { cfg.Pipeline.Build.BuildType.Cluster.DockerConfig = &latest.DockerConfig{ SecretName: "secret", } - err = Set(cfg) + err = Set(cfg, true) t.CheckNoError(err) t.CheckDeepEqual("secret", cfg.Build.Cluster.DockerConfig.SecretName) @@ -270,7 +270,7 @@ func TestCustomBuildWithCluster(t *testing.T) { }, } - err := Set(cfg) + err := Set(cfg, true) testutil.CheckError(t, false, err) testutil.CheckDeepEqual(t, (*latest.KanikoArtifact)(nil), cfg.Build.Artifacts[0].KanikoArtifact) @@ -290,7 +290,7 @@ func TestSetDefaultsOnCloudBuild(t *testing.T) { }, } - err := Set(cfg) + err := Set(cfg, true) testutil.CheckError(t, false, err) testutil.CheckDeepEqual(t, defaultCloudBuildDockerImage, cfg.Build.GoogleCloudBuild.DockerImage) @@ -302,7 +302,7 @@ func TestSetDefaultsOnCloudBuild(t *testing.T) { func TestSetDefaultsOnLocalBuild(t *testing.T) { cfg := &latest.SkaffoldConfig{} - err := Set(cfg) + err := Set(cfg, true) testutil.CheckError(t, false, err) testutil.CheckDeepEqual(t, 1, *cfg.Build.LocalBuild.Concurrency) @@ -324,7 +324,7 @@ func TestSetPortForwardLocalPort(t *testing.T) { }, }, } - err := Set(cfg) + err := Set(cfg, true) testutil.CheckError(t, false, err) testutil.CheckDeepEqual(t, 8080, cfg.PortForward[0].LocalPort) testutil.CheckDeepEqual(t, 9000, cfg.PortForward[1].LocalPort) @@ -344,7 +344,7 @@ func TestSetDefaultPortForwardAddress(t *testing.T) { }, }, } - err := Set(cfg) + err := Set(cfg, true) testutil.CheckError(t, false, err) testutil.CheckDeepEqual(t, "0.0.0.0", cfg.PortForward[0].Address) testutil.CheckDeepEqual(t, constants.DefaultPortForwardAddress, cfg.PortForward[1].Address) @@ -383,7 +383,7 @@ func TestSetLogsConfig(t *testing.T) { }, } - err := Set(&cfg) + err := Set(&cfg, true) t.CheckNoError(err) t.CheckDeepEqual(test.expected, cfg.Deploy.Logs) diff --git a/pkg/skaffold/schema/profiles_test.go b/pkg/skaffold/schema/profiles_test.go index 1705458f032..19d30b819b0 100644 --- a/pkg/skaffold/schema/profiles_test.go +++ b/pkg/skaffold/schema/profiles_test.go @@ -65,8 +65,9 @@ profiles: parsed, err := ParseConfig(tmpDir.Path("skaffold.yaml")) t.CheckNoError(err) + t.CheckTrue(len(parsed) > 0) - skaffoldConfig := parsed.(*latest.SkaffoldConfig) + skaffoldConfig := parsed[0].(*latest.SkaffoldConfig) err = ApplyProfiles(skaffoldConfig, cfg.SkaffoldOptions{ Profiles: []string{"patches"}, }) @@ -96,8 +97,9 @@ profiles: parsed, err := ParseConfig(tmp.Path("skaffold.yaml")) t.CheckNoError(err) + t.CheckTrue(len(parsed) > 0) - skaffoldConfig := parsed.(*latest.SkaffoldConfig) + skaffoldConfig := parsed[0].(*latest.SkaffoldConfig) err = ApplyProfiles(skaffoldConfig, cfg.SkaffoldOptions{ Profiles: []string{"patches"}, }) @@ -788,8 +790,9 @@ profiles: parsed, err := ParseConfig(tmpDir.Path("skaffold.yaml")) t.RequireNoError(err) + t.CheckTrue(len(parsed) > 0) - skaffoldConfig := parsed.(*latest.SkaffoldConfig) + skaffoldConfig := parsed[0].(*latest.SkaffoldConfig) t.CheckDeepEqual(2, len(skaffoldConfig.Profiles)) t.CheckDeepEqual("simple1", skaffoldConfig.Profiles[0].Name) diff --git a/pkg/skaffold/schema/samples_test.go b/pkg/skaffold/schema/samples_test.go index d5985c21d0b..54b947980bd 100644 --- a/pkg/skaffold/schema/samples_test.go +++ b/pkg/skaffold/schema/samples_test.go @@ -75,13 +75,16 @@ func TestParseSamples(t *testing.T) { func checkSkaffoldConfig(t *testutil.T, yaml []byte) { configFile := t.TempFile("skaffold.yaml", yaml) - cfg, err := ParseConfigAndUpgrade(configFile, latest.Version) + parsed, err := ParseConfigAndUpgrade(configFile, latest.Version) t.CheckNoError(err) - - err = defaults.Set(cfg.(*latest.SkaffoldConfig)) - t.CheckNoError(err) - - err = validation.Process(cfg.(*latest.SkaffoldConfig)) + var cfgs []*latest.SkaffoldConfig + for _, p := range parsed { + cfg := p.(*latest.SkaffoldConfig) + err = defaults.Set(cfg, true) + t.CheckNoError(err) + cfgs = append(cfgs, cfg) + } + err = validation.Process(cfgs) t.CheckNoError(err) } diff --git a/pkg/skaffold/schema/validation/validation.go b/pkg/skaffold/schema/validation/validation.go index cd2095b72e7..ea4015d2408 100644 --- a/pkg/skaffold/schema/validation/validation.go +++ b/pkg/skaffold/schema/validation/validation.go @@ -24,6 +24,8 @@ import ( "strings" "time" + "github.com/docker/docker/api/types" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build/misc" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/docker" sErrors "github.com/GoogleContainerTools/skaffold/pkg/skaffold/errors" @@ -32,7 +34,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/util" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/yamltags" "github.com/GoogleContainerTools/skaffold/proto" - "github.com/docker/docker/api/types" ) var ( diff --git a/pkg/skaffold/schema/validation/validation_test.go b/pkg/skaffold/schema/validation/validation_test.go index 0db46e54eb4..a19febb0aec 100644 --- a/pkg/skaffold/schema/validation/validation_test.go +++ b/pkg/skaffold/schema/validation/validation_test.go @@ -87,7 +87,7 @@ func TestValidateSchema(t *testing.T) { } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { - err := Process(test.cfg) + err := Process([]*latest.SkaffoldConfig{test.cfg}) t.CheckError(test.shouldErr, err) }) @@ -427,13 +427,13 @@ func TestValidateNetworkMode(t *testing.T) { t.Override(&validateYamltags, func(interface{}) error { return nil }) err := Process( - &latest.SkaffoldConfig{ + []*latest.SkaffoldConfig{{ Pipeline: latest.Pipeline{ Build: latest.BuildConfig{ Artifacts: test.artifacts, }, }, - }) + }}) t.CheckError(test.shouldErr, err) }) @@ -543,14 +543,15 @@ func TestValidateNetworkModeDockerContainerExists(t *testing.T) { return docker.NewLocalDaemon(fakeClient, nil, false, nil), nil }) - err := ProcessWithRunContext( - &latest.SkaffoldConfig{ - Pipeline: latest.Pipeline{ + err := ProcessWithRunContext(&runcontext.RunContext{ + Pipelines: runcontext.NewPipelines([]latest.Pipeline{ + { Build: latest.BuildConfig{ Artifacts: test.artifacts, }, }, - }, &runcontext.RunContext{}) + }), + }) t.CheckError(test.shouldErr, err) }) @@ -648,13 +649,13 @@ func TestValidateSyncRules(t *testing.T) { t.Override(&validateYamltags, func(interface{}) error { return nil }) err := Process( - &latest.SkaffoldConfig{ + []*latest.SkaffoldConfig{{ Pipeline: latest.Pipeline{ Build: latest.BuildConfig{ Artifacts: test.artifacts, }, }, - }) + }}) t.CheckError(test.shouldErr, err) }) @@ -793,13 +794,13 @@ func TestValidateImageNames(t *testing.T) { t.Override(&validateYamltags, func(interface{}) error { return nil }) err := Process( - &latest.SkaffoldConfig{ + []*latest.SkaffoldConfig{{ Pipeline: latest.Pipeline{ Build: latest.BuildConfig{ Artifacts: test.artifacts, }, }, - }) + }}) t.CheckError(test.shouldErr, err) }) @@ -896,13 +897,13 @@ func TestValidateJibPluginType(t *testing.T) { t.Override(&validateYamltags, func(interface{}) error { return nil }) err := Process( - &latest.SkaffoldConfig{ + []*latest.SkaffoldConfig{{ Pipeline: latest.Pipeline{ Build: latest.BuildConfig{ Artifacts: test.artifacts, }, }, - }) + }}) t.CheckError(test.shouldErr, err) }) @@ -928,7 +929,7 @@ func TestValidateLogsConfig(t *testing.T) { t.Override(&validateYamltags, func(interface{}) error { return nil }) err := Process( - &latest.SkaffoldConfig{ + []*latest.SkaffoldConfig{{ Pipeline: latest.Pipeline{ Deploy: latest.DeployConfig{ Logs: latest.LogsConfig{ @@ -936,7 +937,7 @@ func TestValidateLogsConfig(t *testing.T) { }, }, }, - }) + }}) t.CheckError(test.shouldErr, err) }) @@ -1039,19 +1040,27 @@ func setDependencies(a []*latest.Artifact, d map[int][]int) { } func TestValidateUniqueDependencyAliases(t *testing.T) { - artifacts := []*latest.Artifact{ - { - ImageName: "artifact1", - Dependencies: []*latest.ArtifactDependency{ - {Alias: "alias2", ImageName: "artifact2a"}, - {Alias: "alias2", ImageName: "artifact2b"}, - }, - }, - { - ImageName: "artifact2", - Dependencies: []*latest.ArtifactDependency{ - {Alias: "alias1", ImageName: "artifact1"}, - {Alias: "alias2", ImageName: "artifact1"}, + cfgs := []*latest.SkaffoldConfig{ + { + Pipeline: latest.Pipeline{ + Build: latest.BuildConfig{ + Artifacts: []*latest.Artifact{ + { + ImageName: "artifact1", + Dependencies: []*latest.ArtifactDependency{ + {Alias: "alias2", ImageName: "artifact2a"}, + {Alias: "alias2", ImageName: "artifact2b"}, + }, + }, + { + ImageName: "artifact2", + Dependencies: []*latest.ArtifactDependency{ + {Alias: "alias1", ImageName: "artifact1"}, + {Alias: "alias2", ImageName: "artifact1"}, + }, + }, + }, + }, }, }, } @@ -1059,53 +1068,61 @@ func TestValidateUniqueDependencyAliases(t *testing.T) { fmt.Errorf(`invalid build dependency for artifact "artifact1": alias "alias2" repeated`), fmt.Errorf(`unknown build dependency "artifact2a" for artifact "artifact1"`), } - errs := validateArtifactDependencies(artifacts) + errs := validateArtifactDependencies(cfgs) testutil.CheckDeepEqual(t, expected, errs, cmp.Comparer(errorsComparer)) } func TestValidateValidDependencyAliases(t *testing.T) { - artifacts := []*latest.Artifact{ - { - ImageName: "artifact1", - }, + cfgs := []*latest.SkaffoldConfig{ { - ImageName: "artifact2", - ArtifactType: latest.ArtifactType{ - DockerArtifact: &latest.DockerArtifact{}, - }, - Dependencies: []*latest.ArtifactDependency{ - {Alias: "ARTIFACT_1", ImageName: "artifact1"}, - {Alias: "1_ARTIFACT", ImageName: "artifact1"}, - }, - }, - { - ImageName: "artifact3", - ArtifactType: latest.ArtifactType{ - DockerArtifact: &latest.DockerArtifact{}, - }, - Dependencies: []*latest.ArtifactDependency{ - {Alias: "artifact!", ImageName: "artifact1"}, - {Alias: "artifact#1", ImageName: "artifact1"}, - }, - }, - { - ImageName: "artifact4", - ArtifactType: latest.ArtifactType{ - CustomArtifact: &latest.CustomArtifact{}, - }, - Dependencies: []*latest.ArtifactDependency{ - {Alias: "alias1", ImageName: "artifact1"}, - {Alias: "alias2", ImageName: "artifact2"}, - }, - }, - { - ImageName: "artifact5", - ArtifactType: latest.ArtifactType{ - BuildpackArtifact: &latest.BuildpackArtifact{}, - }, - Dependencies: []*latest.ArtifactDependency{ - {Alias: "artifact!", ImageName: "artifact1"}, - {Alias: "artifact#1", ImageName: "artifact1"}, + Pipeline: latest.Pipeline{ + Build: latest.BuildConfig{ + Artifacts: []*latest.Artifact{ + { + ImageName: "artifact1", + }, + { + ImageName: "artifact2", + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{}, + }, + Dependencies: []*latest.ArtifactDependency{ + {Alias: "ARTIFACT_1", ImageName: "artifact1"}, + {Alias: "1_ARTIFACT", ImageName: "artifact1"}, + }, + }, + { + ImageName: "artifact3", + ArtifactType: latest.ArtifactType{ + DockerArtifact: &latest.DockerArtifact{}, + }, + Dependencies: []*latest.ArtifactDependency{ + {Alias: "artifact!", ImageName: "artifact1"}, + {Alias: "artifact#1", ImageName: "artifact1"}, + }, + }, + { + ImageName: "artifact4", + ArtifactType: latest.ArtifactType{ + CustomArtifact: &latest.CustomArtifact{}, + }, + Dependencies: []*latest.ArtifactDependency{ + {Alias: "alias1", ImageName: "artifact1"}, + {Alias: "alias2", ImageName: "artifact2"}, + }, + }, + { + ImageName: "artifact5", + ArtifactType: latest.ArtifactType{ + BuildpackArtifact: &latest.BuildpackArtifact{}, + }, + Dependencies: []*latest.ArtifactDependency{ + {Alias: "artifact!", ImageName: "artifact1"}, + {Alias: "artifact#1", ImageName: "artifact1"}, + }, + }, + }, + }, }, }, } @@ -1114,7 +1131,7 @@ func TestValidateValidDependencyAliases(t *testing.T) { fmt.Errorf(`invalid build dependency for artifact "artifact3": alias "artifact!" doesn't match required pattern %q`, dependencyAliasPattern), fmt.Errorf(`invalid build dependency for artifact "artifact3": alias "artifact#1" doesn't match required pattern %q`, dependencyAliasPattern), } - errs := validateArtifactDependencies(artifacts) + errs := validateArtifactDependencies(cfgs) testutil.CheckDeepEqual(t, expected, errs, cmp.Comparer(errorsComparer)) } @@ -1169,11 +1186,11 @@ func TestValidateTaggingPolicy(t *testing.T) { t.Override(&validateYamltags, func(interface{}) error { return nil }) err := Process( - &latest.SkaffoldConfig{ + []*latest.SkaffoldConfig{{ Pipeline: latest.Pipeline{ Build: test.cfg, }, - }) + }}) t.CheckError(test.shouldErr, err) }) diff --git a/pkg/skaffold/schema/versions.go b/pkg/skaffold/schema/versions.go index 9b3e696f93f..7c8d0a56cf3 100644 --- a/pkg/skaffold/schema/versions.go +++ b/pkg/skaffold/schema/versions.go @@ -146,51 +146,48 @@ func IsSkaffoldConfig(file string) bool { // ParseConfig reads a configuration file. func ParseConfig(filename string) ([]util.VersionedConfig, error) { - var sl []util.VersionedConfig - buffer, err := misc.ReadConfiguration(filename) + // TODO: update this function to parse the input as multiple configs + buf, err := misc.ReadConfiguration(filename) if err != nil { return nil, fmt.Errorf("read skaffold config: %w", err) } - arr := bytes.Split(buffer, []byte("---")) - for _, buf := range arr { - // This is to quickly check that it's possibly a skaffold.yaml, - // without parsing the whole file. - if !bytes.Contains(buf, []byte("apiVersion")) { - return nil, errors.New("missing apiVersion") - } + // This is to quickly check that it's possibly a skaffold.yaml, + // without parsing the whole file. + if !bytes.Contains(buf, []byte("apiVersion")) { + return nil, errors.New("missing apiVersion") + } - apiVersion := &APIVersion{} - if err := yaml.Unmarshal(buf, apiVersion); err != nil { - return nil, fmt.Errorf("parsing api version: %w", err) - } + apiVersion := &APIVersion{} + if err := yaml.Unmarshal(buf, apiVersion); err != nil { + return nil, fmt.Errorf("parsing api version: %w", err) + } - factory, present := SchemaVersions.Find(apiVersion.Version) - if !present { - return nil, fmt.Errorf("unknown api version: %q", apiVersion.Version) - } + factory, present := SchemaVersions.Find(apiVersion.Version) + if !present { + return nil, fmt.Errorf("unknown api version: %q", apiVersion.Version) + } - // Remove all top-level keys starting with `.` so they can be used as YAML anchors - parsed := make(map[string]interface{}) - if err := yaml.UnmarshalStrict(buf, parsed); err != nil { - return nil, fmt.Errorf("unable to parse YAML: %w", err) - } - for field := range parsed { - if strings.HasPrefix(field, ".") { - delete(parsed, field) - } - } - buf, err = yaml.Marshal(parsed) - if err != nil { - return nil, fmt.Errorf("unable to re-marshal YAML without dotted keys: %w", err) + // Remove all top-level keys starting with `.` so they can be used as YAML anchors + parsed := make(map[string]interface{}) + if err := yaml.UnmarshalStrict(buf, parsed); err != nil { + return nil, fmt.Errorf("unable to parse YAML: %w", err) + } + for field := range parsed { + if strings.HasPrefix(field, ".") { + delete(parsed, field) } + } + buf, err = yaml.Marshal(parsed) + if err != nil { + return nil, fmt.Errorf("unable to re-marshal YAML without dotted keys: %w", err) + } - cfg := factory() - if err := yaml.UnmarshalStrict(buf, cfg); err != nil { - return nil, fmt.Errorf("unable to parse config: %w", err) - } - sl = append(sl, cfg) + cfg := factory() + if err := yaml.UnmarshalStrict(buf, cfg); err != nil { + return nil, fmt.Errorf("unable to parse config: %w", err) } - return sl, nil + + return []util.VersionedConfig{cfg}, nil } // ParseConfigAndUpgrade reads a configuration file and upgrades it to a given version. diff --git a/pkg/skaffold/schema/versions_test.go b/pkg/skaffold/schema/versions_test.go index 985438ef6dd..0bf7b6a331e 100644 --- a/pkg/skaffold/schema/versions_test.go +++ b/pkg/skaffold/schema/versions_test.go @@ -342,14 +342,15 @@ func TestParseConfigAndUpgrade(t *testing.T) { Write("skaffold.yaml", fmt.Sprintf("apiVersion: %s\nkind: Config\n%s", test.apiVersion, test.config)) cfg, err := ParseConfigAndUpgrade(tmpDir.Path("skaffold.yaml"), latest.Version) - if cfg != nil { - config := cfg.(*latest.SkaffoldConfig) - err := defaults.Set(config) - + var expected util.VersionedConfig + if len(cfg) > 0 { + config := cfg[0].(*latest.SkaffoldConfig) + err := defaults.Set(config, true) t.CheckNoError(err) + expected = config } - t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, cfg) + t.CheckErrorAndDeepEqual(test.shouldErr, err, test.expected, expected) }) } } diff --git a/pkg/skaffold/test/test_test.go b/pkg/skaffold/test/test_test.go index 4aa068bac30..88da7a2c1a4 100644 --- a/pkg/skaffold/test/test_test.go +++ b/pkg/skaffold/test/test_test.go @@ -41,7 +41,7 @@ func TestNoTestDependencies(t *testing.T) { t.Override(&docker.NewAPIClient, func(docker.Config) (docker.LocalDaemon, error) { return nil, nil }) cfg := &mockConfig{} - deps, err := NewTester(cfg, true).TestDependencies() + deps, err := NewTester(cfg, func(imageName string) (bool, error) { return true, nil }).TestDependencies() t.CheckNoError(err) t.CheckEmpty(deps) @@ -60,7 +60,7 @@ func TestTestDependencies(t *testing.T) { {StructureTests: []string{"test3.yaml"}}, }, } - deps, err := NewTester(cfg, true).TestDependencies() + deps, err := NewTester(cfg, func(imageName string) (bool, error) { return true, nil }).TestDependencies() expectedDeps := tmpDir.Paths("tests/test1.yaml", "tests/test2.yaml", "test3.yaml") t.CheckNoError(err) @@ -77,7 +77,7 @@ func TestWrongPattern(t *testing.T) { }}, } - tester := NewTester(cfg, true) + tester := NewTester(cfg, func(imageName string) (bool, error) { return true, nil }) _, err := tester.TestDependencies() t.CheckError(true, err) @@ -94,7 +94,7 @@ func TestNoTest(t *testing.T) { testutil.Run(t, "", func(t *testutil.T) { cfg := &mockConfig{} - tester := NewTester(cfg, true) + tester := NewTester(cfg, func(imageName string) (bool, error) { return true, nil }) err := tester.Test(context.Background(), ioutil.Discard, nil) t.CheckNoError(err) @@ -107,7 +107,7 @@ func TestIgnoreDockerNotFound(t *testing.T) { return nil, errors.New("not found") }) - tester := NewTester(&mockConfig{}, true) + tester := NewTester(&mockConfig{}, func(imageName string) (bool, error) { return true, nil }) t.CheckNil(tester) }) @@ -142,7 +142,7 @@ func TestTestSuccess(t *testing.T) { } imagesAreLocal := true - err := NewTester(cfg, imagesAreLocal).Test(context.Background(), ioutil.Discard, []build.Artifact{{ + err := NewTester(cfg, func(imageName string) (bool, error) { return imagesAreLocal, nil }).Test(context.Background(), ioutil.Discard, []build.Artifact{{ ImageName: "image", Tag: "image:tag", }}) @@ -167,7 +167,7 @@ func TestTestSuccessRemoteImage(t *testing.T) { } imagesAreLocal := false - err := NewTester(cfg, imagesAreLocal).Test(context.Background(), ioutil.Discard, []build.Artifact{{ + err := NewTester(cfg, func(imageName string) (bool, error) { return imagesAreLocal, nil }).Test(context.Background(), ioutil.Discard, []build.Artifact{{ ImageName: "image", Tag: "image:tag", }}) @@ -192,7 +192,7 @@ func TestTestFailureRemoteImage(t *testing.T) { } imagesAreLocal := false - err := NewTester(cfg, imagesAreLocal).Test(context.Background(), ioutil.Discard, []build.Artifact{{ + err := NewTester(cfg, func(imageName string) (bool, error) { return imagesAreLocal, nil }).Test(context.Background(), ioutil.Discard, []build.Artifact{{ ImageName: "image", Tag: "image:tag", }}) @@ -219,7 +219,7 @@ func TestTestFailure(t *testing.T) { }, } - err := NewTester(cfg, true).Test(context.Background(), ioutil.Discard, []build.Artifact{{ + err := NewTester(cfg, func(imageName string) (bool, error) { return true, nil }).Test(context.Background(), ioutil.Discard, []build.Artifact{{ ImageName: "broken-image", Tag: "broken-image:tag", }}) @@ -244,7 +244,7 @@ func TestTestMuted(t *testing.T) { } var buf bytes.Buffer - err := NewTester(cfg, true).Test(context.Background(), &buf, []build.Artifact{{ + err := NewTester(cfg, func(imageName string) (bool, error) { return true, nil }).Test(context.Background(), &buf, []build.Artifact{{ ImageName: "image", Tag: "image:tag", }}) @@ -265,10 +265,6 @@ type mockConfig struct { muted config.Muted } -func (c *mockConfig) Muted() config.Muted { return c.muted } -func (c *mockConfig) GetWorkingDir() string { return c.workingDir } -func (c *mockConfig) Pipeline() latest.Pipeline { - var pipeline latest.Pipeline - pipeline.Test = c.tests - return pipeline -} +func (c *mockConfig) Muted() config.Muted { return c.muted } +func (c *mockConfig) GetWorkingDir() string { return c.workingDir } +func (c *mockConfig) TestCases() []*latest.TestCase { return c.tests } diff --git a/pkg/skaffold/trigger/triggers_test.go b/pkg/skaffold/trigger/triggers_test.go index e00e2f800f9..a8acf2d0b4a 100644 --- a/pkg/skaffold/trigger/triggers_test.go +++ b/pkg/skaffold/trigger/triggers_test.go @@ -262,10 +262,6 @@ type mockConfig struct { artifacts []*latest.Artifact } -func (c *mockConfig) Trigger() string { return c.trigger } -func (c *mockConfig) WatchPollInterval() int { return c.watchPollInterval } -func (c *mockConfig) Pipeline() latest.Pipeline { - var pipeline latest.Pipeline - pipeline.Build.Artifacts = c.artifacts - return pipeline -} +func (c *mockConfig) Trigger() string { return c.trigger } +func (c *mockConfig) WatchPollInterval() int { return c.watchPollInterval } +func (c *mockConfig) Artifacts() []*latest.Artifact { return c.artifacts } diff --git a/pkg/skaffold/yaml/yaml.go b/pkg/skaffold/yaml/yaml.go index 0eb06f4e61b..cc69845618f 100644 --- a/pkg/skaffold/yaml/yaml.go +++ b/pkg/skaffold/yaml/yaml.go @@ -19,6 +19,7 @@ package yaml import ( "bytes" "io" + "reflect" yaml "gopkg.in/yaml.v3" ) @@ -55,3 +56,28 @@ func Marshal(in interface{}) (out []byte, err error) { } return b.Bytes(), nil } + +// MarshalWithSeparator is same as yaml.Marshal except for slice or array types where each element is encoded individually and separated by "---". +func MarshalWithSeparator(in interface{}) (out []byte, err error) { + var b bytes.Buffer + encoder := yaml.NewEncoder(&b) + encoder.SetIndent(2) + + switch reflect.TypeOf(in).Kind() { + case reflect.Array: + fallthrough + case reflect.Slice: + s := reflect.ValueOf(in) + for i := 0; i < s.Len(); i++ { + if err := encoder.Encode(s.Index(i).Interface()); err != nil { + return nil, err + } + } + default: + if err := encoder.Encode(in); err != nil { + return nil, err + } + } + + return b.Bytes(), nil +} From bbaa66ed6fbfaf349df1cdb2b0c6d7b4ab75baec Mon Sep 17 00:00:00 2001 From: gsquared94 Date: Mon, 21 Dec 2020 18:08:14 -0800 Subject: [PATCH 3/9] revert example change --- .../examples/microservices/skaffold.yaml | 42 ++++++------------- 1 file changed, 12 insertions(+), 30 deletions(-) diff --git a/integration/examples/microservices/skaffold.yaml b/integration/examples/microservices/skaffold.yaml index 60b39d8ba75..87a0c297d3d 100644 --- a/integration/examples/microservices/skaffold.yaml +++ b/integration/examples/microservices/skaffold.yaml @@ -7,42 +7,24 @@ build: requires: - image: base alias: BASE + - image: leeroy-app + context: leeroy-app + requires: + - image: base + alias: BASE + - image: base + context: base deploy: kubectl: manifests: - leeroy-web/kubernetes/* + - leeroy-app/kubernetes/* portForward: - resourceType: deployment resourceName: leeroy-web port: 8080 localPort: 9000 - ---- - -apiVersion: skaffold/v2beta11 -kind: Config -build: - artifacts: - - image: leeroy-app - context: leeroy-app - requires: - - image: base - alias: BASE -deploy: - kubectl: - manifests: - - leeroy-app/kubernetes/* -portForward: -- resourceType: deployment - resourceName: leeroy-app - port: http - localPort: 9001 - ---- - -apiVersion: skaffold/v2beta11 -kind: Config -build: - artifacts: - - image: base - context: base \ No newline at end of file + - resourceType: deployment + resourceName: leeroy-app + port: http + localPort: 9001 \ No newline at end of file From 1db41c25bc50c4f241bd34c2a765b4fdb2bbe299 Mon Sep 17 00:00:00 2001 From: gsquared94 Date: Mon, 21 Dec 2020 19:15:45 -0800 Subject: [PATCH 4/9] fix for non-managed images --- pkg/skaffold/build/cache/cache.go | 3 ++- pkg/skaffold/runner/new.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/pkg/skaffold/build/cache/cache.go b/pkg/skaffold/build/cache/cache.go index 511bdecc7b5..1e8b22e442b 100644 --- a/pkg/skaffold/build/cache/cache.go +++ b/pkg/skaffold/build/cache/cache.go @@ -65,6 +65,7 @@ type Config interface { docker.Config Pipeline(imageName string) (latest.Pipeline, bool) GetPipelines() []latest.Pipeline + DefaultPipeline() latest.Pipeline GetCluster() config.Cluster CacheArtifacts() bool CacheFile() string @@ -104,7 +105,7 @@ func NewCache(cfg Config, isLocalImage func(imageName string) (bool, error), dep importMissingImage := func(imageName string) (bool, error) { pipeline, found := cfg.Pipeline(imageName) if !found { - return false, fmt.Errorf("unknown pipeline for image %q", imageName) + pipeline = cfg.DefaultPipeline() } if pipeline.Build.GoogleCloudBuild != nil || pipeline.Build.Cluster != nil { diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index d99a4b4dd94..8bb0f5cc8c7 100644 --- a/pkg/skaffold/runner/new.go +++ b/pkg/skaffold/runner/new.go @@ -167,7 +167,7 @@ func setupTrigger(triggerName string, setIntent func(bool), setAutoTrigger func( func isImageLocal(runCtx *runcontext.RunContext, imageName string) (bool, error) { pipeline, found := runCtx.Pipeline(imageName) if !found { - return false, fmt.Errorf("unknown pipeline for image %q", imageName) + pipeline = runCtx.DefaultPipeline() } if pipeline.Build.GoogleCloudBuild != nil || pipeline.Build.Cluster != nil { return false, nil From 3631640875bb0c47ffadde5feed766bcafd341e9 Mon Sep 17 00:00:00 2001 From: gsquared94 Date: Tue, 22 Dec 2020 10:51:42 -0800 Subject: [PATCH 5/9] fix `InitMeterFromConfig` --- pkg/skaffold/instrumentation/meter.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/skaffold/instrumentation/meter.go b/pkg/skaffold/instrumentation/meter.go index ec3b22a3e28..9a5e033a2c6 100644 --- a/pkg/skaffold/instrumentation/meter.go +++ b/pkg/skaffold/instrumentation/meter.go @@ -112,7 +112,7 @@ func init() { } func InitMeterFromConfig(configs []*latest.SkaffoldConfig) { - meter.PlatformType = yamltags.GetYamlTag(configs[0].Build.BuildType) + meter.PlatformType = yamltags.GetYamlTag(configs[0].Build.BuildType) // TODO: support multiple build types in events. for _, config := range configs { for _, artifact := range config.Pipeline.Build.Artifacts { meter.Builders[yamltags.GetYamlTag(artifact.ArtifactType)]++ @@ -120,8 +120,8 @@ func InitMeterFromConfig(configs []*latest.SkaffoldConfig) { meter.SyncType[yamltags.GetYamlTag(artifact.Sync)] = true } } - meter.Deployers = yamltags.GetYamlTags(config.Deploy.DeployType) - meter.BuildArtifacts = len(config.Pipeline.Build.Artifacts) + meter.Deployers = append(meter.Deployers, yamltags.GetYamlTags(config.Deploy.DeployType)...) + meter.BuildArtifacts += len(config.Pipeline.Build.Artifacts) } } From 2d4156360262ad3f0bcf794fb2cd433171004119 Mon Sep 17 00:00:00 2001 From: gsquared94 Date: Tue, 22 Dec 2020 11:25:38 -0800 Subject: [PATCH 6/9] fix message in `fix` command --- cmd/skaffold/app/cmd/find_configs.go | 1 + cmd/skaffold/app/cmd/fix.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/skaffold/app/cmd/find_configs.go b/cmd/skaffold/app/cmd/find_configs.go index b49c3af971b..a2d07c8d85e 100644 --- a/cmd/skaffold/app/cmd/find_configs.go +++ b/cmd/skaffold/app/cmd/find_configs.go @@ -89,6 +89,7 @@ func findConfigs(directory string) (map[string]string, error) { err := walk.From(directory).When(isYaml).Do(func(path string, _ walk.Dirent) error { if cfgs, err := schema.ParseConfig(path); err == nil && len(cfgs) > 0 { + // all configs defined in the same file should have the same version pathToVersion[path] = cfgs[0].GetVersion() } return nil diff --git a/cmd/skaffold/app/cmd/fix.go b/cmd/skaffold/app/cmd/fix.go index a705d088d5d..704e7adb39e 100644 --- a/cmd/skaffold/app/cmd/fix.go +++ b/cmd/skaffold/app/cmd/fix.go @@ -91,7 +91,7 @@ func fix(out io.Writer, configFile string, toVersion string, overwrite bool) err if err := ioutil.WriteFile(configFile, newCfg, 0644); err != nil { return fmt.Errorf("writing config file: %w", err) } - color.Default.Fprintf(out, "New config at version %s generated and written to %s\n", latest.Version, opts.ConfigurationFile) + color.Default.Fprintf(out, "New config at version %s generated and written to %s\n", toVersion, opts.ConfigurationFile) } else { out.Write(newCfg) } From 585e3cee20a9b6983a8f74c4e1a4e34f38e5c770 Mon Sep 17 00:00:00 2001 From: gsquared94 Date: Tue, 22 Dec 2020 11:40:07 -0800 Subject: [PATCH 7/9] address PR feedback --- cmd/skaffold/app/cmd/runner.go | 1 + pkg/skaffold/runner/dev_test.go | 1 - pkg/skaffold/runner/runcontext/context.go | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/cmd/skaffold/app/cmd/runner.go b/cmd/skaffold/app/cmd/runner.go index 83e84128404..00a6774aeb9 100644 --- a/cmd/skaffold/app/cmd/runner.go +++ b/cmd/skaffold/app/cmd/runner.go @@ -105,6 +105,7 @@ func runContext(opts config.SkaffoldOptions) (*runcontext.RunContext, []*latest. configs = append(configs, config) } + // TODO: Should support per-config kubecontext. Right now we constrain all configs to define the same kubecontext. kubectx.ConfigureKubeConfig(opts.KubeConfig, opts.KubeContext, configs[0].Deploy.KubeContext) if err := validation.Process(configs); err != nil { diff --git a/pkg/skaffold/runner/dev_test.go b/pkg/skaffold/runner/dev_test.go index fe2968b0585..7b1f9d426c3 100644 --- a/pkg/skaffold/runner/dev_test.go +++ b/pkg/skaffold/runner/dev_test.go @@ -140,7 +140,6 @@ func TestDevFailFirstCycle(t *testing.T) { artifacts := []*latest.Artifact{{ ImageName: "img", }} - // runner := createRunner(t, test.testBench).WithMonitor(test.monitor) runner := createRunner(t, test.testBench, test.monitor, artifacts) test.testBench.firstMonitor = test.monitor.Run diff --git a/pkg/skaffold/runner/runcontext/context.go b/pkg/skaffold/runner/runcontext/context.go index abed331fcc6..2da986d105a 100644 --- a/pkg/skaffold/runner/runcontext/context.go +++ b/pkg/skaffold/runner/runcontext/context.go @@ -54,9 +54,9 @@ func (ps Pipelines) All() []latest.Pipeline { return ps.pipelines } -// Default returns the first `latest.Pipeline`. +// Head returns the first `latest.Pipeline`. func (ps Pipelines) Head() latest.Pipeline { - return ps.pipelines[0] //there always exists atleast one pipeline. + return ps.pipelines[0] // there always exists atleast one pipeline. } // Select returns the first `latest.Pipeline` that matches the given artifact `imageName`. From ac33eae6fda46ba9d16c8ebc1c751da065ded07e Mon Sep 17 00:00:00 2001 From: gsquared94 Date: Tue, 22 Dec 2020 15:01:38 -0800 Subject: [PATCH 8/9] add UTs --- pkg/skaffold/build/builder_mux.go | 2 +- pkg/skaffold/build/builder_mux_test.go | 124 +++++++++++++++++++++++++ pkg/skaffold/runner/new.go | 3 +- pkg/skaffold/yaml/yaml.go | 2 +- pkg/skaffold/yaml/yaml_test.go | 59 ++++++++++++ 5 files changed, 187 insertions(+), 3 deletions(-) create mode 100644 pkg/skaffold/build/builder_mux_test.go create mode 100644 pkg/skaffold/yaml/yaml_test.go diff --git a/pkg/skaffold/build/builder_mux.go b/pkg/skaffold/build/builder_mux.go index 668203e3e06..4d05c0a65e9 100644 --- a/pkg/skaffold/build/builder_mux.go +++ b/pkg/skaffold/build/builder_mux.go @@ -39,7 +39,7 @@ type Config interface { } // NewBuilderMux returns an implementation of `build.BuilderMux`. -func NewBuilderMux(cfg Config, store ArtifactStore, builder func(p latest.Pipeline) (PipelineBuilder, error)) (Builder, error) { +func NewBuilderMux(cfg Config, store ArtifactStore, builder func(p latest.Pipeline) (PipelineBuilder, error)) (*BuilderMux, error) { pipelines := cfg.GetPipelines() m := make(map[string]PipelineBuilder) var sl []PipelineBuilder diff --git a/pkg/skaffold/build/builder_mux_test.go b/pkg/skaffold/build/builder_mux_test.go new file mode 100644 index 00000000000..6fb4436bf58 --- /dev/null +++ b/pkg/skaffold/build/builder_mux_test.go @@ -0,0 +1,124 @@ +/* +Copyright 2020 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package build + +import ( + "context" + "errors" + "io" + "testing" + + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/latest" + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestNewBuilderMux(t *testing.T) { + tests := []struct { + description string + pipelines []latest.Pipeline + pipeBuilder func(latest.Pipeline) (PipelineBuilder, error) + shouldErr bool + expectedBuilders []string + }{ + { + description: "only local builder", + pipelines: []latest.Pipeline{ + {Build: latest.BuildConfig{BuildType: latest.BuildType{LocalBuild: &latest.LocalBuild{}}}}, + }, + pipeBuilder: newMockPipelineBuilder, + expectedBuilders: []string{"local"}, + }, + { + description: "only cluster builder", + pipelines: []latest.Pipeline{ + {Build: latest.BuildConfig{BuildType: latest.BuildType{Cluster: &latest.ClusterDetails{}}}}, + }, + pipeBuilder: newMockPipelineBuilder, + expectedBuilders: []string{"cluster"}, + }, + { + description: "only gcb builder", + pipelines: []latest.Pipeline{ + {Build: latest.BuildConfig{BuildType: latest.BuildType{GoogleCloudBuild: &latest.GoogleCloudBuild{}}}}, + }, + pipeBuilder: newMockPipelineBuilder, + expectedBuilders: []string{"gcb"}, + }, + { + description: "multiple builders", + pipelines: []latest.Pipeline{ + {Build: latest.BuildConfig{BuildType: latest.BuildType{LocalBuild: &latest.LocalBuild{}}}}, + {Build: latest.BuildConfig{BuildType: latest.BuildType{Cluster: &latest.ClusterDetails{}}}}, + }, + pipeBuilder: newMockPipelineBuilder, + expectedBuilders: []string{"local", "cluster"}, + }, + } + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + cfg := &mockConfig{pipelines: test.pipelines} + + b, err := NewBuilderMux(cfg, nil, test.pipeBuilder) + t.CheckError(test.shouldErr, err) + if test.shouldErr { + return + } + t.CheckTrue(len(b.builders) == len(test.expectedBuilders)) + for i := range b.builders { + t.CheckDeepEqual(test.expectedBuilders[i], b.builders[i].(*mockPipelineBuilder).builderType) + } + }) + } +} + +type mockConfig struct { + pipelines []latest.Pipeline +} + +func (m *mockConfig) GetPipelines() []latest.Pipeline { + return m.pipelines +} + +type mockPipelineBuilder struct { + concurrency int + builderType string +} + +func (m *mockPipelineBuilder) PreBuild(ctx context.Context, out io.Writer) error { return nil } + +func (m *mockPipelineBuilder) Build(ctx context.Context, out io.Writer, artifact *latest.Artifact) ArtifactBuilder { + return nil +} + +func (m *mockPipelineBuilder) PostBuild(ctx context.Context, out io.Writer) error { return nil } + +func (m *mockPipelineBuilder) Concurrency() int { return m.concurrency } + +func (m *mockPipelineBuilder) Prune(context.Context, io.Writer) error { return nil } + +func newMockPipelineBuilder(p latest.Pipeline) (PipelineBuilder, error) { + switch { + case p.Build.BuildType.LocalBuild != nil: + return &mockPipelineBuilder{builderType: "local"}, nil + case p.Build.BuildType.Cluster != nil: + return &mockPipelineBuilder{builderType: "cluster"}, nil + case p.Build.BuildType.GoogleCloudBuild != nil: + return &mockPipelineBuilder{builderType: "gcb"}, nil + default: + return nil, errors.New("invalid config") + } +} diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index 8bb0f5cc8c7..44d5ce71899 100644 --- a/pkg/skaffold/runner/new.go +++ b/pkg/skaffold/runner/new.go @@ -58,7 +58,8 @@ func NewForConfig(runCtx *runcontext.RunContext) (*SkaffoldRunner, error) { } store := build.NewArtifactStore() - builder, err := build.NewBuilderMux(runCtx, store, func(p latest.Pipeline) (build.PipelineBuilder, error) { + var builder build.Builder + builder, err = build.NewBuilderMux(runCtx, store, func(p latest.Pipeline) (build.PipelineBuilder, error) { return getBuilder(runCtx, store, p) }) if err != nil { diff --git a/pkg/skaffold/yaml/yaml.go b/pkg/skaffold/yaml/yaml.go index cc69845618f..2419f95dd2d 100644 --- a/pkg/skaffold/yaml/yaml.go +++ b/pkg/skaffold/yaml/yaml.go @@ -57,7 +57,7 @@ func Marshal(in interface{}) (out []byte, err error) { return b.Bytes(), nil } -// MarshalWithSeparator is same as yaml.Marshal except for slice or array types where each element is encoded individually and separated by "---". +// MarshalWithSeparator is same as Marshal except for slice or array types where each element is encoded individually and separated by "---". func MarshalWithSeparator(in interface{}) (out []byte, err error) { var b bytes.Buffer encoder := yaml.NewEncoder(&b) diff --git a/pkg/skaffold/yaml/yaml_test.go b/pkg/skaffold/yaml/yaml_test.go new file mode 100644 index 00000000000..e2e92ccf0a8 --- /dev/null +++ b/pkg/skaffold/yaml/yaml_test.go @@ -0,0 +1,59 @@ +/* +Copyright 2020 The Skaffold Authors + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package yaml + +import ( + "testing" + + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func TestMarshalWithSeparator(t *testing.T) { + type Data struct { + Foo string `yaml:"foo"` + } + + tests := []struct { + description string + input []Data + expected string + }{ + { + description: "single element slice", + input: []Data{ + {Foo: "foo"}, + }, + expected: "foo: foo\n", + }, + { + description: "multi element slice", + input: []Data{ + {Foo: "foo1"}, + {Foo: "foo2"}, + }, + expected: "foo: foo1\n---\nfoo: foo2\n", + }, + } + + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + output, err := MarshalWithSeparator(test.input) + t.CheckNoError(err) + t.CheckDeepEqual(string(output), test.expected) + }) + } +} From 5ecea52aabaf4f73f042867b099e5b951bb2a9eb Mon Sep 17 00:00:00 2001 From: gsquared94 Date: Tue, 22 Dec 2020 15:23:56 -0800 Subject: [PATCH 9/9] rename `Pipeline` to `PipelineForImage` --- pkg/skaffold/build/builder_mux.go | 9 +++------ pkg/skaffold/build/cache/cache.go | 4 ++-- pkg/skaffold/build/cache/retrieve_test.go | 8 ++++---- pkg/skaffold/diagnose/diagnose_test.go | 2 +- pkg/skaffold/event/util.go | 3 ++- pkg/skaffold/kubernetes/log.go | 4 ++-- pkg/skaffold/kubernetes/log_test.go | 2 +- pkg/skaffold/runner/new.go | 2 +- pkg/skaffold/runner/runcontext/context.go | 2 +- 9 files changed, 17 insertions(+), 19 deletions(-) diff --git a/pkg/skaffold/build/builder_mux.go b/pkg/skaffold/build/builder_mux.go index 4d05c0a65e9..98dd7b47782 100644 --- a/pkg/skaffold/build/builder_mux.go +++ b/pkg/skaffold/build/builder_mux.go @@ -42,14 +42,14 @@ type Config interface { func NewBuilderMux(cfg Config, store ArtifactStore, builder func(p latest.Pipeline) (PipelineBuilder, error)) (*BuilderMux, error) { pipelines := cfg.GetPipelines() m := make(map[string]PipelineBuilder) - var sl []PipelineBuilder + var pb []PipelineBuilder minConcurrency := -1 for _, p := range pipelines { b, err := builder(p) if err != nil { return nil, fmt.Errorf("creating builder: %w", err) } - sl = append(sl, b) + pb = append(pb, b) for _, a := range p.Build.Artifacts { m[a.ImageName] = b } @@ -61,11 +61,8 @@ func NewBuilderMux(cfg Config, store ArtifactStore, builder func(p latest.Pipeli minConcurrency = concurrency } } - if minConcurrency > len(m) { - minConcurrency = 0 // if specified concurrency is greater than maximum number of parallel jobs, then just set it to unlimited - } - return &BuilderMux{builders: sl, byImageName: m, store: store, concurrency: minConcurrency}, nil + return &BuilderMux{builders: pb, byImageName: m, store: store, concurrency: minConcurrency}, nil } // Build executes the specific image builder for each artifact in the given artifact slice. diff --git a/pkg/skaffold/build/cache/cache.go b/pkg/skaffold/build/cache/cache.go index 1e8b22e442b..d35ca1e5612 100644 --- a/pkg/skaffold/build/cache/cache.go +++ b/pkg/skaffold/build/cache/cache.go @@ -63,7 +63,7 @@ type DependencyLister func(ctx context.Context, artifact *latest.Artifact) ([]st type Config interface { docker.Config - Pipeline(imageName string) (latest.Pipeline, bool) + PipelineForImage(imageName string) (latest.Pipeline, bool) GetPipelines() []latest.Pipeline DefaultPipeline() latest.Pipeline GetCluster() config.Cluster @@ -103,7 +103,7 @@ func NewCache(cfg Config, isLocalImage func(imageName string) (bool, error), dep } importMissingImage := func(imageName string) (bool, error) { - pipeline, found := cfg.Pipeline(imageName) + pipeline, found := cfg.PipelineForImage(imageName) if !found { pipeline = cfg.DefaultPipeline() } diff --git a/pkg/skaffold/build/cache/retrieve_test.go b/pkg/skaffold/build/cache/retrieve_test.go index d86663d7705..80a4aab1d87 100644 --- a/pkg/skaffold/build/cache/retrieve_test.go +++ b/pkg/skaffold/build/cache/retrieve_test.go @@ -345,7 +345,7 @@ type mockConfig struct { pipeline latest.Pipeline } -func (c *mockConfig) CacheArtifacts() bool { return true } -func (c *mockConfig) CacheFile() string { return c.cacheFile } -func (c *mockConfig) Mode() config.RunMode { return c.mode } -func (c *mockConfig) Pipeline(string) (latest.Pipeline, bool) { return c.pipeline, true } +func (c *mockConfig) CacheArtifacts() bool { return true } +func (c *mockConfig) CacheFile() string { return c.cacheFile } +func (c *mockConfig) Mode() config.RunMode { return c.mode } +func (c *mockConfig) PipelineForImage(string) (latest.Pipeline, bool) { return c.pipeline, true } diff --git a/pkg/skaffold/diagnose/diagnose_test.go b/pkg/skaffold/diagnose/diagnose_test.go index 4d8c1de6ce2..c1867dc475d 100644 --- a/pkg/skaffold/diagnose/diagnose_test.go +++ b/pkg/skaffold/diagnose/diagnose_test.go @@ -101,7 +101,7 @@ type mockConfig struct { artifacts []*latest.Artifact } -func (c *mockConfig) Pipeline() latest.Pipeline { +func (c *mockConfig) PipelineForImage() latest.Pipeline { var pipeline latest.Pipeline pipeline.Build.Artifacts = c.artifacts return pipeline diff --git a/pkg/skaffold/event/util.go b/pkg/skaffold/event/util.go index 4b0912eb46a..ee8952a758f 100644 --- a/pkg/skaffold/event/util.go +++ b/pkg/skaffold/event/util.go @@ -35,7 +35,8 @@ func initializeMetadata(pipelines []latest.Pipeline, kubeContext string) *proto. Deploy: &proto.DeployMetadata{}, } - // all pipelines are constrained to have the same build type. + // TODO: Event metadata should support multiple build types. + // All pipelines are currently constrained to have the same build type. switch { case pipelines[0].Build.LocalBuild != nil: m.Build.Type = proto.BuildType_LOCAL diff --git a/pkg/skaffold/kubernetes/log.go b/pkg/skaffold/kubernetes/log.go index 361c4a45e09..12a0d997f44 100644 --- a/pkg/skaffold/kubernetes/log.go +++ b/pkg/skaffold/kubernetes/log.go @@ -49,7 +49,7 @@ type LogAggregator struct { } type Config interface { - Pipeline(imageName string) (latest.Pipeline, bool) + PipelineForImage(imageName string) (latest.Pipeline, bool) DefaultPipeline() latest.Pipeline } @@ -183,7 +183,7 @@ func (a *LogAggregator) prefix(pod *v1.Pod, container v1.ContainerStatus) string var c latest.Pipeline var present bool for _, container := range pod.Spec.Containers { - if c, present = a.config.Pipeline(stripTag(container.Image)); present { + if c, present = a.config.PipelineForImage(stripTag(container.Image)); present { break } } diff --git a/pkg/skaffold/kubernetes/log_test.go b/pkg/skaffold/kubernetes/log_test.go index 7df7f0ce9cb..d93d70a2d40 100644 --- a/pkg/skaffold/kubernetes/log_test.go +++ b/pkg/skaffold/kubernetes/log_test.go @@ -216,7 +216,7 @@ type mockConfig struct { log latest.LogsConfig } -func (c *mockConfig) Pipeline(string) (latest.Pipeline, bool) { +func (c *mockConfig) PipelineForImage(string) (latest.Pipeline, bool) { var pipeline latest.Pipeline pipeline.Deploy.Logs = c.log return pipeline, true diff --git a/pkg/skaffold/runner/new.go b/pkg/skaffold/runner/new.go index 44d5ce71899..a31fea6d135 100644 --- a/pkg/skaffold/runner/new.go +++ b/pkg/skaffold/runner/new.go @@ -166,7 +166,7 @@ func setupTrigger(triggerName string, setIntent func(bool), setAutoTrigger func( } func isImageLocal(runCtx *runcontext.RunContext, imageName string) (bool, error) { - pipeline, found := runCtx.Pipeline(imageName) + pipeline, found := runCtx.PipelineForImage(imageName) if !found { pipeline = runCtx.DefaultPipeline() } diff --git a/pkg/skaffold/runner/runcontext/context.go b/pkg/skaffold/runner/runcontext/context.go index 2da986d105a..f326b99503c 100644 --- a/pkg/skaffold/runner/runcontext/context.go +++ b/pkg/skaffold/runner/runcontext/context.go @@ -117,7 +117,7 @@ func NewPipelines(pipelines []latest.Pipeline) Pipelines { return Pipelines{pipelines: pipelines, pipelinesByImageName: m} } -func (rc *RunContext) Pipeline(imageName string) (latest.Pipeline, bool) { +func (rc *RunContext) PipelineForImage(imageName string) (latest.Pipeline, bool) { return rc.Pipelines.Select(imageName) }