From 0d2146461fca46313c93b0d489518c60d2b4a82e Mon Sep 17 00:00:00 2001 From: roy wang Date: Sat, 29 Aug 2020 15:04:23 +0900 Subject: [PATCH 1/3] add labels for workload add e2e test & unit test Signed-off-by: roy wang --- .../applicationconfiguration/render.go | 8 ++++++ .../applicationconfiguration/render_test.go | 16 +++++++++++ pkg/oam/labels.go | 28 +++++++++++++++++++ test/e2e-test/component_version_test.go | 2 +- test/e2e-test/containerized_workload_test.go | 18 +++++++++++- 5 files changed, 70 insertions(+), 2 deletions(-) create mode 100644 pkg/oam/labels.go diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go index 985a0ee4..829072a2 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render.go @@ -34,6 +34,7 @@ import ( "sigs.k8s.io/controller-runtime/pkg/client" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) @@ -130,6 +131,13 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati return nil, errors.Wrapf(err, errFmtRenderWorkload, acc.ComponentName) } + labels := map[string]string{ + oam.LabelAppName: ac.Name, + oam.LabelAppComponent: acc.ComponentName, + oam.LabelAppComponentRevision: componentRevisionName, + } + w.SetLabels(labels) + // pass through labels and annotation from app-config to workload util.PassLabelAndAnnotation(ac, w) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index dfecf2c2..bfafeacf 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -39,6 +39,7 @@ import ( "github.com/crossplane/oam-kubernetes-runtime/apis/core" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) @@ -194,6 +195,11 @@ func TestRenderComponents(t *testing.T) { w.SetNamespace(namespace) w.SetName(workloadName) w.SetOwnerReferences([]metav1.OwnerReference{*ref}) + w.SetLabels(map[string]string{ + oam.LabelAppComponent: componentName, + oam.LabelAppName: acName, + oam.LabelAppComponentRevision: "", + }) return w }(), Traits: []*Trait{ @@ -257,6 +263,11 @@ func TestRenderComponents(t *testing.T) { w.SetNamespace(namespace) w.SetName(componentName) w.SetOwnerReferences([]metav1.OwnerReference{*ref}) + w.SetLabels(map[string]string{ + oam.LabelAppComponent: componentName, + oam.LabelAppName: acName, + oam.LabelAppComponentRevision: revisionName, + }) return w }(), Traits: []*Trait{ @@ -311,6 +322,11 @@ func TestRenderComponents(t *testing.T) { w.SetNamespace(namespace) w.SetName(revisionName2) w.SetOwnerReferences([]metav1.OwnerReference{*ref}) + w.SetLabels(map[string]string{ + oam.LabelAppComponent: componentName, + oam.LabelAppName: acName, + oam.LabelAppComponentRevision: revisionName2, + }) return w }(), Traits: []*Trait{ diff --git a/pkg/oam/labels.go b/pkg/oam/labels.go new file mode 100644 index 00000000..dd9de5f4 --- /dev/null +++ b/pkg/oam/labels.go @@ -0,0 +1,28 @@ +/* +Copyright 2019 The Crossplane 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 oam + +// Label key strings. +// AppConfig controller will add these labels into workloads. +const ( + // LabelAppName records the name of AppConfig + LabelAppName = "app.oam.dev/name" + // LabelAppComponent records the name of Component + LabelAppComponent = "app.oam.dev/component" + // LabelAppComponentRevision records the revision name of Component + LabelAppComponentRevision = "app.oam.dev/revision" +) diff --git a/test/e2e-test/component_version_test.go b/test/e2e-test/component_version_test.go index 196e9f63..5849bf44 100644 --- a/test/e2e-test/component_version_test.go +++ b/test/e2e-test/component_version_test.go @@ -293,7 +293,7 @@ var _ = Describe("Versioning mechanism of components", func() { Eventually(func() string { k8sClient.Get(ctx, client.ObjectKey{Namespace: namespace, Name: componentName}, cwWlV2) return cwWlV2.Spec.Containers[0].Image - }, time.Second*30, time.Microsecond*500).Should(Equal(imageV2)) + }, time.Second*60, time.Microsecond*500).Should(Equal(imageV2)) }) }) diff --git a/test/e2e-test/containerized_workload_test.go b/test/e2e-test/containerized_workload_test.go index 80b9cf87..767cd97e 100644 --- a/test/e2e-test/containerized_workload_test.go +++ b/test/e2e-test/containerized_workload_test.go @@ -16,6 +16,7 @@ import ( logf "sigs.k8s.io/controller-runtime/pkg/log" "github.com/crossplane/oam-kubernetes-runtime/apis/core/v1alpha2" + "github.com/crossplane/oam-kubernetes-runtime/pkg/oam" "github.com/crossplane/oam-kubernetes-runtime/pkg/oam/util" ) @@ -60,7 +61,8 @@ var _ = Describe("ContainerizedWorkload", func() { }) It("apply an application config", func() { - label := map[string]string{"workload": "containerized-workload"} + fakeLabelKey := "workload" + label := map[string]string{fakeLabelKey: "containerized-workload"} // create a workload definition wd := v1alpha2.WorkloadDefinition{ ObjectMeta: metav1.ObjectMeta{ @@ -192,6 +194,20 @@ var _ = Describe("ContainerizedWorkload", func() { logf.Log.Info("Creating application config", "Name", appConfig.Name, "Namespace", appConfig.Namespace) Expect(k8sClient.Create(ctx, &appConfig)).Should(BeNil()) // Verification + By("Checking containerizedworkload is created") + cw := &v1alpha2.ContainerizedWorkload{} + Eventually(func() error { + return k8sClient.Get(ctx, client.ObjectKey{Name: workloadInstanceName, Namespace: namespace}, cw) + }, time.Second*15, time.Millisecond*500).Should(BeNil()) + + By("Checking lables") + cwLabels := cw.GetLabels() + Expect(cwLabels).Should(SatisfyAll( + HaveKey(fakeLabelKey), // propogated from appConfig + HaveKey(oam.LabelAppComponent), + HaveKey(oam.LabelAppComponentRevision), + HaveKey(oam.LabelAppName))) + By("Checking deployment is created") objectKey := client.ObjectKey{ Name: workloadInstanceName, From 808e7f097c28b5ec5d71e87a5c9f9b1119767206 Mon Sep 17 00:00:00 2001 From: roy wang Date: Sat, 29 Aug 2020 21:56:38 +0900 Subject: [PATCH 2/3] use merging instead of overriding add unit test Signed-off-by: roy wang --- .../applicationconfiguration/render.go | 15 +++++- .../applicationconfiguration/render_test.go | 49 +++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go index 829072a2..e12f530b 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render.go @@ -131,12 +131,12 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati return nil, errors.Wrapf(err, errFmtRenderWorkload, acc.ComponentName) } - labels := map[string]string{ + compInfoLabels := map[string]string{ oam.LabelAppName: ac.Name, oam.LabelAppComponent: acc.ComponentName, oam.LabelAppComponentRevision: componentRevisionName, } - w.SetLabels(labels) + addWorkloadLabels(w, compInfoLabels) // pass through labels and annotation from app-config to workload util.PassLabelAndAnnotation(ac, w) @@ -477,6 +477,17 @@ func fillValue(obj *unstructured.Unstructured, fs []string, val interface{}) err return nil } +func addWorkloadLabels(w *unstructured.Unstructured, labels map[string]string) { + workloadLabels := w.GetLabels() + if workloadLabels == nil { + workloadLabels = map[string]string{} + } + for k, v := range labels { + workloadLabels[k] = v + } + w.SetLabels(workloadLabels) +} + func (r *components) getDataInput(ctx context.Context, s *dagSource) (interface{}, bool, error) { obj := s.ObjectRef key := types.NamespacedName{ diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index bfafeacf..f7a37b7e 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -1056,3 +1056,52 @@ func TestMatchValue(t *testing.T) { }) } } + +func TestAddWorkloadLabels(t *testing.T) { + workload1 := new(unstructured.Unstructured) + wantWorkload1 := new(unstructured.Unstructured) + wantWorkload1.SetLabels(map[string]string{ + "newKey": "newValue", + }) + workload2 := new(unstructured.Unstructured) + wantWorkload2 := new(unstructured.Unstructured) + workload2.SetLabels(map[string]string{ + "key": "value", + }) + wantWorkload2.SetLabels(map[string]string{ + "key": "value", + "newKey": "newValue", + }) + + cases := map[string]struct { + workload *unstructured.Unstructured + newLabels map[string]string + want *unstructured.Unstructured + }{ + "add labels to workload without labels": { + workload1, + map[string]string{ + "newKey": "newValue", + }, + wantWorkload1, + }, + "add labels to workload with labels": { + workload2, + map[string]string{ + "newKey": "newValue", + }, + wantWorkload2, + }, + } + + for name, tc := range cases { + workload := tc.workload + wantWorkload := tc.want + t.Run(name, func(t *testing.T) { + addWorkloadLabels(tc.workload, tc.newLabels) + if diff := cmp.Diff(wantWorkload, workload); diff != "" { + t.Error(diff) + } + }) + } +} From febeae2a122e3dc8e7d9233e8c28d02657cf5d6f Mon Sep 17 00:00:00 2001 From: roy wang Date: Mon, 31 Aug 2020 17:20:46 +0900 Subject: [PATCH 3/3] move AddLabel func into oam/helper Signed-off-by: roy wang --- .../applicationconfiguration/render.go | 13 +---- .../applicationconfiguration/render_test.go | 49 ------------------- pkg/oam/util/helper.go | 36 ++++++++------ pkg/oam/util/helper_test.go | 48 ++++++++++++++++++ 4 files changed, 70 insertions(+), 76 deletions(-) diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render.go b/pkg/controller/v1alpha2/applicationconfiguration/render.go index e12f530b..3d378448 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render.go @@ -136,7 +136,7 @@ func (r *components) renderComponent(ctx context.Context, acc v1alpha2.Applicati oam.LabelAppComponent: acc.ComponentName, oam.LabelAppComponentRevision: componentRevisionName, } - addWorkloadLabels(w, compInfoLabels) + util.AddLabels(w, compInfoLabels) // pass through labels and annotation from app-config to workload util.PassLabelAndAnnotation(ac, w) @@ -477,17 +477,6 @@ func fillValue(obj *unstructured.Unstructured, fs []string, val interface{}) err return nil } -func addWorkloadLabels(w *unstructured.Unstructured, labels map[string]string) { - workloadLabels := w.GetLabels() - if workloadLabels == nil { - workloadLabels = map[string]string{} - } - for k, v := range labels { - workloadLabels[k] = v - } - w.SetLabels(workloadLabels) -} - func (r *components) getDataInput(ctx context.Context, s *dagSource) (interface{}, bool, error) { obj := s.ObjectRef key := types.NamespacedName{ diff --git a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go index f7a37b7e..bfafeacf 100644 --- a/pkg/controller/v1alpha2/applicationconfiguration/render_test.go +++ b/pkg/controller/v1alpha2/applicationconfiguration/render_test.go @@ -1056,52 +1056,3 @@ func TestMatchValue(t *testing.T) { }) } } - -func TestAddWorkloadLabels(t *testing.T) { - workload1 := new(unstructured.Unstructured) - wantWorkload1 := new(unstructured.Unstructured) - wantWorkload1.SetLabels(map[string]string{ - "newKey": "newValue", - }) - workload2 := new(unstructured.Unstructured) - wantWorkload2 := new(unstructured.Unstructured) - workload2.SetLabels(map[string]string{ - "key": "value", - }) - wantWorkload2.SetLabels(map[string]string{ - "key": "value", - "newKey": "newValue", - }) - - cases := map[string]struct { - workload *unstructured.Unstructured - newLabels map[string]string - want *unstructured.Unstructured - }{ - "add labels to workload without labels": { - workload1, - map[string]string{ - "newKey": "newValue", - }, - wantWorkload1, - }, - "add labels to workload with labels": { - workload2, - map[string]string{ - "newKey": "newValue", - }, - wantWorkload2, - }, - } - - for name, tc := range cases { - workload := tc.workload - wantWorkload := tc.want - t.Run(name, func(t *testing.T) { - addWorkloadLabels(tc.workload, tc.newLabels) - if diff := cmp.Diff(wantWorkload, workload); diff != "" { - t.Error(diff) - } - }) - } -} diff --git a/pkg/oam/util/helper.go b/pkg/oam/util/helper.go index 7ce1f3d5..4f49bc85 100644 --- a/pkg/oam/util/helper.go +++ b/pkg/oam/util/helper.go @@ -218,21 +218,6 @@ type labelAnnotationObject interface { // PassLabelAndAnnotation passes through labels and annotation objectMeta from the parent to the child object func PassLabelAndAnnotation(parentObj oam.Object, childObj labelAnnotationObject) { - mergeMap := func(src, dst map[string]string) map[string]string { - if len(src) == 0 { - return dst - } - // make sure dst is initialized - if dst == nil { - dst = map[string]string{} - } - for k, v := range src { - if _, exist := dst[k]; !exist { - dst[k] = v - } - } - return dst - } // pass app-config labels childObj.SetLabels(mergeMap(parentObj.GetLabels(), childObj.GetLabels())) // pass app-config annotation @@ -356,3 +341,24 @@ func UnpackRevisionData(rev *appsv1.ControllerRevision) (*v1alpha2.Component, er err = json.Unmarshal(rev.Data.Raw, &comp) return &comp, err } + +// AddLabels will merge labels with existing labels +func AddLabels(o *unstructured.Unstructured, labels map[string]string) { + o.SetLabels(mergeMap(o.GetLabels(), labels)) +} + +func mergeMap(src, dst map[string]string) map[string]string { + if len(src) == 0 { + return dst + } + // make sure dst is initialized + if dst == nil { + dst = map[string]string{} + } + for k, v := range src { + if _, exist := dst[k]; !exist { + dst[k] = v + } + } + return dst +} diff --git a/pkg/oam/util/helper_test.go b/pkg/oam/util/helper_test.go index e0479cae..5987f9ec 100644 --- a/pkg/oam/util/helper_test.go +++ b/pkg/oam/util/helper_test.go @@ -1108,3 +1108,51 @@ var _ = Describe("TestPassThroughObjMeta", func() { Expect(gotLabels).Should(BeEquivalentTo(wantLabels)) }) }) + +var _ = Describe("TestAddLabels", func() { + It("Test AddLabels", func() { + obj1 := new(unstructured.Unstructured) + wantObj1 := new(unstructured.Unstructured) + wantObj1.SetLabels(map[string]string{ + "newKey": "newValue", + }) + obj2 := new(unstructured.Unstructured) + wantObj2 := new(unstructured.Unstructured) + obj2.SetLabels(map[string]string{ + "key": "value", + }) + wantObj2.SetLabels(map[string]string{ + "key": "value", + "newKey": "newValue", + }) + + cases := map[string]struct { + obj *unstructured.Unstructured + newLabels map[string]string + want *unstructured.Unstructured + }{ + "add labels to workload without labels": { + obj1, + map[string]string{ + "newKey": "newValue", + }, + wantObj1, + }, + "add labels to workload with labels": { + obj2, + map[string]string{ + "newKey": "newValue", + }, + wantObj2, + }, + } + + for name, tc := range cases { + By("Running test case: " + name) + obj := tc.obj + wantObj := tc.want + util.AddLabels(obj, tc.newLabels) + Expect(obj.GetLabels()).Should(Equal(wantObj.GetLabels())) + } + }) +})