pool: allow configuration of built-in pools with non-k8s names

The built-in pools device_health_metrics and .nfs created by ceph
need to be configured for replicas, failure domain, etc.
To support this, we allow the pool to be created as a CR.
Since K8s does not support underscores in the resource names
the operator must translate this special pool name into
the name expected by ceph.

This also sets the basis for allowing filesystem data
pools to specify the desired pool name instead of requiring
a generated name.

Signed-off-by: Travis Nielsen <tnielsen@redhat.com>

Documentation/ceph-pool-crd.md

@@ -197,6 +197,11 @@ stretched) then you will have 2 replicas per datacenter where each replica ends
 * `deviceClass`: Sets up the CRUSH rule for the pool to distribute data only on the specified device class. If left empty or unspecified, the pool will use the cluster's default CRUSH root, which usually distributes data over all OSDs, regardless of their class.
 * `crushRoot`: The root in the crush map to be used by the pool. If left empty or unspecified, the default root will be used. Creating a crush hierarchy for the OSDs currently requires the Rook toolbox to run the Ceph tools described [here](http://docs.ceph.com/docs/master/rados/operations/crush-map/#modifying-the-crush-map).
 * `enableRBDStats`: Enables collecting RBD per-image IO statistics by enabling dynamic OSD performance counters. Defaults to false. For more info see the [ceph documentation](https://docs.ceph.com/docs/master/mgr/prometheus/#rbd-io-statistics).
+* `name`: The name of Ceph pools is based on the `metadata.name` of the CephBlockPool CR. Some built-in Ceph pools
+  require names that are incompatible with K8s resource names. These special pools can be configured
+  by setting this `name` to override the name of the Ceph pool that is created instead of using the `metadata.name` for the pool.
+  Two pool names are supported: `device_health_metrics` and `.nfs`. See the example
+  [device health metrics pool](https://github.com/rook/rook/blob/{{ branchName }}/deploy/examples/pool-device-health-metrics.yaml).
 
 * `parameters`: Sets any [parameters](https://docs.ceph.com/docs/master/rados/operations/pools/#set-pool-values) listed to the given pool
   * `target_size_ratio:` gives a hint (%) to Ceph in terms of expected consumption of the total cluster capacity of a given pool, for more info see the [ceph documentation](https://docs.ceph.com/docs/master/rados/operations/placement-groups/#specifying-expected-pool-size)

deploy/charts/rook-ceph/templates/resources.yaml

@@ -30,7 +30,7 @@ spec:
             metadata:
               type: object
             spec:
-              description: PoolSpec represents the spec of ceph pool
+              description: NamedPoolSpec allows a Ceph pool to be created with a non-default name
               properties:
                 compressionMode:
                   description: 'DEPRECATED: use Parameters instead, e.g., Parameters["compression_mode"] = "force" The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force) Do NOT set a default value for kubebuilder as this will override the Parameters'
@@ -110,6 +110,12 @@ spec:
                         type: object
                       type: array
                   type: object
+                name:
+                  description: The desired name of the pool if different from the default name.
+                  enum:
+                    - device_health_metrics
+                    - .nfs
+                  type: string
                 parameters:
                   additionalProperties:
                     type: string

deploy/examples/cluster-test.yaml

@@ -51,3 +51,15 @@ spec:
         timeout: 600s
   disruptionManagement:
     managePodBudgets: true
+---
+apiVersion: ceph.rook.io/v1
+kind: CephBlockPool
+metadata:
+  name: device-health-metrics
+  namespace: rook-ceph # namespace:cluster
+spec:
+  name: device_health_metrics
+  failureDomain: host
+  replicated:
+    size: 1
+    requireSafeReplicaSize: false

deploy/examples/crds.yaml

@@ -33,7 +33,7 @@ spec:
             metadata:
               type: object
             spec:
-              description: PoolSpec represents the spec of ceph pool
+              description: NamedPoolSpec allows a Ceph pool to be created with a non-default name
               properties:
                 compressionMode:
                   description: 'DEPRECATED: use Parameters instead, e.g., Parameters["compression_mode"] = "force" The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force) Do NOT set a default value for kubebuilder as this will override the Parameters'
@@ -113,6 +113,12 @@ spec:
                         type: object
                       type: array
                   type: object
+                name:
+                  description: The desired name of the pool if different from the default name.
+                  enum:
+                    - device_health_metrics
+                    - .nfs
+                  type: string
                 parameters:
                   additionalProperties:
                     type: string

deploy/examples/nfs.yaml

@@ -1,3 +1,27 @@
+#################################################################################################################
+# Create a Ceph pool with settings for replication in production environments. A minimum of 3 OSDs on
+# different hosts are required in this example.
+#  kubectl create -f pool.yaml
+#################################################################################################################
+
+apiVersion: ceph.rook.io/v1
+kind: CephBlockPool
+metadata:
+  name: builtin-nfs
+  namespace: rook-ceph # namespace:cluster
+spec:
+  # The required pool name ".nfs" cannot be specified as a K8s resource name, thus we override
+  # the pool name created in Ceph with this name property
+  name: .nfs
+  failureDomain: host
+  replicated:
+    size: 3
+    requireSafeReplicaSize: true
+  parameters:
+    compression_mode: none
+  mirroring:
+    enabled: false
+---
 apiVersion: ceph.rook.io/v1
 kind: CephNFS
 metadata:

deploy/examples/pool-device-health-metrics.yaml

@@ -0,0 +1,21 @@
+apiVersion: ceph.rook.io/v1
+kind: CephBlockPool
+metadata:
+  # If the built-in Ceph pool for health metrics needs to be configured with alternate
+  # settings, create this pool with any of the pool properties. Create this pool immediately
+  # with the cluster CR, or else some properties may not be applied when Ceph creates the
+  # pool by default.
+  name: device-health-metrics
+  namespace: rook-ceph # namespace:cluster
+spec:
+  # The required pool name with underscores cannot be specified as a K8s resource name, thus we override
+  # the pool name created in Ceph with this name property.
+  name: device_health_metrics
+  failureDomain: host
+  replicated:
+    size: 3
+    requireSafeReplicaSize: true
+  parameters:
+    compression_mode: none
+  mirroring:
+    enabled: false

pkg/apis/ceph.rook.io/v1/pool.go

@@ -60,7 +60,7 @@ func (p *CephBlockPool) ValidateCreate() error {
 	return nil
 }
 
-func validatePoolSpec(ps PoolSpec) error {
+func validatePoolSpec(ps NamedPoolSpec) error {
 	// Checks if either ErasureCoded or Replicated fields are set
 	if ps.ErasureCoded.CodingChunks <= 0 && ps.ErasureCoded.DataChunks <= 0 && ps.Replicated.TargetSizeRatio <= 0 && ps.Replicated.Size <= 0 {
 		return errors.New("invalid create: either of erasurecoded or replicated fields should be set")

pkg/apis/ceph.rook.io/v1/pool_test.go

@@ -28,10 +28,12 @@ func TestValidatePoolSpec(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "ec-pool",
 		},
-		Spec: PoolSpec{
-			ErasureCoded: ErasureCodedSpec{
-				CodingChunks: 1,
-				DataChunks:   2,
+		Spec: NamedPoolSpec{
+			PoolSpec: PoolSpec{
+				ErasureCoded: ErasureCodedSpec{
+					CodingChunks: 1,
+					DataChunks:   2,
+				},
 			},
 		},
 	}
@@ -48,8 +50,10 @@ func TestCephBlockPoolValidateUpdate(t *testing.T) {
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "ec-pool",
 		},
-		Spec: PoolSpec{
-			Replicated: ReplicatedSpec{RequireSafeReplicaSize: true, Size: 3},
+		Spec: NamedPoolSpec{
+			PoolSpec: PoolSpec{
+				Replicated: ReplicatedSpec{RequireSafeReplicaSize: true, Size: 3},
+			},
 		},
 	}
 	up := p.DeepCopy()

pkg/apis/ceph.rook.io/v1/types.go

@@ -565,7 +565,7 @@ type CrashCollectorSpec struct {
 type CephBlockPool struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata"`
-	Spec              PoolSpec `json:"spec"`
+	Spec              NamedPoolSpec `json:"spec"`
 	// +kubebuilder:pruning:PreserveUnknownFields
 	Status *CephBlockPoolStatus `json:"status,omitempty"`
 }
@@ -585,6 +585,17 @@ const (
 	DefaultCRUSHRoot = "default"
 )
 
+// NamedPoolSpec allows a Ceph pool to be created with a non-default name
+type NamedPoolSpec struct {
+	// The desired name of the pool if different from the default name.
+	// +kubebuilder:validation:Enum=device_health_metrics;.nfs
+	// +optional
+	Name string `json:"name,omitempty"`
+
+	// The core pool configuration
+	PoolSpec `json:",inline"`
+}
+
 // PoolSpec represents the spec of ceph pool
 type PoolSpec struct {
 	// The failure domain: osd/host/(region or zone if available) - technically also any type in the crush map

pkg/daemon/ceph/client/crush_rule_test.go

@@ -32,27 +32,27 @@ func TestBuildStretchClusterCrushRule(t *testing.T) {
 	err := json.Unmarshal([]byte(testCrushMap), &crushMap)
 	assert.NoError(t, err)
 
-	pool := &cephv1.PoolSpec{
+	pool := cephv1.PoolSpec{
 		FailureDomain: "datacenter",
 		CrushRoot:     cephv1.DefaultCRUSHRoot,
 		Replicated: cephv1.ReplicatedSpec{
 			ReplicasPerFailureDomain: 2,
 		},
 	}
 
-	rule := buildTwoStepCrushRule(crushMap, "stretched", *pool)
+	rule := buildTwoStepCrushRule(crushMap, "stretched", pool)
 	assert.Equal(t, 2, rule.ID)
 }
 
 func TestBuildCrushSteps(t *testing.T) {
-	pool := &cephv1.PoolSpec{
+	pool := cephv1.PoolSpec{
 		FailureDomain: "datacenter",
 		CrushRoot:     cephv1.DefaultCRUSHRoot,
 		Replicated: cephv1.ReplicatedSpec{
 			ReplicasPerFailureDomain: 2,
 		},
 	}
-	steps := buildTwoStepCrushSteps(*pool)
+	steps := buildTwoStepCrushSteps(pool)
 	assert.Equal(t, 4, len(steps))
 	assert.Equal(t, cephv1.DefaultCRUSHRoot, steps[0].ItemName)
 	assert.Equal(t, "datacenter", steps[1].Type)

pkg/daemon/ceph/client/mirror.go

@@ -104,17 +104,17 @@ func CreateRBDMirrorBootstrapPeer(context *clusterd.Context, clusterInfo *Cluste
 }
 
 // enablePoolMirroring turns on mirroring on that pool by specifying the mirroring type
-func enablePoolMirroring(context *clusterd.Context, clusterInfo *ClusterInfo, pool cephv1.PoolSpec, poolName string) error {
-	logger.Infof("enabling mirroring type %q for pool %q", pool.Mirroring.Mode, poolName)
+func enablePoolMirroring(context *clusterd.Context, clusterInfo *ClusterInfo, pool cephv1.NamedPoolSpec) error {
+	logger.Infof("enabling mirroring type %q for pool %q", pool.Mirroring.Mode, pool.Name)
 
 	// Build command
-	args := []string{"mirror", "pool", "enable", poolName, pool.Mirroring.Mode}
+	args := []string{"mirror", "pool", "enable", pool.Name, pool.Mirroring.Mode}
 	cmd := NewRBDCommand(context, clusterInfo, args)
 
 	// Run command
 	output, err := cmd.Run()
 	if err != nil {
-		return errors.Wrapf(err, "failed to enable mirroring type %q for pool %q. %s", pool.Mirroring.Mode, poolName, output)
+		return errors.Wrapf(err, "failed to enable mirroring type %q for pool %q. %s", pool.Mirroring.Mode, pool.Name, output)
 	}
 
 	return nil
@@ -246,17 +246,17 @@ func removeSnapshotSchedule(context *clusterd.Context, clusterInfo *ClusterInfo,
 	return nil
 }
 
-func enableSnapshotSchedules(context *clusterd.Context, clusterInfo *ClusterInfo, poolSpec cephv1.PoolSpec, poolName string) error {
+func enableSnapshotSchedules(context *clusterd.Context, clusterInfo *ClusterInfo, pool cephv1.NamedPoolSpec) error {
 	logger.Info("resetting current snapshot schedules")
 	// Reset any existing schedules
-	err := removeSnapshotSchedules(context, clusterInfo, poolSpec, poolName)
+	err := removeSnapshotSchedules(context, clusterInfo, pool)
 	if err != nil {
 		logger.Errorf("failed to remove snapshot schedules. %v", err)
 	}
 
 	// Enable all the snap schedules
-	for _, snapSchedule := range poolSpec.Mirroring.SnapshotSchedules {
-		err := enableSnapshotSchedule(context, clusterInfo, snapSchedule, poolName)
+	for _, snapSchedule := range pool.Mirroring.SnapshotSchedules {
+		err := enableSnapshotSchedule(context, clusterInfo, snapSchedule, pool.Name)
 		if err != nil {
 			return errors.Wrap(err, "failed to enable snapshot schedule")
 		}
@@ -266,16 +266,16 @@ func enableSnapshotSchedules(context *clusterd.Context, clusterInfo *ClusterInfo
 }
 
 // removeSnapshotSchedules removes all the existing snapshot schedules
-func removeSnapshotSchedules(context *clusterd.Context, clusterInfo *ClusterInfo, poolSpec cephv1.PoolSpec, poolName string) error {
+func removeSnapshotSchedules(context *clusterd.Context, clusterInfo *ClusterInfo, pool cephv1.NamedPoolSpec) error {
 	// Get the list of existing snapshot schedule
-	existingSnapshotSchedules, err := listSnapshotSchedules(context, clusterInfo, poolName)
+	existingSnapshotSchedules, err := listSnapshotSchedules(context, clusterInfo, pool.Name)
 	if err != nil {
 		return errors.Wrap(err, "failed to list snapshot schedule(s)")
 	}
 
 	// Remove each schedule
 	for _, existingSnapshotSchedule := range existingSnapshotSchedules {
-		err := removeSnapshotSchedule(context, clusterInfo, existingSnapshotSchedule, poolName)
+		err := removeSnapshotSchedule(context, clusterInfo, existingSnapshotSchedule, pool.Name)
 		if err != nil {
 			return errors.Wrapf(err, "failed to remove snapshot schedule %v", existingSnapshotSchedule)
 		}

pkg/daemon/ceph/client/mirror_test.go

@@ -58,22 +58,26 @@ func TestCreateRBDMirrorBootstrapPeer(t *testing.T) {
 	assert.Equal(t, bootstrapPeerToken, string(token))
 }
 func TestEnablePoolMirroring(t *testing.T) {
-	pool := "pool-test"
-	poolSpec := cephv1.PoolSpec{Mirroring: cephv1.MirroringSpec{Mode: "image"}}
+	pool := cephv1.NamedPoolSpec{
+		Name: "pool-test",
+		PoolSpec: cephv1.PoolSpec{
+			Mirroring: cephv1.MirroringSpec{Mode: "image"},
+		},
+	}
 	executor := &exectest.MockExecutor{}
 	executor.MockExecuteCommandWithOutput = func(command string, args ...string) (string, error) {
 		if args[0] == "mirror" {
 			assert.Equal(t, "pool", args[1])
 			assert.Equal(t, "enable", args[2])
-			assert.Equal(t, pool, args[3])
-			assert.Equal(t, poolSpec.Mirroring.Mode, args[4])
+			assert.Equal(t, pool.Name, args[3])
+			assert.Equal(t, pool.Mirroring.Mode, args[4])
 			return "", nil
 		}
 		return "", errors.New("unknown command")
 	}
 	context := &clusterd.Context{Executor: executor}
 
-	err := enablePoolMirroring(context, AdminTestClusterInfo("mycluster"), poolSpec, pool)
+	err := enablePoolMirroring(context, AdminTestClusterInfo("mycluster"), pool)
 	assert.NoError(t, err)
 }
 
@@ -279,7 +283,6 @@ func TestRemoveSnapshotSchedule(t *testing.T) {
 }
 
 func TestRemoveSnapshotSchedules(t *testing.T) {
-	pool := "pool-test"
 	interval := "24h"
 	startTime := "14:00:00-05:00"
 	executor := &exectest.MockExecutor{}
@@ -297,8 +300,17 @@ func TestRemoveSnapshotSchedules(t *testing.T) {
 	}
 
 	context := &clusterd.Context{Executor: executor}
-	poolSpec := &cephv1.PoolSpec{Mirroring: cephv1.MirroringSpec{SnapshotSchedules: []cephv1.SnapshotScheduleSpec{{Interval: interval, StartTime: startTime}}}}
-	err := removeSnapshotSchedules(context, AdminTestClusterInfo("mycluster"), *poolSpec, pool)
+	pool := cephv1.NamedPoolSpec{
+		Name: "pool-test",
+		PoolSpec: cephv1.PoolSpec{
+			Mirroring: cephv1.MirroringSpec{
+				SnapshotSchedules: []cephv1.SnapshotScheduleSpec{
+					{Interval: interval, StartTime: startTime},
+				},
+			},
+		},
+	}
+	err := removeSnapshotSchedules(context, AdminTestClusterInfo("mycluster"), pool)
 	assert.NoError(t, err)
 }