diff --git a/pkg/operator/ceph/cluster/mgr/mgr.go b/pkg/operator/ceph/cluster/mgr/mgr.go index 11dba2a51476..9f9b5d870bd4 100644 --- a/pkg/operator/ceph/cluster/mgr/mgr.go +++ b/pkg/operator/ceph/cluster/mgr/mgr.go @@ -365,14 +365,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 if 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) } diff --git a/pkg/operator/ceph/cluster/mgr/mgr_test.go b/pkg/operator/ceph/cluster/mgr/mgr_test.go index e0d1dc7e4dff..4d302d990705 100644 --- a/pkg/operator/ceph/cluster/mgr/mgr_test.go +++ b/pkg/operator/ceph/cluster/mgr/mgr_test.go @@ -24,15 +24,14 @@ 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/apis/rook.io" "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" @@ -42,8 +41,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) { @@ -174,7 +174,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)) } } @@ -230,7 +230,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" @@ -381,3 +381,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) + }) +}