diff --git a/pkg/skaffold/deploy/kubectl/labels.go b/pkg/skaffold/deploy/kubectl/labels.go index 1f45ea19287..ea1bca80a8a 100644 --- a/pkg/skaffold/deploy/kubectl/labels.go +++ b/pkg/skaffold/deploy/kubectl/labels.go @@ -72,7 +72,10 @@ func (r *labelsSetter) Visit(o map[interface{}]interface{}, k interface{}, v int } for k, v := range r.labels { - labels[k] = v + // Don't replace existing label. + if _, present := labels[k]; !present { + labels[k] = v + } } return false diff --git a/pkg/skaffold/deploy/kubectl/labels_test.go b/pkg/skaffold/deploy/kubectl/labels_test.go index 326b564d551..b62b116652c 100644 --- a/pkg/skaffold/deploy/kubectl/labels_test.go +++ b/pkg/skaffold/deploy/kubectl/labels_test.go @@ -1,5 +1,5 @@ /* -Copyright 2019 The Skaffold Authors +Copyright 2020 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. @@ -63,7 +63,6 @@ kind: Pod metadata: labels: key0: value0 - key1: ignored name: getting-started spec: containers: @@ -87,6 +86,7 @@ spec: `)} resultManifest, err := manifests.SetLabels(map[string]string{ + "key0": "should-be-ignored", "key1": "value1", "key2": "value2", }) diff --git a/pkg/skaffold/deploy/labels.go b/pkg/skaffold/deploy/labels.go index ae2879846ee..bf200f6afd3 100644 --- a/pkg/skaffold/deploy/labels.go +++ b/pkg/skaffold/deploy/labels.go @@ -95,8 +95,8 @@ func labelDeployResults(labels map[string]string, results []Artifact) { func addLabels(labels map[string]string, accessor metav1.Object) { kv := make(map[string]string) - copyMap(kv, accessor.GetLabels()) copyMap(kv, labels) + copyMap(kv, accessor.GetLabels()) accessor.SetLabels(kv) } diff --git a/pkg/skaffold/deploy/labels_test.go b/pkg/skaffold/deploy/labels_test.go new file mode 100644 index 00000000000..756d6c1c0a4 --- /dev/null +++ b/pkg/skaffold/deploy/labels_test.go @@ -0,0 +1,117 @@ +/* +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 ( + "testing" + + v1 "k8s.io/api/apps/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/client-go/dynamic" + fakedynclient "k8s.io/client-go/dynamic/fake" + "k8s.io/client-go/kubernetes" + fakeclient "k8s.io/client-go/kubernetes/fake" + "k8s.io/client-go/kubernetes/scheme" + + k8s "github.com/GoogleContainerTools/skaffold/pkg/skaffold/kubernetes" + "github.com/GoogleContainerTools/skaffold/testutil" +) + +func mockClient(m kubernetes.Interface) func() (kubernetes.Interface, error) { + return func() (kubernetes.Interface, error) { + return m, nil + } +} + +func mockDynamicClient(m dynamic.Interface) func() (dynamic.Interface, error) { + return func() (dynamic.Interface, error) { + return m, nil + } +} + +func TestLabelDeployResults(t *testing.T) { + tests := []struct { + description string + existingLabels map[string]string + appliedLabels map[string]string + expectedLabels map[string]string + }{ + { + description: "set labels", + existingLabels: map[string]string{}, + appliedLabels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + expectedLabels: map[string]string{ + "key1": "value1", + "key2": "value2", + }, + }, + { + description: "add labels", + existingLabels: map[string]string{ + "key0": "value0", + }, + appliedLabels: map[string]string{ + "key0": "should-be-ignored", + "key1": "value1", + }, + expectedLabels: map[string]string{ + "key0": "value0", + "key1": "value1", + }, + }, + } + + for _, test := range tests { + testutil.Run(t, test.description, func(t *testutil.T) { + dep := &v1.Deployment{ + TypeMeta: metav1.TypeMeta{ + APIVersion: "apps/v1", + Kind: "Deployment", + }, + ObjectMeta: metav1.ObjectMeta{ + Name: "foo", + Labels: test.existingLabels, + }, + } + + // Mock Kubernetes + client := fakeclient.NewSimpleClientset(dep) + client.Resources = append(client.Resources, &metav1.APIResourceList{ + GroupVersion: dep.APIVersion, + APIResources: []metav1.APIResource{{ + Kind: dep.Kind, + Name: "deployments", + }}, + }) + t.Override(&k8s.Client, mockClient(client)) + dynClient := fakedynclient.NewSimpleDynamicClient(scheme.Scheme, dep) + t.Override(&k8s.DynamicClient, mockDynamicClient(dynClient)) + + // Patch labels + labelDeployResults(test.appliedLabels, []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()) + }) + } +} diff --git a/vendor/k8s.io/client-go/dynamic/fake/simple.go b/vendor/k8s.io/client-go/dynamic/fake/simple.go new file mode 100644 index 00000000000..819b8d9c90c --- /dev/null +++ b/vendor/k8s.io/client-go/dynamic/fake/simple.go @@ -0,0 +1,370 @@ +/* +Copyright 2018 The Kubernetes 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 fake + +import ( + "strings" + + "k8s.io/apimachinery/pkg/api/meta" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + "k8s.io/apimachinery/pkg/runtime/schema" + "k8s.io/apimachinery/pkg/runtime/serializer" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/watch" + "k8s.io/client-go/dynamic" + "k8s.io/client-go/testing" +) + +func NewSimpleDynamicClient(scheme *runtime.Scheme, objects ...runtime.Object) *FakeDynamicClient { + // In order to use List with this client, you have to have the v1.List registered in your scheme. Neat thing though + // it does NOT have to be the *same* list + scheme.AddKnownTypeWithName(schema.GroupVersionKind{Group: "fake-dynamic-client-group", Version: "v1", Kind: "List"}, &unstructured.UnstructuredList{}) + + codecs := serializer.NewCodecFactory(scheme) + o := testing.NewObjectTracker(scheme, codecs.UniversalDecoder()) + for _, obj := range objects { + if err := o.Add(obj); err != nil { + panic(err) + } + } + + cs := &FakeDynamicClient{scheme: scheme} + cs.AddReactor("*", "*", testing.ObjectReaction(o)) + cs.AddWatchReactor("*", func(action testing.Action) (handled bool, ret watch.Interface, err error) { + gvr := action.GetResource() + ns := action.GetNamespace() + watch, err := o.Watch(gvr, ns) + if err != nil { + return false, nil, err + } + return true, watch, nil + }) + + return cs +} + +// Clientset implements clientset.Interface. Meant to be embedded into a +// struct to get a default implementation. This makes faking out just the method +// you want to test easier. +type FakeDynamicClient struct { + testing.Fake + scheme *runtime.Scheme +} + +type dynamicResourceClient struct { + client *FakeDynamicClient + namespace string + resource schema.GroupVersionResource +} + +var _ dynamic.Interface = &FakeDynamicClient{} + +func (c *FakeDynamicClient) Resource(resource schema.GroupVersionResource) dynamic.NamespaceableResourceInterface { + return &dynamicResourceClient{client: c, resource: resource} +} + +func (c *dynamicResourceClient) Namespace(ns string) dynamic.ResourceInterface { + ret := *c + ret.namespace = ns + return &ret +} + +func (c *dynamicResourceClient) Create(obj *unstructured.Unstructured, opts metav1.CreateOptions, subresources ...string) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootCreateAction(c.resource, obj), obj) + + case len(c.namespace) == 0 && len(subresources) > 0: + accessor, err := meta.Accessor(obj) + if err != nil { + return nil, err + } + name := accessor.GetName() + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootCreateSubresourceAction(c.resource, name, strings.Join(subresources, "/"), obj), obj) + + case len(c.namespace) > 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewCreateAction(c.resource, c.namespace, obj), obj) + + case len(c.namespace) > 0 && len(subresources) > 0: + accessor, err := meta.Accessor(obj) + if err != nil { + return nil, err + } + name := accessor.GetName() + uncastRet, err = c.client.Fake. + Invokes(testing.NewCreateSubresourceAction(c.resource, name, strings.Join(subresources, "/"), c.namespace, obj), obj) + + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} + +func (c *dynamicResourceClient) Update(obj *unstructured.Unstructured, opts metav1.UpdateOptions, subresources ...string) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootUpdateAction(c.resource, obj), obj) + + case len(c.namespace) == 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootUpdateSubresourceAction(c.resource, strings.Join(subresources, "/"), obj), obj) + + case len(c.namespace) > 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewUpdateAction(c.resource, c.namespace, obj), obj) + + case len(c.namespace) > 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewUpdateSubresourceAction(c.resource, strings.Join(subresources, "/"), c.namespace, obj), obj) + + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} + +func (c *dynamicResourceClient) UpdateStatus(obj *unstructured.Unstructured, opts metav1.UpdateOptions) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootUpdateSubresourceAction(c.resource, "status", obj), obj) + + case len(c.namespace) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewUpdateSubresourceAction(c.resource, "status", c.namespace, obj), obj) + + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} + +func (c *dynamicResourceClient) Delete(name string, opts *metav1.DeleteOptions, subresources ...string) error { + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + _, err = c.client.Fake. + Invokes(testing.NewRootDeleteAction(c.resource, name), &metav1.Status{Status: "dynamic delete fail"}) + + case len(c.namespace) == 0 && len(subresources) > 0: + _, err = c.client.Fake. + Invokes(testing.NewRootDeleteSubresourceAction(c.resource, strings.Join(subresources, "/"), name), &metav1.Status{Status: "dynamic delete fail"}) + + case len(c.namespace) > 0 && len(subresources) == 0: + _, err = c.client.Fake. + Invokes(testing.NewDeleteAction(c.resource, c.namespace, name), &metav1.Status{Status: "dynamic delete fail"}) + + case len(c.namespace) > 0 && len(subresources) > 0: + _, err = c.client.Fake. + Invokes(testing.NewDeleteSubresourceAction(c.resource, strings.Join(subresources, "/"), c.namespace, name), &metav1.Status{Status: "dynamic delete fail"}) + } + + return err +} + +func (c *dynamicResourceClient) DeleteCollection(opts *metav1.DeleteOptions, listOptions metav1.ListOptions) error { + var err error + switch { + case len(c.namespace) == 0: + action := testing.NewRootDeleteCollectionAction(c.resource, listOptions) + _, err = c.client.Fake.Invokes(action, &metav1.Status{Status: "dynamic deletecollection fail"}) + + case len(c.namespace) > 0: + action := testing.NewDeleteCollectionAction(c.resource, c.namespace, listOptions) + _, err = c.client.Fake.Invokes(action, &metav1.Status{Status: "dynamic deletecollection fail"}) + + } + + return err +} + +func (c *dynamicResourceClient) Get(name string, opts metav1.GetOptions, subresources ...string) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootGetAction(c.resource, name), &metav1.Status{Status: "dynamic get fail"}) + + case len(c.namespace) == 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootGetSubresourceAction(c.resource, strings.Join(subresources, "/"), name), &metav1.Status{Status: "dynamic get fail"}) + + case len(c.namespace) > 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewGetAction(c.resource, c.namespace, name), &metav1.Status{Status: "dynamic get fail"}) + + case len(c.namespace) > 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewGetSubresourceAction(c.resource, c.namespace, strings.Join(subresources, "/"), name), &metav1.Status{Status: "dynamic get fail"}) + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} + +func (c *dynamicResourceClient) List(opts metav1.ListOptions) (*unstructured.UnstructuredList, error) { + var obj runtime.Object + var err error + switch { + case len(c.namespace) == 0: + obj, err = c.client.Fake. + Invokes(testing.NewRootListAction(c.resource, schema.GroupVersionKind{Group: "fake-dynamic-client-group", Version: "v1", Kind: "" /*List is appended by the tracker automatically*/}, opts), &metav1.Status{Status: "dynamic list fail"}) + + case len(c.namespace) > 0: + obj, err = c.client.Fake. + Invokes(testing.NewListAction(c.resource, schema.GroupVersionKind{Group: "fake-dynamic-client-group", Version: "v1", Kind: "" /*List is appended by the tracker automatically*/}, c.namespace, opts), &metav1.Status{Status: "dynamic list fail"}) + + } + + if obj == nil { + return nil, err + } + + label, _, _ := testing.ExtractFromListOptions(opts) + if label == nil { + label = labels.Everything() + } + + retUnstructured := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(obj, retUnstructured, nil); err != nil { + return nil, err + } + entireList, err := retUnstructured.ToList() + if err != nil { + return nil, err + } + + list := &unstructured.UnstructuredList{} + list.SetResourceVersion(entireList.GetResourceVersion()) + for i := range entireList.Items { + item := &entireList.Items[i] + metadata, err := meta.Accessor(item) + if err != nil { + return nil, err + } + if label.Matches(labels.Set(metadata.GetLabels())) { + list.Items = append(list.Items, *item) + } + } + return list, nil +} + +func (c *dynamicResourceClient) Watch(opts metav1.ListOptions) (watch.Interface, error) { + switch { + case len(c.namespace) == 0: + return c.client.Fake. + InvokesWatch(testing.NewRootWatchAction(c.resource, opts)) + + case len(c.namespace) > 0: + return c.client.Fake. + InvokesWatch(testing.NewWatchAction(c.resource, c.namespace, opts)) + + } + + panic("math broke") +} + +// TODO: opts are currently ignored. +func (c *dynamicResourceClient) Patch(name string, pt types.PatchType, data []byte, opts metav1.PatchOptions, subresources ...string) (*unstructured.Unstructured, error) { + var uncastRet runtime.Object + var err error + switch { + case len(c.namespace) == 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootPatchAction(c.resource, name, pt, data), &metav1.Status{Status: "dynamic patch fail"}) + + case len(c.namespace) == 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewRootPatchSubresourceAction(c.resource, name, pt, data, subresources...), &metav1.Status{Status: "dynamic patch fail"}) + + case len(c.namespace) > 0 && len(subresources) == 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewPatchAction(c.resource, c.namespace, name, pt, data), &metav1.Status{Status: "dynamic patch fail"}) + + case len(c.namespace) > 0 && len(subresources) > 0: + uncastRet, err = c.client.Fake. + Invokes(testing.NewPatchSubresourceAction(c.resource, c.namespace, name, pt, data, subresources...), &metav1.Status{Status: "dynamic patch fail"}) + + } + + if err != nil { + return nil, err + } + if uncastRet == nil { + return nil, err + } + + ret := &unstructured.Unstructured{} + if err := c.client.scheme.Convert(uncastRet, ret, nil); err != nil { + return nil, err + } + return ret, err +} diff --git a/vendor/modules.txt b/vendor/modules.txt index b3c6e68cdeb..faba1aef51f 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -822,6 +822,7 @@ k8s.io/apimachinery/third_party/forked/golang/reflect k8s.io/client-go/discovery k8s.io/client-go/discovery/fake k8s.io/client-go/dynamic +k8s.io/client-go/dynamic/fake k8s.io/client-go/informers k8s.io/client-go/informers/admissionregistration k8s.io/client-go/informers/admissionregistration/v1