From 1e5bd33ad007a82e491e788d7ab781ba98114902 Mon Sep 17 00:00:00 2001 From: David Gageot Date: Thu, 21 Jun 2018 11:34:13 +0200 Subject: [PATCH] Remove duplication --- pkg/skaffold/runner/label/label.go | 11 +++--- pkg/skaffold/runner/runner.go | 63 ++++++++++++++++-------------- 2 files changed, 40 insertions(+), 34 deletions(-) diff --git a/pkg/skaffold/runner/label/label.go b/pkg/skaffold/runner/label/label.go index 2846b8a0f48..d18be3ed745 100644 --- a/pkg/skaffold/runner/label/label.go +++ b/pkg/skaffold/runner/label/label.go @@ -26,6 +26,7 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/constants" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/labels" "github.com/sirupsen/logrus" clientgo "k8s.io/client-go/kubernetes" @@ -44,20 +45,20 @@ import ( type withLabels struct { deploy.Deployer - labels map[string]string + labellers []labels.Labeller } // WithLabels creates a deployer that sets labels on deployed resources. -func WithLabels(d deploy.Deployer, labels map[string]string) deploy.Deployer { +func WithLabels(d deploy.Deployer, labellers ...labels.Labeller) deploy.Deployer { return &withLabels{ - Deployer: d, - labels: labels, + Deployer: d, + labellers: labellers, } } func (w *withLabels) Deploy(ctx context.Context, out io.Writer, artifacts []build.Artifact) ([]deploy.Artifact, error) { dRes, err := w.Deployer.Deploy(ctx, out, artifacts) - labelDeployResults(w.labels, dRes) + labelDeployResults(labels.Merge(w.labellers...), dRes) return dRes, err } diff --git a/pkg/skaffold/runner/runner.go b/pkg/skaffold/runner/runner.go index 4cb07f9a84f..8c789cc2e08 100644 --- a/pkg/skaffold/runner/runner.go +++ b/pkg/skaffold/runner/runner.go @@ -29,7 +29,6 @@ import ( "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" kubectx "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes/context" - "github.com/GoogleContainerTools/skaffold/pkg/skaffold/labels" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/runner/label" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/schema/v1alpha2" "github.com/GoogleContainerTools/skaffold/pkg/skaffold/watch" @@ -72,7 +71,7 @@ func NewForConfig(opts *config.SkaffoldOptions, cfg *config.SkaffoldConfig) (*Sk return nil, errors.Wrap(err, "parsing skaffold deploy config") } - deployer = label.WithLabels(deployer, labels.Merge(opts, builder, deployer, tagger)) + deployer = label.WithLabels(deployer, opts, builder, deployer, tagger) builder, deployer = WithTimings(builder, deployer) if opts.Notification { deployer = WithNotification(deployer) @@ -194,43 +193,26 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*v1 colorPicker := kubernetes.NewColorPicker(artifacts) logger := kubernetes.NewLogAggregator(out, imageList, colorPicker) - onChange := func(changedPaths []string) error { + onDeployChange := func(changedPaths []string) error { logger.Mute() - defer logger.Unmute() - - changedArtifacts := depMap.ArtifactsForPaths(changedPaths) - - bRes, err := r.Builder.Build(ctx, out, r.Tagger, changedArtifacts) - if err != nil { - if r.builds == nil { - return errors.Wrap(err, "exiting dev mode because the first build failed") - } - - logrus.Warnln("Skipping Deploy due to build error:", err) - return nil - } - - // Update which images are logged. - for _, build := range bRes { - imageList.Add(build.Tag) - } - // Make sure all artifacts are redeployed. Not only those that were just rebuilt. - r.builds = mergeWithPreviousBuilds(bRes, r.builds) + _, err := r.Deploy(ctx, out, r.builds) - _, err = r.Deploy(ctx, out, r.builds) + logger.Unmute() return err } - onDeployChange := func(changedPaths []string) error { + onArtifactChange := func(changedPaths []string) error { logger.Mute() - defer logger.Unmute() - _, err := r.Deploy(ctx, out, r.builds) + changedArtifacts := depMap.ArtifactsForPaths(changedPaths) + _, err = r.buildAndDeploy(ctx, out, changedArtifacts, imageList) + + logger.Unmute() return err } - if err := onChange(depMap.Paths()); err != nil { + if err := onArtifactChange(depMap.Paths()); err != nil { return nil, errors.Wrap(err, "first build") } @@ -242,7 +224,7 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*v1 // Watch files and rebuild g, watchCtx := errgroup.WithContext(ctx) g.Go(func() error { - return watcher.Start(watchCtx, out, onChange) + return watcher.Start(watchCtx, out, onArtifactChange) }) g.Go(func() error { return deployWatcher.Start(watchCtx, ioutil.Discard, onDeployChange) @@ -251,6 +233,29 @@ func (r *SkaffoldRunner) Dev(ctx context.Context, out io.Writer, artifacts []*v1 return r.builds, g.Wait() } +// buildAndDeploy builds a subset of the artifacts and deploys everything. +func (r *SkaffoldRunner) buildAndDeploy(ctx context.Context, out io.Writer, artifacts []*v1alpha2.Artifact, images *kubernetes.ImageList) ([]deploy.Artifact, error) { + bRes, err := r.Builder.Build(ctx, out, r.Tagger, artifacts) + if err != nil { + if r.builds == nil { + return nil, errors.Wrap(err, "exiting dev mode because the first build failed") + } + + logrus.Warnln("Skipping Deploy due to build error:", err) + return nil, nil + } + + // Update which images are logged. + for _, build := range bRes { + images.Add(build.Tag) + } + + // Make sure all artifacts are redeployed. Not only those that were just rebuilt. + r.builds = mergeWithPreviousBuilds(bRes, r.builds) + + return r.Deploy(ctx, out, r.builds) +} + func mergeWithPreviousBuilds(builds, previous []build.Artifact) []build.Artifact { updatedBuilds := map[string]bool{} for _, build := range builds {