Skip to content

Commit

Permalink
Merge pull request #1985 from devopsmindset/fix/replication-group-eng…
Browse files Browse the repository at this point in the history
…ine-version

Fix(replicationgroup): Wrong engine version format
  • Loading branch information
MisterMX authored Jan 31, 2024
2 parents 63be74d + 877ba78 commit 23d7da1
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 2 deletions.
30 changes: 30 additions & 0 deletions pkg/controller/cache/replicationgroup/managed.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package replicationgroup
import (
"context"
"reflect"
"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 +63,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 @@ -191,6 +194,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 @@ -210,6 +214,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 @@ -229,6 +234,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 @@ -342,3 +355,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 @@ -37,14 +37,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 @@ -126,6 +127,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 @@ -736,6 +741,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 23d7da1

Please sign in to comment.