fix appRevision label getting overridden (#1293)

* fix lable getting overidden

* randomize namespace

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 {

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"))
+	})
 })

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)
 	})