garbage collection mechanism for AppRevision (#1501)

* WIP refactor gc func

WIP write gc code

WIP update chart,cmd args

add cleanupRevison func into garbageCollection

add more test

finish unit-test

refactor use func implements

Complete e2e test

WIP rewrite some logic

add func test and rewirte context pointing to func

fix cilint

refactor some function

fix typo

fix ci error

change gc logic

after change gc number, fix all test

add check appRevision collection

WIP finish most code

* add rollout batch to fix rollout e2e-tet

* fix component name in rollout

* ignore gc error, just log the error

pkg/controller/core.oam.dev/oamruntime_controller.go

@@ -48,6 +48,10 @@ type Args struct {
 	// The default value is 50.
 	RevisionLimit int
 
+	// AppRevisionLimit is the maximum number of application revisions that will be maintained.
+	// The default value is 10.
+	AppRevisionLimit int
+
 	// ApplyMode indicates whether workloads and traits should be
 	// affected if no spec change is made in the ApplicationConfiguration.
 	ApplyMode ApplyOnceOnlyMode

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

@@ -54,11 +54,12 @@ const (
 // Reconciler reconciles a Application object
 type Reconciler struct {
 	client.Client
-	dm         discoverymapper.DiscoveryMapper
-	pd         *definition.PackageDiscover
-	Log        logr.Logger
-	Scheme     *runtime.Scheme
-	applicator apply.Applicator
+	dm               discoverymapper.DiscoveryMapper
+	pd               *definition.PackageDiscover
+	Log              logr.Logger
+	Scheme           *runtime.Scheme
+	applicator       apply.Applicator
+	appRevisionLimit int
 }
 
 // +kubebuilder:rbac:groups=core.oam.dev,resources=applications,verbs=get;list;watch;create;update;patch;delete
@@ -181,11 +182,9 @@ func (r *Reconciler) Reconcile(req ctrl.Request) (ctrl.Result, error) {
 	app.Status.Services = appCompStatus
 	app.Status.SetConditions(readyCondition("HealthCheck"))
 	app.Status.Phase = common.ApplicationRunning
-	err = handler.garbageCollection(ctx)
+	err = garbageCollection(ctx, handler)
 	if err != nil {
 		applog.Error(err, "[Garbage collection]")
-		app.Status.SetConditions(errorCondition("GarbageCollection", err))
-		return handler.handleErr(err)
 	}
 	// Gather status of components
 	var refComps []v1alpha1.TypedReference
@@ -233,12 +232,13 @@ func (r *Reconciler) UpdateStatus(ctx context.Context, app *v1beta1.Application,
 // Setup adds a controller that reconciles AppRollout.
 func Setup(mgr ctrl.Manager, args core.Args, _ logging.Logger) error {
 	reconciler := Reconciler{
-		Client:     mgr.GetClient(),
-		Log:        ctrl.Log.WithName("Application"),
-		Scheme:     mgr.GetScheme(),
-		dm:         args.DiscoveryMapper,
-		pd:         args.PackageDiscover,
-		applicator: apply.NewAPIApplicator(mgr.GetClient()),
+		Client:           mgr.GetClient(),
+		Log:              ctrl.Log.WithName("Application"),
+		Scheme:           mgr.GetScheme(),
+		dm:               args.DiscoveryMapper,
+		pd:               args.PackageDiscover,
+		applicator:       apply.NewAPIApplicator(mgr.GetClient()),
+		appRevisionLimit: args.AppRevisionLimit,
 	}
 	return reconciler.SetupWithManager(mgr)
 }

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

@@ -587,9 +587,26 @@ func (h *appHandler) recodeTrackedResource(resourceName string, resource runtime
 	return nil
 }
 
+type garbageCollectFunc func(ctx context.Context, h *appHandler) error
+
+// 1. collect useless across-namespace resource
+// 2. collect appRevision
+func garbageCollection(ctx context.Context, h *appHandler) error {
+	collectFuncs := []garbageCollectFunc{
+		garbageCollectFunc(gcAcrossNamespaceResource),
+		garbageCollectFunc(cleanUpApplicationRevision),
+	}
+	for _, collectFunc := range collectFuncs {
+		if err := collectFunc(ctx, h); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
 // Now if workloads or traits are in the same namespace with application, applicationContext will take over gc workloads and traits.
 // Here we cover the case in witch a cross namespace component or one of its cross namespace trait is removed from an application.
-func (h *appHandler) garbageCollection(ctx context.Context) error {
+func gcAcrossNamespaceResource(ctx context.Context, h *appHandler) error {
 	rt := new(v1beta1.ResourceTracker)
 	err := h.r.Get(ctx, ctypes.NamespacedName{Name: h.generateResourceTrackerName()}, rt)
 	if err != nil {

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

@@ -18,9 +18,11 @@ package application
 
 import (
 	"context"
+	"sort"
 
 	"github.com/pkg/errors"
 	apiequality "k8s.io/apimachinery/pkg/api/equality"
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/utils/pointer"
 	"sigs.k8s.io/controller-runtime/pkg/client"
@@ -291,3 +293,81 @@ func ComputeAppRevisionHash(appRevision *v1beta1.ApplicationRevision) (string, e
 	// compute the hash of the entire structure
 	return utils.ComputeSpecHash(&appRevisionHash)
 }
+
+// cleanUpApplicationRevision check all appRevisions of the application, remove them if the number of them exceed the limit
+func cleanUpApplicationRevision(ctx context.Context, h *appHandler) error {
+	listOpts := []client.ListOption{
+		client.InNamespace(h.app.Namespace),
+		client.MatchingLabels{oam.LabelAppName: h.app.Name},
+	}
+	appRevisionList := new(v1beta1.ApplicationRevisionList)
+	// controller-runtime will cache all appRevision by default, there is no need to watch or own appRevision in manager
+	if err := h.r.List(ctx, appRevisionList, listOpts...); err != nil {
+		return err
+	}
+	usingRevision, err := gatherUsingAppRevision(ctx, h)
+	if err != nil {
+		return err
+	}
+	needKill := len(appRevisionList.Items) - h.r.appRevisionLimit - len(usingRevision)
+	if needKill <= 0 {
+		return nil
+	}
+	h.logger.Info("application controller cleanup old appRevisions", "needKillNum", needKill)
+	sortedRevision := appRevisionList.Items
+	sort.Sort(historiesByRevision(sortedRevision))
+
+	for _, rev := range sortedRevision {
+		if needKill <= 0 {
+			break
+		}
+		// we shouldn't delete the revision witch appContext pointing to
+		if usingRevision[rev.Name] {
+			continue
+		}
+		if err := h.r.Delete(ctx, rev.DeepCopy()); err != nil && !apierrors.IsNotFound(err) {
+			return err
+		}
+		needKill--
+	}
+	return nil
+}
+
+// gatherUsingAppRevision get all using appRevisions include app's status pointing to and appContext point to
+func gatherUsingAppRevision(ctx context.Context, h *appHandler) (map[string]bool, error) {
+	listOpts := []client.ListOption{
+		client.InNamespace(h.app.Namespace),
+		client.MatchingLabels{oam.LabelAppName: h.app.Name},
+	}
+	usingRevision := map[string]bool{}
+	if h.app.Status.LatestRevision != nil && len(h.app.Status.LatestRevision.Name) != 0 {
+		usingRevision[h.app.Status.LatestRevision.Name] = true
+	}
+	appContextList := new(v1alpha2.ApplicationContextList)
+	err := h.r.List(ctx, appContextList, listOpts...)
+	if err != nil {
+		return nil, err
+	}
+	for _, appContext := range appContextList.Items {
+		usingRevision[appContext.Spec.ApplicationRevisionName] = true
+	}
+	appDeployUsingRevision, err := utils.CheckAppDeploymentUsingAppRevision(ctx, h.r, h.app.Namespace, h.app.Name)
+	if err != nil {
+		return usingRevision, err
+	}
+	for _, revName := range appDeployUsingRevision {
+		usingRevision[revName] = true
+	}
+	return usingRevision, nil
+}
+
+type historiesByRevision []v1beta1.ApplicationRevision
+
+func (h historiesByRevision) Len() int      { return len(h) }
+func (h historiesByRevision) Swap(i, j int) { h[i], h[j] = h[j], h[i] }
+func (h historiesByRevision) Less(i, j int) bool {
+	// the appRevision is generated by vela,  the error always is nil, so ignore it
+	ir, _ := util.ExtractRevisionNum(h[i].Name)
+	ij, _ := util.ExtractRevisionNum(h[j].Name)
+	return ir < ij
+}

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

@@ -0,0 +1,427 @@
+/*
+Copyright 2021 The KubeVela 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 application
+
+import (
+	"context"
+	"encoding/json"
+	"fmt"
+	"time"
+
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/common"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	apierrors "k8s.io/apimachinery/pkg/api/errors"
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/types"
+	ctrl "sigs.k8s.io/controller-runtime"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/ghodss/yaml"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1alpha2"
+	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
+	"github.com/oam-dev/kubevela/pkg/oam"
+	"github.com/oam-dev/kubevela/pkg/oam/util"
+)
+
+var _ = Describe("Test application controller clean up ", func() {
+	ctx := context.TODO()
+	namespace := "clean-up-revision"
+
+	cd := &v1beta1.ComponentDefinition{}
+	cdDefJson, _ := yaml.YAMLToJSON([]byte(normalCompDefYaml))
+
+	BeforeEach(func() {
+		ns := v1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: namespace,
+			},
+		}
+		Expect(k8sClient.Create(ctx, &ns)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))
+
+		Expect(json.Unmarshal(cdDefJson, cd)).Should(BeNil())
+		Expect(k8sClient.Create(ctx, cd.DeepCopy())).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))
+	})
+
+	AfterEach(func() {
+		By("[TEST] Clean up resources after an integration test")
+	})
+
+	It("Test clean up appRevision", func() {
+		appName := "app-1"
+		appKey := types.NamespacedName{Namespace: namespace, Name: appName}
+		app := getApp(appName, namespace, "normal-worker")
+		Expect(k8sClient.Create(ctx, app)).Should(BeNil())
+		checkApp := new(v1beta1.Application)
+		for i := 0; i < appRevisionLimit+1; i++ {
+			Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
+			property := fmt.Sprintf(`{"cmd":["sleep","1000"],"image":"busybox:%d"}`, i)
+			checkApp.Spec.Components[0].Properties = runtime.RawExtension{Raw: []byte(property)}
+			Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil())
+			_, err := reconciler.Reconcile(ctrl.Request{NamespacedName: appKey})
+			Expect(err).Should(BeNil())
+		}
+		appContext := new(v1alpha2.ApplicationContext)
+		Expect(k8sClient.Get(ctx, appKey, appContext)).Should(BeNil())
+		listOpts := []client.ListOption{
+			client.InNamespace(namespace),
+			client.MatchingLabels{
+				oam.LabelAppName: appName,
+			},
+		}
+		appRevisionList := new(v1beta1.ApplicationRevisionList)
+		Eventually(func() error {
+			err := k8sClient.List(ctx, appRevisionList, listOpts...)
+			if err != nil {
+				return err
+			}
+			if len(appRevisionList.Items) != appRevisionLimit+1 {
+				return fmt.Errorf("error appRevison number wants %d, actually %d", appRevisionLimit+1, len(appRevisionList.Items))
+			}
+			return nil
+		}, time.Second*30, time.Microsecond*300).Should(BeNil())
+
+		By("create new appRevision will remove appRevison1")
+		Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
+		property := fmt.Sprintf(`{"cmd":["sleep","1000"],"image":"busybox:%d"}`, 6)
+		checkApp.Spec.Components[0].Properties = runtime.RawExtension{Raw: []byte(property)}
+		Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil())
+		_, err := reconciler.Reconcile(ctrl.Request{NamespacedName: appKey})
+		Expect(err).Should(BeNil())
+		deletedRevison := new(v1beta1.ApplicationRevision)
+		revKey := types.NamespacedName{Namespace: namespace, Name: appName + "-v1"}
+		Eventually(func() error {
+			err := k8sClient.List(ctx, appRevisionList, listOpts...)
+			if err != nil {
+				return err
+			}
+			if len(appRevisionList.Items) != appRevisionLimit+1 {
+				return fmt.Errorf("error appRevison number wants %d, actually %d", appRevisionLimit+1, len(appRevisionList.Items))
+			}
+			err = k8sClient.Get(ctx, revKey, deletedRevison)
+			if err == nil || !apierrors.IsNotFound(err) {
+				return fmt.Errorf("haven't clean up the oldest revision")
+			}
+			return nil
+		}, time.Second*30, time.Microsecond*300).Should(BeNil())
+
+		By("update app again will gc appRevision2")
+		Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
+		property = fmt.Sprintf(`{"cmd":["sleep","1000"],"image":"busybox:%d"}`, 7)
+		checkApp.Spec.Components[0].Properties = runtime.RawExtension{Raw: []byte(property)}
+		Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil())
+		_, err = reconciler.Reconcile(ctrl.Request{NamespacedName: appKey})
+		Expect(err).Should(BeNil())
+		Eventually(func() error {
+			err := k8sClient.List(ctx, appRevisionList, listOpts...)
+			if err != nil {
+				return err
+			}
+			if len(appRevisionList.Items) != appRevisionLimit+1 {
+				return fmt.Errorf("error appRevison number wants %d, actually %d", appRevisionLimit+1, len(appRevisionList.Items))
+			}
+			revKey = types.NamespacedName{Namespace: namespace, Name: appName + "-v2"}
+			err = k8sClient.Get(ctx, revKey, deletedRevison)
+			if err == nil || !apierrors.IsNotFound(err) {
+				return fmt.Errorf("haven't clean up the  revision-2")
+			}
+			return nil
+		}, time.Second*30, time.Microsecond*300).Should(BeNil())
+	})
+
+	It("Test clean up rollout appRevision", func() {
+		appName := "app-2"
+		appKey := types.NamespacedName{Namespace: namespace, Name: appName}
+		app := getApp(appName, namespace, "normal-worker")
+		metav1.SetMetaDataAnnotation(&app.ObjectMeta, oam.AnnotationAppRollout, "true")
+		metav1.SetMetaDataAnnotation(&app.ObjectMeta, oam.AnnotationRollingComponent, "comp1")
+		Expect(k8sClient.Create(ctx, app)).Should(BeNil())
+		checkApp := new(v1beta1.Application)
+		for i := 0; i < appRevisionLimit+1; i++ {
+			Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
+			property := fmt.Sprintf(`{"cmd":["sleep","1000"],"image":"busybox:%d"}`, i)
+			checkApp.Spec.Components[0].Properties = runtime.RawExtension{Raw: []byte(property)}
+			Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil())
+			_, err := reconciler.Reconcile(ctrl.Request{NamespacedName: appKey})
+			Expect(err).Should(BeNil())
+		}
+		appContext := new(v1alpha2.ApplicationContext)
+		Expect(k8sClient.Get(ctx, appKey, appContext)).Should(util.NotFoundMatcher{})
+		listOpts := []client.ListOption{
+			client.InNamespace(namespace),
+			client.MatchingLabels{
+				oam.LabelAppName: appName,
+			},
+		}
+		appRevisionList := new(v1beta1.ApplicationRevisionList)
+		Eventually(func() error {
+			err := k8sClient.List(ctx, appRevisionList, listOpts...)
+			if err != nil {
+				return err
+			}
+			if len(appRevisionList.Items) != appRevisionLimit+1 {
+				return fmt.Errorf("error appRevison number wants %d, actually %d", appRevisionLimit+1, len(appRevisionList.Items))
+			}
+			return nil
+		}, time.Second*30, time.Microsecond*300).Should(BeNil())
+
+		By("create new appRevision will remove appRevison1")
+		Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
+		property := fmt.Sprintf(`{"cmd":["sleep","1000"],"image":"busybox:%d"}`, 6)
+		checkApp.Spec.Components[0].Properties = runtime.RawExtension{Raw: []byte(property)}
+		Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil())
+		_, err := reconciler.Reconcile(ctrl.Request{NamespacedName: appKey})
+		Expect(err).Should(BeNil())
+		deletedRevison := new(v1beta1.ApplicationRevision)
+		revKey := types.NamespacedName{Namespace: namespace, Name: appName + "-v1"}
+		Eventually(func() error {
+			err := k8sClient.List(ctx, appRevisionList, listOpts...)
+			if err != nil {
+				return err
+			}
+			if len(appRevisionList.Items) != appRevisionLimit+1 {
+				return fmt.Errorf("error appRevison number wants %d, actually %d", appRevisionLimit+1, len(appRevisionList.Items))
+			}
+			err = k8sClient.Get(ctx, revKey, deletedRevison)
+			if err == nil || !apierrors.IsNotFound(err) {
+				return fmt.Errorf("haven't clean up the oldest revision")
+			}
+			if res, err := util.CheckAppRevision(appRevisionList.Items, []int{2, 3, 4, 5, 6, 7}); err != nil || !res {
+				return fmt.Errorf("appRevision collection mismatch")
+			}
+			return nil
+		}, time.Second*30, time.Microsecond*300).Should(BeNil())
+
+		By("update app again will gc appRevision2")
+		Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
+		property = fmt.Sprintf(`{"cmd":["sleep","1000"],"image":"busybox:%d"}`, 7)
+		checkApp.Spec.Components[0].Properties = runtime.RawExtension{Raw: []byte(property)}
+		Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil())
+		_, err = reconciler.Reconcile(ctrl.Request{NamespacedName: appKey})
+		Expect(err).Should(BeNil())
+		Eventually(func() error {
+			err := k8sClient.List(ctx, appRevisionList, listOpts...)
+			if err != nil {
+				return err
+			}
+			if len(appRevisionList.Items) != appRevisionLimit+1 {
+				return fmt.Errorf("error appRevison number wants %d, actually %d", appRevisionLimit+1, len(appRevisionList.Items))
+			}
+			revKey = types.NamespacedName{Namespace: namespace, Name: appName + "-v2"}
+			err = k8sClient.Get(ctx, revKey, deletedRevison)
+			if err == nil || !apierrors.IsNotFound(err) {
+				return fmt.Errorf("haven't clean up the  revision-2")
+			}
+			if res, err := util.CheckAppRevision(appRevisionList.Items, []int{3, 4, 5, 6, 7, 8}); err != nil || !res {
+				return fmt.Errorf("appRevision collection mismatch")
+			}
+			return nil
+		}, time.Second*30, time.Microsecond*300).Should(BeNil())
+	})
+
+	It("Test clean up appDeployment using appRevision", func() {
+		appName := "app-4"
+		appKey := types.NamespacedName{Namespace: namespace, Name: appName}
+		app := getApp(appName, namespace, "normal-worker")
+		metav1.SetMetaDataAnnotation(&app.ObjectMeta, oam.AnnotationAppRollout, "true")
+		metav1.SetMetaDataAnnotation(&app.ObjectMeta, oam.AnnotationRollingComponent, "comp1")
+		Expect(k8sClient.Create(ctx, app)).Should(BeNil())
+		checkApp := new(v1beta1.Application)
+		for i := 0; i < appRevisionLimit+1; i++ {
+			Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
+			property := fmt.Sprintf(`{"cmd":["sleep","1000"],"image":"busybox:%d"}`, i)
+			checkApp.Spec.Components[0].Properties = runtime.RawExtension{Raw: []byte(property)}
+			Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil())
+			_, err := reconciler.Reconcile(ctrl.Request{NamespacedName: appKey})
+			Expect(err).Should(BeNil())
+		}
+		appContext := new(v1alpha2.ApplicationContext)
+		Expect(k8sClient.Get(ctx, appKey, appContext)).Should(util.NotFoundMatcher{})
+		listOpts := []client.ListOption{
+			client.InNamespace(namespace),
+			client.MatchingLabels{
+				oam.LabelAppName: appName,
+			},
+		}
+		appRevisionList := new(v1beta1.ApplicationRevisionList)
+		Eventually(func() error {
+			err := k8sClient.List(ctx, appRevisionList, listOpts...)
+			if err != nil {
+				return err
+			}
+			if len(appRevisionList.Items) != appRevisionLimit+1 {
+				return fmt.Errorf("error appRevison number wants %d, actually %d", appRevisionLimit+1, len(appRevisionList.Items))
+			}
+			return nil
+		}, time.Second*30, time.Microsecond*300).Should(BeNil())
+
+		By("create new appRevision will remove appRevison1")
+		Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
+		property := fmt.Sprintf(`{"cmd":["sleep","1000"],"image":"busybox:%d"}`, 6)
+		checkApp.Spec.Components[0].Properties = runtime.RawExtension{Raw: []byte(property)}
+		Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil())
+		_, err := reconciler.Reconcile(ctrl.Request{NamespacedName: appKey})
+		Expect(err).Should(BeNil())
+		deletedRevison := new(v1beta1.ApplicationRevision)
+		revKey := types.NamespacedName{Namespace: namespace, Name: appName + "-v1"}
+		Eventually(func() error {
+			err := k8sClient.List(ctx, appRevisionList, listOpts...)
+			if err != nil {
+				return err
+			}
+			if len(appRevisionList.Items) != appRevisionLimit+1 {
+				return fmt.Errorf("error appRevison number wants %d, actually %d", appRevisionLimit+1, len(appRevisionList.Items))
+			}
+			err = k8sClient.Get(ctx, revKey, deletedRevison)
+			if err == nil || !apierrors.IsNotFound(err) {
+				return fmt.Errorf("haven't clean up the oldest revision")
+			}
+			if res, err := util.CheckAppRevision(appRevisionList.Items, []int{2, 3, 4, 5, 6, 7}); err != nil || !res {
+				return fmt.Errorf("appRevision collection mismatch")
+			}
+			return nil
+		}, time.Second*30, time.Microsecond*300).Should(BeNil())
+
+		By("update create appDeploy check gc logic")
+		appDeploy := &v1beta1.AppDeployment{
+			TypeMeta: metav1.TypeMeta{
+				APIVersion: v1beta1.AppDeploymentKindAPIVersion,
+				Kind:       v1beta1.AppDeploymentKind,
+			},
+			ObjectMeta: metav1.ObjectMeta{
+				Namespace: namespace,
+				Name:      "app-deploy",
+			},
+			Spec: v1beta1.AppDeploymentSpec{
+				AppRevisions: []v1beta1.AppRevision{
+					{
+						RevisionName: appName + "-v2",
+					},
+				},
+			},
+		}
+		Expect(k8sClient.Create(ctx, appDeploy)).Should(BeNil())
+		// give informer some time to cache
+		time.Sleep(2 * time.Second)
+		for i := 7; i < 9; i++ {
+			Expect(k8sClient.Get(ctx, appKey, checkApp)).Should(BeNil())
+			property = fmt.Sprintf(`{"cmd":["sleep","1000"],"image":"busybox:%d"}`, i)
+			checkApp.Spec.Components[0].Properties = runtime.RawExtension{Raw: []byte(property)}
+			Expect(k8sClient.Update(ctx, checkApp)).Should(BeNil())
+			_, err = reconciler.Reconcile(ctrl.Request{NamespacedName: appKey})
+			Expect(err).Should(BeNil())
+		}
+		Eventually(func() error {
+			err := k8sClient.List(ctx, appRevisionList, listOpts...)
+			if err != nil {
+				return err
+			}
+			if len(appRevisionList.Items) != appRevisionLimit+2 {
+				return fmt.Errorf("error appRevison number wants %d, actually %d", appRevisionLimit+2, len(appRevisionList.Items))
+			}
+			revKey = types.NamespacedName{Namespace: namespace, Name: appName + "-v3"}
+			err = k8sClient.Get(ctx, revKey, deletedRevison)
+			if err == nil || !apierrors.IsNotFound(err) {
+				return fmt.Errorf("haven't clean up the  revision-3")
+			}
+			if res, err := util.CheckAppRevision(appRevisionList.Items, []int{2, 4, 5, 6, 7, 8, 9}); err != nil || !res {
+				return fmt.Errorf("appRevision collection mismatch")
+			}
+			return nil
+		}, time.Second*30, time.Microsecond*300).Should(BeNil())
+	})
+})
+
+var _ = Describe("Test gatherUsingAppRevision func", func() {
+	ctx := context.TODO()
+	namespace := "clean-up-revision"
+
+	cd := &v1beta1.ComponentDefinition{}
+	cdDefJson, _ := yaml.YAMLToJSON([]byte(normalCompDefYaml))
+
+	BeforeEach(func() {
+		ns := v1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: namespace,
+			},
+		}
+		Expect(k8sClient.Create(ctx, &ns)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))
+
+		Expect(json.Unmarshal(cdDefJson, cd)).Should(BeNil())
+		Expect(k8sClient.Create(ctx, cd.DeepCopy())).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))
+	})
+
+	AfterEach(func() {
+		By("[TEST] Clean up resources after an integration test")
+	})
+
+	It("get gatherUsingAppRevision func logic", func() {
+		appName := "app-3"
+		app := getApp(appName, namespace, "normal-worker")
+		app.Status.LatestRevision = &common.Revision{
+			Name: appName + "-v1",
+		}
+		Expect(k8sClient.Create(ctx, app)).Should(BeNil())
+		appContext := getAppContext(namespace, appName+"-ctx", appName+"-v2")
+		appContext.Labels = map[string]string{
+			oam.LabelAppName: appName,
+		}
+		Expect(k8sClient.Create(ctx, appContext)).Should(BeNil())
+		handler := appHandler{
+			r:      reconciler,
+			app:    app,
+			logger: reconciler.Log.WithValues("application", "gatherUsingAppRevision-func-test"),
+		}
+		Eventually(func() error {
+			using, err := gatherUsingAppRevision(ctx, &handler)
+			if err != nil {
+				return err
+			}
+			if len(using) != 2 {
+				return fmt.Errorf("wrong revision number")
+			}
+			if !using[appName+"-v1"] {
+				return fmt.Errorf("revison1 not include")
+			}
+			if !using[appName+"-v2"] {
+				return fmt.Errorf("revison2 not include")
+			}
+			return nil
+		}, time.Second*60, time.Microsecond).Should(BeNil())
+	})
+})
+
+func getAppContext(namespace, name string, pointingRev string) *v1alpha2.ApplicationContext {
+	return &v1alpha2.ApplicationContext{
+		TypeMeta: metav1.TypeMeta{
+			APIVersion: v1alpha2.ApplicationContextKindAPIVersion,
+			Kind:       v1alpha2.ApplicationContextKind,
+		},
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      "app-rollout",
+		},
+		Spec: v1alpha2.ApplicationContextSpec{
+			ApplicationRevisionName: pointingRev,
+		},
+	}
+}

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

@@ -61,6 +61,7 @@ var testScheme = runtime.NewScheme()
 var reconciler *Reconciler
 var stop = make(chan struct{})
 var ctlManager ctrl.Manager
+var appRevisionLimit = 5
 
 // TODO: create a mock client and add UT to cover all the failure cases
 
@@ -122,11 +123,12 @@ var _ = BeforeSuite(func(done Done) {
 	pd, err := definition.NewPackageDiscover(cfg)
 	Expect(err).To(BeNil())
 	reconciler = &Reconciler{
-		Client: k8sClient,
-		Log:    ctrl.Log.WithName("Application-Test"),
-		Scheme: testScheme,
-		dm:     dm,
-		pd:     pd,
+		Client:           k8sClient,
+		Log:              ctrl.Log.WithName("Application-Test"),
+		Scheme:           testScheme,
+		dm:               dm,
+		pd:               pd,
+		appRevisionLimit: appRevisionLimit,
 	}
 	// setup the controller manager since we need the component handler to run in the background
 	ctlManager, err = ctrl.NewManager(cfg, ctrl.Options{