mgr: do not set the balancer mode on pacific

On Pacific, Ceph's default is "upmap", so we should let it be
like this.
This lets the user change the mode is desired.
On Octopus though, Rook continues to force the mode to "upmap".

Closes: #9062
Signed-off-by: Sébastien Han <seb@redhat.com>

pkg/operator/ceph/cluster/mgr/mgr.go

@@ -361,14 +361,18 @@ func (c *Cluster) enableCrashModule() error {
 func (c *Cluster) enableBalancerModule() error {
 	// The order MATTERS, always configure this module first, then turn it on
 
-	// This sets min compat client to luminous and the balancer module mode
-	err := cephclient.ConfigureBalancerModule(c.context, c.clusterInfo, balancerModuleMode)
-	if err != nil {
-		return errors.Wrapf(err, "failed to configure module %q", balancerModuleName)
+	// This enables the balancer module mode only in versions older than Pacific
+	// This let's the user change the default mode is desired
+	if !c.clusterInfo.CephVersion.IsAtLeastPacific() {
+		// This sets min compat client to luminous and the balancer module mode
+		err := cephclient.ConfigureBalancerModule(c.context, c.clusterInfo, balancerModuleMode)
+		if err != nil {
+			return errors.Wrapf(err, "failed to configure module %q", balancerModuleName)
+		}
 	}
 
 	// This turns "on" the balancer
-	err = cephclient.MgrEnableModule(c.context, c.clusterInfo, balancerModuleName, false)
+	err := cephclient.MgrEnableModule(c.context, c.clusterInfo, balancerModuleName, false)
 	if err != nil {
 		return errors.Wrapf(err, "failed to turn on mgr %q module", balancerModuleName)
 	}

pkg/operator/ceph/cluster/mgr/mgr_test.go

@@ -24,14 +24,13 @@ import (
 	"testing"
 	"time"
 
+	"github.com/pkg/errors"
+	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1"
 	"github.com/rook/rook/pkg/client/clientset/versioned/scheme"
 	"github.com/rook/rook/pkg/clusterd"
 	cephclient "github.com/rook/rook/pkg/daemon/ceph/client"
 	cephver "github.com/rook/rook/pkg/operator/ceph/version"
-	"sigs.k8s.io/controller-runtime/pkg/client/fake"
-
-	monitoringv1 "github.com/prometheus-operator/prometheus-operator/pkg/apis/monitoring/v1"
 	testopk8s "github.com/rook/rook/pkg/operator/k8sutil/test"
 	testop "github.com/rook/rook/pkg/operator/test"
 	exectest "github.com/rook/rook/pkg/util/exec/test"
@@ -40,8 +39,9 @@ import (
 	apps "k8s.io/api/apps/v1"
 	policyv1 "k8s.io/api/policy/v1"
 	policyv1beta1 "k8s.io/api/policy/v1beta1"
-	"k8s.io/apimachinery/pkg/api/errors"
+	kerrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"sigs.k8s.io/controller-runtime/pkg/client/fake"
 )
 
 func TestStartMgr(t *testing.T) {
@@ -172,7 +172,7 @@ func validateServices(t *testing.T, c *Cluster) {
 			assert.Equal(t, ds.Spec.Ports[0].Port, int32(c.spec.Dashboard.Port))
 		}
 	} else {
-		assert.True(t, errors.IsNotFound(err))
+		assert.True(t, kerrors.IsNotFound(err))
 	}
 }
 
@@ -227,7 +227,7 @@ func TestMgrSidecarReconcile(t *testing.T) {
 	assert.True(t, calledMgrStat)
 	assert.False(t, calledMgrDump)
 	_, err = c.context.Clientset.CoreV1().Services(c.clusterInfo.Namespace).Get(context.TODO(), "rook-ceph-mgr", metav1.GetOptions{})
-	assert.True(t, errors.IsNotFound(err))
+	assert.True(t, kerrors.IsNotFound(err))
 
 	// nothing is updated when the requested mgr is not the active mgr
 	activeMgr = "b"
@@ -378,3 +378,58 @@ func TestApplyMonitoringLabels(t *testing.T) {
 	applyMonitoringLabels(c, sm)
 	assert.Nil(t, sm.Spec.Endpoints[0].RelabelConfigs)
 }
+
+func TestCluster_enableBalancerModule(t *testing.T) {
+	c := &Cluster{
+		context:     &clusterd.Context{Executor: &exectest.MockExecutor{}, Clientset: testop.New(t, 3)},
+		clusterInfo: cephclient.AdminClusterInfo("mycluster"),
+	}
+
+	t.Run("on octopus we configure the balancer AND enable the upmap mode", func(t *testing.T) {
+		c.clusterInfo.CephVersion = cephver.Octopus
+		executor := &exectest.MockExecutor{
+			MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) {
+				logger.Infof("Command: %s %v", command, args)
+				if command == "ceph" {
+					if args[0] == "osd" && args[1] == "set-require-min-compat-client" {
+						return "", nil
+					}
+					if args[0] == "balancer" && args[1] == "mode" {
+						return "", nil
+					}
+					if args[0] == "balancer" && args[1] == "on" {
+						return "", nil
+					}
+				}
+				return "", errors.New("unknown command")
+			},
+		}
+		c.context.Executor = executor
+		err := c.enableBalancerModule()
+		assert.NoError(t, err)
+	})
+
+	t.Run("on pacific we configure the balancer ONLY and don't set a mode", func(t *testing.T) {
+		c.clusterInfo.CephVersion = cephver.Pacific
+		executor := &exectest.MockExecutor{
+			MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) {
+				logger.Infof("Command: %s %v", command, args)
+				if command == "ceph" {
+					if args[0] == "osd" && args[1] == "set-require-min-compat-client" {
+						return "", nil
+					}
+					if args[0] == "balancer" && args[1] == "mode" {
+						return "", errors.New("balancer mode must not be set")
+					}
+					if args[0] == "balancer" && args[1] == "on" {
+						return "", nil
+					}
+				}
+				return "", errors.New("unknown command")
+			},
+		}
+		c.context.Executor = executor
+		err := c.enableBalancerModule()
+		assert.NoError(t, err)
+	})
+}