[KO-321] [KO-320] Added a flag to disable PDB (#286)

* Added a flag to disable PDB

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
 	}

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

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 {

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"

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

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,

config/rbac/role.yaml

@@ -117,6 +117,7 @@ rules:
   - poddisruptionbudgets
   verbs:
   - create
+  - delete
   - get
   - patch
   - update

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

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

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,
 		)
 

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

helm-charts/aerospike-kubernetes-operator/templates/aerospike-operator-manager-clusterrole.yaml

@@ -122,6 +122,7 @@ rules:
   - poddisruptionbudgets
   verbs:
   - create
+  - delete
   - get
   - patch
   - update