From eb17bb490f99fcafbbf6c3405e5d0ed241c43742 Mon Sep 17 00:00:00 2001 From: Artem Torubarov Date: Mon, 2 Mar 2026 16:16:08 +0100 Subject: [PATCH] rgw: not update shared pools if prev pool is not empty Signed-off-by: Artem Torubarov (cherry picked from commit 845293c3cd779d0f42bd50a46354aa3af0a34a85) --- .../charts/rook-ceph/templates/resources.yaml | 40 ++++++ deploy/examples/crds.yaml | 40 ++++++ pkg/apis/ceph.rook.io/v1/types.go | 13 ++ pkg/daemon/ceph/client/rados.go | 17 +++ pkg/operator/ceph/object/objectstore.go | 123 +++++++++++++----- pkg/operator/ceph/object/rgw.go | 4 +- pkg/operator/ceph/object/shared_pools_test.go | 20 ++- 7 files changed, 222 insertions(+), 35 deletions(-) diff --git a/deploy/charts/rook-ceph/templates/resources.yaml b/deploy/charts/rook-ceph/templates/resources.yaml index a6ffe9f4c..5774f04a6 100644 --- a/deploy/charts/rook-ceph/templates/resources.yaml +++ b/deploy/charts/rook-ceph/templates/resources.yaml @@ -14636,11 +14636,19 @@ spec: description: |- The data pool used to store ObjectStore data that cannot use erasure coding (ex: multi-part uploads). If dataPoolName is not erasure coded, then there is no need for dataNonECPoolName. + maxLength: 128 type: string + x-kubernetes-validations: + - message: pool placement non-EC pool is immutable + rule: self == oldSelf dataPoolName: description: The data pool used to store ObjectStore objects data. + maxLength: 128 minLength: 1 type: string + x-kubernetes-validations: + - message: pool placement data pool is immutable + rule: self == oldSelf default: description: |- Sets given placement as default. Only one placement in the list can be marked as default. @@ -14648,10 +14656,15 @@ spec: type: boolean metadataPoolName: description: The metadata pool used to store ObjectStore bucket index. + maxLength: 128 minLength: 1 type: string + x-kubernetes-validations: + - message: pool placement metadata pool is immutable + rule: self == oldSelf name: description: Pool placement name. Name can be arbitrary. Placement with name "default" will be used as default. + maxLength: 63 minLength: 1 pattern: ^[a-zA-Z0-9._/-]+$ type: string @@ -14664,6 +14677,7 @@ spec: properties: dataPoolName: description: DataPoolName is the data pool used to store ObjectStore objects data. + maxLength: 128 minLength: 1 type: string name: @@ -14673,6 +14687,7 @@ spec: one of the valid x-amz-storage-class values for better compatibility: REDUCED_REDUNDANCY | STANDARD_IA | ONEZONE_IA | INTELLIGENT_TIERING | GLACIER | DEEP_ARCHIVE | OUTPOSTS | GLACIER_IR | SNOW | EXPRESS_ONEZONE See AWS docs: https://aws.amazon.com/de/s3/storage-classes/ + maxLength: 64 minLength: 1 pattern: ^[a-zA-Z0-9._/-]+$ type: string @@ -14680,13 +14695,18 @@ spec: - dataPoolName - name type: object + maxItems: 10 type: array required: - dataPoolName - metadataPoolName - name type: object + maxItems: 10 type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map preserveRadosNamespaceDataOnDelete: description: Whether the RADOS namespaces should be preserved on deletion of the object store type: boolean @@ -15716,11 +15736,19 @@ spec: description: |- The data pool used to store ObjectStore data that cannot use erasure coding (ex: multi-part uploads). If dataPoolName is not erasure coded, then there is no need for dataNonECPoolName. + maxLength: 128 type: string + x-kubernetes-validations: + - message: pool placement non-EC pool is immutable + rule: self == oldSelf dataPoolName: description: The data pool used to store ObjectStore objects data. + maxLength: 128 minLength: 1 type: string + x-kubernetes-validations: + - message: pool placement data pool is immutable + rule: self == oldSelf default: description: |- Sets given placement as default. Only one placement in the list can be marked as default. @@ -15728,10 +15756,15 @@ spec: type: boolean metadataPoolName: description: The metadata pool used to store ObjectStore bucket index. + maxLength: 128 minLength: 1 type: string + x-kubernetes-validations: + - message: pool placement metadata pool is immutable + rule: self == oldSelf name: description: Pool placement name. Name can be arbitrary. Placement with name "default" will be used as default. + maxLength: 63 minLength: 1 pattern: ^[a-zA-Z0-9._/-]+$ type: string @@ -15744,6 +15777,7 @@ spec: properties: dataPoolName: description: DataPoolName is the data pool used to store ObjectStore objects data. + maxLength: 128 minLength: 1 type: string name: @@ -15753,6 +15787,7 @@ spec: one of the valid x-amz-storage-class values for better compatibility: REDUCED_REDUNDANCY | STANDARD_IA | ONEZONE_IA | INTELLIGENT_TIERING | GLACIER | DEEP_ARCHIVE | OUTPOSTS | GLACIER_IR | SNOW | EXPRESS_ONEZONE See AWS docs: https://aws.amazon.com/de/s3/storage-classes/ + maxLength: 64 minLength: 1 pattern: ^[a-zA-Z0-9._/-]+$ type: string @@ -15760,13 +15795,18 @@ spec: - dataPoolName - name type: object + maxItems: 10 type: array required: - dataPoolName - metadataPoolName - name type: object + maxItems: 10 type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map preserveRadosNamespaceDataOnDelete: description: Whether the RADOS namespaces should be preserved on deletion of the object store type: boolean diff --git a/deploy/examples/crds.yaml b/deploy/examples/crds.yaml index 53a745702..ecd9fb2b9 100644 --- a/deploy/examples/crds.yaml +++ b/deploy/examples/crds.yaml @@ -14626,11 +14626,19 @@ spec: description: |- The data pool used to store ObjectStore data that cannot use erasure coding (ex: multi-part uploads). If dataPoolName is not erasure coded, then there is no need for dataNonECPoolName. + maxLength: 128 type: string + x-kubernetes-validations: + - message: pool placement non-EC pool is immutable + rule: self == oldSelf dataPoolName: description: The data pool used to store ObjectStore objects data. + maxLength: 128 minLength: 1 type: string + x-kubernetes-validations: + - message: pool placement data pool is immutable + rule: self == oldSelf default: description: |- Sets given placement as default. Only one placement in the list can be marked as default. @@ -14638,10 +14646,15 @@ spec: type: boolean metadataPoolName: description: The metadata pool used to store ObjectStore bucket index. + maxLength: 128 minLength: 1 type: string + x-kubernetes-validations: + - message: pool placement metadata pool is immutable + rule: self == oldSelf name: description: Pool placement name. Name can be arbitrary. Placement with name "default" will be used as default. + maxLength: 63 minLength: 1 pattern: ^[a-zA-Z0-9._/-]+$ type: string @@ -14654,6 +14667,7 @@ spec: properties: dataPoolName: description: DataPoolName is the data pool used to store ObjectStore objects data. + maxLength: 128 minLength: 1 type: string name: @@ -14663,6 +14677,7 @@ spec: one of the valid x-amz-storage-class values for better compatibility: REDUCED_REDUNDANCY | STANDARD_IA | ONEZONE_IA | INTELLIGENT_TIERING | GLACIER | DEEP_ARCHIVE | OUTPOSTS | GLACIER_IR | SNOW | EXPRESS_ONEZONE See AWS docs: https://aws.amazon.com/de/s3/storage-classes/ + maxLength: 64 minLength: 1 pattern: ^[a-zA-Z0-9._/-]+$ type: string @@ -14670,13 +14685,18 @@ spec: - dataPoolName - name type: object + maxItems: 10 type: array required: - dataPoolName - metadataPoolName - name type: object + maxItems: 10 type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map preserveRadosNamespaceDataOnDelete: description: Whether the RADOS namespaces should be preserved on deletion of the object store type: boolean @@ -15703,11 +15723,19 @@ spec: description: |- The data pool used to store ObjectStore data that cannot use erasure coding (ex: multi-part uploads). If dataPoolName is not erasure coded, then there is no need for dataNonECPoolName. + maxLength: 128 type: string + x-kubernetes-validations: + - message: pool placement non-EC pool is immutable + rule: self == oldSelf dataPoolName: description: The data pool used to store ObjectStore objects data. + maxLength: 128 minLength: 1 type: string + x-kubernetes-validations: + - message: pool placement data pool is immutable + rule: self == oldSelf default: description: |- Sets given placement as default. Only one placement in the list can be marked as default. @@ -15715,10 +15743,15 @@ spec: type: boolean metadataPoolName: description: The metadata pool used to store ObjectStore bucket index. + maxLength: 128 minLength: 1 type: string + x-kubernetes-validations: + - message: pool placement metadata pool is immutable + rule: self == oldSelf name: description: Pool placement name. Name can be arbitrary. Placement with name "default" will be used as default. + maxLength: 63 minLength: 1 pattern: ^[a-zA-Z0-9._/-]+$ type: string @@ -15731,6 +15764,7 @@ spec: properties: dataPoolName: description: DataPoolName is the data pool used to store ObjectStore objects data. + maxLength: 128 minLength: 1 type: string name: @@ -15740,6 +15774,7 @@ spec: one of the valid x-amz-storage-class values for better compatibility: REDUCED_REDUNDANCY | STANDARD_IA | ONEZONE_IA | INTELLIGENT_TIERING | GLACIER | DEEP_ARCHIVE | OUTPOSTS | GLACIER_IR | SNOW | EXPRESS_ONEZONE See AWS docs: https://aws.amazon.com/de/s3/storage-classes/ + maxLength: 64 minLength: 1 pattern: ^[a-zA-Z0-9._/-]+$ type: string @@ -15747,13 +15782,18 @@ spec: - dataPoolName - name type: object + maxItems: 10 type: array required: - dataPoolName - metadataPoolName - name type: object + maxItems: 10 type: array + x-kubernetes-list-map-keys: + - name + x-kubernetes-list-type: map preserveRadosNamespaceDataOnDelete: description: Whether the RADOS namespaces should be preserved on deletion of the object store type: boolean diff --git a/pkg/apis/ceph.rook.io/v1/types.go b/pkg/apis/ceph.rook.io/v1/types.go index cd11d39c1..2ac91ad43 100755 --- a/pkg/apis/ceph.rook.io/v1/types.go +++ b/pkg/apis/ceph.rook.io/v1/types.go @@ -1798,6 +1798,9 @@ type ObjectSharedPoolsSpec struct { // is provided during bucket creation. // If default placement is not provided, spec.sharedPools.dataPoolName and spec.sharedPools.MetadataPoolName will be used as default pools. // If spec.sharedPools are also empty, then RGW pools (spec.dataPool and spec.metadataPool) will be used as defaults. + // +listType=map + // +listMapKey=name + // +kubebuilder:validation:MaxItems=10 // +optional PoolPlacements []PoolPlacementSpec `json:"poolPlacements,omitempty"` } @@ -1806,6 +1809,7 @@ type PoolPlacementSpec struct { // Pool placement name. Name can be arbitrary. Placement with name "default" will be used as default. // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=63 // +kubebuilder:validation:Pattern=`^[a-zA-Z0-9._/-]+$` Name string `json:"name"` @@ -1817,21 +1821,28 @@ type PoolPlacementSpec struct { // The metadata pool used to store ObjectStore bucket index. // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=128 + // +kubebuilder:validation:XValidation:message="pool placement metadata pool is immutable",rule="self == oldSelf" MetadataPoolName string `json:"metadataPoolName"` // The data pool used to store ObjectStore objects data. // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=128 + // +kubebuilder:validation:XValidation:message="pool placement data pool is immutable",rule="self == oldSelf" DataPoolName string `json:"dataPoolName"` // The data pool used to store ObjectStore data that cannot use erasure coding (ex: multi-part uploads). // If dataPoolName is not erasure coded, then there is no need for dataNonECPoolName. + // +kubebuilder:validation:MaxLength=128 + // +kubebuilder:validation:XValidation:message="pool placement non-EC pool is immutable",rule="self == oldSelf" // +optional DataNonECPoolName string `json:"dataNonECPoolName,omitempty"` // StorageClasses can be selected by user to override dataPoolName during object creation. // Each placement has default STANDARD StorageClass pointing to dataPoolName. // This list allows defining additional StorageClasses on top of default STANDARD storage class. + // +kubebuilder:validation:MaxItems=10 // +optional StorageClasses []PlacementStorageClassSpec `json:"storageClasses,omitempty"` } @@ -1844,12 +1855,14 @@ type PlacementStorageClassSpec struct { // See AWS docs: https://aws.amazon.com/de/s3/storage-classes/ // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=64 // +kubebuilder:validation:Pattern=`^[a-zA-Z0-9._/-]+$` Name string `json:"name"` // DataPoolName is the data pool used to store ObjectStore objects data. // +kubebuilder:validation:Required // +kubebuilder:validation:MinLength=1 + // +kubebuilder:validation:MaxLength=128 DataPoolName string `json:"dataPoolName"` } diff --git a/pkg/daemon/ceph/client/rados.go b/pkg/daemon/ceph/client/rados.go index f7b4c4ba9..e8fe36749 100644 --- a/pkg/daemon/ceph/client/rados.go +++ b/pkg/daemon/ceph/client/rados.go @@ -21,6 +21,7 @@ import ( "encoding/json" "fmt" "math" + "strings" "time" "github.com/pkg/errors" @@ -174,6 +175,22 @@ func findLockerWithCookie(lockers []radosLockerInfo, cookie string) *radosLocker return nil } +func RadosNamespaceHasObjects(context *clusterd.Context, clusterInfo *ClusterInfo, pool, namespace string) (bool, error) { + radosArgs := []string{ + "--pool", pool, + "--namespace", namespace, + "ls", + } + command, args := FinalizeCephCommandArgs(RadosTool, clusterInfo, radosArgs, context.ConfigDir) + // Pipe through "head -c 1" so only 1 byte is buffered regardless of object count. + shellCmd := command + " " + strings.Join(args, " ") + " 2>/dev/null | head -c 1" + output, err := context.Executor.ExecuteCommandWithTimeout(exec.CephCommandsTimeout, "sh", "-c", shellCmd) + if err != nil { + return false, errors.Wrapf(err, "failed to check for objects in rados://%s/%s", pool, namespace) + } + return len(output) > 0, nil +} + // RadosRemoveObject idempotently removes a rados object from the given pool and namespace. func RadosRemoveObject( context *clusterd.Context, clusterInfo *ClusterInfo, diff --git a/pkg/operator/ceph/object/objectstore.go b/pkg/operator/ceph/object/objectstore.go index 720a73da3..2c12fa48b 100644 --- a/pkg/operator/ceph/object/objectstore.go +++ b/pkg/operator/ceph/object/objectstore.go @@ -72,6 +72,30 @@ var ( // An user with system privileges for dashboard service DashboardUser = "dashboard-admin" + + zonePoolNSSuffix = map[string]string{ + "domain_root": ".meta.root", + "control_pool": ".control", + "gc_pool": ".log.gc", + "lc_pool": ".log.lc", + "log_pool": ".log", + "intent_log_pool": ".log.intent", + "usage_log_pool": ".log.usage", + "roles_pool": ".meta.roles", + "reshard_pool": ".log.reshard", + "user_keys_pool": ".meta.users.keys", + "user_email_pool": ".meta.users.email", + "user_swift_pool": ".meta.users.swift", + "user_uid_pool": ".meta.users.uid", + "otp_pool": ".otp", + "notif_pool": ".log.notif", + "topics_pool": ".meta.topics", // introduced in Ceph v19 + "account_pool": ".meta.account", // introduced in Ceph v19 + "group_pool": ".meta.group", // introduced in Ceph v19 + "restore_pool": ".log.restore", // introduced in Ceph v20 + "bucket_logging_pool": ".log.bucket-logging", // introduced in Ceph v20 + "dedup_pool": ".dedup", // introduced in Ceph v20 + } ) type idType struct { @@ -909,35 +933,28 @@ func adjustZoneDefaultPools(objContext *Context, zone map[string]interface{}, sp // add zone namespace to metadata pool to safely share accorss rgw instances or zones. // in non-multisite case zone name equals to rgw instance name defaultMetaPool = defaultMetaPool + ":" + name - zonePoolNSSuffix := map[string]string{ - "domain_root": ".meta.root", - "control_pool": ".control", - "gc_pool": ".log.gc", - "lc_pool": ".log.lc", - "log_pool": ".log", - "intent_log_pool": ".log.intent", - "usage_log_pool": ".log.usage", - "roles_pool": ".meta.roles", - "reshard_pool": ".log.reshard", - "user_keys_pool": ".meta.users.keys", - "user_email_pool": ".meta.users.email", - "user_swift_pool": ".meta.users.swift", - "user_uid_pool": ".meta.users.uid", - "otp_pool": ".otp", - "notif_pool": ".log.notif", - "topics_pool": ".meta.topics", // introduced in Ceph v19 - "account_pool": ".meta.account", // introduced in Ceph v19 - "group_pool": ".meta.group", // introduced in Ceph v19 - "restore_pool": ".log.restore", // introduced in Ceph v20 - "bucket_logging_pool": ".log.bucket-logging", // introduced in Ceph v20 - "dedup_pool": ".dedup", // introduced in Ceph v20 - } for pool, nsSuffix := range zonePoolNSSuffix { // replace rgw internal index pools with namespaced metadata pool namespacedPool := defaultMetaPool + nsSuffix + + // check if old pool has data BEFORE overwriting the zone property + prev, _ := zone[pool].(string) + if prev != "" && prev != namespacedPool { + empty, err := checkPoolIsEmpty(objContext, prev) + if err != nil { + return nil, fmt.Errorf("zone pool field %q: unable to check old pool %q: %w", pool, prev, err) + } + if !empty { + log.NamedWarning(objContext.NsName(), logger, + "zone pool field %q is being remapped from %q which still contains data", pool, prev) + continue + } + } + prev, err := updateObjProperty(zone, namespacedPool, pool) if err != nil { log.NamedInfo(objContext.NsName(), logger, "unable to apply rados namespace to shared pool: %v", err) + continue } if namespacedPool != prev { log.NamedDebug(objContext.NsName(), logger, "update shared pool %s for zone %s: %s -> %s", pool, name, prev, namespacedPool) @@ -962,6 +979,45 @@ func adjustZoneDefaultPools(objContext *Context, zone map[string]interface{}, sp return zone, nil } +func ZoneJsonPoolKeys() sets.Set[string] { + s := sets.New[string]() + for k := range zonePoolNSSuffix { + s.Insert(k) + } + return s +} + +func checkPoolIsEmpty(objContext *Context, name string) (bool, error) { + poolName, namespace, isNamespaced := strings.Cut(name, ":") + + poolExists, err := cephclient.IsPoolPresent(objContext.Context, objContext.clusterInfo, poolName) + if err != nil { + return false, fmt.Errorf("failed to check if pool %q exists: %w", poolName, err) + } + if !poolExists { + return true, nil + } + + if isNamespaced { + hasObjects, err := cephclient.RadosNamespaceHasObjects(objContext.Context, objContext.clusterInfo, poolName, namespace) + if err != nil { + return false, fmt.Errorf("failed to check pool %q namespace %q: %w", poolName, namespace, err) + } + return !hasObjects, nil + } + + stats, err := cephclient.GetPoolStats(objContext.Context, objContext.clusterInfo) + if err != nil { + return false, fmt.Errorf("failed to get pool stats: %w", err) + } + for _, p := range stats.Pools { + if p.Name == poolName && p.Stats.Objects > 0 { + return false, nil + } + } + return true, nil +} + // configurePoolsConcurrently checks if operator pod resources are set or not func configurePoolsConcurrently() bool { // if operator resources are specified return false as it will lead to operator pod killed due to resource limit @@ -1187,20 +1243,25 @@ func enableRGWDashboard(context *Context) error { func disableRGWDashboard(context *Context) { log.NamedInfo(context.NsName(), logger, "disabling the dashboard api user and secret key") - _, _, err := GetUser(context, DashboardUser) - if err != nil { - log.NamedInfo(context.NsName(), logger, "unable to fetch the user %q details from this objectstore %q", DashboardUser, context.Name) - } else { - log.NamedInfo(context.NsName(), logger, "deleting rgw dashboard user") - _, err = DeleteUser(context, DashboardUser) + // GetUser and DeleteUser are radosgw-admin non-raw commands that trigger full RGW + // initialization and auto-create pools. Guard them so we don't create ghost pools + // when called during object store deletion after pools are already removed. + if poolsExistForNonRawOps(context) { + _, _, err := GetUser(context, DashboardUser) if err != nil { - log.NamedWarning(context.NsName(), logger, "failed to delete ceph user %q. %v", DashboardUser, err) + log.NamedInfo(context.NsName(), logger, "unable to fetch the user %q details from this objectstore %q", DashboardUser, context.Name) + } else { + log.NamedInfo(context.NsName(), logger, "deleting rgw dashboard user") + _, err = DeleteUser(context, DashboardUser) + if err != nil { + log.NamedWarning(context.NsName(), logger, "failed to delete ceph user %q. %v", DashboardUser, err) + } } } args := []string{"dashboard", "reset-rgw-api-access-key"} cephCmd := cephclient.NewCephCommand(context.Context, context.clusterInfo, args) - _, err = cephCmd.RunWithTimeout(exec.CephCommandsTimeout) + _, err := cephCmd.RunWithTimeout(exec.CephCommandsTimeout) if err != nil { log.NamedWarning(context.NsName(), logger, "failed to reset user accesskey for user %q. %v", DashboardUser, err) } diff --git a/pkg/operator/ceph/object/rgw.go b/pkg/operator/ceph/object/rgw.go index 86e4e7fb4..545803afb 100644 --- a/pkg/operator/ceph/object/rgw.go +++ b/pkg/operator/ceph/object/rgw.go @@ -283,9 +283,7 @@ func (c *clusterConfig) deleteStore() { if objContext != nil { objContext.Endpoint = c.store.Status.Info["endpoint"] - if poolsExistForNonRawOps(objContext) { - go disableRGWDashboard(objContext) - } + disableRGWDashboard(objContext) err = deleteRealmAndPools(objContext, c.store.Spec) if err != nil { diff --git a/pkg/operator/ceph/object/shared_pools_test.go b/pkg/operator/ceph/object/shared_pools_test.go index 3d1fc8a0d..41e132dc0 100644 --- a/pkg/operator/ceph/object/shared_pools_test.go +++ b/pkg/operator/ceph/object/shared_pools_test.go @@ -4,9 +4,12 @@ import ( "encoding/json" "reflect" "testing" + "time" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + "github.com/rook/rook/pkg/clusterd" cephclient "github.com/rook/rook/pkg/daemon/ceph/client" + exectest "github.com/rook/rook/pkg/util/exec/test" "github.com/stretchr/testify/assert" ) @@ -1145,7 +1148,22 @@ func Test_adjustZoneDefaultPools(t *testing.T) { srcZone := map[string]interface{}{} err := json.Unmarshal([]byte(tt.args.beforeJSON), &srcZone) assert.NoError(t, err) - objContext := &Context{clusterInfo: &cephclient.ClusterInfo{Namespace: "test"}} + // Provide a mock executor so checkPoolIsEmpty can run ceph/rados commands (best-effort). + executor := &exectest.MockExecutor{ + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { + if args[0] == "osd" && args[1] == "lspools" { + return `[]`, nil + } + return `{"pools":[]}`, nil + }, + MockExecuteCommandWithTimeout: func(timeout time.Duration, command string, args ...string) (string, error) { + return "", nil + }, + } + objContext := &Context{ + Context: &clusterd.Context{Executor: executor}, + clusterInfo: cephclient.AdminTestClusterInfo("test"), + } changedZone, err := adjustZoneDefaultPools(objContext, srcZone, tt.args.spec) // check that source was not modified