Merge pull request #12906 from rook/mergify/bp/release-1.12/pr-12895

mgr: Allow more than two mgrs (backport #12895)
rook · Sep 14, 2023 · c1d25e7 · c1d25e7
2 parents 559f660 + 19ffd74
commit c1d25e7
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 18 deletions.
diff --git a/Documentation/CRDs/specification.md b/Documentation/CRDs/specification.md
@@ -7218,7 +7218,7 @@ int
 </td>
 <td>
 <em>(Optional)</em>
-<p>Count is the number of manager to run</p>
+<p>Count is the number of manager daemons to run</p>
 </td>
 </tr>
 <tr>

diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml
@@ -1323,8 +1323,8 @@ spec:
                       description: AllowMultiplePerNode allows to run multiple managers on the same node (not recommended)
                       type: boolean
                     count:
-                      description: Count is the number of manager to run
-                      maximum: 2
+                      description: Count is the number of manager daemons to run
+                      maximum: 5
                       minimum: 0
                       type: integer
                     modules:

diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml
@@ -1321,8 +1321,8 @@ spec:
                       description: AllowMultiplePerNode allows to run multiple managers on the same node (not recommended)
                       type: boolean
                     count:
-                      description: Count is the number of manager to run
-                      maximum: 2
+                      description: Count is the number of manager daemons to run
+                      maximum: 5
                       minimum: 0
                       type: integer
                     modules:

diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go
@@ -581,9 +581,9 @@ type MonZoneSpec struct {
 
 // MgrSpec represents options to configure a ceph mgr
 type MgrSpec struct {
-	// Count is the number of manager to run
+	// Count is the number of manager daemons to run
 	// +kubebuilder:validation:Minimum=0
-	// +kubebuilder:validation:Maximum=2
+	// +kubebuilder:validation:Maximum=5
 	// +optional
 	Count int `json:"count,omitempty"`
 	// AllowMultiplePerNode allows to run multiple managers on the same node (not recommended)

diff --git a/pkg/operator/ceph/cluster/mgr/mgr.go b/pkg/operator/ceph/cluster/mgr/mgr.go
@@ -44,7 +44,6 @@ var logger = capnslog.NewPackageLogger("github.com/rook/rook", "op-mgr")
 const (
 	AppName                = "rook-ceph-mgr"
 	serviceAccountName     = "rook-ceph-mgr"
-	maxMgrCount            = 2
 	PrometheusModuleName   = "prometheus"
 	crashModuleName        = "crash"
 	PgautoscalerModuleName = "pg_autoscaler"
@@ -97,9 +96,6 @@ func (c *Cluster) getReplicas() int {
 func (c *Cluster) getDaemonIDs() []string {
 	var daemonIDs []string
 	replicas := c.getReplicas()
-	if replicas > maxMgrCount {
-		replicas = maxMgrCount
-	}
 	for i := 0; i < replicas; i++ {
 		daemonIDs = append(daemonIDs, k8sutil.IndexToName(i))
 	}
@@ -201,14 +197,39 @@ func (c *Cluster) Start() error {
 }
 
 func (c *Cluster) removeExtraMgrs(daemonIDs []string) {
+	options := metav1.ListOptions{LabelSelector: "app=" + AppName}
+	mgrDeployments, err := c.context.Clientset.AppsV1().Deployments(c.clusterInfo.Namespace).List(c.clusterInfo.Context, options)
+	if err != nil {
+		logger.Warningf("failed to check for extra mgrs. %v", err)
+		return
+	}
+	if len(mgrDeployments.Items) == len(daemonIDs) {
+		logger.Debugf("expected number %d of mgrs found", len(daemonIDs))
+		return
+	}
+
 	// In case the mgr count was reduced, delete the extra mgrs
-	for i := maxMgrCount - 1; i >= len(daemonIDs); i-- {
-		mgrName := fmt.Sprintf("%s-%s", AppName, k8sutil.IndexToName(i))
-		err := c.context.Clientset.AppsV1().Deployments(c.clusterInfo.Namespace).Delete(c.clusterInfo.Context, mgrName, metav1.DeleteOptions{})
-		if err == nil {
-			logger.Infof("removed extra mgr %q", mgrName)
-		} else if !kerrors.IsNotFound(err) {
-			logger.Warningf("failed to remove extra mgr %q. %v", mgrName, err)
+	for _, mgrDeployment := range mgrDeployments.Items {
+		id, ok := mgrDeployment.Labels[controller.DaemonIDLabel]
+		if !ok {
+			// skipping evaluation of non-mgr daemon that mistakenly matched the mgr labels
+			continue
+		}
+		found := false
+		for _, daemonID := range daemonIDs {
+			if id == daemonID {
+				// mark the mgr as found if the ID matches
+				found = true
+				break
+			}
+		}
+		if !found {
+			err := c.context.Clientset.AppsV1().Deployments(c.clusterInfo.Namespace).Delete(c.clusterInfo.Context, mgrDeployment.Name, metav1.DeleteOptions{})
+			if err == nil {
+				logger.Infof("removed extra mgr %q", mgrDeployment.Name)
+			} else {
+				logger.Warningf("failed to remove extra mgr %q. %v", mgrDeployment.Name, err)
+			}
 		}
 	}
 }