Feat: optimize empty patch request (#5600)

Signed-off-by: Somefive <yd219913@alibaba-inc.com>

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

@@ -26,6 +26,7 @@ import (
 	"github.com/crossplane/crossplane-runtime/pkg/meta"
 	"github.com/pkg/errors"
 	corev1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/api/equality"
 	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -121,6 +122,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl.Resu
 		}
 		return r.result(client.IgnoreNotFound(err)).ret()
 	}
+	ctx = withOriginalApp(ctx, app)
 
 	if !r.matchControllerRequirement(app) {
 		logCtx.Info("skip app: not match the controller requirement of app")
@@ -410,6 +412,9 @@ func (r *Reconciler) endWithNegativeCondition(ctx context.Context, app *v1beta1.
 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 {
@@ -423,6 +428,9 @@ func (r *Reconciler) patchStatus(ctx context.Context, app *v1beta1.Application,
 func (r *Reconciler) updateStatus(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 !r.disableStatusUpdate {
@@ -632,3 +640,23 @@ func (r *Reconciler) matchControllerRequirement(app *v1beta1.Application) bool {
 	}
 	return true
 }
+
+const (
+	// ComponentNamespaceContextKey is the key in context that defines the override namespace of component
+	ComponentNamespaceContextKey contextKey = iota
+	// ComponentContextKey is the key in context that records the component
+	ComponentContextKey
+	// ReplicaKeyContextKey is the key in context that records the replica key
+	ReplicaKeyContextKey
+	// OriginalAppKey is the key in the context that records the in coming original app
+	OriginalAppKey
+)
+
+func withOriginalApp(ctx context.Context, app *v1beta1.Application) context.Context {
+	return context.WithValue(ctx, OriginalAppKey, app.DeepCopy())
+}
+
+func originalAppFrom(ctx context.Context) (*v1beta1.Application, bool) {
+	app, ok := ctx.Value(OriginalAppKey).(*v1beta1.Application)
+	return app, ok
+}

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

@@ -66,7 +66,7 @@ import (
 	pkgutils "github.com/oam-dev/kubevela/pkg/utils"
 )
 
-type contextKey string
+type contextKey int
 
 const (
 	// ConfigMapKeyComponents is the key in ConfigMap Data field for containing data of components
@@ -79,12 +79,6 @@ const (
 	ManifestKeyTraits = "Traits"
 	// ManifestKeyScopes is the key in Component Manifest for containing scope cr reference.
 	ManifestKeyScopes = "Scopes"
-	// ComponentNamespaceContextKey is the key in context that defines the override namespace of component
-	ComponentNamespaceContextKey = contextKey("component-namespace")
-	// ComponentContextKey is the key in context that records the component
-	ComponentContextKey = contextKey("component")
-	// ReplicaKeyContextKey is the key in context that records the replica key
-	ReplicaKeyContextKey = contextKey("replica-key")
 )
 
 const rolloutTraitName = "rollout"

pkg/utils/apply/apply.go

@@ -193,6 +193,9 @@ func (a *APIApplicator) Apply(ctx context.Context, desired client.Object, ao ...
 		if err != nil {
 			return errors.Wrap(err, "cannot calculate patch by computing a three way diff")
 		}
+		if isEmptyPatch(patch) {
+			return nil
+		}
 		if applyAct.dryRun {
 			return errors.Wrapf(a.c.Patch(ctx, desired, patch, client.DryRunAll), "cannot patch object")
 		}

pkg/utils/apply/apply_test.go

@@ -144,7 +144,7 @@ func TestAPIApplicator(t *testing.T) {
 					return tc.args.existing, tc.args.creatorErr
 				}),
 				patcher: patcherFn(func(c, m client.Object, a *applyAction) (client.Patch, error) {
-					return nil, tc.args.patcherErr
+					return client.RawPatch(types.MergePatchType, []byte(`err`)), tc.args.patcherErr
 				}),
 				c: tc.c,
 			}

pkg/utils/apply/patch.go

@@ -169,3 +169,11 @@ func getOriginalConfiguration(obj runtime.Object) ([]byte, error) {
 	}
 	return []byte(original), nil
 }
+
+func isEmptyPatch(patch client.Patch) bool {
+	if patch == nil {
+		return true
+	}
+	data, _ := patch.Data(nil)
+	return data != nil && string(data) == "{}"
+}

test/e2e-multicluster-test/multicluster_test.go

@@ -884,7 +884,7 @@ var _ = Describe("Test multicluster scenario", func() {
 					}
 				}
 				g.Expect(cnt).Should(Equal(2))
-			}).WithTimeout(10 * time.Second).WithPolling(2 * time.Second).Should(Succeed())
+			}).WithTimeout(30 * time.Second).WithPolling(2 * time.Second).Should(Succeed())
 
 			By("try update application again")
 			Expect(k8sClient.Get(hubCtx, key, app)).Should(Succeed())