addressing comments and adding testcases

api/v1/aerospikecluster_mutating_webhook.go

@@ -95,7 +95,7 @@ func (c *AerospikeCluster) setDefaults(asLog logr.Logger) error {
 
 	// Set common aerospikeConfig defaults
 	// Update configMap
-	if err := c.setDefaultAerospikeConfigs(asLog, *c.Spec.AerospikeConfig, DefaultRackID); err != nil {
+	if err := c.setDefaultAerospikeConfigs(asLog, *c.Spec.AerospikeConfig, nil); err != nil {
 		return err
 	}
 
@@ -280,7 +280,7 @@ func (c *AerospikeCluster) updateRacksAerospikeConfigFromGlobal(asLog logr.Logge
 
 		// Set defaults in updated rack config
 		// Above merge function may have overwritten defaults.
-		if err := c.setDefaultAerospikeConfigs(asLog, AerospikeConfigSpec{Value: m}, rack.ID); err != nil {
+		if err := c.setDefaultAerospikeConfigs(asLog, AerospikeConfigSpec{Value: m}, &rack.ID); err != nil {
 			return err
 		}
 
@@ -291,7 +291,7 @@ func (c *AerospikeCluster) updateRacksAerospikeConfigFromGlobal(asLog logr.Logge
 }
 
 func (c *AerospikeCluster) setDefaultAerospikeConfigs(asLog logr.Logger,
-	configSpec AerospikeConfigSpec, rackID int) error {
+	configSpec AerospikeConfigSpec, rackID *int) error {
 	config := configSpec.Value
 
 	// namespace conf
@@ -362,7 +362,8 @@ func (n *AerospikeNetworkPolicy) setNetworkNamespace(namespace string) {
 // Helper
 // *****************************************************************************
 
-func setDefaultNsConf(asLog logr.Logger, configSpec AerospikeConfigSpec, rackEnabledNsList []string, rackID int) error {
+func setDefaultNsConf(asLog logr.Logger, configSpec AerospikeConfigSpec,
+	rackEnabledNsList []string, rackID *int) error {
 	config := configSpec.Value
 	// namespace conf
 	nsConf, ok := config["namespaces"]
@@ -397,16 +398,28 @@ func setDefaultNsConf(asLog logr.Logger, configSpec AerospikeConfigSpec, rackEna
 		}
 
 		if nsName, ok := nsMap["name"]; ok {
-			if _, ok := nsName.(string); ok {
-				if isNameExist(rackEnabledNsList, nsName.(string)) {
-					nsMap["rack-id"] = rackID
+			if name, ok := nsName.(string); ok {
+				if isNameExist(rackEnabledNsList, name) {
+					// Add rack-id only for rackEnabled namespaces
+					if rackID != nil {
+						// Add rack-id only in rack specific config, not in global config
+						defaultConfs := map[string]interface{}{"rack-id": *rackID}
+						if err := setDefaultsInConfigMap(
+							asLog, nsMap, defaultConfs,
+						); err != nil {
+							return fmt.Errorf(
+								"failed to set default aerospikeConfig.namespaces rack config: %v",
+								err,
+							)
+						}
+					}
 				} else {
 					// User may have added this key or may have patched object with new smaller rackEnabledNamespace list
 					// but left namespace defaults. This key should be removed then only controller will detect
 					// that some namespace is removed from rackEnabledNamespace list and cluster needs rolling restart
 					asLog.Info(
 						"Name aerospikeConfig.namespaces.name not found in rackEnabled namespace list. "+
-							"Namespace will not have defaultRackID",
+							"Namespace will not have any rackID",
 						"nsName", nsName, "rackEnabledNamespaces",
 						rackEnabledNsList,
 					)

api/v1/utils.go

@@ -63,7 +63,7 @@ const (
 	AerospikeInitContainerRegistryEnvVar           = "AEROSPIKE_KUBERNETES_INIT_REGISTRY"
 	AerospikeInitContainerDefaultRegistry          = "docker.io"
 	AerospikeInitContainerDefaultRegistryNamespace = "tanmayj10"
-	AerospikeInitContainerDefaultRepoAndTag        = "aerospike-kubernetes-init:2.1.0-dev1"
+	AerospikeInitContainerDefaultRepoAndTag        = "aerospike-kubernetes-init:2.1.0-dev3"
 	AerospikeAppLabel                              = "app"
 	AerospikeCustomResourceLabel                   = "aerospike.com/cr"
 	AerospikeRackIDLabel                           = "aerospike.com/rack-id"

controllers/aero_info_calls.go

@@ -346,7 +346,7 @@ func (r *SingleClusterReconciler) setDynamicConfig(
 			return reconcileError(err)
 		}
 
-		r.Log.Info("Generated dynamic commands", "commands", fmt.Sprintf("%v", asConfCmds), "pod", podName)
+		r.Log.Info("Generated dynamic config commands", "commands", fmt.Sprintf("%v", asConfCmds), "pod", podName)
 
 		if err := deployment.SetConfigCommandsOnHosts(r.Log, r.getClientPolicy(), allHostConns,
 			[]*deployment.HostConn{host}, asConfCmds); err != nil {
@@ -362,13 +362,13 @@ func (r *SingleClusterReconciler) setDynamicConfig(
 			if patchErr := r.patchPodStatus(
 				context.TODO(), patches,
 			); patchErr != nil {
-				return reconcileError(fmt.Errorf("error updating status: %v", patchErr))
+				return reconcileError(fmt.Errorf("error updating status: %v, dynamic config command error: %v", patchErr, err))
 			}
 
 			return reconcileError(err)
 		}
 
-		if err := r.updatePod(podName); err != nil {
+		if err := r.updateAerospikeConfInPod(podName); err != nil {
 			return reconcileError(err)
 		}
 	}

controllers/pod.go

@@ -112,12 +112,12 @@ func (r *SingleClusterReconciler) getRollingRestartTypeMap(rackState *RackState,
 				return nil, nil, err
 			}
 
-			v, err := lib.CompareVersions(version, "7.0.0")
+			v, err := lib.CompareVersions(version, "6.0.0")
 			if err != nil {
 				return nil, nil, err
 			}
 
-			// If version >= 7.0.0, then we can update config dynamically.
+			// If version >= 6.0.0, then we can update config dynamically.
 			if v >= 0 {
 				// If dynamic commands have failed in previous retry, then we should not try to update config dynamically.
 				if !podStatus.DynamicConfigFailed {
@@ -128,7 +128,7 @@ func (r *SingleClusterReconciler) getRollingRestartTypeMap(rackState *RackState,
 					}
 				}
 			} else {
-				r.Log.Info("Dynamic config change not supported for version < 7.0.0", "currentVersion", version)
+				r.Log.Info("Dynamic config change not supported for version < 6.0.0", "currentVersion", version)
 			}
 		}
 
@@ -394,14 +394,14 @@ func (r *SingleClusterReconciler) restartPods(
 	return reconcileSuccess()
 }
 
-func (r *SingleClusterReconciler) updatePod(podName string) error {
-	r.Log.Info("Updating pod", "pod", podName)
+func (r *SingleClusterReconciler) updateAerospikeConfInPod(podName string) error {
+	r.Log.Info("Updating aerospike config file in pod", "pod", podName)
 
 	if err := r.restartASDOrUpdateAerospikeConf(podName, noRestartUpdateConf); err != nil {
 		return err
 	}
 
-	r.Log.V(1).Info("Pod Updated", "podName", podName)
+	r.Log.V(1).Info("Updated aerospike config file in pod", "podName", podName)
 
 	return nil
 }

controllers/rack.go

@@ -397,14 +397,13 @@ func (r *SingleClusterReconciler) upgradeOrRollingRestartRack(found *appsv1.Stat
 			return found, res
 		}
 	} else {
-		var needRollingRestartRack, needDynamicUpdateRack, restartTypeMap,
-			dynamicConfDiffPerPod, nErr = r.needRollingRestartRack(rackState, ignorablePodNames)
+		var rollingRestartInfo, nErr = r.getRollingRestartInfo(rackState, ignorablePodNames)
 		if nErr != nil {
 			return found, reconcileError(nErr)
 		}
 
-		if needRollingRestartRack {
-			found, res = r.rollingRestartRack(found, rackState, ignorablePodNames, restartTypeMap, failedPods)
+		if rollingRestartInfo.needRestart {
+			found, res = r.rollingRestartRack(found, rackState, ignorablePodNames, rollingRestartInfo.restartTypeMap, failedPods)
 			if !res.isSuccess {
 				if res.err != nil {
 					r.Log.Error(
@@ -424,8 +423,9 @@ func (r *SingleClusterReconciler) upgradeOrRollingRestartRack(found *appsv1.Stat
 			}
 		}
 
-		if len(failedPods) == 0 && needDynamicUpdateRack {
-			res = r.updateDynamicConfig(rackState, ignorablePodNames, restartTypeMap, dynamicConfDiffPerPod)
+		if len(failedPods) == 0 && rollingRestartInfo.needUpdateConf {
+			res = r.updateDynamicConfig(rackState, ignorablePodNames,
+				rollingRestartInfo.restartTypeMap, rollingRestartInfo.dynamicConfDiffPerPod)
 			if !res.isSuccess {
 				if res.err != nil {
 					r.Log.Error(
@@ -469,8 +469,8 @@ func (r *SingleClusterReconciler) updateDynamicConfig(rackState *RackState,
 	r.Log.Info("Update dynamic config in Aerospike pods")
 
 	r.Recorder.Eventf(
-		r.aeroCluster, corev1.EventTypeNormal, "RackDynamicUpdate",
-		"[rack-%d] Started Dynamic update", rackState.Rack.ID,
+		r.aeroCluster, corev1.EventTypeNormal, "DynamicConfigUpdate",
+		"[rack-%d] Started dynamic config update", rackState.Rack.ID,
 	)
 
 	var (
@@ -504,8 +504,8 @@ func (r *SingleClusterReconciler) updateDynamicConfig(rackState *RackState,
 	}
 
 	r.Recorder.Eventf(
-		r.aeroCluster, corev1.EventTypeNormal, "RackDynamicUpdate",
-		"[rack-%d] Finished Dynamic update", rackState.Rack.ID,
+		r.aeroCluster, corev1.EventTypeNormal, "DynamicConfigUpdate",
+		"[rack-%d] Finished Dynamic config update", rackState.Rack.ID,
 	)
 
 	return reconcileSuccess()
@@ -1215,15 +1215,22 @@ func (r *SingleClusterReconciler) handleK8sNodeBlockListPods(statefulSet *appsv1
 	return statefulSet, reconcileSuccess()
 }
 
-func (r *SingleClusterReconciler) needRollingRestartRack(rackState *RackState, ignorablePodNames sets.Set[string]) (
-	needRestart, needUpdateConf bool, restartTypeMap map[string]RestartType,
-	dynamicConfDiffPerPod map[string]asconfig.DynamicConfigMap, err error,
+type rollingRestartInfo struct {
+	restartTypeMap              map[string]RestartType
+	dynamicConfDiffPerPod       map[string]asconfig.DynamicConfigMap
+	needRestart, needUpdateConf bool
+}
+
+func (r *SingleClusterReconciler) getRollingRestartInfo(rackState *RackState, ignorablePodNames sets.Set[string]) (
+	info *rollingRestartInfo, err error,
 ) {
-	restartTypeMap, dynamicConfDiffPerPod, err = r.getRollingRestartTypeMap(rackState, ignorablePodNames)
+	restartTypeMap, dynamicConfDiffPerPod, err := r.getRollingRestartTypeMap(rackState, ignorablePodNames)
 	if err != nil {
-		return needRestart, needUpdateConf, nil, nil, err
+		return nil, err
 	}
 
+	needRestart, needUpdateConf := false, false
+
 	for _, restartType := range restartTypeMap {
 		switch restartType {
 		case noRestart:
@@ -1235,7 +1242,14 @@ func (r *SingleClusterReconciler) needRollingRestartRack(rackState *RackState, i
 		}
 	}
 
-	return needRestart, needUpdateConf, restartTypeMap, dynamicConfDiffPerPod, nil
+	info = &rollingRestartInfo{
+		needRestart:           needRestart,
+		needUpdateConf:        needUpdateConf,
+		restartTypeMap:        restartTypeMap,
+		dynamicConfDiffPerPod: dynamicConfDiffPerPod,
+	}
+
+	return info, nil
 }
 
 func (r *SingleClusterReconciler) isRackUpgradeNeeded(rackID int, ignorablePodNames sets.Set[string]) (

controllers/reconciler.go

@@ -463,10 +463,13 @@ func (r *SingleClusterReconciler) updateAccessControlStatus() error {
 		return err
 	}
 
-	// AerospikeAccessControl
-	statusAerospikeAccessControl := lib.DeepCopy(
-		r.aeroCluster.Spec.AerospikeAccessControl,
-	).(*asdbv1.AerospikeAccessControlSpec)
+	var statusAerospikeAccessControl *asdbv1.AerospikeAccessControlSpec
+	if r.aeroCluster.Spec.AerospikeAccessControl != nil {
+		// AerospikeAccessControl
+		statusAerospikeAccessControl = lib.DeepCopy(
+			r.aeroCluster.Spec.AerospikeAccessControl,
+		).(*asdbv1.AerospikeAccessControlSpec)
+	}
 
 	newAeroCluster.Status.AerospikeClusterStatusSpec.AerospikeAccessControl = statusAerospikeAccessControl
 

go.mod

@@ -5,7 +5,7 @@ go 1.21
 toolchain go1.21.8
 
 require (
-	github.com/aerospike/aerospike-management-lib v1.2.1-0.20240325134810-f8046fe9872e
+	github.com/aerospike/aerospike-management-lib v1.3.1-0.20240404063536-2adfbedf9687
 	github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d
 	github.com/evanphx/json-patch v4.12.0+incompatible
 	github.com/go-logr/logr v1.3.0

go.sum

@@ -2,6 +2,8 @@ github.com/aerospike/aerospike-client-go/v7 v7.1.0 h1:yvCTKdbpqZxHvv7sWsFHV1j49j
 github.com/aerospike/aerospike-client-go/v7 v7.1.0/go.mod h1:AkHiKvCbqa1c16gCNGju3c5X/yzwLVvblNczqjxNwNk=
 github.com/aerospike/aerospike-management-lib v1.2.1-0.20240325134810-f8046fe9872e h1:Q/AfYe++0ouO5csLS8l99kCQqJJvDKlfHwhuWbECpaQ=
 github.com/aerospike/aerospike-management-lib v1.2.1-0.20240325134810-f8046fe9872e/go.mod h1:E4dk798IikCp9a8fugpYoeQVIXuvdxogHvt6sKhaORQ=
+github.com/aerospike/aerospike-management-lib v1.3.1-0.20240404063536-2adfbedf9687 h1:d7oDvHmiKhq4rzcD/w3z9tP3wH0+iaDvxKDk3IYuqeU=
+github.com/aerospike/aerospike-management-lib v1.3.1-0.20240404063536-2adfbedf9687/go.mod h1:E4dk798IikCp9a8fugpYoeQVIXuvdxogHvt6sKhaORQ=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
 github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
 github.com/asaskevich/govalidator v0.0.0-20210307081110-f21760c49a8d h1:Byv0BzEl3/e6D5CLfI0j/7hiIEtvGVFPCZ7Ei2oq8iQ=

test/cluster_test.go

@@ -74,9 +74,68 @@ var _ = Describe(
 				ScaleDownWithMigrateFillDelay(ctx)
 			},
 		)
+		Context(
+			"UpdateClusterPre600", func() {
+				UpdateClusterPre600(ctx)
+			},
+		)
 	},
 )
 
+func UpdateClusterPre600(ctx goctx.Context) {
+	Context(
+		"UpdateClusterPre600", func() {
+			clusterNamespacedName := getNamespacedName(
+				"deploy-cluster-pre6", namespace,
+			)
+
+			BeforeEach(
+				func() {
+					image := fmt.Sprintf(
+						"aerospike/aerospike-server-enterprise:%s", pre6Version,
+					)
+					aeroCluster, err := getAeroClusterConfig(
+						clusterNamespacedName, image,
+					)
+					Expect(err).ToNot(HaveOccurred())
+
+					err = deployCluster(k8sClient, ctx, aeroCluster)
+					Expect(err).ToNot(HaveOccurred())
+				},
+			)
+
+			AfterEach(
+				func() {
+					aeroCluster, err := getCluster(
+						k8sClient, ctx, clusterNamespacedName,
+					)
+					Expect(err).ToNot(HaveOccurred())
+
+					_ = deleteCluster(k8sClient, ctx, aeroCluster)
+				},
+			)
+
+			It(
+				"UpdateReplicationFactor: should fail for updating namespace replication-factor on server"+
+					"before 6.0.0. Cannot be updated", func() {
+					aeroCluster, err := getCluster(k8sClient, ctx, clusterNamespacedName)
+					Expect(err).ToNot(HaveOccurred())
+
+					namespaceConfig :=
+						aeroCluster.Spec.AerospikeConfig.Value["namespaces"].([]interface{})[0].(map[string]interface{})
+					namespaceConfig["replication-factor"] = 5
+					aeroCluster.Spec.AerospikeConfig.Value["namespaces"].([]interface{})[0] = namespaceConfig
+
+					err = k8sClient.Update(
+						ctx, aeroCluster,
+					)
+					Expect(err).Should(HaveOccurred())
+				},
+			)
+		},
+	)
+}
+
 func ScaleDownWithMigrateFillDelay(ctx goctx.Context) {
 	Context(
 		"ScaleDownWithMigrateFillDelay", func() {
@@ -920,6 +979,24 @@ func UpdateClusterTest(ctx goctx.Context) {
 					)
 					Expect(err).ToNot(HaveOccurred())
 
+					By("UpdateReplicationFactor: should update namespace replication-factor on non-SC namespace")
+
+					aeroCluster, err := getCluster(
+						k8sClient, ctx,
+						clusterNamespacedName,
+					)
+					Expect(err).ToNot(HaveOccurred())
+					nsList := aeroCluster.Spec.AerospikeConfig.Value["namespaces"].([]interface{})
+					namespaceConfig := nsList[len(nsList)-1].(map[string]interface{})
+					// aeroCluster.Spec.AerospikeConfig.Value["namespaces"].([]interface{})[0].(map[string]interface{})
+					namespaceConfig["replication-factor"] = 3
+					aeroCluster.Spec.AerospikeConfig.Value["namespaces"].([]interface{})[len(nsList)-1] = namespaceConfig
+
+					err = k8sClient.Update(
+						ctx, aeroCluster,
+					)
+					Expect(err).ToNot(HaveOccurred())
+
 					By("Scaling up along with modifying Namespace storage Dynamically")
 
 					err = scaleUpClusterTestWithNSDeviceHandling(
@@ -1067,7 +1144,8 @@ func UpdateClusterTest(ctx goctx.Context) {
 							Context(
 								"Namespace", func() {
 									It(
-										"UpdateReplicationFactor: should fail for updating namespace replication-factor. Cannot be updated",
+										"UpdateReplicationFactor: should fail for updating namespace"+
+											"replication-factor on SC namespace. Cannot be updated",
 										func() {
 											aeroCluster, err := getCluster(
 												k8sClient, ctx,

test/services_test.go

@@ -129,9 +129,7 @@ func createLoadBalancer() *asdbv1.LoadBalancerSpec {
 		),
 	)
 
-	result := lib.DeepCopy(&lb).(*asdbv1.LoadBalancerSpec)
-
-	return result
+	return lib.DeepCopy(&lb).(*asdbv1.LoadBalancerSpec)
 }
 
 func loadBalancerName(aeroCluster *asdbv1.AerospikeCluster) types.NamespacedName {