Blocking image upgrade along with on-demand operations

api/v1/aerospikecluster_mutating_webhook.go

@@ -100,12 +100,6 @@ func (c *AerospikeCluster) setDefaults(asLog logr.Logger) error {
 		return err
 	}
 
-	// Set defaults for user specified operations
-	// Populate operation id if not present
-	if err := setDefaultOperation(&c.Spec.Operations); err != nil {
-		return err
-	}
-
 	// Update racks configuration using global values where required.
 	if err := c.updateRacks(asLog); err != nil {
 		return err
@@ -855,18 +849,3 @@ func setNamespaceDefault(networks []string, namespace string) {
 		networks[idx] = netName
 	}
 }
-
-func setDefaultOperation(operations *[]OperationSpec) error {
-	for i := range *operations {
-		if (*operations)[i].ID == "" {
-			id, err := randomString(5)
-			if err != nil {
-				return err
-			}
-
-			(*operations)[i].ID = id
-		}
-	}
-
-	return nil
-}

api/v1/aerospikecluster_types.go

@@ -142,11 +142,11 @@ const (
 )
 
 type OperationSpec struct {
-	// OperationKind is the type of operation to be performed on the Aerospike cluster.
+	// Kind is the type of operation to be performed on the Aerospike cluster.
 	// +kubebuilder:validation:Enum=WarmRestart;PodRestart
 	Kind OperationKind `json:"kind"`
 	// +kubebuilder:validation:MaxLength=20
-	// +optional
+	// +kubebuilder:validation:MinLength=1
 	ID      string   `json:"id"`
 	PodList []string `json:"podList,omitempty"`
 }

api/v1/aerospikecluster_validating_webhook.go

@@ -118,7 +118,7 @@ func (c *AerospikeCluster) ValidateUpdate(oldObj runtime.Object) (admission.Warn
 	}
 
 	if err := validateOperationUpdate(
-		old.Spec.Operations, c.Spec.Operations,
+		&old.Spec, &c.Spec, &c.Status,
 	); err != nil {
 		return nil, err
 	}
@@ -283,20 +283,6 @@ func (c *AerospikeCluster) validateOperation() error {
 		return fmt.Errorf("operation cannot be added during aerospike cluster creation")
 	}
 
-	allPodNames := GetAllPodNames(c.Status.Pods)
-
-	podSet := sets.New(c.Spec.Operations[0].PodList...)
-	if !allPodNames.IsSuperset(podSet) {
-		return fmt.Errorf("invalid pod names in operation %v", podSet.Difference(allPodNames).UnsortedList())
-	}
-
-	// Don't allow any operation along with cluster scale up or racks added or removed
-	// New pods won't be available for operation
-	if !reflect.DeepEqual(c.Spec.Operations, c.Status.Operations) &&
-		(c.Spec.Size > c.Status.Size || len(c.Spec.RackConfig.Racks) != len(c.Status.RackConfig.Racks)) {
-		return fmt.Errorf("cannot perform any on-demand operation along with cluster scale-up or rack addition/removal")
-	}
-
 	return nil
 }
 
@@ -2399,16 +2385,47 @@ func (c *AerospikeCluster) validateEnableDynamicConfigUpdate() error {
 	return nil
 }
 
-func validateOperationUpdate(oldOps, newOps []OperationSpec) error {
-	if len(oldOps) == 0 || len(newOps) == 0 {
+func validateOperationUpdate(oldSpec, newSpec *AerospikeClusterSpec, status *AerospikeClusterStatus) error {
+	if len(newSpec.Operations) == 0 {
 		return nil
 	}
 
-	oldOp := oldOps[0]
-	newOp := newOps[0]
+	newOp := &newSpec.Operations[0]
+
+	var oldOp *OperationSpec
+
+	if len(oldSpec.Operations) != 0 {
+		oldOp = &oldSpec.Operations[0]
+	}
 
-	if oldOp.ID == newOp.ID && !reflect.DeepEqual(oldOp, newOp) {
-		return fmt.Errorf("operation %s cannot be updated", newOp.ID)
+	if !reflect.DeepEqual(oldOp, newOp) {
+		if oldOp != nil && oldOp.ID == newOp.ID {
+			return fmt.Errorf("operation %s cannot be updated", newOp.ID)
+		}
+	}
+
+	allPodNames := GetAllPodNames(status.Pods)
+
+	podSet := sets.New(newSpec.Operations[0].PodList...)
+	if !allPodNames.IsSuperset(podSet) {
+		return fmt.Errorf("invalid pod names in operation %v", podSet.Difference(allPodNames).UnsortedList())
+	}
+
+	// Don't allow any on-demand operation along with these cluster change:
+	// 1- scale up
+	// 2- racks added or removed
+	// 3- image update
+	// New pods won't be available for operation
+	if !reflect.DeepEqual(newSpec.Operations, status.Operations) {
+		switch {
+		case newSpec.Size > status.Size:
+			return fmt.Errorf("cannot perform any on-demand operation along with cluster scale-up")
+		case len(newSpec.RackConfig.Racks) != len(status.RackConfig.Racks) ||
+			len(newSpec.RackConfig.Racks) != len(oldSpec.RackConfig.Racks):
+			return fmt.Errorf("cannot perform any on-demand operation along with rack addition/removal")
+		case newSpec.Image != status.Image || newSpec.Image != oldSpec.Image:
+			return fmt.Errorf("cannot perform any on-demand operation along with image update")
+		}
 	}
 
 	return nil

api/v1/utils.go

@@ -1,10 +1,8 @@
 package v1
 
 import (
-	"crypto/rand"
 	"errors"
 	"fmt"
-	"math/big"
 	"os"
 	"regexp"
 	"strings"
@@ -599,21 +597,3 @@ func GetAllPodNames(pods map[string]AerospikePodStatus) sets.Set[string] {
 
 	return podNames
 }
-
-// charset contains the characters to use for the random string
-const charset = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ"
-
-// randomString generates a random string of length n
-func randomString(n int) (string, error) {
-	b := make([]byte, n)
-	for i := range b {
-		num, err := rand.Int(rand.Reader, big.NewInt(int64(len(charset))))
-		if err != nil {
-			return "", err
-		}
-
-		b[i] = charset[num.Int64()]
-	}
-
-	return string(b), nil
-}

config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml

@@ -319,10 +319,11 @@ spec:
                   properties:
                     id:
                       maxLength: 20
+                      minLength: 1
                       type: string
                     kind:
-                      description: OperationKind is the type of operation to be performed
-                        on the Aerospike cluster.
+                      description: Kind is the type of operation to be performed on
+                        the Aerospike cluster.
                       enum:
                       - WarmRestart
                       - PodRestart
@@ -332,6 +333,7 @@ spec:
                         type: string
                       type: array
                   required:
+                  - id
                   - kind
                   type: object
                 maxItems: 1
@@ -9682,10 +9684,11 @@ spec:
                   properties:
                     id:
                       maxLength: 20
+                      minLength: 1
                       type: string
                     kind:
-                      description: OperationKind is the type of operation to be performed
-                        on the Aerospike cluster.
+                      description: Kind is the type of operation to be performed on
+                        the Aerospike cluster.
                       enum:
                       - WarmRestart
                       - PodRestart
@@ -9695,6 +9698,7 @@ spec:
                         type: string
                       type: array
                   required:
+                  - id
                   - kind
                   type: object
                 type: array

controllers/pod.go

@@ -1536,22 +1536,36 @@ func (r *SingleClusterReconciler) updateOperationStatus(restartedASDPodNames, re
 			if len(statusOp.PodList) != 0 {
 				statusPods := sets.New(statusOp.PodList...)
 
-				if statusOp.Kind == asdbv1.OperationWarmRestart && quickRestartsSet != nil {
-					statusOp.PodList = statusPods.Union(quickRestartsSet.Intersection(specPods)).UnsortedList()
+				if statusOp.Kind == asdbv1.OperationWarmRestart {
+					if quickRestartsSet != nil {
+						statusOp.PodList = statusPods.Union(quickRestartsSet.Intersection(specPods)).UnsortedList()
+					}
+
+					if podRestartsSet != nil {
+						statusOp.PodList = statusPods.Union(podRestartsSet.Intersection(specPods)).UnsortedList()
+					}
 				}
 
 				if statusOp.Kind == asdbv1.OperationPodRestart && podRestartsSet != nil {
 					statusOp.PodList = statusPods.Union(podRestartsSet.Intersection(specPods)).UnsortedList()
 				}
 			}
+
+			break
 		}
 	}
 
 	if !opFound {
 		var podList []string
 
-		if specOp.Kind == asdbv1.OperationWarmRestart && quickRestartsSet != nil {
-			podList = quickRestartsSet.Intersection(specPods).UnsortedList()
+		if specOp.Kind == asdbv1.OperationWarmRestart {
+			if quickRestartsSet != nil {
+				podList = quickRestartsSet.Intersection(specPods).UnsortedList()
+			}
+
+			if podRestartsSet != nil {
+				podList = append(podList, podRestartsSet.Intersection(specPods).UnsortedList()...)
+			}
 		}
 
 		if specOp.Kind == asdbv1.OperationPodRestart && podRestartsSet != nil {

helm-charts/aerospike-kubernetes-operator/crds/customresourcedefinition_aerospikeclusters.asdb.aerospike.com.yaml

@@ -319,10 +319,11 @@ spec:
                   properties:
                     id:
                       maxLength: 20
+                      minLength: 1
                       type: string
                     kind:
-                      description: OperationKind is the type of operation to be performed
-                        on the Aerospike cluster.
+                      description: Kind is the type of operation to be performed on
+                        the Aerospike cluster.
                       enum:
                       - WarmRestart
                       - PodRestart
@@ -332,6 +333,7 @@ spec:
                         type: string
                       type: array
                   required:
+                  - id
                   - kind
                   type: object
                 maxItems: 1
@@ -9682,10 +9684,11 @@ spec:
                   properties:
                     id:
                       maxLength: 20
+                      minLength: 1
                       type: string
                     kind:
-                      description: OperationKind is the type of operation to be performed
-                        on the Aerospike cluster.
+                      description: Kind is the type of operation to be performed on
+                        the Aerospike cluster.
                       enum:
                       - WarmRestart
                       - PodRestart
@@ -9695,6 +9698,7 @@ spec:
                         type: string
                       type: array
                   required:
+                  - id
                   - kind
                   type: object
                 type: array