From 3396198e9c0aa2685a41a7cc8b6a627c487de869 Mon Sep 17 00:00:00 2001 From: Santosh Date: Wed, 10 Jun 2026 13:39:40 +0530 Subject: [PATCH] core: delete stale MDS and RGW pdbs When RGW and MDS intances are reduce to 1, the older RGW and MDS PDB instance (with maxUnavailabe=1) are still available. This blocks node drain when the node, on which the single RGW and MDS instances are running, is drained. This PR deletes any stale MDS and RGW PDB when the only 1 MDS and 1 RGW instances are available. Signed-off-by: Santosh --- .../disruption/clusterdisruption/pools.go | 15 ++ .../clusterdisruption/pools_test.go | 190 ++++++++++++++++++ 2 files changed, 205 insertions(+) diff --git a/pkg/operator/ceph/disruption/clusterdisruption/pools.go b/pkg/operator/ceph/disruption/clusterdisruption/pools.go index 7933ed602..6b03bc510 100644 --- a/pkg/operator/ceph/disruption/clusterdisruption/pools.go +++ b/pkg/operator/ceph/disruption/clusterdisruption/pools.go @@ -21,6 +21,7 @@ import ( cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" "github.com/rook/rook/pkg/operator/ceph/cluster/osd/topology" + log "github.com/rook/rook/pkg/util/log" "github.com/pkg/errors" policyv1 "k8s.io/api/policy/v1" @@ -117,6 +118,13 @@ func (r *ReconcileClusterDisruption) reconcileCephObjectStore(cephObjectStoreLis rgwCount := objectStore.Spec.Gateway.Instances minAvailable := &intstr.IntOrString{IntVal: rgwCount - 1} if minAvailable.IntVal < 1 { + stalePDB := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: pdbName, Namespace: namespace}, + } + log.NamespacedInfo(namespace, logger, "deleting stale PDB %q with %d instance(s)", pdbName, rgwCount) + if err := r.deletePDB(stalePDB); err != nil { + return errors.Wrapf(err, "failed to delete stale pdb %q", pdbName) + } continue } blockOwnerDeletion := false @@ -166,6 +174,13 @@ func (r *ReconcileClusterDisruption) reconcileCephFilesystem(cephFilesystemList minAvailable.IntVal++ } if minAvailable.IntVal < 1 { + stalePDB := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: pdbName, Namespace: namespace}, + } + log.NamespacedInfo(namespace, logger, "deleting stale PDB %q", pdbName) + if err := r.deletePDB(stalePDB); err != nil { + return errors.Wrapf(err, "failed to delete stale pdb %q", pdbName) + } continue } blockOwnerDeletion := false diff --git a/pkg/operator/ceph/disruption/clusterdisruption/pools_test.go b/pkg/operator/ceph/disruption/clusterdisruption/pools_test.go index 3417d30a7..064313556 100644 --- a/pkg/operator/ceph/disruption/clusterdisruption/pools_test.go +++ b/pkg/operator/ceph/disruption/clusterdisruption/pools_test.go @@ -17,11 +17,19 @@ limitations under the License. package clusterdisruption import ( + "context" "testing" "github.com/stretchr/testify/assert" cephv1 "github.com/rook/rook/pkg/apis/ceph.rook.io/v1" + "github.com/rook/rook/pkg/client/clientset/versioned/scheme" + "github.com/rook/rook/pkg/operator/ceph/disruption/controllerconfig" + policyv1 "k8s.io/api/policy/v1" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/types" + "k8s.io/apimachinery/pkg/util/intstr" + "sigs.k8s.io/controller-runtime/pkg/client/fake" ) func TestGetMinimumFailureDomain(t *testing.T) { @@ -49,3 +57,185 @@ func TestGetMinimumFailureDomain(t *testing.T) { assert.Equal(t, "host", getMinimumFailureDomain(poolList)) } + +func TestReconcileCephObjectStorePDB(t *testing.T) { + ns := "rook-ceph" + storeName := "my-store" + pdbName := "rook-ceph-rgw-" + storeName + + s := scheme.Scheme + err := policyv1.AddToScheme(s) + assert.NoError(t, err) + + t.Run("scale down from 2 to 1 deletes stale PDB", func(t *testing.T) { + existingPDB := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: pdbName, Namespace: ns}, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &intstr.IntOrString{IntVal: 1}, + }, + } + client := fake.NewClientBuilder().WithScheme(s).WithObjects(existingPDB).Build() + r := &ReconcileClusterDisruption{ + client: client, + scheme: s, + context: &controllerconfig.Context{OpManagerContext: context.TODO()}, + } + + objectStoreList := &cephv1.CephObjectStoreList{ + Items: []cephv1.CephObjectStore{ + { + ObjectMeta: metav1.ObjectMeta{Name: storeName, Namespace: ns}, + Spec: cephv1.ObjectStoreSpec{Gateway: cephv1.GatewaySpec{Instances: 1}}, + }, + }, + } + + err := r.reconcileCephObjectStore(objectStoreList) + assert.NoError(t, err) + + pdb := &policyv1.PodDisruptionBudget{} + err = client.Get(context.TODO(), types.NamespacedName{Name: pdbName, Namespace: ns}, pdb) + assert.Error(t, err, "PDB should be deleted") + }) + + t.Run("scale down with no existing PDB does not error", func(t *testing.T) { + client := fake.NewClientBuilder().WithScheme(s).Build() + r := &ReconcileClusterDisruption{ + client: client, + scheme: s, + context: &controllerconfig.Context{OpManagerContext: context.TODO()}, + } + + objectStoreList := &cephv1.CephObjectStoreList{ + Items: []cephv1.CephObjectStore{ + { + ObjectMeta: metav1.ObjectMeta{Name: storeName, Namespace: ns}, + Spec: cephv1.ObjectStoreSpec{Gateway: cephv1.GatewaySpec{Instances: 1}}, + }, + }, + } + + err := r.reconcileCephObjectStore(objectStoreList) + assert.NoError(t, err) + }) + + t.Run("2 instances creates PDB", func(t *testing.T) { + client := fake.NewClientBuilder().WithScheme(s).Build() + r := &ReconcileClusterDisruption{ + client: client, + scheme: s, + context: &controllerconfig.Context{OpManagerContext: context.TODO()}, + } + + objectStoreList := &cephv1.CephObjectStoreList{ + Items: []cephv1.CephObjectStore{ + { + ObjectMeta: metav1.ObjectMeta{Name: storeName, Namespace: ns}, + Spec: cephv1.ObjectStoreSpec{Gateway: cephv1.GatewaySpec{Instances: 2}}, + }, + }, + } + + err := r.reconcileCephObjectStore(objectStoreList) + assert.NoError(t, err) + + pdb := &policyv1.PodDisruptionBudget{} + err = client.Get(context.TODO(), types.NamespacedName{Name: pdbName, Namespace: ns}, pdb) + assert.NoError(t, err) + assert.Equal(t, int32(1), pdb.Spec.MinAvailable.IntVal) + }) +} + +func TestReconcileCephFilesystemPDB(t *testing.T) { + ns := "rook-ceph" + fsName := "my-fs" + pdbName := "rook-ceph-mds-" + fsName + + s := scheme.Scheme + err := policyv1.AddToScheme(s) + assert.NoError(t, err) + + t.Run("scale down from 2 to 1 active with no standby deletes stale PDB", func(t *testing.T) { + existingPDB := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{Name: pdbName, Namespace: ns}, + Spec: policyv1.PodDisruptionBudgetSpec{ + MinAvailable: &intstr.IntOrString{IntVal: 1}, + }, + } + client := fake.NewClientBuilder().WithScheme(s).WithObjects(existingPDB).Build() + r := &ReconcileClusterDisruption{ + client: client, + scheme: s, + context: &controllerconfig.Context{OpManagerContext: context.TODO()}, + } + + fsList := &cephv1.CephFilesystemList{ + Items: []cephv1.CephFilesystem{ + { + ObjectMeta: metav1.ObjectMeta{Name: fsName, Namespace: ns}, + Spec: cephv1.FilesystemSpec{ + MetadataServer: cephv1.MetadataServerSpec{ActiveCount: 1, ActiveStandby: false}, + }, + }, + }, + } + + err := r.reconcileCephFilesystem(fsList) + assert.NoError(t, err) + + pdb := &policyv1.PodDisruptionBudget{} + err = client.Get(context.TODO(), types.NamespacedName{Name: pdbName, Namespace: ns}, pdb) + assert.Error(t, err, "PDB should be deleted") + }) + + t.Run("scale down with no existing PDB does not error", func(t *testing.T) { + client := fake.NewClientBuilder().WithScheme(s).Build() + r := &ReconcileClusterDisruption{ + client: client, + scheme: s, + context: &controllerconfig.Context{OpManagerContext: context.TODO()}, + } + + fsList := &cephv1.CephFilesystemList{ + Items: []cephv1.CephFilesystem{ + { + ObjectMeta: metav1.ObjectMeta{Name: fsName, Namespace: ns}, + Spec: cephv1.FilesystemSpec{ + MetadataServer: cephv1.MetadataServerSpec{ActiveCount: 1, ActiveStandby: false}, + }, + }, + }, + } + + err := r.reconcileCephFilesystem(fsList) + assert.NoError(t, err) + }) + + t.Run("1 active with standby creates PDB", func(t *testing.T) { + client := fake.NewClientBuilder().WithScheme(s).Build() + r := &ReconcileClusterDisruption{ + client: client, + scheme: s, + context: &controllerconfig.Context{OpManagerContext: context.TODO()}, + } + + fsList := &cephv1.CephFilesystemList{ + Items: []cephv1.CephFilesystem{ + { + ObjectMeta: metav1.ObjectMeta{Name: fsName, Namespace: ns}, + Spec: cephv1.FilesystemSpec{ + MetadataServer: cephv1.MetadataServerSpec{ActiveCount: 1, ActiveStandby: true}, + }, + }, + }, + } + + err := r.reconcileCephFilesystem(fsList) + assert.NoError(t, err) + + pdb := &policyv1.PodDisruptionBudget{} + err = client.Get(context.TODO(), types.NamespacedName{Name: pdbName, Namespace: ns}, pdb) + assert.NoError(t, err) + assert.Equal(t, int32(1), pdb.Spec.MinAvailable.IntVal) + }) +}