update placement and decision wiht patcher

Signed-off-by: haoqing0110 <qhao@redhat.com>
open-cluster-management-io · Jul 6, 2023 · 37c85a6 · 37c85a6
1 parent 48289fd
commit 37c85a6
Show file tree

Hide file tree

Showing 2 changed files with 90 additions and 84 deletions.
diff --git a/pkg/placement/controllers/scheduling/scheduling_controller.go b/pkg/placement/controllers/scheduling/scheduling_controller.go
@@ -14,7 +14,6 @@ import (
 	"github.com/openshift/library-go/pkg/operator/events"
 	errorhelpers "github.com/openshift/library-go/pkg/operator/v1helpers"
 	corev1 "k8s.io/api/core/v1"
-	apiequality "k8s.io/apimachinery/pkg/api/equality"
 	"k8s.io/apimachinery/pkg/api/errors"
 	"k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -40,6 +39,7 @@ import (
 	clusterapiv1beta1 "open-cluster-management.io/api/cluster/v1beta1"
 	clusterapiv1beta2 "open-cluster-management.io/api/cluster/v1beta2"
 
+	"open-cluster-management.io/ocm/pkg/common/patcher"
 	"open-cluster-management.io/ocm/pkg/common/queue"
 	"open-cluster-management.io/ocm/pkg/placement/controllers/framework"
 	"open-cluster-management.io/ocm/pkg/placement/helpers"
@@ -383,9 +383,11 @@ func (c *schedulingController) updateStatus(
 		return nil
 	}
 
-	_, err := c.clusterClient.ClusterV1beta1().
-		Placements(newPlacement.Namespace).
-		UpdateStatus(ctx, newPlacement, metav1.UpdateOptions{})
+	placementPatcher := patcher.NewPatcher[
+		*clusterapiv1beta1.Placement, clusterapiv1beta1.PlacementSpec, clusterapiv1beta1.PlacementStatus](
+		c.clusterClient.ClusterV1beta1().Placements(newPlacement.Namespace))
+
+	_, err := placementPatcher.PatchStatus(ctx, newPlacement, newPlacement.Status, placement.Status)
 	return err
 }
 
@@ -709,7 +711,7 @@ func (c *schedulingController) createOrUpdatePlacementDecision(
 		return fmt.Errorf("the number of clusterdecisions %q exceeds the max limitation %q", len(clusterDecisions), maxNumOfClusterDecisions)
 	}
 
-	existPlacementDecision, err := c.placementDecisionLister.PlacementDecisions(placement.Namespace).Get(placementDecisionName)
+	existPlacementDecision, err := c.placementDecisionLister.PlacementDecisions(placementDecision.Namespace).Get(placementDecisionName)
 	switch {
 	case errors.IsNotFound(err):
 		var err error
@@ -727,29 +729,20 @@ func (c *schedulingController) createOrUpdatePlacementDecision(
 	}
 
 	// update the status and labels of the placementdecision if decisions change
-	decisionsequal := apiequality.Semantic.DeepEqual(existPlacementDecision.Status.Decisions, clusterDecisions)
-	labelsequal := apiequality.Semantic.DeepEqual(existPlacementDecision.Labels, placementDecision.Labels)
-
-	if decisionsequal && labelsequal {
-		return nil
-	}
+	placementDecisionPatcher := patcher.NewPatcher[
+		*clusterapiv1beta1.PlacementDecision, interface{}, clusterapiv1beta1.PlacementDecisionStatus](
+		c.clusterClient.ClusterV1beta1().PlacementDecisions(placementDecision.Namespace))
 
 	newPlacementDecision := existPlacementDecision.DeepCopy()
-	if !labelsequal {
-		newPlacementDecision.Labels = placementDecision.Labels
-		_, err = c.clusterClient.ClusterV1beta1().PlacementDecisions(newPlacementDecision.Namespace).
-			Update(ctx, newPlacementDecision, metav1.UpdateOptions{})
-		if err != nil {
-			return err
-		}
+	newPlacementDecision.Labels = placementDecision.Labels
+	newPlacementDecision.Status.Decisions = clusterDecisions
+	_, err = placementDecisionPatcher.PatchLabelAnnotations(ctx, newPlacementDecision, newPlacementDecision.ObjectMeta, existPlacementDecision.ObjectMeta)
+	if err != nil {
+		return err
 	}
-	if !decisionsequal {
-		newPlacementDecision.Status.Decisions = clusterDecisions
-		_, err = c.clusterClient.ClusterV1beta1().PlacementDecisions(newPlacementDecision.Namespace).
-			UpdateStatus(ctx, newPlacementDecision, metav1.UpdateOptions{})
-		if err != nil {
-			return err
-		}
+	_, err = placementDecisionPatcher.PatchStatus(ctx, newPlacementDecision, newPlacementDecision.Status, existPlacementDecision.Status)
+	if err != nil {
+		return err
 	}
 
 	// update the event with warning

diff --git a/pkg/placement/controllers/scheduling/scheduling_controller_test.go b/pkg/placement/controllers/scheduling/scheduling_controller_test.go
@@ -2,6 +2,7 @@ package scheduling
 
 import (
 	"context"
+	"encoding/json"
 	"fmt"
 	"reflect"
 	"sort"
@@ -67,12 +68,13 @@ func TestSchedulingController_sync(t *testing.T) {
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
 				// check if PlacementDecision has been updated
-				testingcommon.AssertActions(t, actions, "create", "update")
+				testingcommon.AssertActions(t, actions, "create", "patch")
 				// check if Placement has been updated
-				actual := actions[1].(clienttesting.UpdateActionImpl).Object
-				placement, ok := actual.(*clusterapiv1beta1.Placement)
-				if !ok {
-					t.Errorf("expected Placement was updated")
+				placement := &clusterapiv1beta1.Placement{}
+				patchData := actions[1].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placement)
+				if err != nil {
+					t.Fatal(err)
 				}
 
 				if placement.Status.NumberOfSelectedClusters != int32(3) {
@@ -122,12 +124,13 @@ func TestSchedulingController_sync(t *testing.T) {
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
 				// check if PlacementDecision has been updated
-				testingcommon.AssertActions(t, actions, "create", "update")
+				testingcommon.AssertActions(t, actions, "create", "patch")
 				// check if Placement has been updated
-				actual := actions[1].(clienttesting.UpdateActionImpl).Object
-				placement, ok := actual.(*clusterapiv1beta1.Placement)
-				if !ok {
-					t.Errorf("expected Placement was updated")
+				placement := &clusterapiv1beta1.Placement{}
+				patchData := actions[1].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placement)
+				if err != nil {
+					t.Fatal(err)
 				}
 
 				if placement.Status.NumberOfSelectedClusters != int32(3) {
@@ -172,12 +175,13 @@ func TestSchedulingController_sync(t *testing.T) {
 				unscheduledDecisions: 0,
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				testingcommon.AssertActions(t, actions, "create", "create", "create", "update")
+				testingcommon.AssertActions(t, actions, "create", "create", "create", "patch")
 				// check if Placement has been updated
-				actual := actions[3].(clienttesting.UpdateActionImpl).Object
-				placement, ok := actual.(*clusterapiv1beta1.Placement)
-				if !ok {
-					t.Errorf("expected Placement was updated")
+				placement := &clusterapiv1beta1.Placement{}
+				patchData := actions[3].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placement)
+				if err != nil {
+					t.Fatal(err)
 				}
 
 				if placement.Status.NumberOfSelectedClusters != int32(4) {
@@ -247,12 +251,13 @@ func TestSchedulingController_sync(t *testing.T) {
 				unscheduledDecisions: 0,
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				testingcommon.AssertActions(t, actions, "create", "create", "update")
+				testingcommon.AssertActions(t, actions, "create", "create", "patch")
 				// check if Placement has been updated
-				actual := actions[2].(clienttesting.UpdateActionImpl).Object
-				placement, ok := actual.(*clusterapiv1beta1.Placement)
-				if !ok {
-					t.Errorf("expected Placement was updated")
+				placement := &clusterapiv1beta1.Placement{}
+				patchData := actions[2].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placement)
+				if err != nil {
+					t.Fatal(err)
 				}
 
 				if placement.Status.NumberOfSelectedClusters != int32(3) {
@@ -304,12 +309,13 @@ func TestSchedulingController_sync(t *testing.T) {
 				unscheduledDecisions: 0,
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				testingcommon.AssertActions(t, actions, "create", "create", "create", "update")
+				testingcommon.AssertActions(t, actions, "create", "create", "create", "patch")
 				// check if Placement has been updated
-				actual := actions[3].(clienttesting.UpdateActionImpl).Object
-				placement, ok := actual.(*clusterapiv1beta1.Placement)
-				if !ok {
-					t.Errorf("expected Placement was updated")
+				placement := &clusterapiv1beta1.Placement{}
+				patchData := actions[3].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placement)
+				if err != nil {
+					t.Fatal(err)
 				}
 
 				if placement.Status.NumberOfSelectedClusters != int32(3) {
@@ -358,7 +364,7 @@ func TestSchedulingController_sync(t *testing.T) {
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
 				// check if PlacementDecision has been updated
-				testingcommon.AssertActions(t, actions, "create", "update")
+				testingcommon.AssertActions(t, actions, "create", "patch")
 				// check if emtpy PlacementDecision has been created
 				actual := actions[0].(clienttesting.CreateActionImpl).Object
 				placementDecision, ok := actual.(*clusterapiv1beta1.PlacementDecision)
@@ -370,10 +376,11 @@ func TestSchedulingController_sync(t *testing.T) {
 					t.Errorf("expecte %d cluster selected, but got %d", 0, len(placementDecision.Status.Decisions))
 				}
 				// check if Placement has been updated
-				actual = actions[1].(clienttesting.UpdateActionImpl).Object
-				placement, ok := actual.(*clusterapiv1beta1.Placement)
-				if !ok {
-					t.Errorf("expected Placement was updated")
+				placement := &clusterapiv1beta1.Placement{}
+				patchData := actions[1].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placement)
+				if err != nil {
+					t.Fatal(err)
 				}
 
 				if placement.Status.NumberOfSelectedClusters != int32(0) {
@@ -404,7 +411,7 @@ func TestSchedulingController_sync(t *testing.T) {
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
 				// check if PlacementDecision has been updated
-				testingcommon.AssertActions(t, actions, "create", "update")
+				testingcommon.AssertActions(t, actions, "create", "patch")
 				// check if emtpy PlacementDecision has been created
 				actual := actions[0].(clienttesting.CreateActionImpl).Object
 				placementDecision, ok := actual.(*clusterapiv1beta1.PlacementDecision)
@@ -416,10 +423,11 @@ func TestSchedulingController_sync(t *testing.T) {
 					t.Errorf("expecte %d cluster selected, but got %d", 0, len(placementDecision.Status.Decisions))
 				}
 				// check if Placement has been updated
-				actual = actions[1].(clienttesting.UpdateActionImpl).Object
-				placement, ok := actual.(*clusterapiv1beta1.Placement)
-				if !ok {
-					t.Errorf("expected Placement was updated")
+				placement := &clusterapiv1beta1.Placement{}
+				patchData := actions[1].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placement)
+				if err != nil {
+					t.Fatal(err)
 				}
 
 				if placement.Status.NumberOfSelectedClusters != int32(0) {
@@ -453,7 +461,7 @@ func TestSchedulingController_sync(t *testing.T) {
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
 				// check if PlacementDecision has been updated
-				testingcommon.AssertActions(t, actions, "create", "update")
+				testingcommon.AssertActions(t, actions, "create", "patch")
 				// check if emtpy PlacementDecision has been created
 				actual := actions[0].(clienttesting.CreateActionImpl).Object
 				placementDecision, ok := actual.(*clusterapiv1beta1.PlacementDecision)
@@ -465,10 +473,11 @@ func TestSchedulingController_sync(t *testing.T) {
 					t.Errorf("expecte %d cluster selected, but got %d", 0, len(placementDecision.Status.Decisions))
 				}
 				// check if Placement has been updated
-				actual = actions[1].(clienttesting.UpdateActionImpl).Object
-				placement, ok := actual.(*clusterapiv1beta1.Placement)
-				if !ok {
-					t.Errorf("expected Placement was updated")
+				placement := &clusterapiv1beta1.Placement{}
+				patchData := actions[1].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placement)
+				if err != nil {
+					t.Fatal(err)
 				}
 
 				if placement.Status.NumberOfSelectedClusters != int32(0) {
@@ -1263,20 +1272,22 @@ func TestBind(t *testing.T) {
 					WithDecisions(newSelectedClusters(128)[:100]...).Build(),
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				testingcommon.AssertActions(t, actions, "update", "create")
+				testingcommon.AssertActions(t, actions, "patch", "create")
 				selectedClusters := newSelectedClusters(128)
-				actual := actions[0].(clienttesting.UpdateActionImpl).Object
-				placementDecision, ok := actual.(*clusterapiv1beta1.PlacementDecision)
-				if !ok {
-					t.Errorf("expected PlacementDecision was updated")
-				}
-				assertClustersSelected(t, placementDecision.Status.Decisions, selectedClusters[:100]...)
+				placementDecision := &clusterapiv1beta1.PlacementDecision{}
+				patchData := actions[0].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placementDecision)
+				if err != nil {
+					t.Fatal(err)
+				}
+				// only update labels
+				assertClustersSelected(t, placementDecision.Status.Decisions)
 				if placementDecision.Labels[clusterapiv1beta1.DecisionGroupNameLabel] != "" {
 					t.Errorf("unexpected PlacementDecision labels %v", placementDecision.Labels)
 				}
 
-				actual = actions[1].(clienttesting.CreateActionImpl).Object
-				placementDecision, ok = actual.(*clusterapiv1beta1.PlacementDecision)
+				actual := actions[1].(clienttesting.CreateActionImpl).Object
+				placementDecision, ok := actual.(*clusterapiv1beta1.PlacementDecision)
 				if !ok {
 					t.Errorf("expected PlacementDecision was updated")
 				}
@@ -1300,11 +1311,12 @@ func TestBind(t *testing.T) {
 					WithDecisions(newSelectedClusters(128)[100:]...).Build(),
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				testingcommon.AssertActions(t, actions, "update", "delete")
-				actual := actions[0].(clienttesting.UpdateActionImpl).Object
-				placementDecision, ok := actual.(*clusterapiv1beta1.PlacementDecision)
-				if !ok {
-					t.Errorf("expected PlacementDecision was updated")
+				testingcommon.AssertActions(t, actions, "patch", "delete")
+				placementDecision := &clusterapiv1beta1.PlacementDecision{}
+				patchData := actions[0].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placementDecision)
+				if err != nil {
+					t.Fatal(err)
 				}
 				assertClustersSelected(t, placementDecision.Status.Decisions, newSelectedClusters(10)...)
 			},
@@ -1326,11 +1338,12 @@ func TestBind(t *testing.T) {
 					WithDecisions(newSelectedClusters(128)[100:]...).Build(),
 			},
 			validateActions: func(t *testing.T, actions []clienttesting.Action) {
-				testingcommon.AssertActions(t, actions, "update", "delete")
-				actual := actions[0].(clienttesting.UpdateActionImpl).Object
-				placementDecision, ok := actual.(*clusterapiv1beta1.PlacementDecision)
-				if !ok {
-					t.Errorf("expected PlacementDecision was updated")
+				testingcommon.AssertActions(t, actions, "patch", "delete")
+				placementDecision := &clusterapiv1beta1.PlacementDecision{}
+				patchData := actions[0].(clienttesting.PatchActionImpl).Patch
+				err := json.Unmarshal(patchData, placementDecision)
+				if err != nil {
+					t.Fatal(err)
 				}
 				if len(placementDecision.Status.Decisions) != 0 {
 					t.Errorf("expecte %d cluster selected, but got %d", 0, len(placementDecision.Status.Decisions))