Skip to content

Support Valkey upgrades on elasticache_global_replication_group #42636

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changelog/42636.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
```release-note:enhancement
resource/aws_elasticache_global_replication_group: Support `engine` attribute for Valkey
```

```release-note:bug
resource/aws_elasticache_replication_group: Correctly set Valkey engine on secondary replication groups using global datastores
```
22 changes: 22 additions & 0 deletions internal/service/elasticache/engine_version.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"github.com/YakDriver/regexache"
"github.com/aws/aws-sdk-go-v2/aws"
gversion "github.com/hashicorp/go-version"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/customdiff"
"github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema"
"github.com/hashicorp/terraform-provider-aws/names"
)
Expand Down Expand Up @@ -109,6 +110,22 @@ func customizeDiffEngineVersionForceNewOnDowngrade(_ context.Context, diff *sche
return engineVersionForceNewOnDowngrade(diff)
}

func customizeDiffEngineForceNewOnDowngrade() schema.CustomizeDiffFunc {
return customdiff.ForceNewIf(names.AttrEngine, func(_ context.Context, diff *schema.ResourceDiff, meta any) bool {
if _, is_global := diff.GetOk("global_replication_group_id"); is_global {
return false
}

if !diff.HasChange(names.AttrEngine) {
return false
}
if old, new := diff.GetChange(names.AttrEngine); old.(string) == engineRedis && new.(string) == engineValkey {
return false
}
return true
})
}

type getChangeDiffer interface {
Get(key string) any
GetChange(key string) (any, any)
Expand Down Expand Up @@ -151,12 +168,17 @@ func engineVersionIsDowngrade(diff getChangeDiffer) (bool, error) {
type forceNewDiffer interface {
Id() string
Get(key string) any
GetOk(key string) (any, bool)
GetChange(key string) (any, any)
HasChange(key string) bool
ForceNew(key string) error
}

func engineVersionForceNewOnDowngrade(diff forceNewDiffer) error {
if _, is_global := diff.GetOk("global_replication_group_id"); is_global {
return nil
}

if diff.Id() == "" || !diff.HasChange(names.AttrEngineVersion) {
return nil
}
Expand Down
4 changes: 4 additions & 0 deletions internal/service/elasticache/engine_version_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -513,6 +513,10 @@ func (d *mockForceNewDiffer) Get(key string) any {
return d.old
}

func (d *mockForceNewDiffer) GetOk(key string) (any, bool) {
return "", false
}

func (d *mockForceNewDiffer) HasChange(key string) bool {
return d.hasChange || d.old != d.new
}
Expand Down
2 changes: 1 addition & 1 deletion internal/service/elasticache/exports_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ var (
EngineVersionIsDowngrade = engineVersionIsDowngrade
GlobalReplicationGroupRegionPrefixFormat = globalReplicationGroupRegionPrefixFormat
NormalizeEngineVersion = normalizeEngineVersion
ParamGroupNameRequiresMajorVersionUpgrade = paramGroupNameRequiresMajorVersionUpgrade
ParamGroupNameRequiresMajorVersionUpgrade = paramGroupNameRequiresEngineOrMajorVersionUpgrade
ValidateClusterEngineVersion = validateClusterEngineVersion
ValidMemcachedVersionString = validMemcachedVersionString
ValidRedisVersionString = validRedisVersionString
Expand Down
68 changes: 53 additions & 15 deletions internal/service/elasticache/global_replication_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,8 +90,10 @@ func resourceGlobalReplicationGroup() *schema.Resource {
Computed: true,
},
names.AttrEngine: {
Type: schema.TypeString,
Computed: true,
Type: schema.TypeString,
Optional: true,
Computed: true,
ValidateFunc: validation.StringInSlice([]string{engineRedis, engineValkey}, true),
},
names.AttrEngineVersion: {
Type: schema.TypeString,
Expand Down Expand Up @@ -210,7 +212,8 @@ func resourceGlobalReplicationGroup() *schema.Resource {

CustomizeDiff: customdiff.All(
customizeDiffGlobalReplicationGroupEngineVersionErrorOnDowngrade,
customizeDiffGlobalReplicationGroupParamGroupNameRequiresMajorVersionUpgrade,
customizeDiffEngineForceNewOnDowngrade(),
customizeDiffGlobalReplicationGroupParamGroupNameRequiresEngineOrMajorVersionUpgrade,
customdiff.ComputedIf("global_node_groups", diffHasChange("num_node_groups")),
),
}
Expand All @@ -233,18 +236,23 @@ of the Global Replication Group and all Replication Group members. The AWS provi
Please use the "-replace" option on the terraform plan and apply commands (see https://www.terraform.io/cli/commands/plan#replace-address).`, diff.Id())
}

func customizeDiffGlobalReplicationGroupParamGroupNameRequiresMajorVersionUpgrade(_ context.Context, diff *schema.ResourceDiff, _ any) error {
return paramGroupNameRequiresMajorVersionUpgrade(diff)
func customizeDiffGlobalReplicationGroupParamGroupNameRequiresEngineOrMajorVersionUpgrade(_ context.Context, diff *schema.ResourceDiff, _ any) error {
return paramGroupNameRequiresEngineOrMajorVersionUpgrade(diff)
}

// parameter_group_name can only be set when doing a major update,
// but we also should allow it to stay set afterwards
func paramGroupNameRequiresMajorVersionUpgrade(diff sdkv2.ResourceDiffer) error {
func paramGroupNameRequiresEngineOrMajorVersionUpgrade(diff sdkv2.ResourceDiffer) error {
o, n := diff.GetChange(names.AttrParameterGroupName)
if o.(string) == n.(string) {
return nil
}

// param group must be able to change on Redis 7.1 to Valkey 7.2 upgrade
if diff.HasChange(names.AttrEngine) {
return nil
}

if diff.Id() == "" {
if !diff.HasChange(names.AttrEngineVersion) {
return errors.New("cannot change parameter group name without upgrading major engine version")
Expand All @@ -262,7 +270,7 @@ func paramGroupNameRequiresMajorVersionUpgrade(diff sdkv2.ResourceDiffer) error
if vDiff[0] == 0 && vDiff[1] == 0 {
return errors.New("cannot change parameter group name without upgrading major engine version")
}
if vDiff[0] != 1 {
if vDiff[0] == 0 {
return fmt.Errorf("cannot change parameter group name on minor engine version upgrade, upgrading from %s to %s", oldVersion.String(), newVersion.String())
}
}
Expand Down Expand Up @@ -318,9 +326,25 @@ func resourceGlobalReplicationGroupCreate(ctx context.Context, d *schema.Resourc
}
}

if v, ok := d.GetOk(names.AttrEngineVersion); ok {
if e, ok := d.GetOk(names.AttrEngine); ok {
if e.(string) == aws.ToString(globalReplicationGroup.Engine) {
log.Printf("[DEBUG] Not updating ElastiCache Global Replication Group (%s) engine: no change from %q", d.Id(), e)
} else {
version := d.Get(names.AttrEngineVersion).(string)
p := d.Get(names.AttrParameterGroupName).(string)
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(e.(string), version, p), names.AttrEngine, d.Timeout(schema.TimeoutUpdate)); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}
}
} else if v, ok := d.GetOk(names.AttrEngineVersion); ok {
requestedVersion, _ := normalizeEngineVersion(v.(string))

// backwards-compatibility; imply redis engine if just given engine version
engine, ok := d.GetOk(names.AttrEngine)
if !ok {
engine = engineRedis
}

engineVersion, err := gversion.NewVersion(aws.ToString(globalReplicationGroup.EngineVersion))
if err != nil {
return sdkdiag.AppendErrorf(diags, "updating ElastiCache Global Replication Group (%s) engine version on creation: error reading engine version: %s", d.Id(), err)
Expand All @@ -335,15 +359,15 @@ func resourceGlobalReplicationGroupCreate(ctx context.Context, d *schema.Resourc
p := d.Get(names.AttrParameterGroupName).(string)

if diff[0] == 1 {
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(v.(string), p), "engine version (major)", d.Timeout(schema.TimeoutCreate)); err != nil {
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(engine.(string), v.(string), p), "engine version (major)", d.Timeout(schema.TimeoutCreate)); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}
} else if diff[1] == 1 {
if p != "" {
return sdkdiag.AppendErrorf(diags, "cannot change parameter group name on minor engine version upgrade, upgrading from %s to %s", engineVersion.String(), requestedVersion.String())
}
if t, _ := regexp.MatchString(`[6-9]\.x`, v.(string)); !t {
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMinorUpdater(v.(string)), "engine version (minor)", d.Timeout(schema.TimeoutCreate)); err != nil {
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMinorUpdater(engine.(string), v.(string)), "engine version (minor)", d.Timeout(schema.TimeoutCreate)); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}
}
Expand Down Expand Up @@ -440,20 +464,32 @@ func resourceGlobalReplicationGroupUpdate(ctx context.Context, d *schema.Resourc
}
}

if d.HasChange(names.AttrEngineVersion) {
if d.HasChange(names.AttrEngine) {
engine := d.Get(names.AttrEngine).(string)
version := d.Get(names.AttrEngineVersion).(string)
p := d.Get(names.AttrParameterGroupName).(string)
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(engine, version, p), "engine version (major)", d.Timeout(schema.TimeoutUpdate)); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}
} else if d.HasChange(names.AttrEngineVersion) {
o, n := d.GetChange(names.AttrEngineVersion)

newVersion, _ := normalizeEngineVersion(n.(string))
oldVersion, _ := gversion.NewVersion(o.(string))
// backwards-compatibility; imply redis engine if just given engine version
engine, ok := d.GetOk(names.AttrEngine)
if !ok {
engine = engineRedis
}

diff := diffVersion(newVersion, oldVersion)
if diff[0] == 1 {
p := d.Get(names.AttrParameterGroupName).(string)
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(n.(string), p), "engine version (major)", d.Timeout(schema.TimeoutUpdate)); err != nil {
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMajorUpdater(engine.(string), n.(string), p), "engine version (major)", d.Timeout(schema.TimeoutUpdate)); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}
} else if diff[1] == 1 {
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMinorUpdater(n.(string)), "engine version (minor)", d.Timeout(schema.TimeoutUpdate)); err != nil {
if err := updateGlobalReplicationGroup(ctx, conn, d.Id(), globalReplicationGroupEngineVersionMinorUpdater(engine.(string), n.(string)), "engine version (minor)", d.Timeout(schema.TimeoutUpdate)); err != nil {
return sdkdiag.AppendFromErr(diags, err)
}
}
Expand Down Expand Up @@ -509,14 +545,16 @@ func globalReplicationGroupDescriptionUpdater(description string) globalReplicat
}
}

func globalReplicationGroupEngineVersionMinorUpdater(version string) globalReplicationGroupUpdater {
func globalReplicationGroupEngineVersionMinorUpdater(engine, version string) globalReplicationGroupUpdater {
return func(input *elasticache.ModifyGlobalReplicationGroupInput) {
input.Engine = aws.String(engine)
input.EngineVersion = aws.String(version)
}
}

func globalReplicationGroupEngineVersionMajorUpdater(version, paramGroupName string) globalReplicationGroupUpdater {
func globalReplicationGroupEngineVersionMajorUpdater(engine, version, paramGroupName string) globalReplicationGroupUpdater {
return func(input *elasticache.ModifyGlobalReplicationGroupInput) {
input.Engine = aws.String(engine)
input.EngineVersion = aws.String(version)
input.CacheParameterGroupName = aws.String(paramGroupName)
}
Expand Down
Loading
Loading