[kubectl] apply labels by patching

Signed-off-by: David Gageot <david@gageot.net>

pkg/skaffold/deploy/deploy.go

@@ -21,15 +21,8 @@ import (
 	"io"
 
 	"github.com/GoogleContainerTools/skaffold/pkg/skaffold/build"
-	"k8s.io/apimachinery/pkg/runtime"
 )
 
-// Artifact contains all information about a completed deployment
-type Artifact struct {
-	Obj       runtime.Object
-	Namespace string
-}
-
 // Deployer is the Deploy API of skaffold and responsible for deploying
 // the build results to a Kubernetes cluster
 type Deployer interface {

pkg/skaffold/deploy/kubectl.go

@@ -17,8 +17,6 @@ limitations under the License.
 package deploy
 
 import (
-	"bufio"
-	"bytes"
 	"context"
 	"io"
 
@@ -84,16 +82,12 @@ func (k *KubectlDeployer) Deploy(ctx context.Context, out io.Writer, builds []bu
 		return errors.Wrap(err, "replacing images in manifests")
 	}
 
-	updated, err := k.kubectl.Apply(ctx, out, manifests)
+	manifests, err = manifests.SetLabels(merge(labellers...))
 	if err != nil {
-		return errors.Wrap(err, "apply")
+		return errors.Wrap(err, "setting labels in manifests")
 	}
 
-	dRes := parseManifestsForDeploys(k.kubectl.Namespace, updated)
-	labels := merge(labellers...)
-	labelDeployResults(labels, dRes)
-
-	return nil
+	return k.kubectl.Apply(ctx, out, manifests)
 }
 
 // Cleanup deletes what was deployed by calling Deploy.
@@ -135,17 +129,6 @@ func (k *KubectlDeployer) manifestFiles(manifests []string) ([]string, error) {
 	return filteredManifests, nil
 }
 
-func parseManifestsForDeploys(namespace string, manifests kubectl.ManifestList) []Artifact {
-	var results []Artifact
-
-	for _, manifest := range manifests {
-		b := bufio.NewReader(bytes.NewReader(manifest))
-		results = append(results, parseReleaseInfo(namespace, b)...)
-	}
-
-	return results
-}
-
 // readManifests reads the manifests to deploy/delete.
 func (k *KubectlDeployer) readManifests(ctx context.Context) (kubectl.ManifestList, error) {
 	manifests, err := k.Dependencies()

pkg/skaffold/deploy/kubectl/cli.go

@@ -49,22 +49,22 @@ func (c *CLI) Delete(ctx context.Context, out io.Writer, manifests ManifestList)
 }
 
 // Apply runs `kubectl apply` on a list of manifests.
-func (c *CLI) Apply(ctx context.Context, out io.Writer, manifests ManifestList) (ManifestList, error) {
+func (c *CLI) Apply(ctx context.Context, out io.Writer, manifests ManifestList) error {
 	// Only redeploy modified or new manifests
 	// TODO(dgageot): should we delete a manifest that was deployed and is not anymore?
 	updated := c.previousApply.Diff(manifests)
 	logrus.Debugln(len(manifests), "manifests to deploy.", len(updated), "are updated or new")
 	c.previousApply = manifests
 	if len(updated) == 0 {
-		return nil, nil
+		return nil
 	}
 
 	// Add --force flag to delete and redeploy image if changes can't be applied
 	if err := c.Run(ctx, updated.Reader(), out, "apply", c.Flags.Apply, "--force", "-f", "-"); err != nil {
-		return nil, errors.Wrap(err, "kubectl apply")
+		return errors.Wrap(err, "kubectl apply")
 	}
 
-	return updated, nil
+	return nil
 }
 
 // ReadManifests reads a list of manifests in yaml format.

pkg/skaffold/deploy/kubectl/labels.go

@@ -0,0 +1,78 @@
+/*
+Copyright 2018 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 kubectl
+
+import (
+	"github.com/pkg/errors"
+	"github.com/sirupsen/logrus"
+)
+
+// SetLabels add labels to a list of Kubernetes manifests.
+func (l *ManifestList) SetLabels(labels map[string]string) (ManifestList, error) {
+	replacer := newLabelsSetter(labels)
+
+	updated, err := l.Visit(replacer)
+	if err != nil {
+		return nil, errors.Wrap(err, "setting labels")
+	}
+
+	logrus.Debugln("manifests with labels", updated.String())
+
+	return updated, nil
+}
+
+type labelsSetter struct {
+	labels map[string]string
+}
+
+func newLabelsSetter(labels map[string]string) *labelsSetter {
+	return &labelsSetter{
+		labels: labels,
+	}
+}
+
+func (r *labelsSetter) Matches(key string) bool {
+	return "metadata" == key
+}
+
+func (r *labelsSetter) NewValue(old interface{}) (bool, interface{}) {
+	if len(r.labels) == 0 {
+		return false, nil
+	}
+
+	metadata, ok := old.(map[interface{}]interface{})
+	if !ok {
+		return false, nil
+	}
+
+	l, present := metadata["labels"]
+	if !present {
+		metadata["labels"] = r.labels
+		return true, metadata
+	}
+
+	labels, ok := l.(map[interface{}]interface{})
+	if !ok {
+		return false, nil
+	}
+
+	for k, v := range r.labels {
+		labels[k] = v
+	}
+
+	return true, metadata
+}

pkg/skaffold/deploy/kubectl/labels_test.go

@@ -0,0 +1,123 @@
+/*
+Copyright 2018 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 kubectl
+
+import (
+	"testing"
+
+	"github.com/GoogleContainerTools/skaffold/testutil"
+)
+
+func TestSetLabels(t *testing.T) {
+	manifests := ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)}
+
+	expected := ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    key1: value1
+    key2: value2
+  name: getting-started
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)}
+
+	resultManifest, err := manifests.SetLabels(map[string]string{
+		"key1": "value1",
+		"key2": "value2",
+	})
+
+	testutil.CheckErrorAndDeepEqual(t, false, err, expected.String(), resultManifest.String())
+}
+
+func TestAddLabels(t *testing.T) {
+	manifests := ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    key0: value0
+    key1: ignored
+  name: getting-started
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)}
+
+	expected := ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  labels:
+    key0: value0
+    key1: value1
+    key2: value2
+  name: getting-started
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)}
+
+	resultManifest, err := manifests.SetLabels(map[string]string{
+		"key1": "value1",
+		"key2": "value2",
+	})
+
+	testutil.CheckErrorAndDeepEqual(t, false, err, expected.String(), resultManifest.String())
+}
+
+func TestSetNoLabel(t *testing.T) {
+	manifests := ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)}
+
+	expected := ManifestList{[]byte(`
+apiVersion: v1
+kind: Pod
+metadata:
+  name: getting-started
+spec:
+  containers:
+  - image: gcr.io/k8s-skaffold/example
+    name: example
+`)}
+
+	resultManifest, err := manifests.SetLabels(nil)
+
+	testutil.CheckErrorAndDeepEqual(t, false, err, expected.String(), resultManifest.String())
+}

pkg/skaffold/deploy/kubectl_test.go

@@ -222,6 +222,8 @@ func TestKubectlRedeploy(t *testing.T) {
 		WithRunInput("kubectl --context kubecontext --namespace testNamespace apply --force -f -", `apiVersion: v1
 kind: Pod
 metadata:
+  labels:
+    skaffold-deployer: kubectl
   name: leeroy-app
 spec:
   containers:
@@ -231,6 +233,8 @@ spec:
 apiVersion: v1
 kind: Pod
 metadata:
+  labels:
+    skaffold-deployer: kubectl
   name: leeroy-web
 spec:
   containers:
@@ -240,6 +244,8 @@ spec:
 		WithRunInput("kubectl --context kubecontext --namespace testNamespace apply --force -f -", `apiVersion: v1
 kind: Pod
 metadata:
+  labels:
+    skaffold-deployer: kubectl
   name: leeroy-app
 spec:
   containers:
@@ -251,25 +257,26 @@ spec:
 		Manifests: []string{"*.yaml"},
 	}
 	deployer := NewKubectlDeployer(tmpDir.Root(), cfg, testKubeContext, testNamespace, "")
+	labellers := []Labeller{deployer}
 
 	// Deploy one manifest
 	err := deployer.Deploy(context.Background(), ioutil.Discard, []build.Artifact{
 		{ImageName: "leeroy-web", Tag: "leeroy-web:v1"},
 		{ImageName: "leeroy-app", Tag: "leeroy-app:v1"},
-	}, nil)
+	}, labellers)
 	testutil.CheckError(t, false, err)
 
 	// Deploy one manifest since only one image is updated
 	err = deployer.Deploy(context.Background(), ioutil.Discard, []build.Artifact{
 		{ImageName: "leeroy-web", Tag: "leeroy-web:v1"},
 		{ImageName: "leeroy-app", Tag: "leeroy-app:v2"},
-	}, nil)
+	}, labellers)
 	testutil.CheckError(t, false, err)
 
 	// Deploy zero manifest since no image is updated
 	err = deployer.Deploy(context.Background(), ioutil.Discard, []build.Artifact{
 		{ImageName: "leeroy-web", Tag: "leeroy-web:v1"},
 		{ImageName: "leeroy-app", Tag: "leeroy-app:v2"},
-	}, nil)
+	}, labellers)
 	testutil.CheckError(t, false, err)
 }

pkg/skaffold/deploy/kustomize.go

@@ -105,16 +105,12 @@ func (k *KustomizeDeployer) Deploy(ctx context.Context, out io.Writer, builds []
 		return errors.Wrap(err, "replacing images in manifests")
 	}
 
-	updated, err := k.kubectl.Apply(ctx, out, manifests)
+	manifests, err = manifests.SetLabels(merge(labellers...))
 	if err != nil {
-		return errors.Wrap(err, "apply")
+		return errors.Wrap(err, "setting labels in manifests")
 	}
 
-	dRes := parseManifestsForDeploys(k.kubectl.Namespace, updated)
-	labels := merge(labellers...)
-	labelDeployResults(labels, dRes)
-
-	return nil
+	return k.kubectl.Apply(ctx, out, manifests)
 }
 
 // Cleanup deletes what was deployed by calling Deploy.

pkg/skaffold/deploy/labels.go

@@ -29,13 +29,20 @@ import (
 	"github.com/sirupsen/logrus"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/runtime"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
 	patch "k8s.io/apimachinery/pkg/util/strategicpatch"
 	"k8s.io/client-go/discovery"
 	"k8s.io/client-go/dynamic"
 )
 
+// Artifact contains all information about a completed deployment
+type Artifact struct {
+	Obj       runtime.Object
+	Namespace string
+}
+
 // Labeller can give key/value labels to set on deployed resources.
 type Labeller interface {
 	Labels() map[string]string