Refactor: organize appHandler, remove unused flags (#6254)

cmd/core/app/options/options.go

@@ -81,7 +81,6 @@ func NewCoreOptions() *CoreOptions {
 			DefRevisionLimit:                             20,
 			AutoGenWorkloadDefinition:                    true,
 			ConcurrentReconciles:                         4,
-			EnableCompatibility:                          false,
 			IgnoreAppWithoutControllerRequirement:        false,
 			IgnoreDefinitionWithoutControllerRequirement: false,
 		},

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

@@ -46,9 +46,6 @@ type Args struct {
 	// AutoGenWorkloadDefinition indicates whether automatic generated workloadDefinition which componentDefinition refers to
 	AutoGenWorkloadDefinition bool
 
-	// EnableCompatibility indicates that will change some functions of controller to adapt to multiple platforms, such as asi.
-	EnableCompatibility bool
-
 	// IgnoreAppWithoutControllerRequirement indicates that application controller will not process the app without 'app.oam.dev/controller-version-require' annotation.
 	IgnoreAppWithoutControllerRequirement bool
 
@@ -66,7 +63,6 @@ func (a *Args) AddFlags(fs *pflag.FlagSet, c *Args) {
 		"definition-revision-limit is the maximum number of component/trait definition useless revisions that will be maintained, if the useless revisions exceed this number, older ones will be GCed first.The default value is 20.")
 	fs.BoolVar(&a.AutoGenWorkloadDefinition, "autogen-workload-definition", c.AutoGenWorkloadDefinition, "Automatic generated workloadDefinition which componentDefinition refers to.")
 	fs.IntVar(&a.ConcurrentReconciles, "concurrent-reconciles", c.ConcurrentReconciles, "concurrent-reconciles is the concurrent reconcile number of the controller. The default value is 4")
-	fs.BoolVar(&a.EnableCompatibility, "enable-asi-compatibility", c.EnableCompatibility, "enable compatibility for asi")
 	fs.BoolVar(&a.IgnoreAppWithoutControllerRequirement, "ignore-app-without-controller-version", c.IgnoreAppWithoutControllerRequirement, "If true, application controller will not process the app without 'app.oam.dev/controller-version-require' annotation")
 	fs.BoolVar(&a.IgnoreDefinitionWithoutControllerRequirement, "ignore-definition-without-controller-version", c.IgnoreDefinitionWithoutControllerRequirement, "If true, trait/component/workflowstep definition controller will not process the definition without 'definition.oam.dev/controller-version-require' annotation")
 }

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

@@ -95,7 +95,6 @@ type Reconciler struct {
 type options struct {
 	appRevisionLimit     int
 	concurrentReconciles int
-	disableStatusUpdate  bool
 	ignoreAppNoCtrlReq   bool
 	controllerVersion    string
 }
@@ -141,7 +140,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	logCtx.AddTag("publish_version", app.GetAnnotations()[oam.AnnotationPublishVersion])
 
 	appParser := appfile.NewApplicationParser(r.Client, r.pd)
-	handler, err := NewAppHandler(logCtx, r, app, appParser)
+	handler, err := NewAppHandler(logCtx, r, app)
 	if err != nil {
 		return r.endWithNegativeCondition(logCtx, app, condition.ReconcileError(err), common.ApplicationStarting)
 	}
@@ -179,14 +178,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 	app.Status.SetConditions(condition.ReadyCondition("Revision"))
 	r.Recorder.Event(app, event.Normal(velatypes.ReasonRevisoned, velatypes.MessageRevisioned))
 
-	if err := handler.UpdateAppLatestRevisionStatus(logCtx); err != nil {
+	if err := handler.UpdateAppLatestRevisionStatus(logCtx, r.patchStatus); err != nil {
 		logCtx.Error(err, "Failed to update application status")
 		return r.endWithNegativeCondition(logCtx, app, condition.ReconcileError(err), common.ApplicationRendering)
 	}
 	logCtx.Info("Successfully apply application revision")
 
 	if err := handler.ApplyPolicies(logCtx, appFile); err != nil {
-		logCtx.Error(err, "[Handle ApplyPolicies]")
+		logCtx.Error(err, "[handle ApplyPolicies]")
 		r.Recorder.Event(app, event.Warning(velatypes.ReasonFailedApply, err))
 		return r.endWithNegativeCondition(logCtx, app, condition.ErrorCondition(common.PolicyCondition.String(), errors.WithMessage(err, "ApplyPolices")), common.ApplicationPolicyGenerating)
 	}
@@ -440,23 +439,26 @@ func (r *Reconciler) endWithNegativeCondition(ctx context.Context, app *v1beta1.
 	return r.result(fmt.Errorf("object level reconcile error, type: %q, msg: %q", string(condition.Type), condition.Message)).ret()
 }
 
+// Application status can be updated by two methods: patch and update.
+type method int
+
+const (
+	patch = iota
+	update
+)
+
+type statusPatcher func(ctx context.Context, app *v1beta1.Application, phase common.ApplicationPhase) error
+
 func (r *Reconciler) patchStatus(ctx context.Context, app *v1beta1.Application, phase common.ApplicationPhase) error {
-	app.Status.Phase = phase
-	updateObservedGeneration(app)
-	if oldApp, ok := originalAppFrom(ctx); ok && oldApp != nil && equality.Semantic.DeepEqual(oldApp.Status, app.Status) {
-		return nil
-	}
-	ctx, cancel := ctrlrec.NewReconcileTerminationContext(ctx)
-	defer cancel()
-	if err := r.Status().Patch(ctx, app, client.Merge); err != nil {
-		// set to -1 to re-run workflow if status is failed to patch
-		executor.StepStatusCache.Store(fmt.Sprintf("%s-%s", app.Name, app.Namespace), -1)
-		return err
-	}
-	return nil
+	return r.writeStatusByMethod(ctx, patch, app, phase)
 }
 
 func (r *Reconciler) updateStatus(ctx context.Context, app *v1beta1.Application, phase common.ApplicationPhase) error {
+	return r.writeStatusByMethod(ctx, update, app, phase)
+}
+
+func (r *Reconciler) writeStatusByMethod(ctx context.Context, method method, app *v1beta1.Application, phase common.ApplicationPhase) error {
+	// pre-check if the status is changed
 	app.Status.Phase = phase
 	updateObservedGeneration(app)
 	if oldApp, ok := originalAppFrom(ctx); ok && oldApp != nil && equality.Semantic.DeepEqual(oldApp.Status, app.Status) {
@@ -464,15 +466,17 @@ func (r *Reconciler) updateStatus(ctx context.Context, app *v1beta1.Application,
 	}
 	ctx, cancel := ctrlrec.NewReconcileTerminationContext(ctx)
 	defer cancel()
-	if !r.disableStatusUpdate {
-		return r.Status().Update(ctx, app)
+	var f func() error
+	switch method {
+	case patch:
+		f = func() error { return r.Status().Patch(ctx, app, client.Merge) }
+	case update:
+		f = func() error { return r.Status().Update(ctx, app) }
+	default:
+		// Should never happen
+		panic("unknown method")
 	}
-	obj, err := app.Unstructured()
-	if err != nil {
-		return err
-	}
-	if err := r.Status().Update(ctx, obj); err != nil {
-		// set to -1 to re-run workflow if status is failed to update
+	if err := f(); err != nil {
 		executor.StepStatusCache.Store(fmt.Sprintf("%s-%s", app.Name, app.Namespace), -1)
 		return err
 	}
@@ -648,7 +652,6 @@ func timeReconcile(app *v1beta1.Application) func() {
 
 func parseOptions(args core.Args) options {
 	return options{
-		disableStatusUpdate:  args.EnableCompatibility,
 		appRevisionLimit:     args.AppRevisionLimit,
 		concurrentReconciles: args.ConcurrentReconciles,
 		ignoreAppNoCtrlReq:   args.IgnoreAppWithoutControllerRequirement,

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

@@ -20,6 +20,7 @@ import (
 	"context"
 	"sync"
 
+	"github.com/kubevela/workflow/pkg/cue/packages"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
@@ -45,7 +46,9 @@ import (
 
 // AppHandler handles application reconcile
 type AppHandler struct {
-	r              *Reconciler
+	client.Client
+	pd *packages.PackageDiscover
+
 	app            *v1beta1.Application
 	currentAppRev  *v1beta1.ApplicationRevision
 	latestAppRev   *v1beta1.ApplicationRevision
@@ -57,13 +60,12 @@ type AppHandler struct {
 	services         []common.ApplicationComponentStatus
 	appliedResources []common.ClusterObjectReference
 	deletedResources []common.ClusterObjectReference
-	parser           *appfile.Parser
 
 	mu sync.Mutex
 }
 
 // NewAppHandler create new app handler
-func NewAppHandler(ctx context.Context, r *Reconciler, app *v1beta1.Application, parser *appfile.Parser) (*AppHandler, error) {
+func NewAppHandler(ctx context.Context, r *Reconciler, app *v1beta1.Application) (*AppHandler, error) {
 	if ctx, ok := ctx.(monitorContext.Context); ok {
 		subCtx := ctx.Fork("create-app-handler", monitorContext.DurationMetric(func(v float64) {
 			metrics.AppReconcileStageDurationHistogram.WithLabelValues("create-app-handler").Observe(v)
@@ -75,10 +77,10 @@ func NewAppHandler(ctx context.Context, r *Reconciler, app *v1beta1.Application,
 		return nil, errors.Wrapf(err, "failed to create resourceKeeper")
 	}
 	return &AppHandler{
-		r:              r,
+		Client:         r.Client,
+		pd:             r.pd,
 		app:            app,
 		resourceKeeper: resourceHandler,
-		parser:         parser,
 	}, nil
 }
 
@@ -247,7 +249,7 @@ func (h *AppHandler) collectTraitHealthStatus(comp *appfile.Component, tr *appfi
 		pCtx.SetCtx(pkgmulticluster.WithCluster(pCtx.GetCtx(), pkgmulticluster.Local))
 	}
 	_accessor := util.NewApplicationResourceNamespaceAccessor(h.app.Namespace, traitOverrideNamespace)
-	templateContext, err := tr.GetTemplateContext(pCtx, h.r.Client, _accessor)
+	templateContext, err := tr.GetTemplateContext(pCtx, h.Client, _accessor)
 	if err != nil {
 		return common.ApplicationTraitStatus{}, nil, errors.WithMessagef(err, "app=%s, comp=%s, trait=%s, get template context error", appName, comp.Name, tr.Name)
 	}
@@ -271,10 +273,10 @@ func (h *AppHandler) collectWorkloadHealthStatus(ctx context.Context, comp *appf
 	)
 	if comp.CapabilityCategory == types.TerraformCategory {
 		var configuration terraforv1beta2.Configuration
-		if err := h.r.Client.Get(ctx, client.ObjectKey{Name: comp.Name, Namespace: accessor.Namespace()}, &configuration); err != nil {
+		if err := h.Client.Get(ctx, client.ObjectKey{Name: comp.Name, Namespace: accessor.Namespace()}, &configuration); err != nil {
 			if kerrors.IsNotFound(err) {
 				var legacyConfiguration terraforv1beta1.Configuration
-				if err := h.r.Client.Get(ctx, client.ObjectKey{Name: comp.Name, Namespace: accessor.Namespace()}, &legacyConfiguration); err != nil {
+				if err := h.Client.Get(ctx, client.ObjectKey{Name: comp.Name, Namespace: accessor.Namespace()}, &legacyConfiguration); err != nil {
 					return false, nil, nil, errors.WithMessagef(err, "app=%s, comp=%s, check health error", appName, comp.Name)
 				}
 				isHealth = setStatus(status, legacyConfiguration.Status.ObservedGeneration, legacyConfiguration.Generation,
@@ -287,7 +289,7 @@ func (h *AppHandler) collectWorkloadHealthStatus(ctx context.Context, comp *appf
 				appRev.Name, configuration.Status.Apply.State, configuration.Status.Apply.Message)
 		}
 	} else {
-		templateContext, err := comp.GetTemplateContext(comp.Ctx, h.r.Client, accessor)
+		templateContext, err := comp.GetTemplateContext(comp.Ctx, h.Client, accessor)
 		if err != nil {
 			return false, nil, nil, errors.WithMessagef(err, "app=%s, comp=%s, get template context error", appName, comp.Name)
 		}
@@ -389,6 +391,7 @@ func setStatus(status *common.ApplicationComponentStatus, observedGeneration, ge
 }
 
 // ApplyPolicies will render policies into manifests from appfile and dispatch them
+// Note the builtin policy like apply-once, shared-resource, etc. is not handled here.
 func (h *AppHandler) ApplyPolicies(ctx context.Context, af *appfile.Appfile) error {
 	if ctx, ok := ctx.(monitorContext.Context); ok {
 		subCtx := ctx.Fork("apply-policies", monitorContext.DurationMetric(func(v float64) {

pkg/controller/core.oam.dev/v1beta1/application/apply_test.go

@@ -224,7 +224,7 @@ var _ = Describe("Test deleter resource", func() {
 				},
 			},
 		}
-		h, err := NewAppHandler(ctx, reconciler, &v1beta1.Application{ObjectMeta: metav1.ObjectMeta{Name: "example", Namespace: "default"}}, nil)
+		h, err := NewAppHandler(ctx, reconciler, &v1beta1.Application{ObjectMeta: metav1.ObjectMeta{Name: "example", Namespace: "default"}})
 		Expect(err).Should(Succeed())
 		h.appliedResources = appliedRsc
 		Expect(h.Delete(ctx, "", common.WorkflowResourceCreator, &u))

pkg/controller/core.oam.dev/v1beta1/application/assemble/assemble.go

@@ -18,47 +18,15 @@ package assemble
 
 import (
 	"github.com/pkg/errors"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
 	"k8s.io/klog/v2"
 
 	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
 	"github.com/oam-dev/kubevela/apis/types"
-	"github.com/oam-dev/kubevela/pkg/appfile"
 	"github.com/oam-dev/kubevela/pkg/oam"
 	"github.com/oam-dev/kubevela/pkg/oam/util"
 )
 
-// NewAppManifests create a AppManifests
-func NewAppManifests(appRevision *v1beta1.ApplicationRevision, parser *appfile.Parser) *AppManifests {
-	return &AppManifests{AppRevision: appRevision, parser: parser}
-}
-
-// AppManifests contains configuration to assemble resources recorded in the ApplicationRevision.
-// 'Assemble' means expand Application(Component and Trait) into K8s resource and get them ready to go, to be emitted
-// into K8s
-type AppManifests struct {
-	AppRevision     *v1beta1.ApplicationRevision
-	WorkloadOptions []WorkloadOption
-
-	componentManifests []*types.ComponentManifest
-	appName            string
-	appNamespace       string
-	appLabels          map[string]string
-	appAnnotations     map[string]string
-	appOwnerRef        *metav1.OwnerReference
-
-	assembledWorkloads map[string]*unstructured.Unstructured
-	assembledTraits    map[string][]*unstructured.Unstructured
-	// key is workload reference, values are the references of scopes the workload belongs to
-	skipWorkloadApplyComp map[string]bool
-
-	finalized bool
-	err       error
-
-	parser *appfile.Parser
-}
-
 // WorkloadOption will be applied to each workloads AFTER it has been assembled by generic rules shown below:
 // 1) use component name as workload name
 // 2) use application namespace as workload namespace if unspecified
@@ -75,78 +43,6 @@ type WorkloadOption interface {
 	ApplyToWorkload(*unstructured.Unstructured, *v1beta1.ComponentDefinition, []*unstructured.Unstructured) error
 }
 
-// WithWorkloadOption add a WorkloadOption to plug in custom logic applied to each workload
-func (am *AppManifests) WithWorkloadOption(wo WorkloadOption) *AppManifests {
-	if am.WorkloadOptions == nil {
-		am.WorkloadOptions = make([]WorkloadOption, 0)
-	}
-	am.WorkloadOptions = append(am.WorkloadOptions, wo)
-	return am
-}
-
-// WithComponentManifests set component manifests with the given one
-func (am *AppManifests) WithComponentManifests(componentManifests []*types.ComponentManifest) *AppManifests {
-	am.componentManifests = componentManifests
-	return am
-}
-
-// AssembledManifests do assemble and merge all assembled resources(except referenced scopes) into one array
-// The result guarantee the order of resources as defined in application originally.
-// If it contains more than one component, the resources are well-orderred and also grouped.
-// For example, if app = comp1 (wl1 + trait1 + trait2) + comp2 (wl2 + trait3 +trait4),
-// the result is [wl1, trait1, trait2, wl2, trait3, trait4]
-func (am *AppManifests) AssembledManifests() ([]*unstructured.Unstructured, error) {
-	if !am.finalized {
-		am.assemble()
-	}
-	if am.err != nil {
-		return nil, am.err
-	}
-	r := make([]*unstructured.Unstructured, 0)
-	for compName, wl := range am.assembledWorkloads {
-		skipApplyWorkload := false
-		ts := am.assembledTraits[compName]
-		for _, t := range ts {
-			r = append(r, t.DeepCopy())
-			if v := t.GetLabels()[oam.LabelManageWorkloadTrait]; v == "true" {
-				skipApplyWorkload = true
-			}
-		}
-		if !skipApplyWorkload {
-			r = append(r, wl.DeepCopy())
-		} else {
-			klog.InfoS("assemble meet a managedByTrait workload, so skip apply it",
-				"namespace", am.AppRevision.Namespace, "appRev", am.AppRevision.Name)
-		}
-	}
-	return r, nil
-}
-
-// GroupAssembledManifests do assemble and return all resources grouped by components
-func (am *AppManifests) GroupAssembledManifests() (
-	map[string]*unstructured.Unstructured,
-	map[string][]*unstructured.Unstructured,
-	error) {
-	if !am.finalized {
-		am.assemble()
-	}
-	if am.err != nil {
-		return nil, nil, am.err
-	}
-	workloads := make(map[string]*unstructured.Unstructured)
-	for k, wl := range am.assembledWorkloads {
-		workloads[k] = wl.DeepCopy()
-	}
-	traits := make(map[string][]*unstructured.Unstructured)
-	for k, ts := range am.assembledTraits {
-		traits[k] = make([]*unstructured.Unstructured, len(ts))
-		for i, t := range ts {
-			traits[k][i] = t.DeepCopy()
-		}
-	}
-	return workloads, traits, nil
-}
-
 // checkAutoDetectComponent will check if the standardWorkload is empty,
 // currently only Helm-based component is possible to be auto-detected
 // TODO implement auto-detect mechanism
@@ -154,35 +50,6 @@ func checkAutoDetectComponent(wl *unstructured.Unstructured) bool {
 	return wl == nil || (len(wl.GetAPIVersion()) == 0 && len(wl.GetKind()) == 0)
 }
 
-func (am *AppManifests) assemble() {
-	if err := am.complete(); err != nil {
-		am.finalizeAssemble(err)
-		return
-	}
-
-	klog.InfoS("Assemble manifests for application", "name", am.appName, "revision", am.AppRevision.GetName())
-	if err := am.validate(); err != nil {
-		am.finalizeAssemble(err)
-		return
-	}
-	for _, comp := range am.componentManifests {
-		klog.InfoS("Assemble manifests for component", "name", comp.Name)
-		wl, traits, err := PrepareBeforeApply(comp, am.AppRevision, am.WorkloadOptions)
-		if err != nil {
-			am.finalizeAssemble(err)
-			return
-		}
-		if wl == nil {
-			klog.Warningf("component without specify workloadDef can not attach traits currently")
-			continue
-		}
-
-		am.assembledWorkloads[comp.Name] = wl
-		am.assembledTraits[comp.Name] = traits
-	}
-	am.finalizeAssemble(nil)
-}
-
 // PrepareBeforeApply will prepare for some necessary info before apply
 func PrepareBeforeApply(comp *types.ComponentManifest, appRev *v1beta1.ApplicationRevision, workloadOpt []WorkloadOption) (*unstructured.Unstructured, []*unstructured.Unstructured, error) {
 	if checkAutoDetectComponent(comp.StandardWorkload) {
@@ -211,55 +78,6 @@ func PrepareBeforeApply(comp *types.ComponentManifest, appRev *v1beta1.Applicati
 	return wl, assembledTraits, nil
 }
 
-func (am *AppManifests) complete() error {
-	if len(am.componentManifests) == 0 {
-		var err error
-		af, err := am.parser.GenerateAppFileFromRevision(am.AppRevision)
-		if err != nil {
-			return errors.WithMessage(err, "fail to generate appfile from revision for app manifests complete")
-		}
-		am.componentManifests, err = af.GenerateComponentManifests()
-		if err != nil {
-			return errors.WithMessage(err, "fail to complete manifests as generate from app revision failed")
-		}
-	}
-	am.appNamespace = am.AppRevision.GetNamespace()
-	am.appLabels = am.AppRevision.GetLabels()
-	am.appName = am.AppRevision.GetLabels()[oam.LabelAppName]
-	am.appAnnotations = am.AppRevision.GetAnnotations()
-	am.appOwnerRef = metav1.GetControllerOf(am.AppRevision)
-
-	am.assembledWorkloads = make(map[string]*unstructured.Unstructured)
-	am.assembledTraits = make(map[string][]*unstructured.Unstructured)
-	am.skipWorkloadApplyComp = make(map[string]bool)
-	return nil
-}
-
-func (am *AppManifests) finalizeAssemble(err error) {
-	am.finalized = true
-	if err == nil {
-		klog.InfoS("Successfully assemble manifests for application", "name", am.appName, "revision", am.AppRevision.GetName(), "namespace", am.appNamespace)
-		return
-	}
-	klog.ErrorS(err, "Failed assembling manifests for application", "name", am.appName, "revision", am.AppRevision.GetName())
-	am.err = errors.WithMessagef(err, "cannot assemble resources' manifests for application %q", am.appName)
-}
-
-// AssembleOptions is highly coulped with AppRevision, should check the AppRevision provides all info
-// required by AssembleOptions
-func (am *AppManifests) validate() error {
-	if am.appOwnerRef == nil {
-		return errors.New("AppRevision must have an Application as owner")
-	}
-	if len(am.AppRevision.Labels[oam.LabelAppName]) == 0 {
-		return errors.New("AppRevision must have app name in the label")
-	}
-	if len(am.AppRevision.Labels[oam.LabelAppRevisionHash]) == 0 {
-		return errors.New("AppRevision must have revision hash in the label")
-	}
-	return nil
-}
-
 func assembleWorkload(compName string, wl *unstructured.Unstructured,
 	labels map[string]string, resources []*unstructured.Unstructured, appRev *v1beta1.ApplicationRevision, wop []WorkloadOption) (*unstructured.Unstructured, error) {
 	// use component name as workload name if workload name is not specified

pkg/controller/core.oam.dev/v1beta1/application/assemble/assemble_suite_test.go

@@ -15,99 +15,3 @@ limitations under the License.
 */
 
 package assemble
-
-import (
-	"os"
-
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-	"sigs.k8s.io/yaml"
-
-	"github.com/oam-dev/kubevela/apis/core.oam.dev/v1beta1"
-	"github.com/oam-dev/kubevela/pkg/oam"
-)
-
-var _ = Describe("Test Assemble Options", func() {
-	It("test assemble", func() {
-		var (
-			compName  = "test-comp"
-			namespace = "default"
-		)
-
-		appRev := &v1beta1.ApplicationRevision{}
-		b, err := os.ReadFile("./testdata/apprevision.yaml")
-		/* appRevision test data is generated based on below application
-		apiVersion: core.oam.dev/v1beta1
-		kind: Application
-		metadata:
-		  name: test-assemble
-		spec:
-		  components:
-		    - name: test-comp
-		      type: webservice
-		      properties:
-		        image: crccheck/hello-world
-		        port: 8000
-		      traits:
-		        - type: ingress
-		          properties:
-		            domain: localhost
-		            http:
-		              "/": 8000
-		*/
-		Expect(err).Should(BeNil())
-		err = yaml.Unmarshal(b, appRev)
-		Expect(err).Should(BeNil())
-
-		ao := NewAppManifests(appRev, appParser)
-		workloads, traits, err := ao.GroupAssembledManifests()
-		Expect(err).Should(BeNil())
-
-		By("Verify amount of result resources")
-		allResources, err := ao.AssembledManifests()
-		Expect(err).Should(BeNil())
-		Expect(len(allResources)).Should(Equal(3))
-
-		By("Verify amount of result grouped resources")
-		Expect(len(workloads)).Should(Equal(1))
-		Expect(len(traits[compName])).Should(Equal(2))
-
-		By("Verify workload metadata (name, namespace, labels, annotations, ownerRef)")
-		wl := workloads[compName]
-		Expect(wl.GetName()).Should(Equal(compName))
-		Expect(wl.GetNamespace()).Should(Equal(namespace))
-		labels := wl.GetLabels()
-		labelKeys := make([]string, 0, len(labels))
-		for k := range labels {
-			labelKeys = append(labelKeys, k)
-		}
-		Expect(labelKeys).Should(ContainElements(
-			oam.LabelAppName,
-			oam.LabelAppRevision,
-			oam.LabelAppRevisionHash,
-			oam.LabelAppComponent,
-			oam.LabelAppComponentRevision,
-			oam.WorkloadTypeLabel,
-			oam.LabelOAMResourceType))
-		Expect(len(wl.GetAnnotations())).Should(Equal(1))
-
-		By("Verify trait metadata (name, namespace, labels, annotations, ownerRef)")
-		trait := traits[compName][0]
-		Expect(trait.GetName()).Should(ContainSubstring(compName))
-		Expect(trait.GetNamespace()).Should(Equal(namespace))
-		labels = trait.GetLabels()
-		labelKeys = make([]string, 0, len(labels))
-		for k := range labels {
-			labelKeys = append(labelKeys, k)
-		}
-		Expect(labelKeys).Should(ContainElements(
-			oam.LabelAppName,
-			oam.LabelAppRevision,
-			oam.LabelAppRevisionHash,
-			oam.LabelAppComponent,
-			oam.LabelAppComponentRevision,
-			oam.TraitTypeLabel,
-			oam.LabelOAMResourceType))
-		Expect(len(wl.GetAnnotations())).Should(Equal(1))
-	})
-})

pkg/controller/core.oam.dev/v1beta1/application/dispatcher.go

@@ -179,7 +179,7 @@ func (h *AppHandler) generateDispatcher(appRev *v1beta1.ApplicationRevision, rea
 					traitType = splitName
 				}
 			}
-			stageType, err = getTraitDispatchStage(h.r.Client, traitType, appRev)
+			stageType, err = getTraitDispatchStage(h.Client, traitType, appRev)
 			if err != nil {
 				return nil, err
 			}

pkg/controller/core.oam.dev/v1beta1/application/generator.go

@@ -95,37 +95,37 @@ func (h *AppHandler) GenerateApplicationSteps(ctx monitorContext.Context,
 		oam.LabelAppNamespace: app.Namespace,
 	}
 	handlerProviders := providers.NewProviders()
-	kube.Install(handlerProviders, h.r.Client, appLabels, &kube.Handlers{
+	kube.Install(handlerProviders, h.Client, appLabels, &kube.Handlers{
 		Apply:  h.Dispatch,
 		Delete: h.Delete,
 	})
-	configprovider.Install(handlerProviders, h.r.Client, func(ctx context.Context, resources []*unstructured.Unstructured, applyOptions []apply.ApplyOption) error {
+	configprovider.Install(handlerProviders, h.Client, func(ctx context.Context, resources []*unstructured.Unstructured, applyOptions []apply.ApplyOption) error {
 		for _, res := range resources {
 			res.SetLabels(util.MergeMapOverrideWithDst(res.GetLabels(), appLabels))
 		}
 		return h.resourceKeeper.Dispatch(ctx, resources, applyOptions)
 	})
-	oamProvider.Install(handlerProviders, app, af, h.r.Client, h.applyComponentFunc(
+	oamProvider.Install(handlerProviders, app, af, h.Client, h.applyComponentFunc(
 		appParser, appRev, af), h.renderComponentFunc(appParser, appRev, af))
 	pCtx := velaprocess.NewContext(generateContextDataFromApp(app, appRev.Name))
 	renderer := func(ctx context.Context, comp common.ApplicationComponent) (*appfile.Component, error) {
 		return appParser.ParseComponentFromRevisionAndClient(ctx, comp, appRev)
 	}
-	multiclusterProvider.Install(handlerProviders, h.r.Client, app, af,
+	multiclusterProvider.Install(handlerProviders, h.Client, app, af,
 		h.applyComponentFunc(appParser, appRev, af),
 		h.checkComponentHealth(appParser, appRev, af),
 		renderer)
 	terraformProvider.Install(handlerProviders, app, renderer)
-	query.Install(handlerProviders, h.r.Client, nil)
+	query.Install(handlerProviders, h.Client, nil)
 
 	instance := generateWorkflowInstance(af, app)
 	executor.InitializeWorkflowInstance(instance)
 	runners, err := generator.GenerateRunners(ctx, instance, wfTypes.StepGeneratorOptions{
 		Providers:       handlerProviders,
-		PackageDiscover: h.r.pd,
+		PackageDiscover: h.pd,
 		ProcessCtx:      pCtx,
-		TemplateLoader:  template.NewWorkflowStepTemplateRevisionLoader(appRev, h.r.Client.RESTMapper()),
-		Client:          h.r.Client,
+		TemplateLoader:  template.NewWorkflowStepTemplateRevisionLoader(appRev, h.Client.RESTMapper()),
+		Client:          h.Client,
 		StepConvertor: map[string]func(step workflowv1alpha1.WorkflowStep) (workflowv1alpha1.WorkflowStep, error){
 			wfTypes.WorkflowStepTypeApplyComponent: func(lstep workflowv1alpha1.WorkflowStep) (workflowv1alpha1.WorkflowStep, error) {
 				copierStep := lstep.DeepCopy()
@@ -319,7 +319,7 @@ func (h *AppHandler) renderComponentFunc(appParser *appfile.Parser, appRev *v1be
 		if err != nil {
 			return nil, nil, err
 		}
-		return renderComponentsAndTraits(h.r.Client, manifest, appRev, clusterName, overrideNamespace)
+		return renderComponentsAndTraits(h.Client, manifest, appRev, clusterName, overrideNamespace)
 	}
 }
 
@@ -335,7 +335,7 @@ func (h *AppHandler) checkComponentHealth(appParser *appfile.Parser, appRev *v1b
 		}
 		wl.Ctx.SetCtx(auth.ContextWithUserInfo(ctx, h.app))
 
-		readyWorkload, readyTraits, err := renderComponentsAndTraits(h.r.Client, manifest, appRev, clusterName, overrideNamespace)
+		readyWorkload, readyTraits, err := renderComponentsAndTraits(h.Client, manifest, appRev, clusterName, overrideNamespace)
 		if err != nil {
 			return false, nil, nil, err
 		}
@@ -378,7 +378,7 @@ func (h *AppHandler) applyComponentFunc(appParser *appfile.Parser, appRev *v1bet
 		}
 		wl.Ctx.SetCtx(auth.ContextWithUserInfo(ctx, h.app))
 
-		readyWorkload, readyTraits, err := renderComponentsAndTraits(h.r.Client, manifest, appRev, clusterName, overrideNamespace)
+		readyWorkload, readyTraits, err := renderComponentsAndTraits(h.Client, manifest, appRev, clusterName, overrideNamespace)
 		if err != nil {
 			return nil, nil, false, err
 		}
@@ -414,7 +414,7 @@ func (h *AppHandler) applyComponentFunc(appParser *appfile.Parser, appRev *v1bet
 		if DisableResourceApplyDoubleCheck {
 			return readyWorkload, readyTraits, isHealth, nil
 		}
-		workload, traits, err := getComponentResources(auth.ContextWithUserInfo(ctx, h.app), manifest, wl.SkipApplyWorkload, h.r.Client)
+		workload, traits, err := getComponentResources(auth.ContextWithUserInfo(ctx, h.app), manifest, wl.SkipApplyWorkload, h.Client)
 		return workload, traits, isHealth, err
 	}
 }
@@ -457,7 +457,7 @@ func (h *AppHandler) prepareWorkloadAndManifests(ctx context.Context,
 			ctxData.Cluster = cluster
 		}
 		// cluster info are secrets stored in the control plane cluster
-		ctxData.ClusterVersion = multicluster.GetVersionInfoFromObject(pkgmulticluster.WithCluster(ctx, types.ClusterLocalName), h.r.Client, ctxData.Cluster)
+		ctxData.ClusterVersion = multicluster.GetVersionInfoFromObject(pkgmulticluster.WithCluster(ctx, types.ClusterLocalName), h.Client, ctxData.Cluster)
 		ctxData.CompRevision, _ = ctrlutil.ComputeSpecHash(comp)
 	})
 	if err != nil {

pkg/controller/core.oam.dev/v1beta1/application/generator_test.go

@@ -113,7 +113,7 @@ var _ = Describe("Test Application workflow generator", func() {
 		_, err = af.GeneratePolicyManifests(context.Background())
 		Expect(err).Should(BeNil())
 
-		handler, err := NewAppHandler(ctx, reconciler, app, appParser)
+		handler, err := NewAppHandler(ctx, reconciler, app)
 		Expect(err).Should(Succeed())
 
 		logCtx := monitorContext.NewTraceContext(ctx, "")
@@ -157,7 +157,7 @@ var _ = Describe("Test Application workflow generator", func() {
 		_, err = af.GeneratePolicyManifests(context.Background())
 		Expect(err).Should(BeNil())
 
-		handler, err := NewAppHandler(ctx, reconciler, app, appParser)
+		handler, err := NewAppHandler(ctx, reconciler, app)
 		Expect(err).Should(Succeed())
 
 		logCtx := monitorContext.NewTraceContext(ctx, "")
@@ -199,7 +199,7 @@ var _ = Describe("Test Application workflow generator", func() {
 		af, err := appParser.GenerateAppFile(ctx, app)
 		Expect(err).Should(BeNil())
 
-		handler, err := NewAppHandler(ctx, reconciler, app, appParser)
+		handler, err := NewAppHandler(ctx, reconciler, app)
 		Expect(err).Should(Succeed())
 
 		logCtx := monitorContext.NewTraceContext(ctx, "")
@@ -241,7 +241,7 @@ var _ = Describe("Test Application workflow generator", func() {
 		af, err := appParser.GenerateAppFile(ctx, app)
 		Expect(err).Should(BeNil())
 
-		handler, err := NewAppHandler(ctx, reconciler, app, appParser)
+		handler, err := NewAppHandler(ctx, reconciler, app)
 		Expect(err).Should(Succeed())
 
 		logCtx := monitorContext.NewTraceContext(ctx, "")
@@ -280,7 +280,7 @@ var _ = Describe("Test Application workflow generator", func() {
 		af, err := appParser.GenerateAppFile(ctx, app)
 		Expect(err).Should(BeNil())
 
-		handler, err := NewAppHandler(ctx, reconciler, app, appParser)
+		handler, err := NewAppHandler(ctx, reconciler, app)
 		Expect(err).Should(Succeed())
 
 		logCtx := monitorContext.NewTraceContext(ctx, "")

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

@@ -114,14 +114,14 @@ func (h *AppHandler) createResourcesConfigMap(ctx context.Context,
 			ConfigMapKeyPolicy:     string(util.MustJSONMarshal(policies)),
 		},
 	}
-	err := h.r.Client.Get(ctx, client.ObjectKey{Name: appRev.Name, Namespace: appRev.Namespace}, &corev1.ConfigMap{})
+	err := h.Client.Get(ctx, client.ObjectKey{Name: appRev.Name, Namespace: appRev.Namespace}, &corev1.ConfigMap{})
 	if err == nil {
 		return nil
 	}
 	if err != nil && !apierrors.IsNotFound(err) {
 		return err
 	}
-	return h.r.Client.Create(ctx, cm)
+	return h.Client.Create(ctx, cm)
 }
 
 // SprintComponentManifest formats and returns the resulting string.
@@ -283,7 +283,7 @@ func (h *AppHandler) getLatestAppRevision(ctx context.Context) error {
 	}
 	latestRevName := h.app.Status.LatestRevision.Name
 	latestAppRev := &v1beta1.ApplicationRevision{}
-	if err := h.r.Get(ctx, client.ObjectKey{Name: latestRevName, Namespace: h.app.Namespace}, latestAppRev); err != nil {
+	if err := h.Get(ctx, client.ObjectKey{Name: latestRevName, Namespace: h.app.Namespace}, latestAppRev); err != nil {
 		klog.ErrorS(err, "Failed to get latest app revision", "appRevisionName", latestRevName)
 		return errors.Wrapf(err, "fail to get latest app revision %s", latestRevName)
 	}
@@ -403,7 +403,7 @@ func (h *AppHandler) currentAppRevIsNew(ctx context.Context) (bool, bool, error)
 		return false, false, nil
 	}
 
-	revs, err := GetAppRevisions(ctx, h.r.Client, h.app.Name, h.app.Namespace)
+	revs, err := GetAppRevisions(ctx, h.Client, h.app.Name, h.app.Namespace)
 	if err != nil {
 		klog.ErrorS(err, "Failed to list app revision", "appName", h.app.Name)
 		return false, false, errors.Wrap(err, "failed to list app revision")
@@ -567,9 +567,9 @@ func (h *AppHandler) FinalizeAndApplyAppRevision(ctx context.Context) error {
 	sharding.PropagateScheduledShardIDLabel(h.app, appRev)
 
 	gotAppRev := &v1beta1.ApplicationRevision{}
-	if err := h.r.Get(ctx, client.ObjectKey{Name: appRev.Name, Namespace: appRev.Namespace}, gotAppRev); err != nil {
+	if err := h.Get(ctx, client.ObjectKey{Name: appRev.Name, Namespace: appRev.Namespace}, gotAppRev); err != nil {
 		if apierrors.IsNotFound(err) {
-			return h.r.Create(ctx, appRev)
+			return h.Create(ctx, appRev)
 		}
 		return err
 	}
@@ -588,12 +588,12 @@ func (h *AppHandler) FinalizeAndApplyAppRevision(ctx context.Context) error {
 		appRev.Spec.Compression.SetType(compression.Zstd)
 	}
 
-	return h.r.Update(ctx, appRev)
+	return h.Update(ctx, appRev)
 }
 
 // UpdateAppLatestRevisionStatus only call to update app's latest revision status after applying manifests successfully
 // otherwise it will override previous revision which is used during applying to do GC jobs
-func (h *AppHandler) UpdateAppLatestRevisionStatus(ctx context.Context) error {
+func (h *AppHandler) UpdateAppLatestRevisionStatus(ctx context.Context, patchStatus statusPatcher) error {
 	if DisableAllApplicationRevision {
 		return nil
 	}
@@ -614,7 +614,7 @@ func (h *AppHandler) UpdateAppLatestRevisionStatus(ctx context.Context) error {
 		Revision:     int64(revNum),
 		RevisionHash: h.currentRevHash,
 	}
-	if err := h.r.patchStatus(ctx, h.app, common.ApplicationRendering); err != nil {
+	if err := patchStatus(ctx, h.app, common.ApplicationRendering); err != nil {
 		klog.InfoS("Failed to update the latest appConfig revision to status", "application", klog.KObj(h.app),
 			"latest revision", revName, "err", err)
 		return err
@@ -635,13 +635,13 @@ func (h *AppHandler) UpdateApplicationRevisionStatus(ctx context.Context, appRev
 	// Versioned the context backend values.
 	if wfStatus.ContextBackend != nil {
 		var cm corev1.ConfigMap
-		if err := h.r.Client.Get(ctx, ktypes.NamespacedName{Namespace: wfStatus.ContextBackend.Namespace, Name: wfStatus.ContextBackend.Name}, &cm); err != nil {
+		if err := h.Client.Get(ctx, ktypes.NamespacedName{Namespace: wfStatus.ContextBackend.Namespace, Name: wfStatus.ContextBackend.Name}, &cm); err != nil {
 			klog.Error(err, "[UpdateApplicationRevisionStatus] failed to load the context values", "ApplicationRevision", appRev.Name)
 		}
 		appRev.Status.WorkflowContext = cm.Data
 	}
 
-	if err := h.r.Client.Status().Update(ctx, appRev); err != nil {
+	if err := h.Client.Status().Update(ctx, appRev); err != nil {
 		if logCtx, ok := ctx.(monitorContext.Context); ok {
 			logCtx.Error(err, "[UpdateApplicationRevisionStatus] failed to update application revision status", "ApplicationRevision", appRev.Name)
 		} else {

pkg/controller/core.oam.dev/v1beta1/application/revision_test.go

@@ -117,7 +117,7 @@ var _ = Describe("test generate revision ", func() {
 		appRevision2 = *appRevision1.DeepCopy()
 		appRevision2.Name = "appRevision2"
 
-		_handler, err := NewAppHandler(ctx, reconciler, &app, nil)
+		_handler, err := NewAppHandler(ctx, reconciler, &app)
 		Expect(err).Should(Succeed())
 		handler = _handler
 	})
@@ -201,12 +201,12 @@ var _ = Describe("test generate revision ", func() {
 		Expect(handler.PrepareCurrentAppRevision(ctx, generatedAppfile)).Should(Succeed())
 		Expect(handler.FinalizeAndApplyAppRevision(ctx)).Should(Succeed())
 		Expect(handler.ProduceArtifacts(context.Background(), comps, nil)).Should(Succeed())
-		Expect(handler.UpdateAppLatestRevisionStatus(ctx)).Should(Succeed())
+		Expect(handler.UpdateAppLatestRevisionStatus(ctx, reconciler.patchStatus)).Should(Succeed())
 
 		curApp := &v1beta1.Application{}
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx,
+				return handler.Get(ctx,
 					types.NamespacedName{Namespace: ns.Name, Name: app.Name},
 					curApp)
 			},
@@ -216,7 +216,7 @@ var _ = Describe("test generate revision ", func() {
 		curAppRevision := &v1beta1.ApplicationRevision{}
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx,
+				return handler.Get(ctx,
 					types.NamespacedName{Namespace: ns.Name, Name: curApp.Status.LatestRevision.Name},
 					curAppRevision)
 			},
@@ -241,7 +241,7 @@ var _ = Describe("test generate revision ", func() {
 		Expect(handler.ProduceArtifacts(context.Background(), comps, nil)).Should(Succeed())
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx,
+				return handler.Get(ctx,
 					types.NamespacedName{Namespace: ns.Name, Name: app.Name},
 					curApp)
 			},
@@ -254,7 +254,7 @@ var _ = Describe("test generate revision ", func() {
 		curAppRevision = &v1beta1.ApplicationRevision{}
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx,
+				return handler.Get(ctx,
 					types.NamespacedName{Namespace: ns.Name, Name: lastRevision},
 					curAppRevision)
 			},
@@ -278,10 +278,10 @@ var _ = Describe("test generate revision ", func() {
 		Expect(handler.PrepareCurrentAppRevision(ctx, generatedAppfile)).Should(Succeed())
 		Expect(handler.FinalizeAndApplyAppRevision(ctx)).Should(Succeed())
 		Expect(handler.ProduceArtifacts(context.Background(), comps, nil)).Should(Succeed())
-		Expect(handler.UpdateAppLatestRevisionStatus(ctx)).Should(Succeed())
+		Expect(handler.UpdateAppLatestRevisionStatus(ctx, reconciler.patchStatus)).Should(Succeed())
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx,
+				return handler.Get(ctx,
 					types.NamespacedName{Namespace: ns.Name, Name: app.Name},
 					curApp)
 			},
@@ -295,7 +295,7 @@ var _ = Describe("test generate revision ", func() {
 		curAppRevision = &v1beta1.ApplicationRevision{}
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx,
+				return handler.Get(ctx,
 					types.NamespacedName{Namespace: ns.Name, Name: curApp.Status.LatestRevision.Name},
 					curAppRevision)
 			},
@@ -322,10 +322,10 @@ var _ = Describe("test generate revision ", func() {
 		Expect(handler.PrepareCurrentAppRevision(ctx, generatedAppfile)).Should(Succeed())
 		Expect(handler.FinalizeAndApplyAppRevision(ctx)).Should(Succeed())
 		Expect(handler.ProduceArtifacts(context.Background(), comps, nil)).Should(Succeed())
-		Expect(handler.UpdateAppLatestRevisionStatus(ctx)).Should(Succeed())
+		Expect(handler.UpdateAppLatestRevisionStatus(ctx, reconciler.patchStatus)).Should(Succeed())
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx,
+				return handler.Get(ctx,
 					types.NamespacedName{Namespace: ns.Name, Name: app.Name},
 					curApp)
 			},
@@ -339,7 +339,7 @@ var _ = Describe("test generate revision ", func() {
 		curAppRevision = &v1beta1.ApplicationRevision{}
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx,
+				return handler.Get(ctx,
 					types.NamespacedName{Namespace: ns.Name, Name: curApp.Status.LatestRevision.Name},
 					curAppRevision)
 			},
@@ -366,19 +366,19 @@ var _ = Describe("test generate revision ", func() {
 		Expect(handler.PrepareCurrentAppRevision(ctx, generatedAppfile)).Should(Succeed())
 		Expect(handler.FinalizeAndApplyAppRevision(ctx)).Should(Succeed())
 		Expect(handler.ProduceArtifacts(context.Background(), comps, nil)).Should(Succeed())
-		Expect(handler.UpdateAppLatestRevisionStatus(ctx)).Should(Succeed())
+		Expect(handler.UpdateAppLatestRevisionStatus(ctx, reconciler.patchStatus)).Should(Succeed())
 
 		curApp := &v1beta1.Application{}
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: app.Name}, curApp)
+				return handler.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,
+				return handler.Get(ctx,
 					types.NamespacedName{Namespace: ns.Name, Name: curApp.Status.LatestRevision.Name},
 					curAppRevision)
 			},
@@ -399,7 +399,7 @@ var _ = Describe("test generate revision ", func() {
 		Expect(handler.ProduceArtifacts(context.Background(), comps, nil)).Should(Succeed())
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: app.Name}, curApp)
+				return handler.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))
@@ -409,7 +409,7 @@ var _ = Describe("test generate revision ", func() {
 		curAppRevision = &v1beta1.ApplicationRevision{}
 		Eventually(
 			func() error {
-				return handler.r.Get(ctx, types.NamespacedName{Namespace: ns.Name, Name: lastRevision}, curAppRevision)
+				return handler.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))
@@ -734,7 +734,7 @@ status: {}
 		Expect(k8sClient.Create(ctx, &apprev)).Should(SatisfyAny(BeNil(), &util.AlreadyExistMatcher{}))
 
 		// prepare handler
-		_handler, err := NewAppHandler(ctx, reconciler, &app, nil)
+		_handler, err := NewAppHandler(ctx, reconciler, &app)
 		Expect(err).Should(Succeed())
 		handler = _handler