[GitHub] [rocketmq-operator] vongosling commented on a change in pull request #3: Add NFS storage class for RocketMQ data persistence

[GitBox <gi...@apache.org>]

vongosling commented on a change in pull request #3: Add NFS storage class for RocketMQ data persistence
URL: https://github.com/apache/rocketmq-operator/pull/3#discussion_r323059911
 
 

 ##########
 File path: pkg/controller/broker/broker_controller.go
 ##########
 @@ -117,57 +119,79 @@ func (r *ReconcileBroker) Reconcile(request reconcile.Request) (reconcile.Result
 		return reconcile.Result{}, err
 	}
 
-	// Check if the deployment already exists, if not create a new one
-	found := &appsv1.Deployment{}
+	// Check if the statefulSet already exists, if not create a new one
+	found := &appsv1.StatefulSet{}
 
-	brokerGroupNum := int(broker.Spec.Size)
+	share.GroupNum = int(broker.Spec.Size)
 	slavePerGroup := broker.Spec.SlavePerGroup
-	reqLogger.Info("brokerGroupNum=" + strconv.Itoa(brokerGroupNum) + ", slavePerGroup=" + strconv.Itoa(slavePerGroup))
+	reqLogger.Info("brokerGroupNum=" + strconv.Itoa(share.GroupNum) + ", slavePerGroup=" + strconv.Itoa(slavePerGroup))
 
-	for brokerClusterIndex := 0; brokerClusterIndex < brokerGroupNum; brokerClusterIndex++ {
-		reqLogger.Info("Check Broker cluster " + strconv.Itoa(brokerClusterIndex+1) + "/" + strconv.Itoa(brokerGroupNum))
-		dep := r.deploymentForMasterBroker(broker, brokerClusterIndex)
+	for brokerClusterIndex := 0; brokerClusterIndex < share.GroupNum; brokerClusterIndex++ {
+		reqLogger.Info("Check Broker cluster " + strconv.Itoa(brokerClusterIndex+1) + "/" + strconv.Itoa(share.GroupNum))
+		dep := r.statefulSetForMasterBroker(broker, brokerClusterIndex)
 		err = r.client.Get(context.TODO(), types.NamespacedName{Name: dep.Name, Namespace: dep.Namespace}, found)
+
 		if err != nil && errors.IsNotFound(err) {
-			reqLogger.Info("Creating a new Master Broker Deployment.", "Deployment.Namespace", dep.Namespace, "Deployment.Name", dep.Name)
+			reqLogger.Info("Creating a new Master Broker StatefulSet.", "StatefulSet.Namespace", dep.Namespace, "StatefulSet.Name", dep.Name)
 			err = r.client.Create(context.TODO(), dep)
 			for slaveIndex := 1; slaveIndex <= slavePerGroup; slaveIndex++ {
 				reqLogger.Info("Check Slave Broker of cluster-" + strconv.Itoa(brokerClusterIndex) + " " + strconv.Itoa(slaveIndex) + "/" + strconv.Itoa(slavePerGroup))
-				slaveDep := r.deploymentForSlaveBroker(broker, brokerClusterIndex, slaveIndex)
+				slaveDep := r.statefulSetForSlaveBroker(broker, brokerClusterIndex, slaveIndex)
 				err = r.client.Get(context.TODO(), types.NamespacedName{Name: slaveDep.Name, Namespace: slaveDep.Namespace}, found)
 				if err != nil && errors.IsNotFound(err) {
-					reqLogger.Info("Creating a new Slave Broker Deployment.", "Deployment.Namespace", slaveDep.Namespace, "Deployment.Name", slaveDep.Name)
+					reqLogger.Info("Creating a new Slave Broker StatefulSet.", "StatefulSet.Namespace", slaveDep.Namespace, "StatefulSet.Name", slaveDep.Name)
 					err = r.client.Create(context.TODO(), slaveDep)
 					if err != nil {
-						reqLogger.Error(err, "Failed to create new Deployment of broker-"+strconv.Itoa(brokerClusterIndex)+"-slave-"+strconv.Itoa(slaveIndex), "Deployment.Namespace", slaveDep.Namespace, "Deployment.Name", slaveDep.Name)
+						reqLogger.Error(err, "Failed to create new StatefulSet of broker-"+strconv.Itoa(brokerClusterIndex)+"-slave-"+strconv.Itoa(slaveIndex), "StatefulSet.Namespace", slaveDep.Namespace, "StatefulSet.Name", slaveDep.Name)
 					}
 				} else if err != nil {
-					reqLogger.Error(err, "Failed to get broker slave Deployment.")
+					reqLogger.Error(err, "Failed to get broker slave StatefulSet.")
 				}
 			}
 			if err != nil {
-				reqLogger.Error(err, "Failed to create new Deployment of "+cons.BrokerClusterPrefix+strconv.Itoa(brokerClusterIndex), "Deployment.Namespace", dep.Namespace, "Deployment.Name", dep.Name)
+				reqLogger.Error(err, "Failed to create new StatefulSet of "+cons.BrokerClusterPrefix+strconv.Itoa(brokerClusterIndex), "StatefulSet.Namespace", dep.Namespace, "StatefulSet.Name", dep.Name)
 			}
 		} else if err != nil {
-			reqLogger.Error(err, "Failed to get broker master Deployment.")
+			reqLogger.Error(err, "Failed to get broker master StatefulSet.")
 		}
+
+		// The following code will restart all brokers to update NAMESRV_ADDR env
 
 Review comment:
   If no live update feature introduced, please prioritize the rolling update.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services