Add labels to render (GoogleContainerTools#3558)

* Add labeller to render Signed-off-by: Armin Buerkle <armin.buerkle@alfatraining.de> * Add labeller to integration test Signed-off-by: Armin Buerkle <armin.buerkle@alfatraining.de> * Add kustomize render labeller unit test Signed-off-by: Armin Buerkle <armin.buerkle@alfatraining.de> * Add kubectl render labeller unit test Signed-off-by: Armin Buerkle <armin.buerkle@alfatraining.de> * rebase from master and add one more test Co-authored-by: Tejal Desai <tejal29@gmail.com>
kissmikijr · Feb 3, 2020 · 2365c31 · 2365c31
1 parent 1a6ac11
commit 2365c31
Show file tree

Hide file tree

Showing 14 changed files with 160 additions and 37 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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,16 +210,16 @@ 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
 	}
 
 	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
@@ -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,38 +528,57 @@ 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: ".",
 				Cfg: latest.Pipeline{
 					Deploy: latest.DeployConfig{
 						DeployType: latest.DeployType{
 							KubectlDeploy: &latest.KubectlDeploy{
-								Manifests: []string{"deployment.yaml"},
+								Manifests: []string{tmpDir.Path("deployment.yaml")},
 							},
 						},
 					},
 				},
 				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
@@ -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
 	}