diff --git a/pkg/apis/ceph.rook.io/v1/pool.go b/pkg/apis/ceph.rook.io/v1/pool.go index bd00b7dc6..c14d74003 100644 --- a/pkg/apis/ceph.rook.io/v1/pool.go +++ b/pkg/apis/ceph.rook.io/v1/pool.go @@ -53,7 +53,7 @@ func (p *ReplicatedSpec) IsTargetRatioEnabled() bool { func (p *CephBlockPool) ValidateCreate() error { logger.Infof("validate create cephblockpool %v", p) - err := ValidateCephBlockPool(&p.Spec) + err := ValidateCephBlockPool(p) if err != nil { return err } @@ -61,14 +61,14 @@ func (p *CephBlockPool) ValidateCreate() error { } // ValidateCephBlockPool validates specifically a CephBlockPool's spec (not just any NamedPoolSpec) -func ValidateCephBlockPool(s *NamedBlockPoolSpec) error { - if s.Name == "device_health_metrics" || s.Name == ".mgr" || s.Name == ".nfs" { - if s.IsErasureCoded() { - return errors.Errorf("invalid CephBlockPool spec: ceph built-in pool %q cannot be erasure coded", s.Name) +func ValidateCephBlockPool(p *CephBlockPool) error { + if p.Spec.Name == "device_health_metrics" || p.Spec.Name == ".mgr" || p.Spec.Name == ".nfs" { + if p.Spec.IsErasureCoded() { + return errors.Errorf("invalid CephBlockPool spec: ceph built-in pool %q cannot be erasure coded", p.Name) } } - return validatePoolSpec(s.ToNamedPoolSpec()) + return validatePoolSpec(p.ToNamedPoolSpec()) } // validate any NamedPoolSpec @@ -98,17 +98,23 @@ func validatePoolSpec(ps NamedPoolSpec) error { return nil } -func (p *NamedBlockPoolSpec) ToNamedPoolSpec() NamedPoolSpec { +func (p *CephBlockPool) ToNamedPoolSpec() NamedPoolSpec { + // If the name is not overridden in the pool spec.name, set it to the name of the pool CR + name := p.Spec.Name + if name == "" { + // Set the name of the pool CR since a name override wasn't specified in the spec + name = p.Name + } return NamedPoolSpec{ - Name: p.Name, - PoolSpec: p.PoolSpec, + Name: name, + PoolSpec: p.Spec.PoolSpec, } } func (p *CephBlockPool) ValidateUpdate(old runtime.Object) error { logger.Info("validate update cephblockpool") ocbp := old.(*CephBlockPool) - err := ValidateCephBlockPool(&p.Spec) + err := ValidateCephBlockPool(p) if err != nil { return err } diff --git a/pkg/apis/ceph.rook.io/v1/pool_test.go b/pkg/apis/ceph.rook.io/v1/pool_test.go index 95de4d917..1b37403c1 100644 --- a/pkg/apis/ceph.rook.io/v1/pool_test.go +++ b/pkg/apis/ceph.rook.io/v1/pool_test.go @@ -37,11 +37,11 @@ func TestValidatePoolSpec(t *testing.T) { }, }, } - err := validatePoolSpec(p.Spec.ToNamedPoolSpec()) + err := validatePoolSpec(p.ToNamedPoolSpec()) assert.NoError(t, err) p.Spec.ErasureCoded.DataChunks = 1 - err = validatePoolSpec(p.Spec.ToNamedPoolSpec()) + err = validatePoolSpec(p.ToNamedPoolSpec()) assert.Error(t, err) } diff --git a/pkg/operator/ceph/pool/controller.go b/pkg/operator/ceph/pool/controller.go index 9c168d436..432f52143 100644 --- a/pkg/operator/ceph/pool/controller.go +++ b/pkg/operator/ceph/pool/controller.go @@ -221,10 +221,6 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile } } - // Set the name of the Ceph pool to be reconciled internally, which - // may be different from the pool CR name. - cephBlockPool.Spec.Name = getCephName(*cephBlockPool) - // DELETE: the CR was deleted if !cephBlockPool.GetDeletionTimestamp().IsZero() { deps, err := cephBlockPoolDependents(r.context, r.clusterInfo, cephBlockPool) @@ -242,8 +238,8 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile r.recorder.Event(cephBlockPool, corev1.EventTypeNormal, string(cephv1.ReconcileStarted), "starting blockpool deletion") - logger.Infof("deleting pool %q", cephBlockPool.Name) - poolSpec := cephBlockPool.Spec.ToNamedPoolSpec() + poolSpec := cephBlockPool.ToNamedPoolSpec() + logger.Infof("deleting pool %q", poolSpec.Name) err = deletePool(r.context, clusterInfo, &poolSpec) if err != nil { return opcontroller.ImmediateRetryResult, *cephBlockPool, errors.Wrapf(err, "failed to delete pool %q. ", cephBlockPool.Name) @@ -299,7 +295,7 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile return reconcile.Result{}, *cephBlockPool, errors.Wrap(err, "failed to enable/disable stats collection for pool(s)") } - poolSpec := cephBlockPool.Spec.ToNamedPoolSpec() + poolSpec := cephBlockPool.ToNamedPoolSpec() checker := newMirrorChecker(r.context, r.client, r.clusterInfo, request.NamespacedName, &poolSpec) // ADD PEERS logger.Debug("reconciling create rbd mirror peer configuration") @@ -361,7 +357,7 @@ func (r *ReconcileCephBlockPool) reconcile(request reconcile.Request) (reconcile } func (r *ReconcileCephBlockPool) reconcileCreatePool(clusterInfo *cephclient.ClusterInfo, cephCluster *cephv1.ClusterSpec, cephBlockPool *cephv1.CephBlockPool) (reconcile.Result, error) { - poolSpec := cephBlockPool.Spec.ToNamedPoolSpec() + poolSpec := cephBlockPool.ToNamedPoolSpec() err := createPool(r.context, clusterInfo, cephCluster, &poolSpec) if err != nil { return opcontroller.ImmediateRetryResult, errors.Wrapf(err, "failed to create pool %q.", cephBlockPool.GetName()) @@ -371,19 +367,6 @@ func (r *ReconcileCephBlockPool) reconcileCreatePool(clusterInfo *cephclient.Clu return reconcile.Result{}, nil } -func getCephName(pool cephv1.CephBlockPool) string { - // If the name is not overridden in the pool spec.name, set it to the name of the pool CR - if pool.Spec.Name == "" { - return pool.Name - } - if pool.Spec.Name == pool.Name { - // The same pool name is set, nothing is being overridden - return pool.Name - } - // The pool names that can be allowed to be overridden are enforced in the crd schema - return pool.Spec.Name -} - // Create the pool func createPool(context *clusterd.Context, clusterInfo *cephclient.ClusterInfo, clusterSpec *cephv1.ClusterSpec, p *cephv1.NamedPoolSpec) error { // Set the application name to rbd by default, but override for special pools @@ -445,7 +428,7 @@ func configureRBDStats(clusterContext *clusterd.Context, clusterInfo *cephclient for _, cephBlockPool := range cephBlockPoolList.Items { if cephBlockPool.GetDeletionTimestamp() == nil && cephBlockPool.Spec.EnableRBDStats { // add to list of CephBlockPool with enableRBDStats set to true and not marked for deletion - enableStatsForPools = append(enableStatsForPools, getCephName(cephBlockPool)) + enableStatsForPools = append(enableStatsForPools, cephBlockPool.ToNamedPoolSpec().Name) } } logger.Debugf("RBD per-image IO statistics will be collected for pools: %v", enableStatsForPools) diff --git a/pkg/operator/ceph/pool/controller_test.go b/pkg/operator/ceph/pool/controller_test.go index 4035c3a1c..5e7930aa2 100644 --- a/pkg/operator/ceph/pool/controller_test.go +++ b/pkg/operator/ceph/pool/controller_test.go @@ -123,22 +123,27 @@ func TestCreatePool(t *testing.T) { func TestCephPoolName(t *testing.T) { t.Run("spec not set", func(t *testing.T) { p := cephv1.CephBlockPool{ObjectMeta: metav1.ObjectMeta{Name: "metapool"}} - name := getCephName(p) + name := p.ToNamedPoolSpec().Name assert.Equal(t, "metapool", name) }) t.Run("same name already set", func(t *testing.T) { p := cephv1.CephBlockPool{ObjectMeta: metav1.ObjectMeta{Name: "metapool"}, Spec: cephv1.NamedBlockPoolSpec{Name: "metapool"}} - name := getCephName(p) + name := p.ToNamedPoolSpec().Name assert.Equal(t, "metapool", name) }) t.Run("override device metrics", func(t *testing.T) { p := cephv1.CephBlockPool{ObjectMeta: metav1.ObjectMeta{Name: "device-metrics"}, Spec: cephv1.NamedBlockPoolSpec{Name: "device_health_metrics"}} - name := getCephName(p) + name := p.ToNamedPoolSpec().Name assert.Equal(t, "device_health_metrics", name) }) + t.Run("override mgr", func(t *testing.T) { + p := cephv1.CephBlockPool{ObjectMeta: metav1.ObjectMeta{Name: "default-mgr"}, Spec: cephv1.NamedBlockPoolSpec{Name: ".mgr"}} + name := p.ToNamedPoolSpec().Name + assert.Equal(t, ".mgr", name) + }) t.Run("override nfs", func(t *testing.T) { p := cephv1.CephBlockPool{ObjectMeta: metav1.ObjectMeta{Name: "default-nfs"}, Spec: cephv1.NamedBlockPoolSpec{Name: ".nfs"}} - name := getCephName(p) + name := p.ToNamedPoolSpec().Name assert.Equal(t, ".nfs", name) }) } diff --git a/pkg/operator/ceph/pool/validate.go b/pkg/operator/ceph/pool/validate.go index 75963a886..c607b917c 100644 --- a/pkg/operator/ceph/pool/validate.go +++ b/pkg/operator/ceph/pool/validate.go @@ -33,7 +33,7 @@ func validatePool(context *clusterd.Context, clusterInfo *cephclient.ClusterInfo return errors.New("missing namespace") } - if err := cephv1.ValidateCephBlockPool(&p.Spec); err != nil { + if err := cephv1.ValidateCephBlockPool(p); err != nil { return err }