From 7287f412d08f8e457df0ffe55c2e716a4c8bd3ea Mon Sep 17 00:00:00 2001 From: David Gageot Date: Mon, 29 Jul 2019 20:48:11 +0200 Subject: [PATCH] Minor changes to kubectl and kustomize deployers (#2537) * Remove duplication Signed-off-by: David Gageot * Make kubectl and kustomise deployers look more the same Signed-off-by: David Gageot * Add missing deploy failure Signed-off-by: David Gageot --- pkg/skaffold/deploy/kubectl.go | 20 ++++----------- pkg/skaffold/deploy/kustomize.go | 34 +++++++++----------------- pkg/skaffold/deploy/transformations.go | 32 ++++++++++++++++++++++++ pkg/skaffold/util/util.go | 9 ++++--- 4 files changed, 54 insertions(+), 41 deletions(-) create mode 100644 pkg/skaffold/deploy/transformations.go diff --git a/pkg/skaffold/deploy/kubectl.go b/pkg/skaffold/deploy/kubectl.go index cc71e3fea64..b2d90573d82 100644 --- a/pkg/skaffold/deploy/kubectl.go +++ b/pkg/skaffold/deploy/kubectl.go @@ -65,16 +65,6 @@ func (k *KubectlDeployer) Labels() map[string]string { } } -type ManifestTransform func(l kubectl.ManifestList, builds []build.Artifact, insecureRegistries map[string]bool) (kubectl.ManifestList, error) - -// Transforms are applied to manifests -var manifestTransforms []ManifestTransform - -// AddManifestTransform adds a transform to be applied when deploying. -func AddManifestTransform(newTransform ManifestTransform) { - manifestTransforms = append(manifestTransforms, newTransform) -} - // Deploy templates the provided manifests with a simple `find and replace` and // runs `kubectl apply` on those manifests func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) error { @@ -83,8 +73,6 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu color.Default.Fprintln(out, err) } - event.DeployInProgress() - manifests, err := k.readManifests(ctx) if err != nil { event.DeployFailed(err) @@ -95,6 +83,8 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu return nil } + event.DeployInProgress() + manifests, err = manifests.ReplaceImages(builds, k.defaultRepo) if err != nil { event.DeployFailed(err) @@ -110,18 +100,18 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu for _, transform := range manifestTransforms { manifests, err = transform(manifests, builds, k.insecureRegistries) if err != nil { + event.DeployFailed(err) return errors.Wrap(err, "unable to transform manifests") } } - err = k.kubectl.Apply(ctx, out, manifests) - if err != nil { + if err := k.kubectl.Apply(ctx, out, manifests); err != nil { event.DeployFailed(err) return errors.Wrap(err, "kubectl error") } event.DeployComplete() - return err + return nil } // Cleanup deletes what was deployed by calling Deploy. diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index 3935c4a4a19..32305b0839a 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -126,12 +126,12 @@ func (k *KustomizeDeployer) Deploy(ctx context.Context, out io.Writer, builds [] for _, transform := range manifestTransforms { manifests, err = transform(manifests, builds, k.insecureRegistries) if err != nil { + event.DeployFailed(err) return errors.Wrap(err, "unable to transform manifests") } } - err = k.kubectl.Apply(ctx, out, manifests) - if err != nil { + if err := k.kubectl.Apply(ctx, out, manifests); err != nil { event.DeployFailed(err) return errors.Wrap(err, "kubectl error") } @@ -154,6 +154,11 @@ func (k *KustomizeDeployer) Cleanup(ctx context.Context, out io.Writer) error { return nil } +// Dependencies lists all the files that can change what needs to be deployed. +func (k *KustomizeDeployer) Dependencies() ([]string, error) { + return dependenciesForKustomization(k.KustomizePath) +} + func dependenciesForKustomization(dir string) ([]string, error) { var deps []string @@ -196,32 +201,22 @@ func dependenciesForKustomization(dir string) ([]string, error) { } } - deps = append(deps, joinPaths(dir, content.Patches)...) - deps = append(deps, joinPaths(dir, content.PatchesStrategicMerge)...) - deps = append(deps, joinPaths(dir, content.CRDs)...) + deps = append(deps, util.AbsolutePaths(dir, content.Patches)...) + deps = append(deps, util.AbsolutePaths(dir, content.PatchesStrategicMerge)...) + deps = append(deps, util.AbsolutePaths(dir, content.CRDs)...) for _, patch := range content.PatchesJSON6902 { deps = append(deps, filepath.Join(dir, patch.Path)) } for _, generator := range content.ConfigMapGenerator { - deps = append(deps, joinPaths(dir, generator.Files)...) + deps = append(deps, util.AbsolutePaths(dir, generator.Files)...) } for _, generator := range content.SecretGenerator { - deps = append(deps, joinPaths(dir, generator.Files)...) + deps = append(deps, util.AbsolutePaths(dir, generator.Files)...) } return deps, nil } -func joinPaths(root string, paths []string) []string { - var list []string - - for _, path := range paths { - list = append(list, filepath.Join(root, path)) - } - - return list -} - // A kustomization config must be at the root of the direectory. Kustomize will // error if more than one of these files exists so order doesn't matter. func findKustomizationConfig(dir string) (string, error) { @@ -245,11 +240,6 @@ func pathExistsLocally(filename string, workingDir string) (bool, os.FileMode) { return false, 0 } -// Dependencies lists all the files that can change what needs to be deployed. -func (k *KustomizeDeployer) Dependencies() ([]string, error) { - return dependenciesForKustomization(k.KustomizePath) -} - func (k *KustomizeDeployer) readManifests(ctx context.Context) (kubectl.ManifestList, error) { cmd := exec.CommandContext(ctx, "kustomize", "build", k.KustomizePath) out, err := util.RunCmdOut(cmd) diff --git a/pkg/skaffold/deploy/transformations.go b/pkg/skaffold/deploy/transformations.go new file mode 100644 index 00000000000..39bfbd5c573 --- /dev/null +++ b/pkg/skaffold/deploy/transformations.go @@ -0,0 +1,32 @@ +/* +Copyright 2019 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 deploy + +import ( + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/build" + "github.com/GoogleContainerTools/skaffold/pkg/skaffold/deploy/kubectl" +) + +type ManifestTransform func(l kubectl.ManifestList, builds []build.Artifact, insecureRegistries map[string]bool) (kubectl.ManifestList, error) + +// Transforms are applied to manifests +var manifestTransforms []ManifestTransform + +// AddManifestTransform adds a transform to be applied when deploying. +func AddManifestTransform(newTransform ManifestTransform) { + manifestTransforms = append(manifestTransforms, newTransform) +} diff --git a/pkg/skaffold/util/util.go b/pkg/skaffold/util/util.go index 14adb9ec41d..b9dcc16873f 100644 --- a/pkg/skaffold/util/util.go +++ b/pkg/skaffold/util/util.go @@ -260,15 +260,16 @@ func CloneThroughYAML(old interface{}, new interface{}) error { // AbsolutePaths prepends each path in paths with workspace if the path isn't absolute func AbsolutePaths(workspace string, paths []string) []string { - var p []string + var list []string + for _, path := range paths { - // TODO(dgageot): this is only done for jib builder. if !filepath.IsAbs(path) { path = filepath.Join(workspace, path) } - p = append(p, path) + list = append(list, path) } - return p + + return list } // IsHiddenDir returns if a directory is hidden.