make DeepEqual judgments more efficient

Signed-off-by: zhzhuang-zju <[email protected]>
karmada-io · Apr 12, 2024 · e28c9c7 · e28c9c7
1 parent da7689f
commit e28c9c7
Show file tree

Hide file tree

Showing 3 changed files with 27 additions and 23 deletions.
diff --git a/cmd/agent/app/agent.go b/cmd/agent/app/agent.go
@@ -24,7 +24,6 @@ import (
 	"strconv"
 
 	"github.com/spf13/cobra"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/dynamic"
 	"k8s.io/client-go/informers"
@@ -433,7 +432,6 @@ func startCertRotationController(ctx controllerscontext.Context) (bool, error) {
 }
 
 func generateClusterInControllerPlane(opts util.ClusterRegisterOption) (*clusterv1alpha1.Cluster, error) {
-	clusterObj := &clusterv1alpha1.Cluster{ObjectMeta: metav1.ObjectMeta{Name: opts.ClusterName}}
 	mutateFunc := func(cluster *clusterv1alpha1.Cluster) {
 		cluster.Spec.SyncMode = clusterv1alpha1.Pull
 		cluster.Spec.APIEndpoint = opts.ClusterAPIEndpoint
@@ -475,9 +473,9 @@ func generateClusterInControllerPlane(opts util.ClusterRegisterOption) (*cluster
 		}
 	}
 	controlPlaneKarmadaClient := karmadaclientset.NewForConfigOrDie(opts.ControlPlaneConfig)
-	cluster, err := util.CreateOrUpdateClusterObject(controlPlaneKarmadaClient, clusterObj, mutateFunc)
+	cluster, err := util.CreateOrUpdateClusterObject(controlPlaneKarmadaClient, opts.ClusterName, mutateFunc)
 	if err != nil {
-		klog.Errorf("Failed to create cluster(%s) object, error: %v", clusterObj.Name, err)
+		klog.Errorf("Failed to create or update cluster(%s) object, error: %v", opts.ClusterName, err)
 		return nil, err
 	}
 

diff --git a/pkg/util/cluster.go b/pkg/util/cluster.go
@@ -136,30 +136,34 @@ func CreateClusterObject(controlPlaneClient karmadaclientset.Interface, clusterO
 }
 
 // CreateOrUpdateClusterObject create cluster object in karmada control plane,
-// if cluster object has been existed and different from input clusterObj, update it.
-func CreateOrUpdateClusterObject(controlPlaneClient karmadaclientset.Interface, clusterObj *clusterv1alpha1.Cluster, mutate func(*clusterv1alpha1.Cluster)) (*clusterv1alpha1.Cluster, error) {
-	cluster, exist, err := GetClusterWithKarmadaClient(controlPlaneClient, clusterObj.Name)
+// if cluster object has been existed, update it if necessary.
+func CreateOrUpdateClusterObject(controlPlaneClient karmadaclientset.Interface, clusterName string, mutate func(*clusterv1alpha1.Cluster)) (*clusterv1alpha1.Cluster, error) {
+	cluster, exist, err := GetClusterWithKarmadaClient(controlPlaneClient, clusterName)
 	if err != nil {
 		return nil, err
 	}
 	if exist {
-		if reflect.DeepEqual(cluster.Spec, clusterObj.Spec) {
-			klog.Warningf("Cluster(%s) already exist and newest", clusterObj.Name)
+		clusterCopy := cluster.DeepCopy()
+		mutate(cluster)
+		if reflect.DeepEqual(clusterCopy.Spec, cluster.Spec) {
+			klog.Warningf("Cluster(%s) already exist and newest", clusterName)
 			return cluster, nil
 		}
-		mutate(cluster)
+
 		cluster, err = updateCluster(controlPlaneClient, cluster)
 		if err != nil {
-			klog.Warningf("Failed to create cluster(%s). error: %v", clusterObj.Name, err)
+			klog.Warningf("Failed to update cluster(%s). error: %v", clusterName, err)
 			return nil, err
 		}
 		return cluster, nil
 	}
 
-	mutate(clusterObj)
-
-	if cluster, err = createCluster(controlPlaneClient, clusterObj); err != nil {
-		klog.Warningf("Failed to create cluster(%s). error: %v", clusterObj.Name, err)
+	cluster = &clusterv1alpha1.Cluster{
+		ObjectMeta: metav1.ObjectMeta{Name: clusterName},
+	}
+	mutate(cluster)
+	if cluster, err = createCluster(controlPlaneClient, cluster); err != nil {
+		klog.Warningf("Failed to create cluster(%s). error: %v", clusterName, err)
 		return nil, err
 	}
 	return cluster, nil

diff --git a/pkg/util/cluster_test.go b/pkg/util/cluster_test.go
@@ -62,7 +62,7 @@ func withID(cluster *clusterv1alpha1.Cluster, id string) *clusterv1alpha1.Cluste
 func TestCreateOrUpdateClusterObject(t *testing.T) {
 	type args struct {
 		controlPlaneClient karmadaclientset.Interface
-		clusterObj         *clusterv1alpha1.Cluster
+		clusterName        string
 		mutate             func(*clusterv1alpha1.Cluster)
 	}
 	tests := []struct {
@@ -75,8 +75,9 @@ func TestCreateOrUpdateClusterObject(t *testing.T) {
 			name: "cluster exist, and update cluster",
 			args: args{
 				controlPlaneClient: karmadaclientsetfake.NewSimpleClientset(withAPIEndPoint(newCluster(ClusterMember1), "https://127.0.0.1:6443")),
-				clusterObj:         newCluster(ClusterMember1),
+				clusterName:        ClusterMember1,
 				mutate: func(cluster *clusterv1alpha1.Cluster) {
+					cluster.Spec.APIEndpoint = "https://127.0.0.1:6443"
 					cluster.Spec.SyncMode = clusterv1alpha1.Pull
 				},
 			},
@@ -86,8 +87,9 @@ func TestCreateOrUpdateClusterObject(t *testing.T) {
 			name: "cluster exist and equal, not update",
 			args: args{
 				controlPlaneClient: karmadaclientsetfake.NewSimpleClientset(withSyncMode(withAPIEndPoint(newCluster(ClusterMember1), "https://127.0.0.1:6443"), clusterv1alpha1.Pull)),
-				clusterObj:         withSyncMode(withAPIEndPoint(newCluster(ClusterMember1), "https://127.0.0.1:6443"), clusterv1alpha1.Pull),
+				clusterName:        ClusterMember1,
 				mutate: func(cluster *clusterv1alpha1.Cluster) {
+					cluster.Spec.APIEndpoint = "https://127.0.0.1:6443"
 					cluster.Spec.SyncMode = clusterv1alpha1.Pull
 				},
 			},
@@ -97,7 +99,7 @@ func TestCreateOrUpdateClusterObject(t *testing.T) {
 			name: "cluster not exist, and create cluster",
 			args: args{
 				controlPlaneClient: karmadaclientsetfake.NewSimpleClientset(),
-				clusterObj:         newCluster(ClusterMember1),
+				clusterName:        ClusterMember1,
 				mutate: func(cluster *clusterv1alpha1.Cluster) {
 					cluster.Spec.SyncMode = clusterv1alpha1.Pull
 				},
@@ -112,7 +114,7 @@ func TestCreateOrUpdateClusterObject(t *testing.T) {
 					c.PrependReactor("get", "*", errorAction)
 					return c
 				}(),
-				clusterObj: newCluster(ClusterMember1),
+				clusterName: ClusterMember1,
 				mutate: func(cluster *clusterv1alpha1.Cluster) {
 					cluster.Spec.SyncMode = clusterv1alpha1.Pull
 				},
@@ -128,7 +130,7 @@ func TestCreateOrUpdateClusterObject(t *testing.T) {
 					c.PrependReactor("create", "*", errorAction)
 					return c
 				}(),
-				clusterObj: newCluster(ClusterMember1),
+				clusterName: ClusterMember1,
 				mutate: func(cluster *clusterv1alpha1.Cluster) {
 					cluster.Spec.SyncMode = clusterv1alpha1.Pull
 				},
@@ -144,7 +146,7 @@ func TestCreateOrUpdateClusterObject(t *testing.T) {
 					c.PrependReactor("update", "*", errorAction)
 					return c
 				}(),
-				clusterObj: newCluster(ClusterMember1),
+				clusterName: ClusterMember1,
 				mutate: func(cluster *clusterv1alpha1.Cluster) {
 					cluster.Spec.SyncMode = clusterv1alpha1.Pull
 				},
@@ -155,7 +157,7 @@ func TestCreateOrUpdateClusterObject(t *testing.T) {
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
-			got, err := CreateOrUpdateClusterObject(tt.args.controlPlaneClient, tt.args.clusterObj, tt.args.mutate)
+			got, err := CreateOrUpdateClusterObject(tt.args.controlPlaneClient, tt.args.clusterName, tt.args.mutate)
 			if (err != nil) != tt.wantErr {
 				t.Errorf("CreateOrUpdateClusterObject() error = %v, wantErr %v", err, tt.wantErr)
 				return