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 · 28cc6f5 · 28cc6f5
1 parent ee0759b
commit 28cc6f5
Show file tree

Hide file tree

Showing 8 changed files with 165 additions and 146 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,19 @@ func ValidatePoolSpec(context *clusterd.Context, clusterInfo *cephclient.Cluster
 
 	// validate pool compression mode if specified
 	if p.CompressionMode != "" {
-		switch p.CompressionMode {
-		case "none", "passive", "aggressive", "force":
-			break
-		default:
-			return errors.Errorf("unrecognized compression mode %q", p.CompressionMode)
+		logger.Warning("compressionMode is DEPRECATED, use Parameters instead")
+	}
+
+	// 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)
+			}
 		}
 	}