From efa5078d81156a0daa8046095a1cbbe38eea3e97 Mon Sep 17 00:00:00 2001 From: Ryan Zhang Date: Thu, 25 Mar 2021 02:28:17 -0700 Subject: [PATCH] fix appRevision label getting overridden (#1293) * fix lable getting overidden * randomize namespace --- .../v1alpha2/application/revision.go | 2 +- .../v1alpha2/application/revision_test.go | 66 +++++++++++++++++++ test/e2e-test/rollout_plan_test.go | 4 +- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go index 8fd69512e..6f0462f14 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision.go @@ -99,7 +99,7 @@ func (h *appHandler) GenerateRevision(ctx context.Context, ac *v1alpha2.Applicat if err != nil { return false, nil, err } - appRev.SetLabels(map[string]string{oam.LabelAppRevisionHash: appRevisionHash}) + util.AddLabels(appRev, map[string]string{oam.LabelAppRevisionHash: appRevisionHash}) // check if the appRevision is different from the existing one if h.app.Status.LatestRevision != nil && h.app.Status.LatestRevision.RevisionHash == appRevisionHash { diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go b/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go index 928289abb..4cc91d280 100644 --- a/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go +++ b/pkg/controller/core.oam.dev/v1alpha2/application/revision_test.go @@ -481,4 +481,70 @@ var _ = Describe("test generate revision ", func() { }, time.Second*5, time.Microsecond*500).Should(Succeed()) Expect(len(curACs.Items)).Should(BeEquivalentTo(1)) }) + + It("Test apply passes all label and annotation from app to appRevision", func() { + By("Apply the application") + appParser := appfile.NewApplicationParser(reconciler.Client, reconciler.dm, reconciler.pd) + ctx = util.SetNamespaceInCtx(ctx, app.Namespace) + labelKey1 := "labelKey1" + app.SetLabels(map[string]string{labelKey1: "true"}) + annoKey1 := "annoKey1" + app.SetAnnotations(map[string]string{annoKey1: "true"}) + generatedAppfile, err := appParser.GenerateAppFile(ctx, app.Name, &app) + Expect(err).Should(Succeed()) + ac, comps, err = appParser.GenerateApplicationConfiguration(generatedAppfile, app.Namespace) + Expect(err).Should(Succeed()) + handler.appfile = generatedAppfile + Expect(ac.Namespace).Should(Equal(app.Namespace)) + Expect(handler.apply(context.Background(), ac, comps)).Should(Succeed()) + + curApp := &v1beta1.Application{} + Eventually( + func() error { + return handler.r.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: app.Name}, curApp) + }, time.Second*10, time.Millisecond*500).Should(BeNil()) + Expect(curApp.Status.LatestRevision.Revision).Should(BeEquivalentTo(1)) + By("Verify the created appRevision is exactly what it is") + curAppRevision := &v1beta1.ApplicationRevision{} + Eventually( + func() error { + return handler.r.Get(ctx, + types.NamespacedName{Namespace: ns.Name, Name: curApp.Status.LatestRevision.Name}, + curAppRevision) + }, + time.Second*5, time.Millisecond*500).Should(BeNil()) + appHash1, err := ComputeAppRevisionHash(curAppRevision) + Expect(err).Should(Succeed()) + Expect(curAppRevision.GetLabels()[oam.LabelAppRevisionHash]).Should(Equal(appHash1)) + Expect(appHash1).Should(Equal(curApp.Status.LatestRevision.RevisionHash)) + Expect(curAppRevision.GetLabels()[labelKey1]).Should(Equal("true")) + Expect(curAppRevision.GetAnnotations()[annoKey1]).Should(Equal("true")) + // there can be annotation change and appContext should have the exact label/annotation as app + annoKey2 := "testKey2" + app.SetAnnotations(map[string]string{annoKey2: "true"}) + labelKey2 := "labelKey2" + app.SetLabels(map[string]string{labelKey2: "true"}) + lastRevision := curApp.Status.LatestRevision.Name + Expect(handler.apply(context.Background(), ac, comps)).Should(Succeed()) + Eventually( + func() error { + return handler.r.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: app.Name}, curApp) + }, time.Second*10, time.Millisecond*500).Should(BeNil()) + // no new revision should be created + Expect(curApp.Status.LatestRevision.Name).Should(Equal(lastRevision)) + Expect(curApp.Status.LatestRevision.RevisionHash).Should(Equal(appHash1)) + By("Verify the appRevision is not changed") + // reset appRev + curAppRevision = &v1beta1.ApplicationRevision{} + Eventually( + func() error { + return handler.r.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: lastRevision}, curAppRevision) + }, time.Second*5, time.Millisecond*500).Should(BeNil()) + Expect(err).Should(Succeed()) + Expect(curAppRevision.GetLabels()[oam.LabelAppRevisionHash]).Should(Equal(appHash1)) + Expect(curAppRevision.GetLabels()[labelKey1]).Should(BeEmpty()) + Expect(curAppRevision.GetLabels()[labelKey2]).Should(Equal("true")) + Expect(curAppRevision.GetAnnotations()[annoKey1]).Should(BeEmpty()) + Expect(curAppRevision.GetAnnotations()[annoKey2]).Should(Equal("true")) + }) }) diff --git a/test/e2e-test/rollout_plan_test.go b/test/e2e-test/rollout_plan_test.go index a85ced63d..043bed757 100644 --- a/test/e2e-test/rollout_plan_test.go +++ b/test/e2e-test/rollout_plan_test.go @@ -3,6 +3,8 @@ package controllers_test import ( "context" "fmt" + "math/rand" + "strconv" "time" . "github.com/onsi/ginkgo" @@ -230,7 +232,7 @@ var _ = Describe("Cloneset based rollout tests", func() { BeforeEach(func() { By("Start to run a test, clean up previous resources") - namespace = "rolling-e2e-test" // + "-" + strconv.FormatInt(rand.Int63(), 16) + namespace = "rolling-e2e-test" + "-" + strconv.FormatInt(rand.Int63(), 16) createNamespace(namespace) })