dont't template source in rollout and rollout first component (#1888)

* fix rollout and only rollout the first component

* determine rollout component before render

* let other resources apply when target worklaod not ready

* immutable resource only update ownerRef

* add annotation

* fix unit test

add handle sourceWorkload

add anotation

add comment

add more e2e-test

Co-authored-by: 天元 <jianbo.sjb@alibaba-inc.com>

pkg/controller/core.oam.dev/v1alpha2/application/apply.go

@@ -226,6 +226,8 @@ func (h *appHandler) handleRollout(ctx context.Context) (reconcile.Result, error
 	var comps []string
 	for _, component := range h.app.Spec.Components {
 		comps = append(comps, component.Name)
+		// TODO rollout only support one component now, and we only rollout the first component in Application
+		break
 	}
 
 	// targetRevision should always points to LatestRevison

pkg/controller/core.oam.dev/v1alpha2/application/assemble/options.go

@@ -22,6 +22,8 @@ import (
 	"reflect"
 	"strings"
 
+	"github.com/oam-dev/kubevela/pkg/oam"
+
 	"github.com/crossplane/crossplane-runtime/pkg/fieldpath"
 	kruisev1alpha1 "github.com/openkruise/kruise-api/apps/v1alpha1"
 	"github.com/pkg/errors"
@@ -116,8 +118,14 @@ func discoverHelmModuleWorkload(ctx context.Context, c client.Reader, assembledW
 // PrepareWorkloadForRollout prepare the workload before it is emit to the k8s. The current approach is to mark it
 // as disabled so that it's spec won't take effect immediately. The rollout controller can take over the resources
 // and enable it on its own since app controller here won't override their change
-func PrepareWorkloadForRollout() WorkloadOption {
+func PrepareWorkloadForRollout(rolloutComp string) WorkloadOption {
 	return WorkloadOptionFn(func(assembledWorkload *unstructured.Unstructured, _ *v1beta1.ComponentDefinition, _ []*unstructured.Unstructured) error {
+
+		compName := assembledWorkload.GetLabels()[oam.LabelAppComponent]
+		if compName != rolloutComp {
+			return nil
+		}
+
 		const (
 			// below are the resources that we know how to disable
 			cloneSetDisablePath            = "spec.updateStrategy.paused"

pkg/controller/core.oam.dev/v1alpha2/application/assemble/options_test.go

@@ -65,7 +65,7 @@ var _ = Describe("Test WorkloadOption", func() {
 				StandardWorkload: cs,
 			}
 			By("Add PrepareWorkloadForRollout WorkloadOption")
-			ao := NewAppManifests(appRev).WithWorkloadOption(PrepareWorkloadForRollout())
+			ao := NewAppManifests(appRev).WithWorkloadOption(PrepareWorkloadForRollout(compName))
 			ao.componentManifests = []*types.ComponentManifest{&comp}
 			workloads, _, _, err := ao.GroupAssembledManifests()
 			Expect(err).Should(BeNil())
@@ -89,7 +89,7 @@ var _ = Describe("Test WorkloadOption", func() {
 				StandardWorkload: sts,
 			}
 			By("Add PrepareWorkloadForRollout WorkloadOption")
-			ao := NewAppManifests(appRev).WithWorkloadOption(PrepareWorkloadForRollout())
+			ao := NewAppManifests(appRev).WithWorkloadOption(PrepareWorkloadForRollout(compName))
 			ao.componentManifests = []*types.ComponentManifest{&comp}
 			workloads, _, _, err := ao.GroupAssembledManifests()
 			Expect(err).Should(BeNil())
@@ -106,7 +106,7 @@ var _ = Describe("Test WorkloadOption", func() {
 
 		It("test rollout Deployment", func() {
 			By("Add PrepareWorkloadForRollout WorkloadOption")
-			ao := NewAppManifests(appRev).WithWorkloadOption(PrepareWorkloadForRollout())
+			ao := NewAppManifests(appRev).WithWorkloadOption(PrepareWorkloadForRollout(compName))
 			workloads, _, _, err := ao.GroupAssembledManifests()
 			Expect(err).Should(BeNil())
 			Expect(len(workloads)).Should(Equal(1))

pkg/controller/core.oam.dev/v1alpha2/application/dispatch/dispatch.go

@@ -20,6 +20,8 @@ import (
 	"context"
 	"reflect"
 
+	v1 "k8s.io/api/core/v1"
+
 	"github.com/pkg/errors"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -78,9 +80,9 @@ func (a *AppManifestsDispatcher) EndAndGC(rt *v1beta1.ResourceTracker) *AppManif
 // For resources exists in two revision, dispatcher will update their owner to the new resource tracker.
 // It's helpful in a rollout scenario where new revision is going to create a new workload while the old one should not
 // be deleted before rollout is terminated.
-func (a *AppManifestsDispatcher) StartAndSkipGC(rt *v1beta1.ResourceTracker) *AppManifestsDispatcher {
-	if rt != nil {
-		a.previousRT = rt.DeepCopy()
+func (a *AppManifestsDispatcher) StartAndSkipGC(previousRT *v1beta1.ResourceTracker) *AppManifestsDispatcher {
+	if previousRT != nil {
+		a.previousRT = previousRT.DeepCopy()
 		a.skipGC = true
 	}
 	return a
@@ -133,7 +135,7 @@ func (a *AppManifestsDispatcher) validateAndComplete(ctx context.Context) error
 	a.namespace = a.appRev.Namespace
 	a.currentRTName = ConstructResourceTrackerName(a.appRevName, a.namespace)
 
-	// if upgrade is enbabled (no matter GC or skip GC), it requires a valid existing resource tracker
+	// if upgrade is enabled (no matter GC or skip GC), it requires a valid existing resource tracker
 	if a.previousRT != nil && a.previousRT.Name != a.currentRTName {
 		klog.InfoS("Validate previous resource tracker exists", "previous", klog.KObj(a.previousRT))
 		gotPreviousRT := &v1beta1.ResourceTracker{}
@@ -194,6 +196,18 @@ func (a *AppManifestsDispatcher) applyAndRecordManifests(ctx context.Context, ma
 		BlockOwnerDeletion: pointer.BoolPtr(true),
 	}
 	for _, rsc := range manifests {
+
+		immutable, err := a.ImmutableResourcesUpdate(ctx, rsc, ownerRef, applyOpts)
+		if immutable {
+			if err != nil {
+				klog.ErrorS(err, "Failed to apply immutable resource with new ownerReference", "object",
+					klog.KObj(rsc), "apiVersion", rsc.GetAPIVersion(), "kind", rsc.GetKind())
+				return errors.Wrapf(err, "cannot apply immutable resource with new ownerReference, name: %q apiVersion: %q kind: %q",
+					rsc.GetName(), rsc.GetAPIVersion(), rsc.GetKind())
+			}
+			continue
+		}
+
 		// each resource applied by dispatcher MUST be controlled by resource tracker
 		setOrOverrideControllerOwner(rsc, ownerRef)
 		if err := a.applicator.Apply(ctx, rsc, applyOpts...); err != nil {
@@ -208,6 +222,30 @@ func (a *AppManifestsDispatcher) applyAndRecordManifests(ctx context.Context, ma
 	return a.updateResourceTrackerStatus(ctx, manifests)
 }
 
+// ImmutableResourcesUpdate only updates the ownerReference
+// TODO(wonderflow): we should allow special fields to be updated. e.g. the resources.requests for bound claims for PV should be able to update
+func (a *AppManifestsDispatcher) ImmutableResourcesUpdate(ctx context.Context, res *unstructured.Unstructured, ownerRef metav1.OwnerReference, applyOpts []apply.ApplyOption) (bool, error) {
+	if res == nil {
+		return false, nil
+	}
+	switch res.GroupVersionKind() {
+	case v1.SchemeGroupVersion.WithKind(reflect.TypeOf(v1.PersistentVolume{}).Name()):
+		pv := new(v1.PersistentVolume)
+		err := a.c.Get(ctx, client.ObjectKey{Name: res.GetName(), Namespace: res.GetNamespace()}, pv)
+		if kerrors.IsNotFound(err) {
+			return false, nil
+		}
+		if err != nil {
+			return true, err
+		}
+		setOrOverrideControllerOwner(pv, ownerRef)
+		pv.SetGroupVersionKind(v1.SchemeGroupVersion.WithKind(reflect.TypeOf(v1.PersistentVolume{}).Name()))
+		return true, a.applicator.Apply(ctx, pv, applyOpts...)
+	default:
+	}
+	return false, nil
+}
+
 func (a *AppManifestsDispatcher) updateResourceTrackerStatus(ctx context.Context, appliedManifests []*unstructured.Unstructured) error {
 	// merge applied resources and already tracked ones
 	if a.currentRT.Status.TrackedResources == nil {
@@ -251,7 +289,13 @@ func (a *AppManifestsDispatcher) updateResourceTrackerStatus(ctx context.Context
 	return nil
 }
 
-func setOrOverrideControllerOwner(obj *unstructured.Unstructured, controllerOwner metav1.OwnerReference) {
+// ObjectOwner is a interface for get and set ownerReference
+type ObjectOwner interface {
+	GetOwnerReferences() []metav1.OwnerReference
+	SetOwnerReferences([]metav1.OwnerReference)
+}
+
+func setOrOverrideControllerOwner(obj ObjectOwner, controllerOwner metav1.OwnerReference) {
 	ownerRefs := []metav1.OwnerReference{controllerOwner}
 	for _, owner := range obj.GetOwnerReferences() {
 		if owner.Controller != nil && *owner.Controller &&

pkg/controller/core.oam.dev/v1alpha2/applicationrollout/applicationrollout_controller.go

@@ -136,6 +136,11 @@ func (r *Reconciler) DoReconcile(ctx context.Context, appRollout *v1beta1.AppRol
 		h.targetRevName = appRollout.Spec.TargetAppRevisionName
 	}
 
+	// 	TODO we only support rollout for one component and it should be specified in componentList
+	if len(appRollout.Spec.ComponentList) == 1 {
+		h.needRollComponent = appRollout.Spec.ComponentList[0]
+	}
+
 	// call assemble func generate source and target manifest
 	if err = h.prepareRollout(ctx); err != nil {
 		return reconcile.Result{}, err
@@ -160,10 +165,11 @@ func (r *Reconciler) DoReconcile(ctx context.Context, appRollout *v1beta1.AppRol
 			return ctrl.Result{}, nil
 		}
 	case v1alpha1.LocatingTargetAppState:
-		// dispatch sourceWorkload
-		err = h.templateSourceManifest(ctx)
-		if err != nil {
-			return reconcile.Result{}, err
+		if h.sourceAppRevision != nil {
+			err = h.handleSourceWorkload(ctx)
+			if err != nil {
+				return reconcile.Result{}, err
+			}
 		}
 		// target manifest haven't template yet, call dispatch template target manifest firstly
 		err = h.templateTargetManifest(ctx)

pkg/controller/core.oam.dev/v1alpha2/applicationrollout/helper.go

@@ -19,11 +19,10 @@ package applicationrollout
 import (
 	"reflect"
 
-	"k8s.io/utils/pointer"
-
 	"github.com/openkruise/kruise-api/apps/v1alpha1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/klog/v2"
+	"k8s.io/utils/pointer"
 
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	oamstd "github.com/oam-dev/kubevela/apis/standard.oam.dev/v1alpha1"
@@ -31,8 +30,14 @@ import (
 	"github.com/oam-dev/kubevela/pkg/oam"
 )
 
-func rolloutWorkloadName() assemble.WorkloadOption {
+func rolloutWorkloadName(rolloutComp string) assemble.WorkloadOption {
 	return assemble.WorkloadOptionFn(func(w *unstructured.Unstructured, _ *v1beta1.ComponentDefinition, _ []*unstructured.Unstructured) error {
+
+		compName := w.GetLabels()[oam.LabelAppComponent]
+		if compName != rolloutComp {
+			return nil
+		}
+
 		// we hard code the behavior depends on the workload group/kind for now. The only in-place upgradable resources
 		// we support is cloneset/statefulset for now. We can easily add more later.
 		if w.GroupVersionKind().Group == v1alpha1.GroupVersion.Group {

pkg/controller/core.oam.dev/v1alpha2/applicationrollout/rollout_handler.go

@@ -19,7 +19,12 @@ package applicationrollout
 import (
 	"context"
 	"fmt"
+	"reflect"
 
+	"k8s.io/utils/pointer"
+
+	"github.com/crossplane/crossplane-runtime/pkg/event"
+	"github.com/pkg/errors"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
@@ -27,13 +32,10 @@ import (
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
-	"github.com/crossplane/crossplane-runtime/pkg/event"
-
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	"github.com/oam-dev/kubevela/apis/standard.oam.dev/v1alpha1"
-	"github.com/oam-dev/kubevela/pkg/controller/core.oam.dev/v1alpha2/application/dispatch"
-
 	"github.com/oam-dev/kubevela/pkg/controller/core.oam.dev/v1alpha2/application/assemble"
+	"github.com/oam-dev/kubevela/pkg/controller/core.oam.dev/v1alpha2/application/dispatch"
 	oamutil "github.com/oam-dev/kubevela/pkg/oam/util"
 	appUtil "github.com/oam-dev/kubevela/pkg/webhook/core.oam.dev/v1alpha2/applicationrollout"
 )
@@ -76,8 +78,8 @@ func (h *rolloutHandler) prepareRollout(ctx context.Context) error {
 
 	// construct a assemble manifest for targetAppRevision
 	targetAssemble := assemble.NewAppManifests(h.targetAppRevision).
-		WithWorkloadOption(rolloutWorkloadName()).
-		WithWorkloadOption(assemble.PrepareWorkloadForRollout())
+		WithWorkloadOption(rolloutWorkloadName(h.needRollComponent)).
+		WithWorkloadOption(assemble.PrepareWorkloadForRollout(h.needRollComponent))
 
 	// in template phase, we should use targetManifests including target workloads/traits to
 	h.targetManifests, err = targetAssemble.AssembledManifests()
@@ -100,8 +102,8 @@ func (h *rolloutHandler) prepareRollout(ctx context.Context) error {
 		}
 		// construct a assemble manifest for sourceAppRevision
 		sourceAssemble := assemble.NewAppManifests(h.sourceAppRevision).
-			WithWorkloadOption(assemble.PrepareWorkloadForRollout()).
-			WithWorkloadOption(rolloutWorkloadName())
+			WithWorkloadOption(assemble.PrepareWorkloadForRollout(h.needRollComponent)).
+			WithWorkloadOption(rolloutWorkloadName(h.needRollComponent))
 		h.sourceWorkloads, _, _, err = sourceAssemble.GroupAssembledManifests()
 		if err != nil {
 			klog.Error("appRollout sourceAppRevision failed to assemble workloads", "appRollout", klog.KRef(h.appRollout.Namespace, h.appRollout.Name))
@@ -118,6 +120,10 @@ func (h *rolloutHandler) prepareRollout(ctx context.Context) error {
 
 // we only support one workload now, so this func is to determine witch component is need to rollout
 func (h *rolloutHandler) determineRolloutComponent() error {
+	if h.needRollComponent != "" {
+		return nil
+	}
+
 	componentList := h.appRollout.Spec.ComponentList
 	// if user not set ComponentList in AppRollout we also find a common component between source and target
 	if len(componentList) == 0 {
@@ -211,38 +217,14 @@ func (h *rolloutHandler) templateTargetManifest(ctx context.Context) error {
 		klog.Errorf("dispatch targetRevision error %s:%v", h.appRollout.Spec.TargetAppRevisionName, err)
 		return err
 	}
-	workload, err := h.extractWorkload(ctx, *h.targetWorkloads[h.needRollComponent])
-	if err != nil {
-		return err
-	}
-	ref := metav1.GetControllerOfNoCopy(workload)
-	if ref != nil && ref.Kind == v1beta1.ResourceTrackerKind {
-		wlPatch := client.MergeFrom(workload.DeepCopy())
-		// guarantee resourceTracker isn't controller owner of workload
-		disableControllerOwner(workload)
-		if err = h.Client.Patch(ctx, workload, wlPatch, client.FieldOwner(h.appRollout.UID)); err != nil {
-			return err
-		}
-	}
-	return nil
-}
 
-// templateTargetManifest call dispatch to template source app revision's manifests to cluster
-func (h *rolloutHandler) templateSourceManifest(ctx context.Context) error {
-
-	// only when sourceAppRevision is not nil, we need template sourceRevision revision
-	if h.sourceAppRevision == nil {
-		return nil
+	// The workload not in target workloads can be not ready for insertSecret case
+	targetWL := h.targetWorkloads[h.needRollComponent]
+	if targetWL == nil {
+		return errors.Errorf("target workload for component %s for app %s is not ready", h.needRollComponent, h.targetAppRevision.Spec.Application.Name)
 	}
 
-	// use source resourceTracker to handle same resource owner transfer
-	dispatcher := dispatch.NewAppManifestsDispatcher(h, h.sourceAppRevision)
-	_, err := dispatcher.Dispatch(ctx, h.sourceManifests)
-	if err != nil {
-		klog.Errorf("dispatch sourceRevision error %s:%v", h.appRollout.Spec.TargetAppRevisionName, err)
-		return err
-	}
-	workload, err := h.extractWorkload(ctx, *h.sourceWorkloads[h.needRollComponent])
+	workload, err := h.extractWorkload(ctx, *targetWL)
 	if err != nil {
 		return err
 	}
@@ -293,3 +275,52 @@ func (h *rolloutHandler) finalizeRollingSucceeded(ctx context.Context) error {
 	}
 	return nil
 }
+
+// this func handle two case
+// 1. handle 1.0.x lagacy workload, their owner is appcontext so let resourceTracker take over it
+// 2. disable resourceTracker controller owner
+func (h *rolloutHandler) handleSourceWorkload(ctx context.Context) error {
+	workload, err := h.extractWorkload(ctx, *h.sourceWorkloads[h.needRollComponent])
+	if err != nil {
+		return err
+	}
+	owners := workload.GetOwnerReferences()
+	var wantOwner []metav1.OwnerReference
+	wlPatch := client.MergeFrom(workload.DeepCopy())
+	for _, owner := range owners {
+		if owner.Kind == v1beta1.ResourceTrackerKind {
+			wantOwner = append(wantOwner, owner)
+		}
+	}
+	// logic here is  compatibility code for 1.0.X lagacy workload, their ownerReference is appcontext, so remove it
+	if len(wantOwner) == 0 {
+		klog.InfoS("meet a lagacy workload, should let resourceTracker take over it")
+		rtName := dispatch.ConstructResourceTrackerName(h.sourceRevName, h.appRollout.Namespace)
+		rt := v1beta1.ResourceTracker{}
+		if err := h.Get(ctx, types.NamespacedName{Name: rtName}, &rt); err != nil {
+			if apierrors.IsNotFound(err) {
+				if err = h.Create(ctx, &rt); err != nil {
+					return err
+				}
+			} else {
+				return err
+			}
+		}
+		ownerRef := metav1.OwnerReference{
+			APIVersion:         v1beta1.SchemeGroupVersion.String(),
+			Kind:               reflect.TypeOf(v1beta1.ResourceTracker{}).Name(),
+			Name:               rt.Name,
+			UID:                rt.UID,
+			Controller:         pointer.BoolPtr(true),
+			BlockOwnerDeletion: pointer.BoolPtr(true),
+		}
+		workload.SetOwnerReferences([]metav1.OwnerReference{ownerRef})
+	}
+	// after last succeed rollout finish, workload's ownerReference controller have been set true
+	// so we should disable the controller owner firstly
+	disableControllerOwner(workload)
+	if err = h.Client.Patch(ctx, workload, wlPatch, client.FieldOwner(h.appRollout.UID)); err != nil {
+		return err
+	}
+	return nil
+}

test/e2e-test/app_embed_rollout_test.go

@@ -91,6 +91,18 @@ var _ = Describe("Cloneset based app embed rollout tests", func() {
 			time.Second*3, time.Millisecond*300).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))
 	}
 
+	CreateIngressDef := func() {
+		By("Install Ingress trait definition")
+		var td v1beta1.TraitDefinition
+		Expect(common.ReadYamlToObject("testdata/rollout/cloneset/ingressDefinition.yaml", &td)).Should(BeNil())
+		// create the traitDefinition if not exist
+		Eventually(
+			func() error {
+				return k8sClient.Create(ctx, &td)
+			},
+			time.Second*3, time.Millisecond*300).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))
+	}
+
 	generateNewApp := func(appName, namespace, compType string, plan *v1alpha1.RolloutPlan) *v1beta1.Application {
 		return &v1beta1.Application{
 			TypeMeta: metav1.TypeMeta{
@@ -121,6 +133,7 @@ var _ = Describe("Cloneset based app embed rollout tests", func() {
 		namespaceName = randomNamespaceName("app-rollout-e2e-test")
 		createNamespace()
 		CreateClonesetDef()
+		CreateIngressDef()
 	})
 
 	AfterEach(func() {
@@ -465,6 +478,76 @@ var _ = Describe("Cloneset based app embed rollout tests", func() {
 		verifyRolloutSucceeded(utils.ConstructRevisionName(appName, 2), "2")
 	})
 
+	It("Test rollout with trait", func() {
+		plan := &v1alpha1.RolloutPlan{
+			RolloutStrategy: v1alpha1.IncreaseFirstRolloutStrategyType,
+			RolloutBatches: []v1alpha1.RolloutBatch{
+				{
+					Replicas: intstr.FromString("50%"),
+				},
+				{
+					Replicas: intstr.FromString("50%"),
+				},
+			},
+			TargetSize: pointer.Int32Ptr(6),
+		}
+		appName = "app-rollout-5"
+		app := generateNewApp(appName, namespaceName, "clonesetservice", plan)
+		ingressProperties := `{"domain":"test-1.example.com","http":{"/":8080}}`
+		app.Spec.Components[0].Traits = []v1beta1.ApplicationTrait{{Type: "ingress", Properties: runtime.RawExtension{Raw: []byte(ingressProperties)}}}
+		Expect(k8sClient.Create(ctx, app)).Should(BeNil())
+		verifyRolloutSucceeded(utils.ConstructRevisionName(appName, 1), "1")
+		updateAppWithCpuAndPlan(app, "2", plan)
+		By("rollout to v2")
+		checkApp := new(v1beta1.Application)
+		verifyRolloutSucceeded(utils.ConstructRevisionName(appName, 2), "2")
+		Expect(k8sClient.Get(ctx, ctypes.NamespacedName{Name: appName, Namespace: namespaceName}, checkApp)).Should(BeNil())
+		Expect(checkApp.Status.LatestRevision.Name).Should(BeEquivalentTo(utils.ConstructRevisionName(appName, 2)))
+		updateAppWithCpuAndPlan(app, "3", plan)
+		By("rollout to v3")
+		verifyRolloutSucceeded(utils.ConstructRevisionName(appName, 3), "3")
+		Expect(k8sClient.Get(ctx, ctypes.NamespacedName{Name: appName, Namespace: namespaceName}, checkApp)).Should(BeNil())
+		Expect(checkApp.Status.LatestRevision.Name).Should(BeEquivalentTo(utils.ConstructRevisionName(appName, 3)))
+	})
+
+	It("Test rollout with another component only rollout first component", func() {
+		plan := &v1alpha1.RolloutPlan{
+			RolloutStrategy: v1alpha1.IncreaseFirstRolloutStrategyType,
+			RolloutBatches: []v1alpha1.RolloutBatch{
+				{
+					Replicas: intstr.FromString("50%"),
+				},
+				{
+					Replicas: intstr.FromString("50%"),
+				},
+			},
+			TargetSize: pointer.Int32Ptr(6),
+		}
+		appName = "app-rollout-6"
+		app := generateNewApp(appName, namespaceName, "clonesetservice", plan)
+		annotherComp := v1beta1.ApplicationComponent{
+			Name: "another-comp",
+			Type: "clonesetservice",
+			Properties: runtime.RawExtension{
+				Raw: []byte(initialProperty),
+			},
+		}
+		app.Spec.Components = append(app.Spec.Components, annotherComp)
+		Expect(k8sClient.Create(ctx, app)).Should(BeNil())
+		verifyRolloutSucceeded(utils.ConstructRevisionName(appName, 1), "1")
+		updateAppWithCpuAndPlan(app, "2", plan)
+
+		checkApp := new(v1beta1.Application)
+		By("verify update rolloutPlan shouldn't create new revision")
+		verifyRolloutSucceeded(utils.ConstructRevisionName(appName, 2), "2")
+		Expect(k8sClient.Get(ctx, ctypes.NamespacedName{Name: appName, Namespace: namespaceName}, checkApp)).Should(BeNil())
+		Expect(checkApp.Status.LatestRevision.Name).Should(BeEquivalentTo(utils.ConstructRevisionName(appName, 2)))
+		updateAppWithCpuAndPlan(app, "3", plan)
+		verifyRolloutSucceeded(utils.ConstructRevisionName(appName, 3), "3")
+		Expect(k8sClient.Get(ctx, ctypes.NamespacedName{Name: appName, Namespace: namespaceName}, checkApp)).Should(BeNil())
+		Expect(checkApp.Status.LatestRevision.Name).Should(BeEquivalentTo(utils.ConstructRevisionName(appName, 3)))
+	})
+
 	//  TODO add more corner case tests
 	//  update application by clean rolloutPlan strategy in the middle of rollout process
 })