Skip to content

Commit

Permalink
addressing comments
Browse files Browse the repository at this point in the history
  • Loading branch information
tanmayja committed Feb 14, 2024
1 parent 5ebb6f2 commit b73f541
Show file tree
Hide file tree
Showing 14 changed files with 113 additions and 107 deletions.
3 changes: 1 addition & 2 deletions api/v1/aerospikecluster_mutating_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import (
"sigs.k8s.io/controller-runtime/pkg/webhook/admission"

"github.com/aerospike/aerospike-kubernetes-operator/pkg/merge"
lib "github.com/aerospike/aerospike-management-lib"
)

//nolint:lll // for readability
Expand Down Expand Up @@ -264,7 +263,7 @@ func (c *AerospikeCluster) updateRacksAerospikeConfigFromGlobal(asLog logr.Logge
)
} else {
// Use the global config.
m = lib.DeepCopy(c.Spec.AerospikeConfig.Value).(map[string]interface{})
m = c.Spec.AerospikeConfig.DeepCopy().Value
}

asLog.V(1).Info(
Expand Down
102 changes: 53 additions & 49 deletions api/v1/aerospikecluster_types.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
Copyright 2023.
Copyright 2024.
Licensed under the Apache License, Version 2.0 (the "License");
you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -867,95 +867,99 @@ func init() {
}

// CopySpecToStatus copy spec in status. Spec to Status DeepCopy doesn't work. It fails in reflect lib.
//
//nolint:dupl // not duplicate
func CopySpecToStatus(spec *AerospikeClusterSpec) (*AerospikeClusterStatusSpec, error) {
status := AerospikeClusterStatusSpec{}

status.Size = spec.Size
status.Image = spec.Image

// Storage
statusStorage := lib.DeepCopy(spec.Storage).(AerospikeStorageSpec)
statusStorage := lib.DeepCopy(&spec.Storage).(*AerospikeStorageSpec)

status.Storage = statusStorage
status.Storage = *statusStorage

if spec.AerospikeAccessControl != nil {
// AerospikeAccessControl
statusAerospikeAccessControl := lib.DeepCopy(
*spec.AerospikeAccessControl,
).(AerospikeAccessControlSpec)
spec.AerospikeAccessControl,
).(*AerospikeAccessControlSpec)

status.AerospikeAccessControl = &statusAerospikeAccessControl
status.AerospikeAccessControl = statusAerospikeAccessControl
}

if spec.AerospikeConfig != nil {
// AerospikeConfig
statusAerospikeConfig := lib.DeepCopy(
*spec.AerospikeConfig,
).(AerospikeConfigSpec)
spec.AerospikeConfig,
).(*AerospikeConfigSpec)

status.AerospikeConfig = &statusAerospikeConfig
status.AerospikeConfig = statusAerospikeConfig
}

if spec.ValidationPolicy != nil {
// ValidationPolicy
statusValidationPolicy := lib.DeepCopy(
*spec.ValidationPolicy,
).(ValidationPolicySpec)
spec.ValidationPolicy,
).(*ValidationPolicySpec)

status.ValidationPolicy = &statusValidationPolicy
status.ValidationPolicy = statusValidationPolicy
}

// RackConfig
statusRackConfig := lib.DeepCopy(spec.RackConfig).(RackConfig)
status.RackConfig = statusRackConfig
statusRackConfig := lib.DeepCopy(&spec.RackConfig).(*RackConfig)
status.RackConfig = *statusRackConfig

// AerospikeNetworkPolicy
statusAerospikeNetworkPolicy := lib.DeepCopy(
spec.AerospikeNetworkPolicy,
).(AerospikeNetworkPolicy)
&spec.AerospikeNetworkPolicy,
).(*AerospikeNetworkPolicy)

status.AerospikeNetworkPolicy = statusAerospikeNetworkPolicy
status.AerospikeNetworkPolicy = *statusAerospikeNetworkPolicy

if spec.OperatorClientCertSpec != nil {
clientCertSpec := lib.DeepCopy(
*spec.OperatorClientCertSpec,
).(AerospikeOperatorClientCertSpec)
spec.OperatorClientCertSpec,
).(*AerospikeOperatorClientCertSpec)

status.OperatorClientCertSpec = &clientCertSpec
status.OperatorClientCertSpec = clientCertSpec
}

// Storage
statusPodSpec := lib.DeepCopy(spec.PodSpec).(AerospikePodSpec)
status.PodSpec = statusPodSpec
statusPodSpec := lib.DeepCopy(&spec.PodSpec).(*AerospikePodSpec)
status.PodSpec = *statusPodSpec

seedsFinderServices := lib.DeepCopy(
spec.SeedsFinderServices,
).(SeedsFinderServices)
&spec.SeedsFinderServices,
).(*SeedsFinderServices)

status.SeedsFinderServices = seedsFinderServices
status.SeedsFinderServices = *seedsFinderServices

// RosterNodeBlockList
if len(spec.RosterNodeBlockList) != 0 {
rosterNodeBlockList := lib.DeepCopy(
spec.RosterNodeBlockList,
).([]string)
&spec.RosterNodeBlockList,
).(*[]string)

status.RosterNodeBlockList = rosterNodeBlockList
status.RosterNodeBlockList = *rosterNodeBlockList
}

return &status, nil
}

// CopyStatusToSpec copy status in spec. Status to Spec DeepCopy doesn't work. It fails in reflect lib.
//
//nolint:dupl // not duplicate
func CopyStatusToSpec(status *AerospikeClusterStatusSpec) (*AerospikeClusterSpec, error) {
spec := AerospikeClusterSpec{}

spec.Size = status.Size
spec.Image = status.Image

// Storage
specStorage := lib.DeepCopy(status.Storage).(AerospikeStorageSpec)
spec.Storage = specStorage
specStorage := lib.DeepCopy(&status.Storage).(*AerospikeStorageSpec)
spec.Storage = *specStorage

if status.AerospikeAccessControl != nil {
// AerospikeAccessControl
Expand All @@ -978,50 +982,50 @@ func CopyStatusToSpec(status *AerospikeClusterStatusSpec) (*AerospikeClusterSpec
if status.ValidationPolicy != nil {
// ValidationPolicy
specValidationPolicy := lib.DeepCopy(
*status.ValidationPolicy,
).(ValidationPolicySpec)
status.ValidationPolicy,
).(*ValidationPolicySpec)

spec.ValidationPolicy = &specValidationPolicy
spec.ValidationPolicy = specValidationPolicy
}

// RackConfig
specRackConfig := lib.DeepCopy(status.RackConfig).(RackConfig)
specRackConfig := lib.DeepCopy(&status.RackConfig).(*RackConfig)

spec.RackConfig = specRackConfig
spec.RackConfig = *specRackConfig

// AerospikeNetworkPolicy
specAerospikeNetworkPolicy := lib.DeepCopy(
status.AerospikeNetworkPolicy,
).(AerospikeNetworkPolicy)
&status.AerospikeNetworkPolicy,
).(*AerospikeNetworkPolicy)

spec.AerospikeNetworkPolicy = specAerospikeNetworkPolicy
spec.AerospikeNetworkPolicy = *specAerospikeNetworkPolicy

if status.OperatorClientCertSpec != nil {
clientCertSpec := lib.DeepCopy(
*status.OperatorClientCertSpec,
).(AerospikeOperatorClientCertSpec)
status.OperatorClientCertSpec,
).(*AerospikeOperatorClientCertSpec)

spec.OperatorClientCertSpec = &clientCertSpec
spec.OperatorClientCertSpec = clientCertSpec
}

// Storage
specPodSpec := lib.DeepCopy(status.PodSpec).(AerospikePodSpec)
specPodSpec := lib.DeepCopy(&status.PodSpec).(*AerospikePodSpec)

spec.PodSpec = specPodSpec
spec.PodSpec = *specPodSpec

seedsFinderServices := lib.DeepCopy(
status.SeedsFinderServices,
).(SeedsFinderServices)
&status.SeedsFinderServices,
).(*SeedsFinderServices)

spec.SeedsFinderServices = seedsFinderServices
spec.SeedsFinderServices = *seedsFinderServices

// RosterNodeBlockList
if len(status.RosterNodeBlockList) != 0 {
rosterNodeBlockList := lib.DeepCopy(
status.RosterNodeBlockList,
).([]string)
&status.RosterNodeBlockList,
).(*[]string)

spec.RosterNodeBlockList = rosterNodeBlockList
spec.RosterNodeBlockList = *rosterNodeBlockList
}

return &spec, nil
Expand Down
7 changes: 1 addition & 6 deletions api/v1/aerospikeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,5 @@ func (c *AerospikeConfigSpec) UnmarshalJSON(b []byte) error {
}

func (c *AerospikeConfigSpec) DeepCopy() *AerospikeConfigSpec {
dst := &AerospikeConfigSpec{
Value: map[string]interface{}{},
}
dst.Value = lib.DeepCopy(c.Value).(map[string]interface{})

return dst
return lib.DeepCopy(c).(*AerospikeConfigSpec)
}
7 changes: 1 addition & 6 deletions api/v1beta1/aerospikeconfig.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,5 @@ func (c *AerospikeConfigSpec) UnmarshalJSON(b []byte) error {
}

func (c *AerospikeConfigSpec) DeepCopy() *AerospikeConfigSpec {
dst := &AerospikeConfigSpec{
Value: map[string]interface{}{},
}
dst.Value = lib.DeepCopy(c.Value).(map[string]interface{})

return dst
return lib.DeepCopy(c).(*AerospikeConfigSpec)
}
3 changes: 1 addition & 2 deletions controllers/aero_info_calls.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/aerospike/aerospike-kubernetes-operator/pkg/jsonpatch"
"github.com/aerospike/aerospike-kubernetes-operator/pkg/utils"
"github.com/aerospike/aerospike-management-lib/asconfig"
libcommons "github.com/aerospike/aerospike-management-lib/commons"
"github.com/aerospike/aerospike-management-lib/deployment"
)

Expand Down Expand Up @@ -302,7 +301,7 @@ func (r *SingleClusterReconciler) setMigrateFillDelay(
}

func (r *SingleClusterReconciler) setDynamicConfig(
dynamicConfDiffPerPod map[string]libcommons.DynamicConfigMap, pods []*corev1.Pod, ignorablePodNames sets.Set[string],
dynamicConfDiffPerPod map[string]asconfig.DynamicConfigMap, pods []*corev1.Pod, ignorablePodNames sets.Set[string],
) reconcileResult {
// This doesn't make actual connection, only objects having connection info are created
allHostConns, err := r.newAllHostConnWithOption(ignorablePodNames)
Expand Down
6 changes: 3 additions & 3 deletions controllers/configmap.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,14 @@ func createPodSpecForRack(
aeroCluster *asdbv1.AerospikeCluster, rack *asdbv1.Rack,
) *asdbv1.AerospikePodSpec {
rackFullPodSpec := lib.DeepCopy(
aeroCluster.Spec.PodSpec,
).(asdbv1.AerospikePodSpec)
&aeroCluster.Spec.PodSpec,
).(*asdbv1.AerospikePodSpec)

rackFullPodSpec.Affinity = rack.PodSpec.Affinity
rackFullPodSpec.Tolerations = rack.PodSpec.Tolerations
rackFullPodSpec.NodeSelector = rack.PodSpec.NodeSelector

return &rackFullPodSpec
return rackFullPodSpec
}

func (r *SingleClusterReconciler) buildConfigTemplate(rack *asdbv1.Rack) (
Expand Down
44 changes: 28 additions & 16 deletions controllers/pod.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ import (
asdbv1 "github.com/aerospike/aerospike-kubernetes-operator/api/v1"
"github.com/aerospike/aerospike-kubernetes-operator/pkg/jsonpatch"
"github.com/aerospike/aerospike-kubernetes-operator/pkg/utils"
lib "github.com/aerospike/aerospike-management-lib"
"github.com/aerospike/aerospike-management-lib/asconfig"
libcommons "github.com/aerospike/aerospike-management-lib/commons"
)

// RestartType is the type of pod restart to use.
Expand Down Expand Up @@ -62,11 +62,11 @@ func mergeRestartType(current, incoming RestartType) RestartType {

// Fetching RestartType of all pods, based on the operation being performed.
func (r *SingleClusterReconciler) getRollingRestartTypeMap(rackState *RackState, ignorablePodNames sets.Set[string]) (
restartTypeMap map[string]RestartType, dynamicConfDiffPerPod map[string]libcommons.DynamicConfigMap, err error) {
restartTypeMap map[string]RestartType, dynamicConfDiffPerPod map[string]asconfig.DynamicConfigMap, err error) {
var addedNSDevices []string

restartTypeMap = make(map[string]RestartType)
dynamicConfDiffPerPod = make(map[string]libcommons.DynamicConfigMap)
dynamicConfDiffPerPod = make(map[string]asconfig.DynamicConfigMap)

pods, err := r.getOrderedRackPodList(rackState.Rack.ID)
if err != nil {
Expand Down Expand Up @@ -95,13 +95,30 @@ func (r *SingleClusterReconciler) getRollingRestartTypeMap(rackState *RackState,
}
}

// If dynamic commands have failed in previous retry, then we should not try to update config dynamically.
if !podStatus.DynamicConfigFailed {
// Fetching all dynamic config change.
dynamicConfDiffPerPod[pods[idx].Name], err = r.handleDynamicConfigChange(rackState, pods[idx])
if err != nil {
return nil, nil, err
serverContainer := getContainer(pods[idx].Spec.Containers, asdbv1.AerospikeServerContainerName)

version, err := asdbv1.GetImageVersion(serverContainer.Image)
if err != nil {
return nil, nil, err
}

v, err := lib.CompareVersions(version, "7.0.0")
if err != nil {
return nil, nil, err
}

// If version >= 7.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 {
// Fetching all dynamic config change.
dynamicConfDiffPerPod[pods[idx].Name], err = r.handleDynamicConfigChange(rackState, pods[idx], version)
if err != nil {
return nil, nil, err
}
}
} else {
r.Log.Info("Dynamic config change not supported for version < 7.0.0", "currentVersion", version)
}
}

Expand Down Expand Up @@ -1271,8 +1288,8 @@ func (r *SingleClusterReconciler) getConfigMap(rackID int) (*corev1.ConfigMap, e
return confMap, nil
}

func (r *SingleClusterReconciler) handleDynamicConfigChange(rackState *RackState, pod *corev1.Pod) (
libcommons.DynamicConfigMap, error) {
func (r *SingleClusterReconciler) handleDynamicConfigChange(rackState *RackState, pod *corev1.Pod, version string) (
asconfig.DynamicConfigMap, error) {
statusFromAnnotation := pod.Annotations["aerospikeConf"]

asConfStatus, err := getFlatConfig(r.Log, statusFromAnnotation)
Expand All @@ -1290,11 +1307,6 @@ func (r *SingleClusterReconciler) handleDynamicConfigChange(rackState *RackState
return nil, fmt.Errorf("failed to load config map by lib: %v", err)
}

version, err := asdbv1.GetImageVersion(r.aeroCluster.Spec.Image)
if err != nil {
return nil, err
}

specToStatusDiffs, err := asconfig.ConfDiff(r.Log, *asConfSpec, *asConfStatus,
true, version)
if err != nil {
Expand Down
Loading

0 comments on commit b73f541

Please sign in to comment.