From b47d2c2f79803830d16f270e5ff7a6cf336680cc Mon Sep 17 00:00:00 2001 From: Artem Torubarov Date: Mon, 23 Feb 2026 11:23:55 +0100 Subject: [PATCH 1/2] rgw: force objectStore controller to wait until zone and sharedPools are ready Fixes race condition for rgw multisite deployments caused by ObjectStore and Zone controllers calling period commit concurrently. See: https://github.com/rook/rook/issues/17013 Signed-off-by: Artem Torubarov --- .../examples/object-test-multisite-realm.yaml | 41 ++++ .../examples/object-test-multisite-store.yaml | 11 ++ pkg/operator/ceph/object/controller.go | 60 ++++-- pkg/operator/ceph/object/controller_test.go | 186 +++++++++++++++++- pkg/operator/ceph/object/objectstore.go | 23 ++- pkg/operator/ceph/object/objectstore_test.go | 2 +- pkg/operator/ceph/object/rgw.go | 20 +- pkg/operator/ceph/object/zone/controller.go | 10 + 8 files changed, 324 insertions(+), 29 deletions(-) create mode 100644 deploy/examples/object-test-multisite-realm.yaml create mode 100644 deploy/examples/object-test-multisite-store.yaml diff --git a/deploy/examples/object-test-multisite-realm.yaml b/deploy/examples/object-test-multisite-realm.yaml new file mode 100644 index 000000000..a23efb860 --- /dev/null +++ b/deploy/examples/object-test-multisite-realm.yaml @@ -0,0 +1,41 @@ +apiVersion: ceph.rook.io/v1 +kind: CephObjectRealm +metadata: + name: test-realm + namespace: rook-ceph +--- +apiVersion: ceph.rook.io/v1 +kind: CephObjectZoneGroup +metadata: + name: test-zonegroup + namespace: rook-ceph +spec: + realm: test-realm +--- +apiVersion: ceph.rook.io/v1 +kind: CephObjectZone +metadata: + name: test-zone + namespace: rook-ceph +spec: + zoneGroup: test-zonegroup + metadataPool: + failureDomain: host + replicated: + size: 1 + requireSafeReplicaSize: false + dataPool: + failureDomain: host + replicated: + size: 1 + requireSafeReplicaSize: false + sharedPools: + poolPlacements: + - name: placement-a + metadataPoolName: rgw-meta-pool + dataPoolName: rgw-data-pool + default: true + - name: placement-b + metadataPoolName: rgw-meta-pool + dataPoolName: rgw-data-pool + preserveRadosNamespaceDataOnDelete: true diff --git a/deploy/examples/object-test-multisite-store.yaml b/deploy/examples/object-test-multisite-store.yaml new file mode 100644 index 000000000..03c942a98 --- /dev/null +++ b/deploy/examples/object-test-multisite-store.yaml @@ -0,0 +1,11 @@ +apiVersion: ceph.rook.io/v1 +kind: CephObjectStore +metadata: + name: test-store + namespace: rook-ceph +spec: + gateway: + port: 80 + instances: 1 + zone: + name: test-zone diff --git a/pkg/operator/ceph/object/controller.go b/pkg/operator/ceph/object/controller.go index b77345730..0c4a87cc4 100644 --- a/pkg/operator/ceph/object/controller.go +++ b/pkg/operator/ceph/object/controller.go @@ -375,22 +375,31 @@ func (r *ReconcileCephObjectStore) reconcile(request reconcile.Request) (reconci return reconcile.Result{}, *cephObjectStore, errors.Wrapf(err, "failed to get latest CephObjectStore %q", request.NamespacedName.String()) } objCtx, err := NewMultisiteContext(r.context, r.clusterInfo, cephObjectStore) - if err != nil { + if err != nil && !kerrors.IsNotFound(err) { return reconcile.Result{}, *cephObjectStore, errors.Wrapf(err, "failed to get object context") } - opsCtx, err := NewMultisiteAdminOpsContext(objCtx, &cephObjectStore.Spec) - if err != nil { - return reconcile.Result{}, *cephObjectStore, errors.Wrapf(err, "failed to get admin ops API context") + + // Check dependents only if we have a working multisite context. + // Context may be nil when zone/zonegroup/realm CRDs are already deleted. + if objCtx != nil { + opsCtx, err := NewMultisiteAdminOpsContext(objCtx, &cephObjectStore.Spec) + if err != nil { + // If admin ops context fails during deletion, skip dependency check. + // The store may never have been fully created (e.g. pools didn't exist), + // so there can't be any dependents. + log.NamedWarning(request.NamespacedName, logger, "failed to get admin ops context during deletion, skipping dependency check: %v", err) + } else { + deps, err := cephObjectStoreDependents(r.context, r.clusterInfo, cephObjectStore, objCtx, opsCtx) + if err != nil { + return reconcile.Result{}, *cephObjectStore, err + } + if !deps.Empty() { + err := reporting.ReportDeletionBlockedDueToDependents(r.opManagerContext, logger, r.client, cephObjectStore, deps) + return opcontroller.WaitForRequeueIfFinalizerBlocked, *cephObjectStore, err + } + reporting.ReportDeletionNotBlockedDueToDependents(r.opManagerContext, logger, r.client, r.recorder, cephObjectStore) + } } - deps, err := cephObjectStoreDependents(r.context, r.clusterInfo, cephObjectStore, objCtx, opsCtx) - if err != nil { - return reconcile.Result{}, *cephObjectStore, err - } - if !deps.Empty() { - err := reporting.ReportDeletionBlockedDueToDependents(r.opManagerContext, logger, r.client, cephObjectStore, deps) - return opcontroller.WaitForRequeueIfFinalizerBlocked, *cephObjectStore, err - } - reporting.ReportDeletionNotBlockedDueToDependents(r.opManagerContext, logger, r.client, r.recorder, cephObjectStore) cfg := clusterConfig{ context: r.context, @@ -479,13 +488,20 @@ func (r *ReconcileCephObjectStore) reconcile(request reconcile.Request) (reconci } // CREATE/UPDATE - _, err = r.reconcileCreateObjectStore(cephObjectStore, request.NamespacedName, cfg) + reconcileResult, err := r.reconcileCreateObjectStore(cephObjectStore, request.NamespacedName, cfg) if err != nil && kerrors.IsNotFound(err) { // A not found error may mean ceph is still initializing, but there might be some other error // so we log the error and requeue log.NamedWarning(request.NamespacedName, logger, "object store %q reconcile failed, ceph may still be initializing. %v", request.NamespacedName.String(), err) return opcontroller.WaitForRequeueIfOperatorNotInitialized, *cephObjectStore, nil } else if err != nil { + // If reconcileCreateObjectStore returned a requeue result (e.g. waiting for zone Ready), + // propagate it instead of setting Failure status. This avoids flapping between + // Progressing and Failure while waiting for dependencies. + if reconcileResult.RequeueAfter > 0 { + log.NamedWarning(request.NamespacedName, logger, "object store %q reconcile: %v. requeueing", request.NamespacedName.String(), err) + return reconcileResult, *cephObjectStore, nil + } result, err := r.setFailedStatus(k8sutil.ObservedGenerationNotAvailable, request.NamespacedName, "failed to create object store deployments", err) return result, *cephObjectStore, err } @@ -556,6 +572,11 @@ func (r *ReconcileCephObjectStore) reconcileCreateObjectStore(cephObjectStore *c if err := UpdateEndpointForAdminOps(objContext, cephObjectStore); err != nil { return r.setFailedStatus(k8sutil.ObservedGenerationNotAvailable, namespacedName, "failed to set endpoint", err) } + + // Validate that the admin ops user credentials exist + if _, _, err := GetAdminOPSUserCredentials(objContext, &cephObjectStore.Spec); err != nil { + return r.setFailedStatus(k8sutil.ObservedGenerationNotAvailable, namespacedName, "failed to get admin ops user credentials", err) + } } else { log.NamedInfo(namespacedName, logger, "reconciling object store deployments") @@ -677,6 +698,17 @@ func (r *ReconcileCephObjectStore) getMultisiteResourceNames(cephObjectStore *ce } log.NamedDebug(nsName, logger, "CephObjectZone resource %s found", zone.Name) + // Wait for the zone to be fully configured (pools, placements, period commit) + // to avoid race condition in period commit in objectStore and Zone controllers + if zone.Status == nil || zone.Status.Phase != k8sutil.ReadyStatus { + gotPhase := "" + if zone.Status != nil { + gotPhase = zone.Status.Phase + } + logger.Infof("CephObjectZone %q is not ready yet (phase %q). waiting for it to be configured", zoneName, gotPhase) + return "", "", "", nil, waitForRequeueIfObjectStoreNotReady, fmt.Errorf("CephObjectZone %q is not ready", zoneName) + } + zonegroup := &cephv1.CephObjectZoneGroup{} err = r.client.Get(r.opManagerContext, types.NamespacedName{Name: zone.Spec.ZoneGroup, Namespace: cephObjectStore.Namespace}, zonegroup) if err != nil { diff --git a/pkg/operator/ceph/object/controller_test.go b/pkg/operator/ceph/object/controller_test.go index 68c538454..a0ff45e49 100644 --- a/pkg/operator/ceph/object/controller_test.go +++ b/pkg/operator/ceph/object/controller_test.go @@ -606,6 +606,9 @@ func TestCephObjectStoreControllerMultisite(t *testing.T) { MetadataPool: metadataPool, DataPool: dataPool, }, + Status: &cephv1.Status{ + Phase: k8sutil.ReadyStatus, + }, } objectZoneGroup := &cephv1.CephObjectZoneGroup{ @@ -799,6 +802,176 @@ func TestCephObjectStoreControllerMultisite(t *testing.T) { }) } +func TestCephObjectStoreControllerZoneNotReady(t *testing.T) { + // Verify that the object store controller requeues when the CephObjectZone + // is not yet Ready. This prevents a race condition where RGW starts before + // the zone controller has configured shared pool placements, causing RGW to + // auto-create unwanted default pools (see https://github.com/rook/rook/issues/17013). + ctx := context.TODO() + capnslog.SetGlobalLogLevel(capnslog.DEBUG) + os.Setenv("ROOK_LOG_LEVEL", "DEBUG") + + zoneName := "zone-a" + zoneGroupName := "zonegroup-a" + realmName := "realm-a" + + cephCluster := &cephv1.CephCluster{ + ObjectMeta: metav1.ObjectMeta{ + Name: namespace, + Namespace: namespace, + }, + Status: cephv1.ClusterStatus{ + Phase: k8sutil.ReadyStatus, + CephStatus: &cephv1.CephStatus{ + Health: "HEALTH_OK", + }, + }, + } + + secrets := map[string][]byte{ + "fsid": []byte(name), + "mon-secret": []byte("monsecret"), + "admin-secret": []byte("adminsecret"), + } + secret := &v1.Secret{ + ObjectMeta: metav1.ObjectMeta{ + Name: "rook-ceph-mon", + Namespace: namespace, + }, + Data: secrets, + Type: k8sutil.RookType, + } + + objectZoneGroup := &cephv1.CephObjectZoneGroup{ + ObjectMeta: metav1.ObjectMeta{ + Name: zoneGroupName, + Namespace: namespace, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "CephObjectZoneGroup", + }, + Spec: cephv1.ObjectZoneGroupSpec{}, + } + objectZoneGroup.Spec.Realm = realmName + + objectRealm := &cephv1.CephObjectRealm{ + ObjectMeta: metav1.ObjectMeta{ + Name: realmName, + Namespace: namespace, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "CephObjectRealm", + }, + Spec: cephv1.ObjectRealmSpec{}, + } + + objectStore := &cephv1.CephObjectStore{ + ObjectMeta: metav1.ObjectMeta{ + Name: store, + Namespace: namespace, + Finalizers: []string{"cephobjectstore.ceph.rook.io"}, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "CephObjectStore", + }, + Spec: cephv1.ObjectStoreSpec{}, + } + objectStore.Spec.Zone.Name = zoneName + objectStore.Spec.Gateway.Port = 80 + + executor := &exectest.MockExecutor{ + MockExecuteCommandWithOutput: func(command string, args ...string) (string, error) { + if args[0] == "status" { + return `{"fsid":"c47cac40-9bee-4d52-823b-ccd803ba5bfe","health":{"checks":{},"status":"HEALTH_OK"},"pgmap":{"num_pgs":100,"pgs_by_state":[{"state_name":"active+clean","count":100}]}}`, nil + } + if args[0] == "versions" { + return dummyVersionsRaw, nil + } + return "", nil + }, + } + + clientset := test.New(t, 3) + + s := scheme.Scheme + s.AddKnownTypes(cephv1.SchemeGroupVersion, &cephv1.CephObjectZone{}, &cephv1.CephObjectZoneList{}, &cephv1.CephCluster{}, &cephv1.CephClusterList{}, &cephv1.CephObjectStore{}, &cephv1.CephObjectStoreList{}) + + req := reconcile.Request{ + NamespacedName: types.NamespacedName{ + Name: store, + Namespace: namespace, + }, + } + + currentAndDesiredCephVersion = func(ctx context.Context, rookImage string, namespace string, jobName string, ownerInfo *k8sutil.OwnerInfo, context *clusterd.Context, cephClusterSpec *cephv1.ClusterSpec, clusterInfo *client.ClusterInfo) (*cephver.CephVersion, *cephver.CephVersion, error) { + return &cephver.Squid, &cephver.Squid, nil + } + + zoneNotReadyPhases := []struct { + name string + status *cephv1.Status + }{ + {"zone status is nil", nil}, + {"zone is reconciling", &cephv1.Status{Phase: k8sutil.ReconcilingStatus}}, + {"zone reconcile failed", &cephv1.Status{Phase: k8sutil.ReconcileFailedStatus}}, + {"zone status is empty", &cephv1.Status{Phase: k8sutil.EmptyStatus}}, + } + + for _, tc := range zoneNotReadyPhases { + t.Run(tc.name, func(t *testing.T) { + objectZone := &cephv1.CephObjectZone{ + ObjectMeta: metav1.ObjectMeta{ + Name: zoneName, + Namespace: namespace, + }, + TypeMeta: metav1.TypeMeta{ + Kind: "CephObjectZone", + }, + Spec: cephv1.ObjectZoneSpec{ + ZoneGroup: zoneGroupName, + }, + Status: tc.status, + } + + object := []runtime.Object{ + objectZone, + objectStore, + objectZoneGroup, + objectRealm, + cephCluster, + } + + cl := fake.NewClientBuilder().WithScheme(s).WithRuntimeObjects(object...).Build() + c := &clusterd.Context{ + Executor: executor, + RookClientset: rookfake.NewSimpleClientset(objectRealm, objectZoneGroup, objectZone, objectStore), + Clientset: clientset, + } + + r := &ReconcileCephObjectStore{ + client: cl, + scheme: s, + context: c, + recorder: events.NewFakeRecorder(50), + opManagerContext: ctx, + } + + _, err := r.context.Clientset.CoreV1().Secrets(namespace).Create(ctx, secret, metav1.CreateOptions{}) + if k8serrors.IsAlreadyExists(err) { + err = nil + } + assert.NoError(t, err) + + res, err := r.Reconcile(ctx, req) + // Reconcile should requeue because the zone is not ready. + // The store stays in Progressing status, preventing RGW from + // starting before zone shared pools are configured. + assert.NoError(t, err) + assert.True(t, res.RequeueAfter > 0, "expected requeue while waiting for zone Ready") + }) + } +} + func TestCephObjectExternalStoreController(t *testing.T) { ctx := context.TODO() capnslog.SetGlobalLogLevel(capnslog.DEBUG) @@ -964,14 +1137,20 @@ func TestCephObjectExternalStoreController(t *testing.T) { } t.Run("create an external object store with missing secret", func(t *testing.T) { + // Clear DeletionTimestamp that was set by the delete subtest above, since + // externalObjectStore is a shared pointer. + externalObjectStore.DeletionTimestamp = nil + externalObjectStore.Finalizers = nil objects := []runtime.Object{ cephCluster, externalObjectStore, } r := getReconciler(objects) res, err := r.Reconcile(ctx, req) - assert.Error(t, err) - assert.False(t, res.Requeue) + // Missing secret is a NotFound error, which triggers a requeue + // (the secret may be created later by another controller or manually). + assert.NoError(t, err) + assert.True(t, res.Requeue) }) t.Run("create an external object store with no external RGW endpoints", func(t *testing.T) { @@ -1203,6 +1382,9 @@ func TestKeyRotation(t *testing.T) { MetadataPool: metadataPool, DataPool: dataPool, }, + Status: &cephv1.Status{ + Phase: k8sutil.ReadyStatus, + }, } objectZoneGroup := &cephv1.CephObjectZoneGroup{ diff --git a/pkg/operator/ceph/object/objectstore.go b/pkg/operator/ceph/object/objectstore.go index 61b73dac3..3370742ee 100644 --- a/pkg/operator/ceph/object/objectstore.go +++ b/pkg/operator/ceph/object/objectstore.go @@ -189,7 +189,16 @@ func deleteSingleSiteRealmAndPools(objContext *Context, spec cephv1.ObjectStoreS if !spec.PreservePoolsOnDelete { if EmptyPool(spec.DataPool) && EmptyPool(spec.MetadataPool) { - log.NamedInfo(objContext.NsName(), logger, "skipping removal of pools since not specified in the object store") + if !IsNeedToCreateObjectStorePools(spec.SharedPools) { + // When shared pools are configured, "radosgw-admin zone delete" may leave behind + // per-zone pools (e.g., store-a.rgw.control, store-a.rgw.meta). + log.NamedInfo(objContext.NsName(), logger, "cleaning up per-zone ghost pools left by realm deletion") + if err := DeletePools(objContext, lastStore, objContext.Name); err != nil { + log.NamedWarning(objContext.NsName(), logger, "failed to clean up ghost pools: %v", err) + } + } else { + log.NamedInfo(objContext.NsName(), logger, "skipping removal of pools since not specified in the object store") + } return nil } err = DeletePools(objContext, lastStore, objContext.Name) @@ -595,12 +604,14 @@ func configureObjectStore(objContext *Context, store *cephv1.CephObjectStore, zo } func deleteRealm(context *Context) error { - // realmArg := fmt.Sprintf("--rgw-realm=%s", context.Name) zoneGroupArg := fmt.Sprintf("--rgw-zonegroup=%s", context.Name) - _, err := RunAdminCommandNoMultisite(context, false, "realm", "delete", realmArg) + + // Delete in reverse order: zone → zonegroup → realm. + // Note: radosgw-admin uses "delete" for zone and zonegroup, but "rm" for realm. + _, err := runAdminCommand(context, false, "zone", "delete") if err != nil { - log.NamedWarning(context.NsName(), logger, "failed to delete rgw realm %q. %v", context.Name, err) + log.NamedWarning(context.NsName(), logger, "failed to delete rgw zone %q. %v", context.Name, err) } _, err = RunAdminCommandNoMultisite(context, false, "zonegroup", "delete", realmArg, zoneGroupArg) @@ -608,9 +619,9 @@ func deleteRealm(context *Context) error { log.NamedWarning(context.NsName(), logger, "failed to delete rgw zonegroup %q. %v", context.Name, err) } - _, err = runAdminCommand(context, false, "zone", "delete") + _, err = RunAdminCommandNoMultisite(context, false, "realm", "rm", realmArg) if err != nil { - log.NamedWarning(context.NsName(), logger, "failed to delete rgw zone %q. %v", context.Name, err) + log.NamedWarning(context.NsName(), logger, "failed to delete rgw realm %q. %v", context.Name, err) } return nil diff --git a/pkg/operator/ceph/object/objectstore_test.go b/pkg/operator/ceph/object/objectstore_test.go index 7c9c03252..3c3b97330 100644 --- a/pkg/operator/ceph/object/objectstore_test.go +++ b/pkg/operator/ceph/object/objectstore_test.go @@ -444,7 +444,7 @@ func deleteStore(t *testing.T, name string, existingStores string, expectedDelet } } if args[0] == "realm" { - if args[1] == "delete" { + if args[1] == "rm" { realmDeleted = true return "", nil } diff --git a/pkg/operator/ceph/object/rgw.go b/pkg/operator/ceph/object/rgw.go index 25d005650..13cbaf365 100644 --- a/pkg/operator/ceph/object/rgw.go +++ b/pkg/operator/ceph/object/rgw.go @@ -270,19 +270,27 @@ func (c *clusterConfig) deleteStore() { } } - // Delete the realm and pools + // Delete the realm and pools. + // NewMultisiteContext may return nil objContext when the CephObjectStore references a + // CephObjectZone (spec.zone.name is set) and the zone/zonegroup/realm CRDs have already + // been garbage-collected. This happens during simultaneous deletion of all multisite CRDs. + // Guard against nil to avoid a panic in that case — realm/pool cleanup is best-effort anyway. objContext, err := NewMultisiteContext(c.context, c.clusterInfo, c.store) if err != nil { log.NamedError(nsName, logger, "failed to set multisite on object store %q. Error: %v", c.store.Name, err) } - objContext.Endpoint = c.store.Status.Info["endpoint"] + if objContext != nil { + objContext.Endpoint = c.store.Status.Info["endpoint"] - go disableRGWDashboard(objContext) + go disableRGWDashboard(objContext) - err = deleteRealmAndPools(objContext, c.store.Spec) - if err != nil { - log.NamedError(nsName, logger, "failed to delete the realm and pools. Error: %v", err) + err = deleteRealmAndPools(objContext, c.store.Spec) + if err != nil { + log.NamedError(nsName, logger, "failed to delete the realm and pools. Error: %v", err) + } + } else { + log.NamedWarning(nsName, logger, "skipping realm/pool deletion: multisite context not available (zone/zonegroup/realm CRDs may already be deleted)") } } diff --git a/pkg/operator/ceph/object/zone/controller.go b/pkg/operator/ceph/object/zone/controller.go index f27716444..21336c1f7 100644 --- a/pkg/operator/ceph/object/zone/controller.go +++ b/pkg/operator/ceph/object/zone/controller.go @@ -206,6 +206,16 @@ func (r *ReconcileObjectZone) reconcile(request reconcile.Request) (reconcile.Re // Make sure an ObjectZoneGroup is present realmName, reconcileResponse, err := r.getCephObjectZoneGroup(cephObjectZone) if err != nil { + // If the zone is being deleted and the zonegroup CRD is already gone (e.g., all multisite + // CRDs deleted simultaneously), remove the finalizer so the zone CRD doesn't get stuck. + if !cephObjectZone.GetDeletionTimestamp().IsZero() && kerrors.IsNotFound(err) { + log.NamedWarning(request.NamespacedName, logger, "zonegroup CRD not found during zone deletion, removing finalizer") + err := opcontroller.RemoveFinalizer(r.opManagerContext, r.client, cephObjectZone) + if err != nil { + return reconcile.Result{}, *cephObjectZone, errors.Wrap(err, "failed to remove finalizer") + } + return reconcile.Result{}, *cephObjectZone, nil + } return reconcileResponse, *cephObjectZone, err } From a7f176b366acb940ce9989b4c7714ee13e24fcb0 Mon Sep 17 00:00:00 2001 From: Artem Torubarov Date: Wed, 25 Feb 2026 14:12:05 +0100 Subject: [PATCH 2/2] rgw: skip non-raw commands on object deletion path when pools not exists Signed-off-by: Artem Torubarov --- ....yaml => object-multisite-realm-test.yaml} | 0 ....yaml => object-multisite-store-test.yaml} | 0 pkg/operator/ceph/object/controller.go | 27 ++++++++-------- pkg/operator/ceph/object/controller_test.go | 8 +++++ pkg/operator/ceph/object/objectstore.go | 31 +++++++++++++------ pkg/operator/ceph/object/rgw.go | 4 ++- 6 files changed, 46 insertions(+), 24 deletions(-) rename deploy/examples/{object-test-multisite-realm.yaml => object-multisite-realm-test.yaml} (100%) rename deploy/examples/{object-test-multisite-store.yaml => object-multisite-store-test.yaml} (100%) diff --git a/deploy/examples/object-test-multisite-realm.yaml b/deploy/examples/object-multisite-realm-test.yaml similarity index 100% rename from deploy/examples/object-test-multisite-realm.yaml rename to deploy/examples/object-multisite-realm-test.yaml diff --git a/deploy/examples/object-test-multisite-store.yaml b/deploy/examples/object-multisite-store-test.yaml similarity index 100% rename from deploy/examples/object-test-multisite-store.yaml rename to deploy/examples/object-multisite-store-test.yaml diff --git a/pkg/operator/ceph/object/controller.go b/pkg/operator/ceph/object/controller.go index 0c4a87cc4..ee8caecdb 100644 --- a/pkg/operator/ceph/object/controller.go +++ b/pkg/operator/ceph/object/controller.go @@ -382,22 +382,23 @@ func (r *ReconcileCephObjectStore) reconcile(request reconcile.Request) (reconci // Check dependents only if we have a working multisite context. // Context may be nil when zone/zonegroup/realm CRDs are already deleted. if objCtx != nil { - opsCtx, err := NewMultisiteAdminOpsContext(objCtx, &cephObjectStore.Spec) - if err != nil { - // If admin ops context fails during deletion, skip dependency check. - // The store may never have been fully created (e.g. pools didn't exist), - // so there can't be any dependents. - log.NamedWarning(request.NamespacedName, logger, "failed to get admin ops context during deletion, skipping dependency check: %v", err) + if !poolsExistForNonRawOps(objCtx) { + log.NamedInfo(request.NamespacedName, logger, "skipping bucket dependency check during deletion") } else { - deps, err := cephObjectStoreDependents(r.context, r.clusterInfo, cephObjectStore, objCtx, opsCtx) + opsCtx, err := NewMultisiteAdminOpsContext(objCtx, &cephObjectStore.Spec) if err != nil { - return reconcile.Result{}, *cephObjectStore, err + log.NamedWarning(request.NamespacedName, logger, "failed to get admin ops context during deletion, skipping dependency check: %v", err) + } else { + deps, err := cephObjectStoreDependents(r.context, r.clusterInfo, cephObjectStore, objCtx, opsCtx) + if err != nil { + return reconcile.Result{}, *cephObjectStore, err + } + if !deps.Empty() { + err := reporting.ReportDeletionBlockedDueToDependents(r.opManagerContext, logger, r.client, cephObjectStore, deps) + return opcontroller.WaitForRequeueIfFinalizerBlocked, *cephObjectStore, err + } + reporting.ReportDeletionNotBlockedDueToDependents(r.opManagerContext, logger, r.client, r.recorder, cephObjectStore) } - if !deps.Empty() { - err := reporting.ReportDeletionBlockedDueToDependents(r.opManagerContext, logger, r.client, cephObjectStore, deps) - return opcontroller.WaitForRequeueIfFinalizerBlocked, *cephObjectStore, err - } - reporting.ReportDeletionNotBlockedDueToDependents(r.opManagerContext, logger, r.client, r.recorder, cephObjectStore) } } diff --git a/pkg/operator/ceph/object/controller_test.go b/pkg/operator/ceph/object/controller_test.go index a0ff45e49..80996a43c 100644 --- a/pkg/operator/ceph/object/controller_test.go +++ b/pkg/operator/ceph/object/controller_test.go @@ -669,6 +669,9 @@ func TestCephObjectStoreControllerMultisite(t *testing.T) { if args[0] == "osd" && args[1] == "pool" && args[2] == "get" { return "", errors.New("test pool does not exit yet") } + if args[0] == "osd" && args[1] == "lspools" { + return `[{"poolnum":1,"poolname":"zone-a.rgw.control"},{"poolnum":2,"poolname":"zone-a.rgw.meta"},{"poolnum":3,"poolname":"zone-a.rgw.log"},{"poolnum":4,"poolname":"zone-a.rgw.buckets.index"},{"poolnum":5,"poolname":"zone-a.rgw.buckets.non-ec"},{"poolnum":6,"poolname":"zone-a.rgw.otp"},{"poolnum":7,"poolname":"zone-a.rgw.buckets.data"},{"poolnum":8,"poolname":".rgw.root"}]`, nil + } if args[0] == "versions" { return dummyVersionsRaw, nil } @@ -761,6 +764,8 @@ func TestCephObjectStoreControllerMultisite(t *testing.T) { }) t.Run("delete the same store", func(t *testing.T) { calledCommitConfigChanges = false + // Simulate that the store was already created and has endpoints in the zone + zoneGroupGetMultisiteJSON = zoneGroupGetMultisiteJSONWithEndpoint // no dependents dependentsChecked := false @@ -1040,6 +1045,9 @@ func TestCephObjectExternalStoreController(t *testing.T) { if args[0] == "osd" && args[1] == "pool" && args[2] == "get" { return "", errors.New("test pool does not exit yet") } + if args[0] == "osd" && args[1] == "lspools" { + return `[{"poolnum":1,"poolname":"default.rgw.control"},{"poolnum":2,"poolname":"default.rgw.meta"},{"poolnum":3,"poolname":"default.rgw.log"},{"poolnum":4,"poolname":"default.rgw.buckets.index"},{"poolnum":5,"poolname":"default.rgw.buckets.non-ec"},{"poolnum":6,"poolname":"default.rgw.otp"},{"poolnum":7,"poolname":"default.rgw.buckets.data"},{"poolnum":8,"poolname":".rgw.root"}]`, nil + } if args[0] == "versions" { return dummyVersionsRaw, nil } diff --git a/pkg/operator/ceph/object/objectstore.go b/pkg/operator/ceph/object/objectstore.go index 3370742ee..dbb8774a8 100644 --- a/pkg/operator/ceph/object/objectstore.go +++ b/pkg/operator/ceph/object/objectstore.go @@ -189,16 +189,7 @@ func deleteSingleSiteRealmAndPools(objContext *Context, spec cephv1.ObjectStoreS if !spec.PreservePoolsOnDelete { if EmptyPool(spec.DataPool) && EmptyPool(spec.MetadataPool) { - if !IsNeedToCreateObjectStorePools(spec.SharedPools) { - // When shared pools are configured, "radosgw-admin zone delete" may leave behind - // per-zone pools (e.g., store-a.rgw.control, store-a.rgw.meta). - log.NamedInfo(objContext.NsName(), logger, "cleaning up per-zone ghost pools left by realm deletion") - if err := DeletePools(objContext, lastStore, objContext.Name); err != nil { - log.NamedWarning(objContext.NsName(), logger, "failed to clean up ghost pools: %v", err) - } - } else { - log.NamedInfo(objContext.NsName(), logger, "skipping removal of pools since not specified in the object store") - } + log.NamedInfo(objContext.NsName(), logger, "skipping removal of pools since not specified in the object store") return nil } err = DeletePools(objContext, lastStore, objContext.Name) @@ -758,6 +749,26 @@ func missingPools(context *Context) ([]string, error) { return missingPools, nil } +// poolsExistForNonRawOps checks whether all object store pools exist. +// Non-raw radosgw-admin commands (ones not in RGW raw_storage_ops_list, see: +// https://github.com/ceph/ceph/blob/649c3131862d310ce0b273dcbe08ebca0c2ec7ec/src/rgw/radosgw-admin/radosgw-admin.cc#L4540-L4567 +// ) +// trigger full RGW initialization which auto-creates default pools — for example user info called by disableRGWDashboard and +// user create called by NewMultisiteAdminOpsContext during store deletion. +// Use this function on delete reconciliation path to skip non-raw commands if pools were not created. +func poolsExistForNonRawOps(context *Context) bool { + missing, err := missingPools(context) + if err != nil { + log.NamedWarning(context.NsName(), logger, "failed to check for missing pools, assuming they don't exist to avoid ghost pool creation: %v", err) + return false + } + if len(missing) > 0 { + log.NamedInfo(context.NsName(), logger, "some object store pools are missing, skipping non-raw radosgw-admin operations to avoid ghost pool creation: %v", missing) + return false + } + return true +} + func CreateObjectStorePools(context *Context, cluster *cephv1.ClusterSpec, metadataPool, dataPool cephv1.PoolSpec) error { if EmptyPool(dataPool) || EmptyPool(metadataPool) { log.NamedInfo(context.NsName(), logger, "no pools specified for the CR, checking for their existence...") diff --git a/pkg/operator/ceph/object/rgw.go b/pkg/operator/ceph/object/rgw.go index 13cbaf365..86e4e7fb4 100644 --- a/pkg/operator/ceph/object/rgw.go +++ b/pkg/operator/ceph/object/rgw.go @@ -283,7 +283,9 @@ func (c *clusterConfig) deleteStore() { if objContext != nil { objContext.Endpoint = c.store.Status.Info["endpoint"] - go disableRGWDashboard(objContext) + if poolsExistForNonRawOps(objContext) { + go disableRGWDashboard(objContext) + } err = deleteRealmAndPools(objContext, c.store.Spec) if err != nil {