Skip to content

Commit

Permalink
fix unnecesary sort
Browse files Browse the repository at this point in the history
fix: engine version change on update

Signed-off-by: Domene Esteban, Nil <nildomenee@gmail.com>
  • Loading branch information
Domene Esteban, Nil {DISB~SANT CUGAT DIA} authored and nilde committed Jan 13, 2024
1 parent 9a50654 commit 63d4e92
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 3 deletions.
31 changes: 30 additions & 1 deletion pkg/controller/cache/replicationgroup/managed.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ package replicationgroup
import (
"context"
"reflect"
"sort"
"strconv"
"strings"

"github.com/aws/aws-sdk-go-v2/aws"
awselasticache "github.com/aws/aws-sdk-go-v2/service/elasticache"
Expand Down Expand Up @@ -61,6 +62,7 @@ const (
errUpdateReplicationGroupTags = "cannot update ElastiCache replication group tags"
errReplicationGroupCacheClusterMinimum = "at least 1 replica is required"
errReplicationGroupCacheClusterMaximum = "maximum of 5 replicas are allowed"
errVersionInput = "unable to parse version number"
)

// SetupReplicationGroup adds a controller that reconciles ReplicationGroups.
Expand Down Expand Up @@ -190,6 +192,7 @@ func (e *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
}

cr.Status.SetConditions(xpv1.Creating())

// Our create request will fail if auth is enabled but transit encryption is
// not. We don't check for the latter here because it's less surprising to
// submit the request as the operator intended and let the reconcile fail
Expand All @@ -209,6 +212,7 @@ func (e *external) Create(ctx context.Context, mg resource.Managed) (managed.Ext
}
if token != nil {
return managed.ExternalCreation{

ConnectionDetails: managed.ConnectionDetails{
xpv1.ResourceCredentialsSecretPasswordKey: []byte(*token),
},
Expand All @@ -228,6 +232,14 @@ func (e *external) Update(ctx context.Context, mg resource.Managed) (managed.Ext
return managed.ExternalUpdate{}, nil
}

// updates the engine version to the required format
var version *string
version, err := getVersion(cr.Spec.ForProvider.EngineVersion)
if err != nil {
return managed.ExternalUpdate{}, errorutils.Wrap(err, errVersionInput)
}
cr.Spec.ForProvider.EngineVersion = version

rsp, err := e.client.DescribeReplicationGroups(ctx, elasticache.NewDescribeReplicationGroupsInput(meta.GetExternalName(cr)))
if err != nil {
return managed.ExternalUpdate{}, errorutils.Wrap(err, errDescribeReplicationGroup)
Expand Down Expand Up @@ -369,3 +381,20 @@ func (e *external) updateReplicationGroupNumCacheClusters(ctx context.Context, r
return nil
}
}

func getVersion(version *string) (*string, error) {
versionSplit := strings.Split(aws.ToString(version), ".")
version1, err := strconv.Atoi(versionSplit[0])
if err != nil {
return nil, errors.Wrap(err, errVersionInput)
}
versionOut := strconv.Itoa(version1)
if len(versionSplit) > 1 {
version2, err := strconv.Atoi(versionSplit[1])
if err != nil {
return nil, errors.Wrap(err, errVersionInput)
}
versionOut += "." + strconv.Itoa(version2)
}
return &versionOut, nil
}
57 changes: 55 additions & 2 deletions pkg/controller/cache/replicationgroup/managed_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,15 @@ import (
)

const (
name = "coolGroup"
name = "coolGroup"
engineVersionToTest = "5.0.2"
)

var (
cacheNodeType = "n1.super.cool"
autoFailoverEnabled = true
cacheParameterGroupName = "coolParamGroup"
engineVersion = "5.0.0"
engineVersion = "5.0"
port = 6379
host = "172.16.0.1"
maintenanceWindow = "tomorrow"
Expand Down Expand Up @@ -140,6 +141,10 @@ func withNumCacheClusters(n int) replicationGroupModifier {
return func(r *v1beta1.ReplicationGroup) { r.Spec.ForProvider.NumCacheClusters = &n }
}

func withEngineVersion(e string) replicationGroupModifier {
return func(r *v1beta1.ReplicationGroup) { r.Spec.ForProvider.EngineVersion = &e }
}

func replicationGroup(rm ...replicationGroupModifier) *v1beta1.ReplicationGroup {
r := &v1beta1.ReplicationGroup{
ObjectMeta: objectMeta,
Expand Down Expand Up @@ -750,6 +755,54 @@ func TestUpdate(t *testing.T) {
),
returnsErr: false,
},
{
name: "IncreaseReplicationsAndCheckBehaviourVersion",
e: &external{client: &fake.MockClient{
MockDescribeReplicationGroups: func(ctx context.Context, _ *elasticache.DescribeReplicationGroupsInput, opts []func(*elasticache.Options)) (*elasticache.DescribeReplicationGroupsOutput, error) {
return &elasticache.DescribeReplicationGroupsOutput{
ReplicationGroups: []types.ReplicationGroup{{
Status: aws.String(v1beta1.StatusAvailable),
MemberClusters: cacheClusters,
AutomaticFailover: types.AutomaticFailoverStatusEnabled,
CacheNodeType: aws.String(cacheNodeType),
SnapshotRetentionLimit: aws.Int32(int32(snapshotRetentionLimit)),
SnapshotWindow: aws.String(snapshotWindow),
ClusterEnabled: aws.Bool(true),
ConfigurationEndpoint: &types.Endpoint{Address: aws.String(host), Port: int32(port)},
}},
}, nil
},
MockDescribeCacheClusters: func(ctx context.Context, _ *elasticache.DescribeCacheClustersInput, opts []func(*elasticache.Options)) (*elasticache.DescribeCacheClustersOutput, error) {
return &elasticache.DescribeCacheClustersOutput{
CacheClusters: []types.CacheCluster{
{EngineVersion: aws.String(engineVersion)},
{EngineVersion: aws.String(engineVersion)},
{EngineVersion: aws.String(engineVersion)},
},
}, nil
},
MockIncreaseReplicaCount: func(ctx context.Context, _ *elasticache.IncreaseReplicaCountInput, opts []func(*elasticache.Options)) (*elasticache.IncreaseReplicaCountOutput, error) {
return &elasticache.IncreaseReplicaCountOutput{}, nil
},
}},
r: replicationGroup(
withEngineVersion(engineVersionToTest),
withReplicationGroupID(name),
withProviderStatus(v1beta1.StatusAvailable),
withConditions(xpv1.Available()),
withMemberClusters(cacheClusters),
withNumCacheClusters(4),
),
want: replicationGroup(
withEngineVersion(engineVersion),
withReplicationGroupID(name),
withProviderStatus(v1beta1.StatusAvailable),
withConditions(xpv1.Available()),
withMemberClusters(cacheClusters),
withNumCacheClusters(4),
),
returnsErr: false,
},
}

for _, tc := range cases {
Expand Down

0 comments on commit 63d4e92

Please sign in to comment.