From 2365c3147ffa716c73017fcdb9fa25968edbd84f Mon Sep 17 00:00:00 2001 From: arminbuerkle <22750465+arminbuerkle@users.noreply.github.com> Date: Thu, 30 Jan 2020 10:47:36 +0100 Subject: [PATCH] Add labels to render (#3558) * Add labeller to render Signed-off-by: Armin Buerkle * Add labeller to integration test Signed-off-by: Armin Buerkle * Add kustomize render labeller unit test Signed-off-by: Armin Buerkle * Add kubectl render labeller unit test Signed-off-by: Armin Buerkle * rebase from master and add one more test Co-authored-by: Tejal Desai --- cmd/skaffold/app/cmd/flags.go | 2 +- docs/content/en/docs/references/cli/_index.md | 2 + integration/render_test.go | 13 ++- pkg/skaffold/deploy/deploy.go | 2 +- pkg/skaffold/deploy/deploy_mux.go | 4 +- pkg/skaffold/deploy/deploy_mux_test.go | 6 +- pkg/skaffold/deploy/helm.go | 2 +- pkg/skaffold/deploy/helm_test.go | 2 +- pkg/skaffold/deploy/kubectl.go | 19 +++-- pkg/skaffold/deploy/kubectl_test.go | 57 ++++++++++--- pkg/skaffold/deploy/kustomize.go | 4 +- pkg/skaffold/deploy/kustomize_test.go | 80 ++++++++++++++++++- pkg/skaffold/runner/render.go | 2 +- pkg/skaffold/runner/runner_test.go | 2 +- 14 files changed, 160 insertions(+), 37 deletions(-) diff --git a/cmd/skaffold/app/cmd/flags.go b/cmd/skaffold/app/cmd/flags.go index 9b69db73479..c5eada7eecd 100644 --- a/cmd/skaffold/app/cmd/flags.go +++ b/cmd/skaffold/app/cmd/flags.go @@ -135,7 +135,7 @@ var FlagRegistry = []Flag{ Value: &opts.CustomLabels, DefValue: []string{}, FlagAddMethod: "StringSliceVar", - DefinedOn: []string{"dev", "run", "debug", "deploy"}, + DefinedOn: []string{"dev", "run", "debug", "deploy", "render"}, }, { Name: "toot", diff --git a/docs/content/en/docs/references/cli/_index.md b/docs/content/en/docs/references/cli/_index.md index 84953b3a558..1ef491ba6ee 100644 --- a/docs/content/en/docs/references/cli/_index.md +++ b/docs/content/en/docs/references/cli/_index.md @@ -654,6 +654,7 @@ The following options can be passed to any command: Options: -d, --default-repo='': Default repository value (overrides global config) -f, --filename='skaffold.yaml': Filename or URL to the pipeline file + -l, --label=[]: Add custom labels to deployed objects. Set multiple times for multiple labels --loud=false: Show the build logs and output -n, --namespace='': Run deployments in the specified namespace --output='': file to write rendered manifests to @@ -670,6 +671,7 @@ Env vars: * `SKAFFOLD_DEFAULT_REPO` (same as `--default-repo`) * `SKAFFOLD_FILENAME` (same as `--filename`) +* `SKAFFOLD_LABEL` (same as `--label`) * `SKAFFOLD_LOUD` (same as `--loud`) * `SKAFFOLD_NAMESPACE` (same as `--namespace`) * `SKAFFOLD_OUTPUT` (same as `--output`) diff --git a/integration/render_test.go b/integration/render_test.go index 60cb9490084..82cf746aed3 100644 --- a/integration/render_test.go +++ b/integration/render_test.go @@ -36,6 +36,7 @@ func TestKubectlRender(t *testing.T) { tests := []struct { description string builds []build.Artifact + labels []deploy.Labeller input string expectedOut string }{ @@ -47,6 +48,7 @@ func TestKubectlRender(t *testing.T) { Tag: "gcr.io/k8s-skaffold/skaffold:test", }, }, + labels: []deploy.Labeller{}, input: `apiVersion: v1 kind: Pod spec: @@ -57,6 +59,8 @@ spec: expectedOut: `apiVersion: v1 kind: Pod metadata: + labels: + skaffold.dev/deployer: kubectl namespace: default spec: containers: @@ -76,6 +80,7 @@ spec: Tag: "gcr.io/project/image2:tag2", }, }, + labels: []deploy.Labeller{}, input: `apiVersion: v1 kind: Pod spec: @@ -88,6 +93,8 @@ spec: expectedOut: `apiVersion: v1 kind: Pod metadata: + labels: + skaffold.dev/deployer: kubectl namespace: default spec: containers: @@ -126,6 +133,8 @@ spec: expectedOut: `apiVersion: v1 kind: Pod metadata: + labels: + skaffold.dev/deployer: kubectl namespace: default spec: containers: @@ -135,6 +144,8 @@ spec: apiVersion: v1 kind: Pod metadata: + labels: + skaffold.dev/deployer: kubectl namespace: default spec: containers: @@ -162,7 +173,7 @@ spec: }, }) var b bytes.Buffer - err := deployer.Render(context.Background(), &b, test.builds, "") + err := deployer.Render(context.Background(), &b, test.builds, test.labels, "") t.CheckNoError(err) t.CheckDeepEqual(test.expectedOut, b.String()) diff --git a/pkg/skaffold/deploy/deploy.go b/pkg/skaffold/deploy/deploy.go index c873127d476..43744323987 100644 --- a/pkg/skaffold/deploy/deploy.go +++ b/pkg/skaffold/deploy/deploy.go @@ -41,7 +41,7 @@ type Deployer interface { // Render generates the Kubernetes manifests replacing the build results and // writes them to the given file path - Render(context.Context, io.Writer, []build.Artifact, string) error + Render(context.Context, io.Writer, []build.Artifact, []Labeller, string) error } type Result struct { diff --git a/pkg/skaffold/deploy/deploy_mux.go b/pkg/skaffold/deploy/deploy_mux.go index 02ce3aee60f..71d4a701133 100644 --- a/pkg/skaffold/deploy/deploy_mux.go +++ b/pkg/skaffold/deploy/deploy_mux.go @@ -98,11 +98,11 @@ func (m DeployerMux) Cleanup(ctx context.Context, w io.Writer) error { return nil } -func (m DeployerMux) Render(ctx context.Context, w io.Writer, as []build.Artifact, filepath string) error { +func (m DeployerMux) Render(ctx context.Context, w io.Writer, as []build.Artifact, ls []Labeller, filepath string) error { resources, buf := []string{}, &bytes.Buffer{} for _, deployer := range m { buf.Reset() - if err := deployer.Render(ctx, buf, as, "" /* never write to files */); err != nil { + if err := deployer.Render(ctx, buf, as, ls, "" /* never write to files */); err != nil { return err } resources = append(resources, buf.String()) diff --git a/pkg/skaffold/deploy/deploy_mux_test.go b/pkg/skaffold/deploy/deploy_mux_test.go index fee6cb0eda7..eb9a6950889 100644 --- a/pkg/skaffold/deploy/deploy_mux_test.go +++ b/pkg/skaffold/deploy/deploy_mux_test.go @@ -49,7 +49,7 @@ func (m *MockDeployer) Deploy(context.Context, io.Writer, []build.Artifact, []La err: m.deployErr, } } -func (m *MockDeployer) Render(_ context.Context, w io.Writer, _ []build.Artifact, _ string) error { +func (m *MockDeployer) Render(_ context.Context, w io.Writer, _ []build.Artifact, _ []Labeller, _ string) error { w.Write([]byte(m.renderResult)) return m.renderErr } @@ -251,7 +251,7 @@ func TestDeployerMux_Render(t *testing.T) { }) buf := &bytes.Buffer{} - err := deployerMux.Render(context.Background(), buf, nil, "") + err := deployerMux.Render(context.Background(), buf, nil, nil, "") testutil.CheckErrorAndDeepEqual(t, test.shouldErr, err, test.expectedRender, buf.String()) }) } @@ -268,7 +268,7 @@ func TestDeployerMux_Render(t *testing.T) { NewMockDeployer().WithRenderResult(test.render2).WithRenderErr(test.err2), }) - err := deployerMux.Render(context.Background(), nil, nil, tempDir.Path("render")) + err := deployerMux.Render(context.Background(), nil, nil, nil, tempDir.Path("render")) testutil.CheckError(t, false, err) file, _ := os.Open(tempDir.Path("render")) diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index a0b200b818c..9e23869877d 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -508,7 +508,7 @@ func generateGetFilesArgs(m map[string]string, valuesSet map[string]bool) []stri return args } -func (h *HelmDeployer) Render(context.Context, io.Writer, []build.Artifact, string) error { +func (h *HelmDeployer) Render(context.Context, io.Writer, []build.Artifact, []Labeller, string) error { return errors.New("not yet implemented") } diff --git a/pkg/skaffold/deploy/helm_test.go b/pkg/skaffold/deploy/helm_test.go index 93ca9b8f847..22972e3e218 100644 --- a/pkg/skaffold/deploy/helm_test.go +++ b/pkg/skaffold/deploy/helm_test.go @@ -867,7 +867,7 @@ func TestHelmRender(t *testing.T) { for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { deployer := NewHelmDeployer(&runcontext.RunContext{}) - actual := deployer.Render(context.Background(), ioutil.Discard, []build.Artifact{}, "tmp/dir") + actual := deployer.Render(context.Background(), ioutil.Discard, []build.Artifact{}, nil, "tmp/dir") t.CheckError(test.shouldErr, actual) }) } diff --git a/pkg/skaffold/deploy/kubectl.go b/pkg/skaffold/deploy/kubectl.go index 68c316c024a..db3ebcd1e6d 100644 --- a/pkg/skaffold/deploy/kubectl.go +++ b/pkg/skaffold/deploy/kubectl.go @@ -72,7 +72,7 @@ func (k *KubectlDeployer) Labels() map[string]string { // runs `kubectl apply` on those manifests func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) *Result { event.DeployInProgress() - manifests, err := k.renderManifests(ctx, out, builds) + manifests, err := k.renderManifests(ctx, out, builds, labellers) if err != nil { event.DeployFailed(err) @@ -84,12 +84,6 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu return NewDeploySuccessResult(nil) } - manifests, err = manifests.SetLabels(merge(k, labellers...)) - if err != nil { - event.DeployFailed(err) - return NewDeployErrorResult(errors.Wrap(err, "setting labels in manifests")) - } - namespaces, err := manifests.CollectNamespaces() if err != nil { event.DeployInfoEvent(errors.Wrap(err, "could not fetch deployed resource namespace. "+ @@ -216,8 +210,8 @@ func (k *KubectlDeployer) readRemoteManifest(ctx context.Context, name string) ( return manifest.Bytes(), nil } -func (k *KubectlDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, filepath string) error { - manifests, err := k.renderManifests(ctx, out, builds) +func (k *KubectlDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller, filepath string) error { + manifests, err := k.renderManifests(ctx, out, builds, labellers) if err != nil { return err } @@ -225,7 +219,7 @@ func (k *KubectlDeployer) Render(ctx context.Context, out io.Writer, builds []bu return dumpToFileOrWriter(manifests.String(), filepath, out) } -func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, builds []build.Artifact) (deploy.ManifestList, error) { +func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller) (deploy.ManifestList, error) { if err := k.kubectl.CheckVersion(ctx); err != nil { color.Default.Fprintln(out, "kubectl client version:", k.kubectl.Version(ctx)) color.Default.Fprintln(out, err) @@ -268,5 +262,10 @@ func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, bu } } + manifests, err = manifests.SetLabels(merge(k, labellers...)) + if err != nil { + return nil, errors.Wrap(err, "setting labels in manifests") + } + return manifests, nil } diff --git a/pkg/skaffold/deploy/kubectl_test.go b/pkg/skaffold/deploy/kubectl_test.go index 4f15c010978..9c5ded9e119 100644 --- a/pkg/skaffold/deploy/kubectl_test.go +++ b/pkg/skaffold/deploy/kubectl_test.go @@ -481,7 +481,9 @@ func TestKubectlRender(t *testing.T) { tests := []struct { description string builds []build.Artifact + labels []Labeller input string + expected string }{ { description: "normal render", @@ -491,12 +493,26 @@ func TestKubectlRender(t *testing.T) { Tag: "gcr.io/k8s-skaffold/skaffold:test", }, }, + labels: []Labeller{}, input: `apiVersion: v1 kind: Pod +metadata: + namespace: default spec: containers: - image: gcr.io/k8s-skaffold/skaffold name: skaffold +`, + expected: `apiVersion: v1 +kind: Pod +metadata: + labels: + skaffold.dev/deployer: kubectl + namespace: default +spec: + containers: + - image: gcr.io/k8s-skaffold/skaffold:test + name: skaffold `, }, { @@ -512,21 +528,39 @@ spec: }, }, input: `apiVersion: v1 - kind: Pod - spec: - containers: - - image: gcr.io/project/image1 - name: image1 - - image: gcr.io/project/image2 - name: image2 - `, +kind: Pod +metadata: + namespace: default +spec: + containers: + - image: gcr.io/project/image1 + name: image1 + - image: gcr.io/project/image2 + name: image2 +`, + expected: `apiVersion: v1 +kind: Pod +metadata: + labels: + skaffold.dev/deployer: kubectl + namespace: default +spec: + containers: + - image: gcr.io/project/image1:tag1 + name: image1 + - image: gcr.io/project/image2:tag2 + name: image2 +`, }, } for _, test := range tests { testutil.Run(t, test.description, func(t *testutil.T) { + tmpDir := t.NewTempDir(). + Write("deployment.yaml", test.input) + t.Override(&util.DefaultExecCommand, testutil. CmdRunOut("kubectl version --client -ojson", kubectlVersion). - AndRunOut("kubectl --context kubecontext create --dry-run -oyaml -f deployment.yaml", test.input)) + AndRunOut("kubectl --context kubecontext create --dry-run -oyaml -f "+tmpDir.Path("deployment.yaml"), test.input)) deployer := NewKubectlDeployer(&runcontext.RunContext{ WorkingDir: ".", @@ -534,7 +568,7 @@ spec: Deploy: latest.DeployConfig{ DeployType: latest.DeployType{ KubectlDeploy: &latest.KubectlDeploy{ - Manifests: []string{"deployment.yaml"}, + Manifests: []string{tmpDir.Path("deployment.yaml")}, }, }, }, @@ -542,8 +576,9 @@ spec: KubeContext: testKubeContext, }) var b bytes.Buffer - err := deployer.Render(context.Background(), &b, test.builds, "") + err := deployer.Render(context.Background(), &b, test.builds, test.labels, "") t.CheckNoError(err) + t.CheckDeepEqual(test.expected, b.String()) }) } } diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index 2616f889977..12718eeb891 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -184,8 +184,8 @@ func (k *KustomizeDeployer) Dependencies() ([]string, error) { return deps.toList(), nil } -func (k *KustomizeDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, filepath string) error { - manifests, err := k.renderManifests(ctx, out, builds, nil) +func (k *KustomizeDeployer) Render(ctx context.Context, out io.Writer, builds []build.Artifact, labellers []Labeller, filepath string) error { + manifests, err := k.renderManifests(ctx, out, builds, labellers) if err != nil { return err } diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index 4c125a7bb19..0614ab66187 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -434,6 +434,13 @@ func TestKustomizeBuildCommandArgs(t *testing.T) { } } +type testLabels struct { + labels map[string]string +} + +func (t *testLabels) Labels() map[string]string { + return t.labels +} func TestKustomizeRender(t *testing.T) { type kustomizationCall struct { folder string @@ -442,8 +449,10 @@ func TestKustomizeRender(t *testing.T) { tests := []struct { description string builds []build.Artifact + labels []Labeller kustomizations []kustomizationCall expected string + shouldErr bool }{ { description: "single kustomization", @@ -462,6 +471,8 @@ func TestKustomizeRender(t *testing.T) { folder: ".", buildResult: `apiVersion: v1 kind: Pod +metadata: + namespace: default spec: containers: - image: gcr.io/project/image1 @@ -473,6 +484,59 @@ spec: }, expected: `apiVersion: v1 kind: Pod +metadata: + labels: + skaffold.dev/deployer: kustomize + namespace: default +spec: + containers: + - image: gcr.io/project/image1:tag1 + name: image1 + - image: gcr.io/project/image2:tag2 + name: image2 +`, + }, + { + description: "single kustomization with user labels", + builds: []build.Artifact{ + { + ImageName: "gcr.io/project/image1", + Tag: "gcr.io/project/image1:tag1", + }, + { + ImageName: "gcr.io/project/image2", + Tag: "gcr.io/project/image2:tag2", + }, + }, + labels: []Labeller{ + &testLabels{ + labels: map[string]string{ + "user/label": "test", + }}, + }, + kustomizations: []kustomizationCall{ + { + folder: ".", + buildResult: `apiVersion: v1 +kind: Pod +metadata: + namespace: default +spec: + containers: + - image: gcr.io/project/image1 + name: image1 + - image: gcr.io/project/image2 + name: image2 +`, + }, + }, + expected: `apiVersion: v1 +kind: Pod +metadata: + labels: + skaffold.dev/deployer: kustomize + user/label: test + namespace: default spec: containers: - image: gcr.io/project/image1:tag1 @@ -498,6 +562,8 @@ spec: folder: "a", buildResult: `apiVersion: v1 kind: Pod +metadata: + namespace: default spec: containers: - image: gcr.io/project/image1 @@ -508,6 +574,8 @@ spec: folder: "b", buildResult: `apiVersion: v1 kind: Pod +metadata: + namespace: default spec: containers: - image: gcr.io/project/image2 @@ -517,6 +585,10 @@ spec: }, expected: `apiVersion: v1 kind: Pod +metadata: + labels: + skaffold.dev/deployer: kustomize + namespace: default spec: containers: - image: gcr.io/project/image1:tag1 @@ -524,6 +596,10 @@ spec: --- apiVersion: v1 kind: Pod +metadata: + labels: + skaffold.dev/deployer: kustomize + namespace: default spec: containers: - image: gcr.io/project/image2:tag2 @@ -561,8 +637,8 @@ spec: }, }) var b bytes.Buffer - err := k.Render(context.Background(), &b, test.builds, "") - t.CheckError(false, err) + err := k.Render(context.Background(), &b, test.builds, test.labels, "") + t.CheckError(test.shouldErr, err) t.CheckDeepEqual(test.expected, b.String()) }) } diff --git a/pkg/skaffold/runner/render.go b/pkg/skaffold/runner/render.go index 6e278137a21..64978779260 100644 --- a/pkg/skaffold/runner/render.go +++ b/pkg/skaffold/runner/render.go @@ -24,5 +24,5 @@ import ( ) func (r *SkaffoldRunner) Render(ctx context.Context, out io.Writer, builds []build.Artifact, filepath string) error { - return r.deployer.Render(ctx, out, builds, filepath) + return r.deployer.Render(ctx, out, builds, r.labellers, filepath) } diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index b4a7b90121a..76a0a99e84f 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -164,7 +164,7 @@ func (t *TestBench) Deploy(_ context.Context, _ io.Writer, artifacts []build.Art return deploy.NewDeploySuccessResult(t.namespaces) } -func (t *TestBench) Render(_ context.Context, _ io.Writer, artifacts []build.Artifact, _ string) error { +func (t *TestBench) Render(_ context.Context, _ io.Writer, artifacts []build.Artifact, _ []deploy.Labeller, _ string) error { return nil }