ceph: remove default value for pool compression

Using a default value for CompressionMode to none effectively overrides any values for Parameters. It is deprecated but still takes precedence. Which means that in its previous form, Parameters was always ignored since CompressionMode was always set to none when empty. Signed-off-by: Sébastien Han <seb@redhat.com>
rook · Oct 13, 2021 · f73241a · f73241a
1 parent ee0759b
commit f73241a
Show file tree

Hide file tree

Showing 8 changed files with 195 additions and 141 deletions.
diff --git a/cluster/charts/rook-ceph/templates/resources.yaml b/cluster/charts/rook-ceph/templates/resources.yaml
@@ -33,8 +33,7 @@ spec:
               description: PoolSpec represents the spec of ceph pool
               properties:
                 compressionMode:
-                  default: none
-                  description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                  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'
                   enum:
                     - none
                     - passive
@@ -4457,8 +4456,7 @@ spec:
                     description: PoolSpec represents the spec of ceph pool
                     properties:
                       compressionMode:
-                        default: none
-                        description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                        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'
                         enum:
                           - none
                           - passive
@@ -4625,8 +4623,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive
@@ -6387,8 +6384,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive
@@ -7326,8 +7322,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive
@@ -7822,8 +7817,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive
@@ -7988,8 +7982,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive

diff --git a/cluster/examples/kubernetes/ceph/crds.yaml b/cluster/examples/kubernetes/ceph/crds.yaml
@@ -36,8 +36,7 @@ spec:
               description: PoolSpec represents the spec of ceph pool
               properties:
                 compressionMode:
-                  default: none
-                  description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                  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'
                   enum:
                     - none
                     - passive
@@ -4456,8 +4455,7 @@ spec:
                     description: PoolSpec represents the spec of ceph pool
                     properties:
                       compressionMode:
-                        default: none
-                        description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                        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'
                         enum:
                           - none
                           - passive
@@ -4624,8 +4622,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive
@@ -6383,8 +6380,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive
@@ -7322,8 +7318,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive
@@ -7815,8 +7810,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive
@@ -7981,8 +7975,7 @@ spec:
                   nullable: true
                   properties:
                     compressionMode:
-                      default: none
-                      description: 'The inline compression mode in Bluestore OSD to set to (options are: none, passive, aggressive, force)'
+                      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'
                       enum:
                         - none
                         - passive

diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go
@@ -601,9 +601,10 @@ type PoolSpec struct {
 	// +nullable
 	DeviceClass string `json:"deviceClass,omitempty"`
 
+	// 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)
 	// +kubebuilder:validation:Enum=none;passive;aggressive;force;""
-	// +kubebuilder:default=none
+	// Do NOT set a default value for kubebuilder as this will override the Parameters
 	// +optional
 	// +nullable
 	CompressionMode string `json:"compressionMode,omitempty"`

diff --git a/pkg/daemon/ceph/client/pool.go b/pkg/daemon/ceph/client/pool.go
@@ -34,7 +34,7 @@ const (
 	confirmFlag             = "--yes-i-really-mean-it"
 	reallyConfirmFlag       = "--yes-i-really-really-mean-it"
 	targetSizeRatioProperty = "target_size_ratio"
-	compressionModeProperty = "compression_mode"
+	CompressionModeProperty = "compression_mode"
 	PgAutoscaleModeProperty = "pg_autoscale_mode"
 	PgAutoscaleModeOn       = "on"
 )
@@ -252,7 +252,7 @@ func setCommonPoolProperties(context *clusterd.Context, clusterInfo *ClusterInfo
 	}
 
 	if pool.IsCompressionEnabled() {
-		pool.Parameters[compressionModeProperty] = pool.CompressionMode
+		pool.Parameters[CompressionModeProperty] = pool.CompressionMode
 	}
 
 	// Apply properties

diff --git a/pkg/operator/ceph/pool/validate.go b/pkg/operator/ceph/pool/validate.go
@@ -139,11 +139,25 @@ func ValidatePoolSpec(context *clusterd.Context, clusterInfo *cephclient.Cluster
 
 	// validate pool compression mode if specified
 	if p.CompressionMode != "" {
+		logger.Warning("compressionMode is DEPRECATED, use Parameters instead")
 		switch p.CompressionMode {
 		case "none", "passive", "aggressive", "force":
 			break
 		default:
-			return errors.Errorf("unrecognized compression mode %q", p.CompressionMode)
+			return errors.Errorf("failed to validate pool spec unknown compression mode %q", p.CompressionMode)
+		}
+	}
+
+	// Test the same for Parameters
+	if p.Parameters != nil {
+		compression, ok := p.Parameters[client.CompressionModeProperty]
+		if ok && compression != "" {
+			switch compression {
+			case "none", "passive", "aggressive", "force":
+				break
+			default:
+				return errors.Errorf("failed to validate pool spec unknown compression mode %q", compression)
+			}
 		}
 	}