Skip to content

Commit

Permalink
addressing comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tanmayja committed Jun 26, 2024
1 parent ebcf676 commit 2b49acf
Show file tree
Hide file tree
Showing 13 changed files with 182 additions and 214 deletions.
4 changes: 2 additions & 2 deletions Jenkinsfile
Original file line number Diff line number Diff line change
Expand Up @@ -80,8 +80,8 @@ pipeline {
sh "./snyk-linux test --severity-threshold=high --fail-on=all"

// Scan the operator images
sh "./snyk-linux container test ${OPERATOR_CONTAINER_IMAGE_CANDIDATE_NAME} --severity-threshold=high --file=Dockerfile --policy-path=.snyk --fail-on=all"
sh "./snyk-linux container test ${OPERATOR_BUNDLE_IMAGE_CANDIDATE_NAME} --severity-threshold=high --file=Dockerfile --policy-path=.snyk --fail-on=all"
sh "./snyk-linux container test ${OPERATOR_CONTAINER_IMAGE_CANDIDATE_NAME} --severity-threshold=high --file=Dockerfile --policy-path=.snyk --fail-on=all"
sh "./snyk-linux container test ${OPERATOR_BUNDLE_IMAGE_CANDIDATE_NAME} --severity-threshold=high --file=Dockerfile --policy-path=.snyk --fail-on=all"
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions api/v1/aerospikecluster_mutating_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -858,13 +858,13 @@ func setNamespaceDefault(networks []string, namespace string) {

func setDefaultOperation(operations *[]OperationSpec) error {
for i := range *operations {
if (*operations)[i].OperationID == "" {
if (*operations)[i].ID == "" {
id, err := randomString(5)
if err != nil {
return err
}

(*operations)[i].OperationID = id
(*operations)[i].ID = id
}
}

Expand Down
37 changes: 19 additions & 18 deletions api/v1/aerospikecluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,30 +124,31 @@ type AerospikeClusterSpec struct { //nolint:govet // for readability
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Kubernetes Node BlockList"
// +kubebuilder:validation:MinItems:=1
K8sNodeBlockList []string `json:"k8sNodeBlockList,omitempty"`
// Operations is a list of on demand operation to be performed on the Aerospike cluster.
// Operations is a list of on-demand operations to be performed on the Aerospike cluster.
// +operator-sdk:csv:customresourcedefinitions:type=spec,displayName="Operations"
// +kubebuilder:validation:MaxItems:=1
Operations []OperationSpec `json:"operations,omitempty"`
}

type OperationType string
type OperationKind string

const (
// OperationQuickRestart specifies the on demand operation is quick restart of aerospike pods.
OperationQuickRestart OperationType = "quickRestart"
// OperationWarmRestart is the on-demand operation that leads to the warm restart of the aerospike pods
// (Restarting ASD in the pods). https://aerospike.com/docs/cloud/kubernetes/operator/Warm-restart
OperationWarmRestart OperationKind = "WarmRestart"

// OperationPodRestart specifies the on demand operation is cold restart of aerospike pods.
OperationPodRestart OperationType = "podRestart"
// OperationPodRestart is the on-demand operation that leads to the restart of aerospike pods.
OperationPodRestart OperationKind = "PodRestart"
)

type OperationSpec struct {
// OperationType is the type of operation to be performed on the Aerospike cluster.
// +kubebuilder:validation:Enum=quickRestart;podRestart
OperationType OperationType `json:"operationType"`
// +kubebuilder:validation:MaxLength=5
// +kubebuilder:validation:MinLength=5
// OperationKind 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
OperationID string `json:"operationID"`
PodList []string `json:"podList,omitempty"`
ID string `json:"id"`
PodList []string `json:"podList,omitempty"`
}

type SeedsFinderServices struct {
Expand Down Expand Up @@ -723,7 +724,7 @@ type AerospikeClusterStatusSpec struct { //nolint:govet // for readability
RosterNodeBlockList []string `json:"rosterNodeBlockList,omitempty"`
// K8sNodeBlockList is a list of Kubernetes nodes which are not used for Aerospike pods.
K8sNodeBlockList []string `json:"k8sNodeBlockList,omitempty"`
// Operations is a list of on demand operation to be performed on the Aerospike cluster.
// Operations is a list of on-demand operation to be performed on the Aerospike cluster.
Operations []OperationSpec `json:"operations,omitempty"`
}

Expand Down Expand Up @@ -1071,9 +1072,9 @@ func CopySpecToStatus(spec *AerospikeClusterSpec) (*AerospikeClusterStatusSpec,
}

if len(spec.Operations) != 0 {
operation := lib.DeepCopy(&spec.Operations).(*[]OperationSpec)
operations := lib.DeepCopy(&spec.Operations).(*[]OperationSpec)

status.Operations = *operation
status.Operations = *operations
}

return &status, nil
Expand Down Expand Up @@ -1176,8 +1177,8 @@ func CopyStatusToSpec(status *AerospikeClusterStatusSpec) (*AerospikeClusterSpec
}

if len(status.Operations) != 0 {
operation := lib.DeepCopy(&status.Operations).(*[]OperationSpec)
spec.Operations = *operation
operations := lib.DeepCopy(&status.Operations).(*[]OperationSpec)
spec.Operations = *operations
}

return &spec, nil
Expand Down
57 changes: 23 additions & 34 deletions api/v1/aerospikecluster_validating_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -280,25 +280,21 @@ func (c *AerospikeCluster) validateOperation() error {
}

if c.Status.AerospikeConfig == nil {
return fmt.Errorf("operation cannot be set on create")
return fmt.Errorf("operation cannot be added during aerospike cluster creation")
}

if len(c.Spec.Operations) > 1 {
return fmt.Errorf("only one operation can be set at a time")
}

allPodNames := GetAllPodNames(c.Name, c.Spec.Size, c.Spec.RackConfig.Racks)
allPodNames := GetAllPodNames(c.Status.Pods)

podSet := sets.NewString(c.Spec.Operations[0].PodList...)
if !allPodNames.IsSuperset(sets.Set[string](podSet)) {
return fmt.Errorf("invalid pod name in operation")
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 racks added or removed")
return fmt.Errorf("cannot perform any on-demand operation along with cluster scale-up or rack addition/removal")
}

return nil
Expand Down Expand Up @@ -1333,20 +1329,22 @@ func validateSecurityConfigUpdate(
func validateEnableSecurityConfig(newConfSpec, oldConfSpec *AerospikeConfigSpec) error {
newConf := newConfSpec.Value
oldConf := oldConfSpec.Value

oldSec, oldSecConfFound := oldConf["security"]
newSec, newSecConfFound := newConf["security"]
if !oldSecConfFound {
return nil
}

if oldSecConfFound && !newSecConfFound {
newSec, newSecConfFound := newConf["security"]
if !newSecConfFound {
return fmt.Errorf("cannot remove cluster security config")
}

if oldSecConfFound {
oldSecFlag, oldEnableSecurityFlagFound := oldSec.(map[string]interface{})["enable-security"]
newSecFlag, newEnableSecurityFlagFound := newSec.(map[string]interface{})["enable-security"]
oldSecFlag, oldEnableSecurityFlagFound := oldSec.(map[string]interface{})["enable-security"]
newSecFlag, newEnableSecurityFlagFound := newSec.(map[string]interface{})["enable-security"]

if oldEnableSecurityFlagFound && oldSecFlag.(bool) && (!newEnableSecurityFlagFound || !newSecFlag.(bool)) {
return fmt.Errorf("cannot disable cluster security in running cluster")
}
if oldEnableSecurityFlagFound && oldSecFlag.(bool) && (!newEnableSecurityFlagFound || !newSecFlag.(bool)) {
return fmt.Errorf("cannot disable cluster security in running cluster")
}

return nil
Expand Down Expand Up @@ -2401,25 +2399,16 @@ func (c *AerospikeCluster) validateEnableDynamicConfigUpdate() error {
return nil
}

func validateOperationUpdate(oldOp, newOp *[]OperationSpec) error {
// Define a key extractor function
keyExtractor := func(op OperationSpec) string {
return op.OperationID
func validateOperationUpdate(oldOps, newOps *[]OperationSpec) error {
if len(*oldOps) == 0 || len(*newOps) == 0 {
return nil
}

// Convert the array of structs to a map
oldOpMap, err := ConvertToMap(*oldOp, keyExtractor)
if err != nil {
return err
}
oldOp := (*oldOps)[0]
newOp := (*newOps)[0]

for idx := range *newOp {
key := (*newOp)[idx].OperationID
if _, ok := oldOpMap[key]; ok {
if !reflect.DeepEqual(oldOpMap[key], (*newOp)[idx]) {
return fmt.Errorf("operation %s cannot be updated", key)
}
}
if oldOp.ID == newOp.ID && !reflect.DeepEqual(oldOp, newOp) {
return fmt.Errorf("operation %s cannot be updated", newOp.ID)
}

return nil
Expand Down
27 changes: 3 additions & 24 deletions api/v1/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"math/big"
"os"
"regexp"
"strconv"
"strings"

"k8s.io/apimachinery/pkg/util/sets"
Expand Down Expand Up @@ -591,31 +590,11 @@ func DistributeItems(totalItems, totalGroups int) []int {
return topology
}

func ConvertToMap[T any](items []T, keyExtractor func(T) string) (map[string]T, error) {
itemMap := make(map[string]T)

for _, item := range items {
key := keyExtractor(item)
if _, ok := itemMap[key]; ok {
return nil, fmt.Errorf("duplicate key %s", key)
}

itemMap[key] = item
}

return itemMap, nil
}

func GetAllPodNames(clusterName string, clusterSize int32, racks []Rack) sets.Set[string] {
func GetAllPodNames(pods map[string]AerospikePodStatus) sets.Set[string] {
podNames := make(sets.Set[string])
topology := DistributeItems(
int(clusterSize), len(racks),
)

for idx := range racks {
for i := 0; i < topology[idx]; i++ {
podNames.Insert(fmt.Sprintf("%s-%s-%d", clusterName, strconv.Itoa(racks[idx].ID), i))
}
for podName := range pods {
podNames.Insert(podName)
}

return podNames
Expand Down
35 changes: 17 additions & 18 deletions config/crd/bases/asdb.aerospike.com_aerospikeclusters.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -313,28 +313,28 @@ spec:
Refer Aerospike documentation for more details.
x-kubernetes-int-or-string: true
operations:
description: Operations is a list of on demand operation to be performed
description: Operations is a list of on-demand operations to be performed
on the Aerospike cluster.
items:
properties:
operationID:
maxLength: 5
minLength: 5
id:
maxLength: 20
type: string
operationType:
description: OperationType is the type of operation to be performed
kind:
description: OperationKind is the type of operation to be performed
on the Aerospike cluster.
enum:
- quickRestart
- podRestart
- WarmRestart
- PodRestart
type: string
podList:
items:
type: string
type: array
required:
- operationType
- kind
type: object
maxItems: 1
type: array
operatorClientCert:
description: Certificates to connect to Aerospike.
Expand Down Expand Up @@ -9676,27 +9676,26 @@ spec:
now part of podSpec"
type: boolean
operations:
description: Operations is a list of on demand operation to be performed
description: Operations is a list of on-demand operation to be performed
on the Aerospike cluster.
items:
properties:
operationID:
maxLength: 5
minLength: 5
id:
maxLength: 20
type: string
operationType:
description: OperationType is the type of operation to be performed
kind:
description: OperationKind is the type of operation to be performed
on the Aerospike cluster.
enum:
- quickRestart
- podRestart
- WarmRestart
- PodRestart
type: string
podList:
items:
type: string
type: array
required:
- operationType
- kind
type: object
type: array
operatorClientCertSpec:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,8 @@ spec:
for more details.
displayName: Max Unavailable
path: maxUnavailable
- description: Operations is a list of on demand operation to be performed on
the Aerospike cluster.
- description: Operations is a list of on-demand operations to be performed
on the Aerospike cluster.
displayName: Operations
path: operations
- description: Certificates to connect to Aerospike.
Expand Down
Loading

0 comments on commit 2b49acf

Please sign in to comment.