From cecb7690dadc177338555183c78c2aee9a9dd0e7 Mon Sep 17 00:00:00 2001
From: res <2232824543@qq.com>
Date: Thu, 11 May 2023 14:57:12 +0800
Subject: [PATCH] =?UTF-8?q?perf:=20=E6=B7=BB=E5=8A=A0=E5=A4=9A=E4=B8=AA?=
 =?UTF-8?q?=E9=9B=86=E7=BE=A4=E6=97=B6=E5=85=A8=E9=83=A8=E5=A4=84=E7=90=86?=
 =?UTF-8?q?=E5=AE=8C=E7=BB=9F=E4=B8=80=E8=BF=94=E5=9B=9E,=E4=BC=98?=
 =?UTF-8?q?=E5=8C=96=E7=9B=B8=E5=85=B3=E4=BB=A3=E7=A0=81?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

---
 pkg/server/handler/clusters/cluster.go | 106 +++++++++++++++----------
 1 file changed, 63 insertions(+), 43 deletions(-)

diff --git a/pkg/server/handler/clusters/cluster.go b/pkg/server/handler/clusters/cluster.go
index 21d7bc90f..4a5182800 100644
--- a/pkg/server/handler/clusters/cluster.go
+++ b/pkg/server/handler/clusters/cluster.go
@@ -97,34 +97,41 @@ func (ch *ClusterHandler) ListClusters(c *gin.Context) {
 
 // AddClusters add the clusters which has deployed crane server. cluster info must has valid & accessible crane server url
 func (ch *ClusterHandler) AddClusters(c *gin.Context) {
-	var r AddClustersRequest
-	if err := c.ShouldBindJSON(&r); err != nil {
+	var request AddClustersRequest
+	if err := c.ShouldBindJSON(&request); err != nil {
 		ginwrapper.WriteResponse(c, err, nil)
 		return
 	}
 
-	if len(r.Clusters) == 0 {
-		ginwrapper.WriteResponse(c, fmt.Errorf("req is empty, check your input para"), nil)
+	if len(request.Clusters) == 0 {
+		ginwrapper.WriteResponse(c, fmt.Errorf("no clusters provided"), nil)
 		return
 	}
 
 	clustersMap, err := ch.getClusterMap()
+
 	if err != nil {
 		ginwrapper.WriteResponse(c, err, nil)
 		return
 	}
 
-	for _, cluster := range r.Clusters {
-		if cluster.CraneUrl == "" || cluster.Name == "" {
-			err := fmt.Errorf("cluster CraneUrl, Name field must not be empty")
-			ginwrapper.WriteResponse(c, err, nil)
-			return
+	var errors []error
+	var addedClusters []*store.Cluster
+
+	for _, cluster := range request.Clusters {
+		if err := validateCluster(cluster); err != nil {
+			errors = append(errors, err)
+			continue
 		}
 
-		if !IsUrl(cluster.CraneUrl) {
-			err := fmt.Errorf("cluster CraneUrl %v is not valid url", cluster.CraneUrl)
-			ginwrapper.WriteResponse(c, err, nil)
-			return
+		if _, ok := clustersMap[cluster.Id]; ok {
+			errors = append(errors, fmt.Errorf("cluster id %v duplicated", cluster.Id))
+			continue
+		}
+
+		if cluster.CraneUrlDuplicated(clustersMap) {
+			errors = append(errors, fmt.Errorf("cluster CraneUrl %v duplicated", cluster.CraneUrl))
+			continue
 		}
 
 		if cluster.Id == "" {
@@ -136,49 +143,62 @@ func (ch *ClusterHandler) AddClusters(c *gin.Context) {
 			cluster.Discount = 100
 		}
 
-		if _, ok := clustersMap[cluster.Id]; ok {
-			err := fmt.Errorf("cluster id %v duplicated", cluster.Id)
-			ginwrapper.WriteResponse(c, err, nil)
-			return
-		}
+		clustersMap[cluster.Id] = cluster
 
-		if cluster.CraneUrlDuplicated(clustersMap) {
-			err := fmt.Errorf("cluster CraneUlr %v duplicated", cluster.CraneUrl)
-			ginwrapper.WriteResponse(c, err, nil)
-			return
-		}
+		addedClusters = append(addedClusters, cluster)
+	}
 
-		clustersMap[cluster.Id] = cluster
+	if len(errors) > 0 {
+		ginwrapper.WriteResponse(c, fmt.Errorf("encountered errors adding clusters: %v", errors), nil)
+		return
 	}
 
-	for _, cluster := range r.Clusters {
-		err := ch.clusterSrv.AddCluster(context.TODO(), cluster)
-		if err != nil {
-			ginwrapper.WriteResponse(c, err, nil)
-			return
+	for _, cluster := range addedClusters {
+		if err := ch.clusterSrv.AddCluster(context.TODO(), cluster); err != nil {
+			errors = append(errors, err)
+			continue
 		}
 
-		if cluster.PreinstallRecommendation && err == nil {
-			err := ch.upsertRecommendationRule(RecommendationRuleWorkloadsName, RecommendationRuleWorkloadsYAML)
-			if err != nil {
-				ginwrapper.WriteResponse(c, err, nil)
-				return
-			}
-
-			err = ch.upsertRecommendationRule(RecommendationRuleIdleNodeName, RecommendationRuleIdleNodeYAML)
-			if err != nil {
-				ginwrapper.WriteResponse(c, err, nil)
-				return
+		if cluster.PreinstallRecommendation {
+			if err := ch.upsertRecommendationRules(); err != nil {
+				errors = append(errors, err)
+				continue
 			}
-		} else if err != nil {
-			ginwrapper.WriteResponse(c, err, nil)
-			return
 		}
 	}
 
+	if len(errors) > 0 {
+		ginwrapper.WriteResponse(c, fmt.Errorf("encountered errors adding clusters: %v", errors), nil)
+		return
+	}
+
 	ginwrapper.WriteResponse(c, nil, nil)
 }
 
+func validateCluster(cluster *store.Cluster) error {
+	if cluster.CraneUrl == "" || cluster.Name == "" {
+		return fmt.Errorf("cluster CraneUrl or Name field is empty")
+	}
+
+	if !IsUrl(cluster.CraneUrl) {
+		return fmt.Errorf("cluster CraneUrl %v is not valid url", cluster.CraneUrl)
+	}
+
+	return nil
+}
+
+func (ch *ClusterHandler) upsertRecommendationRules() error {
+	if err := ch.upsertRecommendationRule(RecommendationRuleWorkloadsName, RecommendationRuleWorkloadsYAML); err != nil {
+		return err
+	}
+
+	if err := ch.upsertRecommendationRule(RecommendationRuleIdleNodeName, RecommendationRuleIdleNodeYAML); err != nil {
+		return err
+	}
+
+	return nil
+}
+
 // UpdateCluster the clusters crane info
 func (ch *ClusterHandler) UpdateCluster(c *gin.Context) {
 	var r store.Cluster