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) + }) +}