Feat: optimize empty patch request

Signed-off-by: Somefive <yd219913@alibaba-inc.com>
kubevela · Mar 6, 2023 · 058beb3 · 058beb3
1 parent 76a8d13
commit 058beb3
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 1 deletion.
diff --git a/pkg/controller/core.oam.dev/v1alpha2/application/application_controller.go b/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,16 @@ func (r *Reconciler) matchControllerRequirement(app *v1beta1.Application) bool {
 	}
 	return true
 }
+
+const (
+	originalAppKey contextKey = "original-app"
+)
+
+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
+}
diff --git a/pkg/utils/apply/apply.go b/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")
 		}

diff --git a/pkg/utils/apply/apply_test.go b/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,
 			}

diff --git a/pkg/utils/apply/patch.go b/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) == "{}"
+}