From a59c977de3cafc33614bdd0873cf41b7a50dc484 Mon Sep 17 00:00:00 2001 From: Sudhanshu Ranjan Date: Fri, 3 May 2024 16:11:53 +0530 Subject: [PATCH] [KO-321] [KO-320] Added a flag to disable PDB (#286) * Added a flag to disable PDB --- api/v1/aerospikecluster_mutating_webhook.go | 2 +- api/v1/aerospikecluster_types.go | 31 ++-- api/v1/aerospikecluster_validating_webhook.go | 13 +- api/v1/utils.go | 1 + api/v1/zz_generated.deepcopy.go | 10 ++ .../asdb.aerospike.com_aerospikeclusters.yaml | 8 + ...rnetes-operator.clusterserviceversion.yaml | 3 + config/rbac/role.yaml | 1 + controllers/aerospikecluster_controller.go | 2 +- controllers/poddistruptionbudget.go | 88 +++++++++- controllers/reconciler.go | 8 +- ..._aerospikeclusters.asdb.aerospike.com.yaml | 8 + ...erospike-operator-manager-clusterrole.yaml | 1 + test/poddisruptionbudget_test.go | 160 ++++++++++++++++-- 14 files changed, 296 insertions(+), 40 deletions(-) diff --git a/api/v1/aerospikecluster_mutating_webhook.go b/api/v1/aerospikecluster_mutating_webhook.go index 10c7222c4..3f3f65d87 100644 --- a/api/v1/aerospikecluster_mutating_webhook.go +++ b/api/v1/aerospikecluster_mutating_webhook.go @@ -72,7 +72,7 @@ func (c *AerospikeCluster) Default(operation v1.Operation) admission.Response { func (c *AerospikeCluster) setDefaults(asLog logr.Logger) error { // Set maxUnavailable default to 1 - if c.Spec.MaxUnavailable == nil { + if !GetBool(c.Spec.DisablePDB) && c.Spec.MaxUnavailable == nil { maxUnavailable := intstr.FromInt(1) c.Spec.MaxUnavailable = &maxUnavailable } diff --git a/api/v1/aerospikecluster_types.go b/api/v1/aerospikecluster_types.go index 699a08726..d1eec5a0f 100644 --- a/api/v1/aerospikecluster_types.go +++ b/api/v1/aerospikecluster_types.go @@ -68,14 +68,17 @@ type AerospikeClusterSpec struct { //nolint:govet // for readability // Aerospike cluster size // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Cluster Size" Size int32 `json:"size"` + // Aerospike server image + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Server Image" + Image string `json:"image"` // MaxUnavailable is the percentage/number of pods that can be allowed to go down or unavailable before application // disruption. This value is used to create PodDisruptionBudget. Defaults to 1. // Refer Aerospike documentation for more details. // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Max Unavailable" MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"` - // Aerospike server image - // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Server Image" - Image string `json:"image"` + // Disable the PodDisruptionBudget creation for the Aerospike cluster. + // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Disable PodDisruptionBudget" + DisablePDB *bool `json:"disablePDB,omitempty"` // Storage specify persistent storage to use for the Aerospike pods // +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Storage" Storage AerospikeStorageSpec `json:"storage,omitempty"` @@ -637,6 +640,8 @@ type AerospikeClusterStatusSpec struct { //nolint:govet // for readability // MaxUnavailable is the percentage/number of pods that can be allowed to go down or unavailable before application // disruption. This value is used to create PodDisruptionBudget. Defaults to 1. MaxUnavailable *intstr.IntOrString `json:"maxUnavailable,omitempty"` + // Disable the PodDisruptionBudget creation for the Aerospike cluster. + DisablePDB *bool `json:"disablePDB,omitempty"` // If set true then multiple pods can be created per Kubernetes Node. // This will create a NodePort service for each Pod. // NodePort, as the name implies, opens a specific port on all the Kubernetes Nodes , @@ -1000,11 +1005,13 @@ func CopySpecToStatus(spec *AerospikeClusterSpec) (*AerospikeClusterStatusSpec, } if spec.EnableDynamicConfigUpdate != nil { - enableDynamicConfigUpdate := lib.DeepCopy( - spec.EnableDynamicConfigUpdate, - ).(*bool) + enableDynamicConfigUpdate := *spec.EnableDynamicConfigUpdate + status.EnableDynamicConfigUpdate = &enableDynamicConfigUpdate + } - status.EnableDynamicConfigUpdate = enableDynamicConfigUpdate + if spec.DisablePDB != nil { + disablePDB := *spec.DisablePDB + status.DisablePDB = &disablePDB } // Storage @@ -1097,11 +1104,13 @@ func CopyStatusToSpec(status *AerospikeClusterStatusSpec) (*AerospikeClusterSpec } if status.EnableDynamicConfigUpdate != nil { - enableDynamicConfigUpdate := lib.DeepCopy( - status.EnableDynamicConfigUpdate, - ).(*bool) + enableDynamicConfigUpdate := *status.EnableDynamicConfigUpdate + spec.EnableDynamicConfigUpdate = &enableDynamicConfigUpdate + } - spec.EnableDynamicConfigUpdate = enableDynamicConfigUpdate + if status.DisablePDB != nil { + disablePDB := *status.DisablePDB + spec.DisablePDB = &disablePDB } // Storage diff --git a/api/v1/aerospikecluster_validating_webhook.go b/api/v1/aerospikecluster_validating_webhook.go index 7cca9aa7a..7b063bdb1 100644 --- a/api/v1/aerospikecluster_validating_webhook.go +++ b/api/v1/aerospikecluster_validating_webhook.go @@ -2222,8 +2222,17 @@ func validateIntOrStringField(value *intstr.IntOrString, fieldPath string) error func (c *AerospikeCluster) validateMaxUnavailable() error { // safe check for corner cases when mutation webhook somehow didn't work - if c.Spec.MaxUnavailable == nil { - return fmt.Errorf("maxUnavailable cannot be nil. Mutation webhook didn't work") + if !GetBool(c.Spec.DisablePDB) && c.Spec.MaxUnavailable == nil { + return fmt.Errorf("maxUnavailable cannot be nil if PDB is not disabled. Mutation webhook didn't work") + } + + if GetBool(c.Spec.DisablePDB) { + if c.Spec.MaxUnavailable != nil { + return fmt.Errorf("maxUnavailable must be nil if PDB is disabled") + } + + // PDB is disabled, so no need to validate further + return nil } if err := validateIntOrStringField(c.Spec.MaxUnavailable, "spec.maxUnavailable"); err != nil { diff --git a/api/v1/utils.go b/api/v1/utils.go index ed5c65040..653145710 100644 --- a/api/v1/utils.go +++ b/api/v1/utils.go @@ -65,6 +65,7 @@ const ( AerospikeInitContainerDefaultRegistryNamespace = "aerospike" AerospikeInitContainerDefaultRepoAndTag = "aerospike-kubernetes-init:2.2.0-dev3" AerospikeAppLabel = "app" + AerospikeAppLabelValue = "aerospike-cluster" AerospikeCustomResourceLabel = "aerospike.com/cr" AerospikeRackIDLabel = "aerospike.com/rack-id" AerospikeAPIVersionLabel = "aerospike.com/api-version" diff --git a/api/v1/zz_generated.deepcopy.go b/api/v1/zz_generated.deepcopy.go index d21f9c6b9..41e05a81f 100644 --- a/api/v1/zz_generated.deepcopy.go +++ b/api/v1/zz_generated.deepcopy.go @@ -159,6 +159,11 @@ func (in *AerospikeClusterSpec) DeepCopyInto(out *AerospikeClusterSpec) { *out = new(intstr.IntOrString) **out = **in } + if in.DisablePDB != nil { + in, out := &in.DisablePDB, &out.DisablePDB + *out = new(bool) + **out = **in + } in.Storage.DeepCopyInto(&out.Storage) if in.AerospikeAccessControl != nil { in, out := &in.AerospikeAccessControl, &out.AerospikeAccessControl @@ -241,6 +246,11 @@ func (in *AerospikeClusterStatusSpec) DeepCopyInto(out *AerospikeClusterStatusSp *out = new(intstr.IntOrString) **out = **in } + if in.DisablePDB != nil { + in, out := &in.DisablePDB, &out.DisablePDB + *out = new(bool) + **out = **in + } if in.MultiPodPerHost != nil { in, out := &in.MultiPodPerHost, &out.MultiPodPerHost *out = new(bool) diff --git a/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml b/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml index 6676d4989..f1fdbf581 100644 --- a/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml +++ b/config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml @@ -281,6 +281,10 @@ spec: - customInterface type: string type: object + disablePDB: + description: Disable the PodDisruptionBudget creation for the Aerospike + cluster. + type: boolean enableDynamicConfigUpdate: description: EnableDynamicConfigUpdate enables dynamic config update flow of the operator. If enabled, operator will try to update the @@ -9601,6 +9605,10 @@ spec: - customInterface type: string type: object + disablePDB: + description: Disable the PodDisruptionBudget creation for the Aerospike + cluster. + type: boolean enableDynamicConfigUpdate: description: EnableDynamicConfigUpdate enables dynamic config update flow of the operator. If enabled, operator will try to update the diff --git a/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml b/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml index 92a168efd..95b650fb0 100644 --- a/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml +++ b/config/manifests/bases/aerospike-kubernetes-operator.clusterserviceversion.yaml @@ -47,6 +47,9 @@ spec: the Aerospike cluster. displayName: Aerospike Network Policy path: aerospikeNetworkPolicy + - description: Disable the PodDisruptionBudget creation for the Aerospike cluster. + displayName: Disable PodDisruptionBudget + path: disablePDB - description: EnableDynamicConfigUpdate enables dynamic config update flow of the operator. If enabled, operator will try to update the Aerospike config dynamically. In case of inconsistent state during dynamic config update, diff --git a/config/rbac/role.yaml b/config/rbac/role.yaml index 4cd176f8d..a7d8fb265 100644 --- a/config/rbac/role.yaml +++ b/config/rbac/role.yaml @@ -117,6 +117,7 @@ rules: - poddisruptionbudgets verbs: - create + - delete - get - patch - update diff --git a/controllers/aerospikecluster_controller.go b/controllers/aerospikecluster_controller.go index 701e9b9a4..110d3032e 100644 --- a/controllers/aerospikecluster_controller.go +++ b/controllers/aerospikecluster_controller.go @@ -86,7 +86,7 @@ type RackState struct { // +kubebuilder:rbac:groups=core,resources=configmaps,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=core,resources=secrets,verbs=get // +kubebuilder:rbac:groups=apps,resources=statefulsets,verbs=get;list;watch;create;update;patch;delete -// +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;create;update;patch +// +kubebuilder:rbac:groups=policy,resources=poddisruptionbudgets,verbs=get;create;update;patch;delete //nolint:lll // marker // +kubebuilder:rbac:groups=asdb.aerospike.com,resources=aerospikeclusters,verbs=get;list;watch;create;update;patch;delete // +kubebuilder:rbac:groups=asdb.aerospike.com,resources=aerospikeclusters/status,verbs=get;update;patch diff --git a/controllers/poddistruptionbudget.go b/controllers/poddistruptionbudget.go index 4d0bb2065..20eafbd3c 100644 --- a/controllers/poddistruptionbudget.go +++ b/controllers/poddistruptionbudget.go @@ -14,6 +14,54 @@ import ( "github.com/aerospike/aerospike-kubernetes-operator/pkg/utils" ) +func (r *SingleClusterReconciler) reconcilePDB() error { + // If spec.DisablePDB is set to true, then we don't need to create PDB + // If it exist then delete it + if asdbv1.GetBool(r.aeroCluster.Spec.DisablePDB) { + if !asdbv1.GetBool(r.aeroCluster.Status.DisablePDB) { + r.Log.Info("PodDisruptionBudget is disabled. Deleting old PodDisruptionBudget") + return r.deletePDB() + } + + r.Log.Info("PodDisruptionBudget is disabled, skipping PodDisruptionBudget creation") + + return nil + } + + // Create or update PodDisruptionBudget + return r.createOrUpdatePDB() +} + +func (r *SingleClusterReconciler) deletePDB() error { + pdb := &v1.PodDisruptionBudget{} + + // Get the PodDisruptionBudget + if err := r.Client.Get( + context.TODO(), types.NamespacedName{ + Name: r.aeroCluster.Name, Namespace: r.aeroCluster.Namespace, + }, pdb, + ); err != nil { + if errors.IsNotFound(err) { + // PodDisruptionBudget is already deleted + return nil + } + + return err + } + + if !isPDBCreatedByOperator(pdb) { + r.Log.Info( + "PodDisruptionBudget is not created/owned by operator. Skipping delete", + "name", getPDBNamespacedName(r.aeroCluster), + ) + + return nil + } + + // Delete the PodDisruptionBudget + return r.Client.Delete(context.TODO(), pdb) +} + func (r *SingleClusterReconciler) createOrUpdatePDB() error { podList, err := r.getClusterPodList() if err != nil { @@ -48,7 +96,7 @@ func (r *SingleClusterReconciler) createOrUpdatePDB() error { return err } - r.Log.Info("Create PodDisruptionBudget") + r.Log.Info("Create PodDisruptionBudget", "name", getPDBNamespacedName(r.aeroCluster)) pdb.SetName(r.aeroCluster.Name) pdb.SetNamespace(r.aeroCluster.Namespace) @@ -75,17 +123,31 @@ func (r *SingleClusterReconciler) createOrUpdatePDB() error { ) } - r.Log.Info("Created new PodDisruptionBudget", "name", - utils.NamespacedName(r.aeroCluster.Namespace, r.aeroCluster.Name)) + r.Log.Info("Created new PodDisruptionBudget", "name", getPDBNamespacedName(r.aeroCluster)) return nil } r.Log.Info( - "PodDisruptionBudget already exist. Updating existing PodDisruptionBudget if required", "name", - utils.NamespacedName(r.aeroCluster.Namespace, r.aeroCluster.Name), + "PodDisruptionBudget already exist. Updating existing PodDisruptionBudget if required", + "name", getPDBNamespacedName(r.aeroCluster), ) + // This will ensure that cluster is not deployed with PDB created by user + // cluster deploy call itself will fail. + // If PDB is not created by operator then no need to even match the spec + if !isPDBCreatedByOperator(pdb) { + r.Log.Info( + "PodDisruptionBudget is not created/owned by operator. Skipping update", + "name", getPDBNamespacedName(r.aeroCluster), + ) + + return fmt.Errorf( + "failed to update PodDisruptionBudget, PodDisruptionBudget is not "+ + "created/owned by operator. name: %s", getPDBNamespacedName(r.aeroCluster), + ) + } + if pdb.Spec.MaxUnavailable.String() != r.aeroCluster.Spec.MaxUnavailable.String() { pdb.Spec.MaxUnavailable = r.aeroCluster.Spec.MaxUnavailable @@ -98,9 +160,21 @@ func (r *SingleClusterReconciler) createOrUpdatePDB() error { ) } - r.Log.Info("Updated PodDisruptionBudget", "name", - utils.NamespacedName(r.aeroCluster.Namespace, r.aeroCluster.Name)) + r.Log.Info("Updated PodDisruptionBudget", "name", getPDBNamespacedName(r.aeroCluster)) } return nil } + +func isPDBCreatedByOperator(pdb *v1.PodDisruptionBudget) bool { + val, ok := pdb.GetLabels()[asdbv1.AerospikeAppLabel] + if ok && val == asdbv1.AerospikeAppLabelValue { + return true + } + + return false +} + +func getPDBNamespacedName(aeroCluster *asdbv1.AerospikeCluster) types.NamespacedName { + return types.NamespacedName{Name: aeroCluster.Name, Namespace: aeroCluster.Namespace} +} diff --git a/controllers/reconciler.go b/controllers/reconciler.go index 0ae486729..e7329b8ec 100644 --- a/controllers/reconciler.go +++ b/controllers/reconciler.go @@ -138,11 +138,11 @@ func (r *SingleClusterReconciler) Reconcile() (result ctrl.Result, recErr error) return res.result, recErr } - if err := r.createOrUpdatePDB(); err != nil { - r.Log.Error(err, "Failed to create PodDisruptionBudget") + if err := r.reconcilePDB(); err != nil { + r.Log.Error(err, "Failed to reconcile PodDisruptionBudget") r.Recorder.Eventf( - r.aeroCluster, corev1.EventTypeWarning, "PodDisruptionBudgetCreateFailed", - "Failed to create PodDisruptionBudget %s/%s", + r.aeroCluster, corev1.EventTypeWarning, "PodDisruptionBudgetReconcileFailed", + "Failed to reconcile PodDisruptionBudget %s/%s", r.aeroCluster.Namespace, r.aeroCluster.Name, ) diff --git a/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml b/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml index 6676d4989..f1fdbf581 100644 --- a/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml +++ b/helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml @@ -281,6 +281,10 @@ spec: - customInterface type: string type: object + disablePDB: + description: Disable the PodDisruptionBudget creation for the Aerospike + cluster. + type: boolean enableDynamicConfigUpdate: description: EnableDynamicConfigUpdate enables dynamic config update flow of the operator. If enabled, operator will try to update the @@ -9601,6 +9605,10 @@ spec: - customInterface type: string type: object + disablePDB: + description: Disable the PodDisruptionBudget creation for the Aerospike + cluster. + type: boolean enableDynamicConfigUpdate: description: EnableDynamicConfigUpdate enables dynamic config update flow of the operator. If enabled, operator will try to update the diff --git a/helm-charts/aerospike-kubernetes-operator/templates/aerospike-operator-manager-clusterrole.yaml b/helm-charts/aerospike-kubernetes-operator/templates/aerospike-operator-manager-clusterrole.yaml index adf721767..05053cfbc 100644 --- a/helm-charts/aerospike-kubernetes-operator/templates/aerospike-operator-manager-clusterrole.yaml +++ b/helm-charts/aerospike-kubernetes-operator/templates/aerospike-operator-manager-clusterrole.yaml @@ -122,6 +122,7 @@ rules: - poddisruptionbudgets verbs: - create + - delete - get - patch - update diff --git a/test/poddisruptionbudget_test.go b/test/poddisruptionbudget_test.go index a864fbade..385b652d1 100644 --- a/test/poddisruptionbudget_test.go +++ b/test/poddisruptionbudget_test.go @@ -6,8 +6,11 @@ import ( . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" policyv1 "k8s.io/api/policy/v1" + "k8s.io/apimachinery/pkg/api/errors" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" "k8s.io/apimachinery/pkg/util/intstr" + "k8s.io/utils/ptr" asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1" ) @@ -16,7 +19,8 @@ var _ = Describe( "PodDisruptionBudget", func() { ctx := context.TODO() aeroCluster := &asdbv1.AerospikeCluster{} - maxAvailable := intstr.FromInt(0) + maxUnavailable := intstr.FromInt(0) + defaultMaxUnavailable := intstr.FromInt(1) clusterNamespacedName := getNamespacedName("pdb-test-cluster", namespace) BeforeEach(func() { @@ -27,37 +31,115 @@ var _ = Describe( AfterEach(func() { Expect(deleteCluster(k8sClient, ctx, aeroCluster)).NotTo(HaveOccurred()) + Expect(deletePDB(ctx, aeroCluster)).NotTo(HaveOccurred()) }) Context("Valid Operations", func() { It("Validate create PDB with default maxUnavailable", func() { err := deployCluster(k8sClient, ctx, aeroCluster) Expect(err).ToNot(HaveOccurred()) - validatePDB(ctx, aeroCluster, 1) + validatePDB(ctx, aeroCluster, defaultMaxUnavailable.IntValue()) }) It("Validate create PDB with specified maxUnavailable", func() { - aeroCluster.Spec.MaxUnavailable = &maxAvailable + aeroCluster.Spec.MaxUnavailable = &maxUnavailable err := deployCluster(k8sClient, ctx, aeroCluster) Expect(err).ToNot(HaveOccurred()) - validatePDB(ctx, aeroCluster, 0) + validatePDB(ctx, aeroCluster, maxUnavailable.IntValue()) }) It("Validate update PDB", func() { err := deployCluster(k8sClient, ctx, aeroCluster) Expect(err).ToNot(HaveOccurred()) - validatePDB(ctx, aeroCluster, 1) + validatePDB(ctx, aeroCluster, defaultMaxUnavailable.IntValue()) aeroCluster, err = getCluster(k8sClient, ctx, clusterNamespacedName) Expect(err).ToNot(HaveOccurred()) // Update maxUnavailable By("Update maxUnavailable to 0") - aeroCluster.Spec.MaxUnavailable = &maxAvailable + aeroCluster.Spec.MaxUnavailable = &maxUnavailable err = updateCluster(k8sClient, ctx, aeroCluster) Expect(err).ToNot(HaveOccurred()) - validatePDB(ctx, aeroCluster, 0) + validatePDB(ctx, aeroCluster, maxUnavailable.IntValue()) + }) + + It("Validate disablePDB, the Operator will not create PDB", func() { + aeroCluster.Spec.DisablePDB = ptr.To(true) + err := deployCluster(k8sClient, ctx, aeroCluster) + Expect(err).ToNot(HaveOccurred()) + + // Validate PDB is not created + _, err = getPDB(ctx, aeroCluster) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + pkgLog.Info("PDB not created as expected") + }) + + It("Validate update disablePDB, the Operator will delete and recreate PDB", func() { + By("Create cluster with PDB enabled") + err := deployCluster(k8sClient, ctx, aeroCluster) + Expect(err).ToNot(HaveOccurred()) + validatePDB(ctx, aeroCluster, defaultMaxUnavailable.IntValue()) + + By("Update disablePDB to true") + aeroCluster, err = getCluster(k8sClient, ctx, clusterNamespacedName) + Expect(err).ToNot(HaveOccurred()) + + aeroCluster.Spec.DisablePDB = ptr.To(true) + aeroCluster.Spec.MaxUnavailable = nil + err = updateCluster(k8sClient, ctx, aeroCluster) + Expect(err).ToNot(HaveOccurred()) + + _, err = getPDB(ctx, aeroCluster) + Expect(err).To(HaveOccurred()) + Expect(errors.IsNotFound(err)).To(BeTrue()) + pkgLog.Info("PDB deleted as expected") + + By("Update disablePDB to false") + aeroCluster, err = getCluster(k8sClient, ctx, clusterNamespacedName) + Expect(err).ToNot(HaveOccurred()) + + aeroCluster.Spec.DisablePDB = ptr.To(false) + err = updateCluster(k8sClient, ctx, aeroCluster) + Expect(err).ToNot(HaveOccurred()) + validatePDB(ctx, aeroCluster, defaultMaxUnavailable.IntValue()) + }) + + It("Validate that non-operator created PDB is not created", func() { + By("Create PDB") + err := createPDB(ctx, aeroCluster, defaultMaxUnavailable.IntValue()) + Expect(err).ToNot(HaveOccurred()) + + By("Create cluster. It should fail as PDB is already created") + // Create cluster should fail as PDB is not created by operator + err = deployCluster(k8sClient, ctx, aeroCluster) + Expect(err).To(HaveOccurred()) + + By("Delete PDB") + err = deletePDB(ctx, aeroCluster) + Expect(err).ToNot(HaveOccurred()) + + By("Wait for cluster to be created. It should pass as PDB is deleted") + // Create cluster should pass as PDB is deleted + err = waitForAerospikeCluster( + k8sClient, ctx, aeroCluster, int(aeroCluster.Spec.Size), retryInterval, + getTimeout(aeroCluster.Spec.Size), []asdbv1.AerospikeClusterPhase{asdbv1.AerospikeClusterCompleted}, + ) + Expect(err).ToNot(HaveOccurred()) + + validatePDB(ctx, aeroCluster, defaultMaxUnavailable.IntValue()) + }) + + It("Validate that cluster is deployed with disabledPDB even if non-operator created PDB is present", func() { + err := createPDB(ctx, aeroCluster, defaultMaxUnavailable.IntValue()) + Expect(err).ToNot(HaveOccurred()) + + // Create cluster with disabledPDB + aeroCluster.Spec.DisablePDB = ptr.To(true) + err = deployCluster(k8sClient, ctx, aeroCluster) + Expect(err).ToNot(HaveOccurred()) }) }) @@ -65,14 +147,24 @@ var _ = Describe( value := intstr.FromInt(3) It("Should fail if maxUnavailable is greater than size", func() { + // Cluster size is 2 aeroCluster.Spec.MaxUnavailable = &value err := deployCluster(k8sClient, ctx, aeroCluster) Expect(err).To(HaveOccurred()) }) It("Should fail if maxUnavailable is greater than RF", func() { - aeroCluster.Spec.Size = 3 - value := intstr.FromInt(3) + // PDB should be < (least rf)). rf is 2 in this test + aeroCluster.Spec.Size = 4 + value := intstr.FromInt(2) + aeroCluster.Spec.MaxUnavailable = &value + err := deployCluster(k8sClient, ctx, aeroCluster) + Expect(err).To(HaveOccurred()) + }) + + It("Should fail if maxUnavailable is given but disablePDB is true", func() { + aeroCluster.Spec.DisablePDB = ptr.To(true) + value := intstr.FromInt(1) aeroCluster.Spec.MaxUnavailable = &value err := deployCluster(k8sClient, ctx, aeroCluster) Expect(err).To(HaveOccurred()) @@ -81,17 +173,57 @@ var _ = Describe( }) func validatePDB(ctx context.Context, aerocluster *asdbv1.AerospikeCluster, expectedMaxUnavailable int) { - pdb := policyv1.PodDisruptionBudget{} - err := k8sClient.Get(ctx, types.NamespacedName{ - Namespace: aerocluster.Namespace, - Name: aerocluster.Name, - }, &pdb) + pdb, err := getPDB(ctx, aerocluster) Expect(err).ToNot(HaveOccurred()) // Validate PDB + pkgLog.Info("Found PDB", "pdb", pdb.Name, + "maxUnavailable", pdb.Spec.MaxUnavailable, + "expectedMaxUnavailable", expectedMaxUnavailable) + Expect(pdb.Spec.MaxUnavailable.IntValue()).To(Equal(expectedMaxUnavailable)) Expect(pdb.Status.ExpectedPods).To(Equal(aerocluster.Spec.Size)) Expect(pdb.Status.CurrentHealthy).To(Equal(aerocluster.Spec.Size)) Expect(pdb.Status.DisruptionsAllowed).To(Equal(int32(expectedMaxUnavailable))) Expect(pdb.Status.DesiredHealthy).To(Equal(aerocluster.Spec.Size - int32(expectedMaxUnavailable))) } + +func getPDB(ctx context.Context, aerocluster *asdbv1.AerospikeCluster) (*policyv1.PodDisruptionBudget, error) { + pdb := &policyv1.PodDisruptionBudget{} + err := k8sClient.Get(ctx, types.NamespacedName{ + Namespace: aerocluster.Namespace, + Name: aerocluster.Name, + }, pdb) + + return pdb, err +} + +func createPDB(ctx context.Context, aerocluster *asdbv1.AerospikeCluster, maxUnavailable int) error { + pdb := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: aerocluster.Name, + Namespace: aerocluster.Namespace, + }, + Spec: policyv1.PodDisruptionBudgetSpec{ + MaxUnavailable: &intstr.IntOrString{Type: intstr.Int, IntVal: int32(maxUnavailable)}, + }, + } + + return k8sClient.Create(ctx, pdb) +} + +func deletePDB(ctx context.Context, aerocluster *asdbv1.AerospikeCluster) error { + pdb := &policyv1.PodDisruptionBudget{ + ObjectMeta: metav1.ObjectMeta{ + Name: aerocluster.Name, + Namespace: aerocluster.Namespace, + }, + } + + err := k8sClient.Delete(ctx, pdb) + if err != nil && !errors.IsNotFound(err) { + return err + } + + return nil +}