From 5ecd3b907b67691a443e9ab8fefc93971d31a7dc Mon Sep 17 00:00:00 2001 From: Nick Kubala Date: Fri, 10 Jul 2020 11:08:57 -0700 Subject: [PATCH] Use annotations instead of labels on created k8s resources --- integration/render_test.go | 8 ++-- pkg/skaffold/build/build.go | 2 + pkg/skaffold/build/cluster/types.go | 4 ++ pkg/skaffold/build/cluster/types_test.go | 3 +- pkg/skaffold/build/gcb/types.go | 4 ++ pkg/skaffold/build/local/types.go | 4 ++ pkg/skaffold/build/tag/custom.go | 4 ++ pkg/skaffold/build/tag/date_time.go | 6 ++- pkg/skaffold/build/tag/env_template.go | 4 ++ pkg/skaffold/build/tag/git_commit.go | 5 +++ pkg/skaffold/build/tag/sha256.go | 5 +++ pkg/skaffold/build/tag/tag.go | 2 + pkg/skaffold/deploy/helm.go | 8 +++- pkg/skaffold/deploy/kubectl.go | 6 ++- pkg/skaffold/deploy/kubectl/labels.go | 47 +++++++++++++--------- pkg/skaffold/deploy/kubectl/labels_test.go | 43 +++++++++++++++----- pkg/skaffold/deploy/kubectl_test.go | 12 +++--- pkg/skaffold/deploy/kustomize.go | 6 ++- pkg/skaffold/deploy/kustomize_test.go | 17 +++++--- pkg/skaffold/deploy/labeller.go | 27 ++++++++----- pkg/skaffold/deploy/labels.go | 35 +++++++++++----- pkg/skaffold/deploy/labels_test.go | 38 +++++++++++++++-- pkg/skaffold/runner/runner_test.go | 1 + pkg/skaffold/sync/sync_test.go | 4 +- 24 files changed, 221 insertions(+), 74 deletions(-) diff --git a/integration/render_test.go b/integration/render_test.go index 773081848af..f570a056788 100644 --- a/integration/render_test.go +++ b/integration/render_test.go @@ -68,7 +68,7 @@ spec: expectedOut: `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl name: my-pod-123 namespace: default @@ -105,7 +105,7 @@ spec: expectedOut: `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl name: my-pod-123 namespace: default @@ -150,7 +150,7 @@ spec: expectedOut: `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl name: my-pod-123 namespace: default @@ -162,7 +162,7 @@ spec: apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl name: my-pod-456 namespace: default diff --git a/pkg/skaffold/build/build.go b/pkg/skaffold/build/build.go index 75d00909a10..1f433070235 100644 --- a/pkg/skaffold/build/build.go +++ b/pkg/skaffold/build/build.go @@ -37,6 +37,8 @@ type Artifact struct { type Builder interface { Labels() map[string]string + Annotations() map[string]string + Build(ctx context.Context, out io.Writer, tags tag.ImageTags, artifacts []*latest.Artifact) ([]Artifact, error) // Prune removes images built with Skaffold diff --git a/pkg/skaffold/build/cluster/types.go b/pkg/skaffold/build/cluster/types.go index 8a4a4a4f319..623f4b58991 100644 --- a/pkg/skaffold/build/cluster/types.go +++ b/pkg/skaffold/build/cluster/types.go @@ -56,6 +56,10 @@ func NewBuilder(runCtx *runcontext.RunContext) (*Builder, error) { // Labels are labels specific to cluster builder. func (b *Builder) Labels() map[string]string { + return map[string]string{} +} + +func (b *Builder) Annotations() map[string]string { return map[string]string{ constants.Labels.Builder: "cluster", } diff --git a/pkg/skaffold/build/cluster/types_test.go b/pkg/skaffold/build/cluster/types_test.go index 3372317f5f9..a86f0c6bdb0 100644 --- a/pkg/skaffold/build/cluster/types_test.go +++ b/pkg/skaffold/build/cluster/types_test.go @@ -106,7 +106,8 @@ func TestNewBuilder(t *testing.T) { } func TestLabels(t *testing.T) { - testutil.CheckDeepEqual(t, map[string]string{"skaffold.dev/builder": "cluster"}, (&Builder{}).Labels()) + testutil.CheckDeepEqual(t, map[string]string{}, (&Builder{}).Labels()) + testutil.CheckDeepEqual(t, map[string]string{"skaffold.dev/builder": "cluster"}, (&Builder{}).Annotations()) } func TestPruneIsNoop(t *testing.T) { diff --git a/pkg/skaffold/build/gcb/types.go b/pkg/skaffold/build/gcb/types.go index 7da63fe4a40..b58802b255e 100644 --- a/pkg/skaffold/build/gcb/types.go +++ b/pkg/skaffold/build/gcb/types.go @@ -93,6 +93,10 @@ func NewBuilder(runCtx *runcontext.RunContext) *Builder { // Labels are labels specific to Google Cloud Build. func (b *Builder) Labels() map[string]string { + return nil +} + +func (b *Builder) Annotations() map[string]string { return map[string]string{ constants.Labels.Builder: "google-cloud-build", } diff --git a/pkg/skaffold/build/local/types.go b/pkg/skaffold/build/local/types.go index 4b32ad55c6d..dd071b5c0ce 100644 --- a/pkg/skaffold/build/local/types.go +++ b/pkg/skaffold/build/local/types.go @@ -95,6 +95,10 @@ func (b *Builder) PushImages() bool { // Labels are labels specific to local builder. func (b *Builder) Labels() map[string]string { + return map[string]string{} +} + +func (b *Builder) Annotations() map[string]string { labels := map[string]string{ constants.Labels.Builder: "local", } diff --git a/pkg/skaffold/build/tag/custom.go b/pkg/skaffold/build/tag/custom.go index dcf8e515859..5626fccf566 100644 --- a/pkg/skaffold/build/tag/custom.go +++ b/pkg/skaffold/build/tag/custom.go @@ -28,6 +28,10 @@ type CustomTag struct { } func (c *CustomTag) Labels() map[string]string { + return map[string]string{} +} + +func (c *CustomTag) Annotations() map[string]string { return map[string]string{ constants.Labels.TagPolicy: "custom", } diff --git a/pkg/skaffold/build/tag/date_time.go b/pkg/skaffold/build/tag/date_time.go index 064b33ccfc1..386fc31230a 100644 --- a/pkg/skaffold/build/tag/date_time.go +++ b/pkg/skaffold/build/tag/date_time.go @@ -44,7 +44,11 @@ func NewDateTimeTagger(format, timezone string) Tagger { } } -func (tagger *dateTimeTagger) Labels() map[string]string { +func (t *dateTimeTagger) Labels() map[string]string { + return map[string]string{} +} + +func (t *dateTimeTagger) Annotations() map[string]string { return map[string]string{ constants.Labels.TagPolicy: "dateTimeTagger", } diff --git a/pkg/skaffold/build/tag/env_template.go b/pkg/skaffold/build/tag/env_template.go index 083bbbc9619..1aa0941a30d 100644 --- a/pkg/skaffold/build/tag/env_template.go +++ b/pkg/skaffold/build/tag/env_template.go @@ -44,6 +44,10 @@ func NewEnvTemplateTagger(t string) (Tagger, error) { } func (t *envTemplateTagger) Labels() map[string]string { + return map[string]string{} +} + +func (t *envTemplateTagger) Annotations() map[string]string { return map[string]string{ constants.Labels.TagPolicy: "envTemplateTagger", } diff --git a/pkg/skaffold/build/tag/git_commit.go b/pkg/skaffold/build/tag/git_commit.go index d4ddf0407ed..51e8f0f3e9a 100644 --- a/pkg/skaffold/build/tag/git_commit.go +++ b/pkg/skaffold/build/tag/git_commit.go @@ -60,6 +60,11 @@ func NewGitCommit(prefix, variant string) (*GitCommit, error) { // Labels are labels specific to the git tagger. func (c *GitCommit) Labels() map[string]string { + return map[string]string{} +} + +// Labels are labels specific to the git tagger. +func (c *GitCommit) Annotations() map[string]string { return map[string]string{ constants.Labels.TagPolicy: "git-commit", } diff --git a/pkg/skaffold/build/tag/sha256.go b/pkg/skaffold/build/tag/sha256.go index c0b4e4f4115..34084cd7729 100644 --- a/pkg/skaffold/build/tag/sha256.go +++ b/pkg/skaffold/build/tag/sha256.go @@ -26,6 +26,11 @@ type ChecksumTagger struct{} // Labels are labels specific to the sha256 tagger. func (c *ChecksumTagger) Labels() map[string]string { + return map[string]string{} +} + +// Labels are labels specific to the sha256 tagger. +func (c *ChecksumTagger) Annotations() map[string]string { return map[string]string{ constants.Labels.TagPolicy: "sha256", } diff --git a/pkg/skaffold/build/tag/tag.go b/pkg/skaffold/build/tag/tag.go index 879898a0772..b57da44cb3d 100644 --- a/pkg/skaffold/build/tag/tag.go +++ b/pkg/skaffold/build/tag/tag.go @@ -24,6 +24,8 @@ type Tagger interface { // Labels produces labels to indicate the used tagger in deployed pods. Labels() map[string]string + Annotations() map[string]string + // GenerateFullyQualifiedImageName resolves the fully qualified image name for an artifact. // The workingDir is the root directory of the artifact with respect to the Skaffold root, // and imageName is the base name of the image. diff --git a/pkg/skaffold/deploy/helm.go b/pkg/skaffold/deploy/helm.go index 57b362ee3e3..ff58237925e 100644 --- a/pkg/skaffold/deploy/helm.go +++ b/pkg/skaffold/deploy/helm.go @@ -92,6 +92,10 @@ func NewHelmDeployer(runCtx *runcontext.RunContext) *HelmDeployer { // Labels returns the Kubernetes labels used by this deployer func (h *HelmDeployer) Labels() map[string]string { + return map[string]string{} +} + +func (h *HelmDeployer) Annotations() map[string]string { return map[string]string{ constants.Labels.Deployer: "helm", } @@ -143,8 +147,8 @@ func (h *HelmDeployer) Deploy(ctx context.Context, out io.Writer, builds []build event.DeployComplete() - labels := merge(h.addSkaffoldLabels, h, labellers...) - labelDeployResults(labels, dRes) + labels, annotations := merge(h.addSkaffoldLabels, h, labellers...) + labelAndAnnotateDeployResults(labels, annotations, dRes) // Collect namespaces in a string namespaces := make([]string, 0, len(nsMap)) diff --git a/pkg/skaffold/deploy/kubectl.go b/pkg/skaffold/deploy/kubectl.go index 34aa10cfed0..dc79868a708 100644 --- a/pkg/skaffold/deploy/kubectl.go +++ b/pkg/skaffold/deploy/kubectl.go @@ -76,6 +76,10 @@ func NewKubectlDeployer(runCtx *runcontext.RunContext) *KubectlDeployer { } func (k *KubectlDeployer) Labels() map[string]string { + return map[string]string{} +} + +func (k *KubectlDeployer) Annotations() map[string]string { return map[string]string{ constants.Labels.Deployer: "kubectl", } @@ -298,7 +302,7 @@ func (k *KubectlDeployer) renderManifests(ctx context.Context, out io.Writer, bu } } - manifests, err = manifests.SetLabels(merge(k.addSkaffoldLabels, k, labellers...)) + manifests, err = manifests.SetLabelsAndAnnotations(merge(k.addSkaffoldLabels, k, labellers...)) if err != nil { return nil, fmt.Errorf("setting labels in manifests: %w", err) } diff --git a/pkg/skaffold/deploy/kubectl/labels.go b/pkg/skaffold/deploy/kubectl/labels.go index 8ef369780ec..fa4a8924b57 100644 --- a/pkg/skaffold/deploy/kubectl/labels.go +++ b/pkg/skaffold/deploy/kubectl/labels.go @@ -23,8 +23,8 @@ import ( ) // SetLabels add labels to a list of Kubernetes manifests. -func (l *ManifestList) SetLabels(labels map[string]string) (ManifestList, error) { - replacer := newLabelsSetter(labels) +func (l *ManifestList) SetLabelsAndAnnotations(labels, annotations map[string]string) (ManifestList, error) { + replacer := newLabelsAndAnnotationsSetter(labels, annotations) updated, err := l.Visit(replacer) if err != nil { @@ -36,45 +36,54 @@ func (l *ManifestList) SetLabels(labels map[string]string) (ManifestList, error) return updated, nil } -type labelsSetter struct { - labels map[string]string +type labelsAndAnnotationsSetter struct { + labels map[string]string + annotations map[string]string } -func newLabelsSetter(labels map[string]string) *labelsSetter { - return &labelsSetter{ - labels: labels, +func newLabelsAndAnnotationsSetter(labels, annotations map[string]string) *labelsAndAnnotationsSetter { + return &labelsAndAnnotationsSetter{ + labels: labels, + annotations: annotations, } } -func (r *labelsSetter) Visit(o map[string]interface{}, k string, v interface{}) bool { +func (r *labelsAndAnnotationsSetter) Visit(o map[string]interface{}, k string, v interface{}) bool { if k != "metadata" { return true } - if len(r.labels) == 0 { - return false - } - metadata, ok := v.(map[string]interface{}) if !ok { return true } - l, present := metadata["labels"] + if visitAndReplace("labels", r.labels, &metadata) { + return true + } + return visitAndReplace("annotations", r.annotations, &metadata) +} + +func visitAndReplace(fieldName string, values map[string]string, metadata *map[string]interface{}) bool { + if len(values) == 0 { + return false + } + + field, present := (*metadata)[fieldName] if !present { - metadata["labels"] = r.labels + (*metadata)[fieldName] = values return false } - labels, ok := l.(map[string]interface{}) + existingValues, ok := field.(map[string]interface{}) if !ok { return true } - for k, v := range r.labels { - // Don't replace existing label. - if _, present := labels[k]; !present { - labels[k] = v + for k, v := range values { + // Don't overwrite existing values + if _, present := existingValues[k]; !present { + existingValues[k] = v } } diff --git a/pkg/skaffold/deploy/kubectl/labels_test.go b/pkg/skaffold/deploy/kubectl/labels_test.go index b62b116652c..475b3e20369 100644 --- a/pkg/skaffold/deploy/kubectl/labels_test.go +++ b/pkg/skaffold/deploy/kubectl/labels_test.go @@ -22,7 +22,7 @@ import ( "github.com/GoogleContainerTools/skaffold/testutil" ) -func TestSetLabels(t *testing.T) { +func TestSetLabelsAndAnnotations(t *testing.T) { manifests := ManifestList{[]byte(` apiVersion: v1 kind: Pod @@ -38,6 +38,9 @@ spec: apiVersion: v1 kind: Pod metadata: + annotations: + key1: value1 + key2: value2 labels: key1: value1 key2: value2 @@ -48,7 +51,10 @@ spec: name: example `)} - resultManifest, err := manifests.SetLabels(map[string]string{ + resultManifest, err := manifests.SetLabelsAndAnnotations(map[string]string{ + "key1": "value1", + "key2": "value2", + }, map[string]string{ "key1": "value1", "key2": "value2", }) @@ -56,11 +62,13 @@ spec: testutil.CheckErrorAndDeepEqual(t, false, err, expected.String(), resultManifest.String()) } -func TestAddLabels(t *testing.T) { +func TestAddLabelsAndAnnotations(t *testing.T) { manifests := ManifestList{[]byte(` apiVersion: v1 kind: Pod metadata: + annotations: + key0: value0 labels: key0: value0 name: getting-started @@ -74,6 +82,10 @@ spec: apiVersion: v1 kind: Pod metadata: + annotations: + key0: value0 + key1: value1 + key2: value2 labels: key0: value0 key1: value1 @@ -85,7 +97,11 @@ spec: name: example `)} - resultManifest, err := manifests.SetLabels(map[string]string{ + resultManifest, err := manifests.SetLabelsAndAnnotations(map[string]string{ + "key0": "should-be-ignored", + "key1": "value1", + "key2": "value2", + }, map[string]string{ "key0": "should-be-ignored", "key1": "value1", "key2": "value2", @@ -94,7 +110,7 @@ spec: testutil.CheckErrorAndDeepEqual(t, false, err, expected.String(), resultManifest.String()) } -func TestSetNoLabel(t *testing.T) { +func TestSetNoLabelOrAnnotation(t *testing.T) { manifests := ManifestList{[]byte(` apiVersion: v1 kind: Pod @@ -117,12 +133,12 @@ spec: name: example `)} - resultManifest, err := manifests.SetLabels(nil) + resultManifest, err := manifests.SetLabelsAndAnnotations(nil, nil) testutil.CheckErrorAndDeepEqual(t, false, err, expected.String(), resultManifest.String()) } -func TestSetNoLabelWhenTypeUnexpected(t *testing.T) { +func TestSetNoLabelOrAnnotationWhenTypeUnexpected(t *testing.T) { manifests := ManifestList{[]byte(` apiVersion: v1 kind: Pod @@ -132,16 +148,17 @@ metadata: 3 apiVersion: v1 kind: Pod metadata: + annotations: 3 labels: 3 name: getting-started `)} - resultManifest, err := manifests.SetLabels(map[string]string{"key0": "value0"}) + resultManifest, err := manifests.SetLabelsAndAnnotations(map[string]string{"key0": "value0"}, map[string]string{"key0": "value0"}) testutil.CheckErrorAndDeepEqual(t, false, err, manifests.String(), resultManifest.String()) } -func TestSetNoLabelInCRDSchema(t *testing.T) { +func TestSetLabelAndAnnotationInCRDSchema(t *testing.T) { manifests := ManifestList{[]byte(`apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: @@ -163,6 +180,9 @@ spec: expected := ManifestList{[]byte(`apiVersion: apiextensions.k8s.io/v1beta1 kind: CustomResourceDefinition metadata: + annotations: + key0: value0 + key1: value1 labels: key0: value0 key1: value1 @@ -181,7 +201,10 @@ spec: metadata: type: object`)} - resultManifest, err := manifests.SetLabels(map[string]string{ + resultManifest, err := manifests.SetLabelsAndAnnotations(map[string]string{ + "key0": "value0", + "key1": "value1", + }, map[string]string{ "key0": "value0", "key1": "value1", }) diff --git a/pkg/skaffold/deploy/kubectl_test.go b/pkg/skaffold/deploy/kubectl_test.go index 84c920a23b5..adadbb7c9b7 100644 --- a/pkg/skaffold/deploy/kubectl_test.go +++ b/pkg/skaffold/deploy/kubectl_test.go @@ -363,7 +363,7 @@ func TestKubectlRedeploy(t *testing.T) { AndRunInput("kubectl --context kubecontext --namespace testNamespace apply -f -", `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl name: leeroy-app spec: @@ -374,7 +374,7 @@ spec: apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl name: leeroy-web spec: @@ -385,7 +385,7 @@ spec: AndRunInput("kubectl --context kubecontext --namespace testNamespace apply -f -", `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl name: leeroy-app spec: @@ -535,7 +535,7 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl namespace: default spec: @@ -570,7 +570,7 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl namespace: default spec: @@ -598,7 +598,7 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kubectl namespace: default spec: diff --git a/pkg/skaffold/deploy/kustomize.go b/pkg/skaffold/deploy/kustomize.go index 6f5d03ea30e..5a8f3615d0f 100644 --- a/pkg/skaffold/deploy/kustomize.go +++ b/pkg/skaffold/deploy/kustomize.go @@ -119,6 +119,10 @@ func NewKustomizeDeployer(runCtx *runcontext.RunContext) *KustomizeDeployer { // Labels returns the labels specific to kustomize. func (k *KustomizeDeployer) Labels() map[string]string { + return map[string]string{} +} + +func (k *KustomizeDeployer) Annotations() map[string]string { return map[string]string{ constants.Labels.Deployer: "kustomize", } @@ -186,7 +190,7 @@ func (k *KustomizeDeployer) renderManifests(ctx context.Context, out io.Writer, } } - manifests, err = manifests.SetLabels(merge(k.addSkaffoldLabels, k, labellers...)) + manifests, err = manifests.SetLabelsAndAnnotations(merge(k.addSkaffoldLabels, k, labellers...)) if err != nil { return nil, fmt.Errorf("setting labels in manifests: %w", err) } diff --git a/pkg/skaffold/deploy/kustomize_test.go b/pkg/skaffold/deploy/kustomize_test.go index 5c51eb31281..a0e67311afc 100644 --- a/pkg/skaffold/deploy/kustomize_test.go +++ b/pkg/skaffold/deploy/kustomize_test.go @@ -470,12 +470,18 @@ func TestKustomizeBuildCommandArgs(t *testing.T) { } type testLabels struct { - labels map[string]string + labels map[string]string + annotations map[string]string } func (t *testLabels) Labels() map[string]string { return t.labels } + +func (t *testLabels) Annotations() map[string]string { + return t.annotations +} + func TestKustomizeRender(t *testing.T) { type kustomizationCall struct { folder string @@ -520,7 +526,7 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kustomize namespace: default spec: @@ -568,8 +574,9 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kustomize + labels: user/label: test namespace: default spec: @@ -621,7 +628,7 @@ spec: expected: `apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kustomize namespace: default spec: @@ -632,7 +639,7 @@ spec: apiVersion: v1 kind: Pod metadata: - labels: + annotations: skaffold.dev/deployer: kustomize namespace: default spec: diff --git a/pkg/skaffold/deploy/labeller.go b/pkg/skaffold/deploy/labeller.go index 9c71bf2dfbf..e41696d8084 100644 --- a/pkg/skaffold/deploy/labeller.go +++ b/pkg/skaffold/deploy/labeller.go @@ -56,8 +56,23 @@ func NewLabeller(opts config.SkaffoldOptions) *DefaultLabeller { func (d *DefaultLabeller) Labels() map[string]string { labels := map[string]string{ - K8sManagedByLabelKey: fmt.Sprintf("skaffold-%s", d.version), - RunIDLabel: d.runID, + RunIDLabel: d.runID, + } + + for _, cl := range d.opts.CustomLabels { + l := strings.SplitN(cl, "=", 2) + if len(l) == 1 { + labels[l[0]] = "" + continue + } + labels[l[0]] = l[1] + } + return labels +} + +func (d *DefaultLabeller) Annotations() map[string]string { + labels := map[string]string{ + K8sManagedByLabelKey: "skaffold", } if d.opts.Cleanup { @@ -73,14 +88,6 @@ func (d *DefaultLabeller) Labels() map[string]string { key := fmt.Sprintf("skaffold.dev/profile.%d", i) labels[key] = profile } - for _, cl := range d.opts.CustomLabels { - l := strings.SplitN(cl, "=", 2) - if len(l) == 1 { - labels[l[0]] = "" - continue - } - labels[l[0]] = l[1] - } return labels } diff --git a/pkg/skaffold/deploy/labels.go b/pkg/skaffold/deploy/labels.go index d2cd3b9a945..9a09c7e1e13 100644 --- a/pkg/skaffold/deploy/labels.go +++ b/pkg/skaffold/deploy/labels.go @@ -41,25 +41,32 @@ type Artifact struct { Namespace string } -// Labeller can give key/value labels to set on deployed resources. +// Labeller can give key/value labels and annotations to set on deployed resources. type Labeller interface { // Labels keys must be prefixed with "skaffold.dev/" Labels() map[string]string + + Annotations() map[string]string } // merge merges the labels from multiple sources. -func merge(addSkaffoldLabels bool, deployer Labeller, sources ...Labeller) map[string]string { +func merge(addSkaffoldLabels bool, deployer Labeller, sources ...Labeller) (map[string]string, map[string]string) { if !addSkaffoldLabels { - return map[string]string{} + return map[string]string{}, map[string]string{} } - merged := deployer.Labels() + mergedLabels := deployer.Labels() for _, src := range sources { - copyMap(merged, src.Labels()) + copyMap(mergedLabels, src.Labels()) } - return merged + mergedAnnotations := deployer.Annotations() + for _, src := range sources { + copyMap(mergedAnnotations, src.Annotations()) + } + + return mergedLabels, mergedAnnotations } // retry 3 times to give the object time to propagate to the API server @@ -68,7 +75,7 @@ const ( sleeptime = 300 * time.Millisecond ) -func labelDeployResults(labels map[string]string, results []Artifact) { +func labelAndAnnotateDeployResults(labels, annotations map[string]string, results []Artifact) { // use the kubectl client to update all k8s objects with a skaffold watermark dynClient, err := kubernetes.DynamicClient() if err != nil { @@ -85,7 +92,7 @@ func labelDeployResults(labels map[string]string, results []Artifact) { for _, res := range results { err = nil for i := 0; i < tries; i++ { - if err = updateRuntimeObject(dynClient, client.Discovery(), labels, res); err == nil { + if err = updateRuntimeObject(dynClient, client.Discovery(), labels, annotations, res); err == nil { break } time.Sleep(sleeptime) @@ -105,7 +112,16 @@ func addLabels(labels map[string]string, accessor metav1.Object) { accessor.SetLabels(kv) } -func updateRuntimeObject(client dynamic.Interface, disco discovery.DiscoveryInterface, labels map[string]string, res Artifact) error { +func addAnnotations(annotations map[string]string, accessor metav1.Object) { + kv := make(map[string]string) + + copyMap(kv, annotations) + copyMap(kv, accessor.GetAnnotations()) + + accessor.SetAnnotations(kv) +} + +func updateRuntimeObject(client dynamic.Interface, disco discovery.DiscoveryInterface, labels, annotations map[string]string, res Artifact) error { originalJSON, _ := json.Marshal(res.Obj) modifiedObj := res.Obj.DeepCopyObject() accessor, err := meta.Accessor(modifiedObj) @@ -115,6 +131,7 @@ func updateRuntimeObject(client dynamic.Interface, disco discovery.DiscoveryInte name := accessor.GetName() addLabels(labels, accessor) + addAnnotations(annotations, accessor) modifiedJSON, _ := json.Marshal(modifiedObj) p, _ := patch.CreateTwoWayMergePatch(originalJSON, modifiedJSON, modifiedObj) diff --git a/pkg/skaffold/deploy/labels_test.go b/pkg/skaffold/deploy/labels_test.go index 756d6c1c0a4..bb41da7f15e 100644 --- a/pkg/skaffold/deploy/labels_test.go +++ b/pkg/skaffold/deploy/labels_test.go @@ -50,6 +50,10 @@ func TestLabelDeployResults(t *testing.T) { existingLabels map[string]string appliedLabels map[string]string expectedLabels map[string]string + + existingAnnotations map[string]string + appliedAnnotations map[string]string + expectedAnnotations map[string]string }{ { description: "set labels", @@ -77,6 +81,32 @@ func TestLabelDeployResults(t *testing.T) { "key1": "value1", }, }, + { + description: "set annotations", + existingAnnotations: map[string]string{}, + appliedAnnotations: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedAnnotations: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + { + description: "add annotations", + existingAnnotations: map[string]string{ + "key0": "value0", + }, + appliedAnnotations: map[string]string{ + "key0": "should-be-ignored", + "key1": "value1", + }, + expectedAnnotations: map[string]string{ + "key0": "value0", + "key1": "value1", + }, + }, } for _, test := range tests { @@ -87,8 +117,9 @@ func TestLabelDeployResults(t *testing.T) { Kind: "Deployment", }, ObjectMeta: metav1.ObjectMeta{ - Name: "foo", - Labels: test.existingLabels, + Name: "foo", + Labels: test.existingLabels, + Annotations: test.existingAnnotations, }, } @@ -106,12 +137,13 @@ func TestLabelDeployResults(t *testing.T) { t.Override(&k8s.DynamicClient, mockDynamicClient(dynClient)) // Patch labels - labelDeployResults(test.appliedLabels, []Artifact{{Obj: dep}}) + labelAndAnnotateDeployResults(test.appliedLabels, test.appliedAnnotations, []Artifact{{Obj: dep}}) // Check modified value modified, err := dynClient.Resource(schema.GroupVersionResource{Group: "apps", Version: "v1", Resource: "deployments"}).Get("foo", metav1.GetOptions{}) t.CheckNoError(err) t.CheckDeepEqual(test.expectedLabels, modified.GetLabels()) + t.CheckDeepEqual(test.expectedAnnotations, modified.GetAnnotations()) }) } } diff --git a/pkg/skaffold/runner/runner_test.go b/pkg/skaffold/runner/runner_test.go index a77ec54d3fd..ce454e853bb 100644 --- a/pkg/skaffold/runner/runner_test.go +++ b/pkg/skaffold/runner/runner_test.go @@ -91,6 +91,7 @@ func (t *TestBench) WithTestErrors(testErrors []error) *TestBench { } func (t *TestBench) Labels() map[string]string { return map[string]string{} } +func (t *TestBench) Annotations() map[string]string { return map[string]string{} } func (t *TestBench) TestDependencies() ([]string, error) { return nil, nil } func (t *TestBench) Dependencies() ([]string, error) { return nil, nil } func (t *TestBench) Cleanup(ctx context.Context, out io.Writer) error { return nil } diff --git a/pkg/skaffold/sync/sync_test.go b/pkg/skaffold/sync/sync_test.go index 449804a334a..2e41b5a650f 100644 --- a/pkg/skaffold/sync/sync_test.go +++ b/pkg/skaffold/sync/sync_test.go @@ -862,7 +862,7 @@ var pod = &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "podname", Labels: map[string]string{ - "app.kubernetes.io/managed-by": "skaffold-dirty", + "app.kubernetes.io/managed-by": "skaffold", }, }, Status: v1.PodStatus{ @@ -882,7 +882,7 @@ var nonRunningPod = &v1.Pod{ ObjectMeta: metav1.ObjectMeta{ Name: "podname", Labels: map[string]string{ - "app.kubernetes.io/managed-by": "skaffold-dirty", + "app.kubernetes.io/managed-by": "skaffold", }, }, Status: v1.PodStatus{