Skip to content

feat(elasticache): support inplace migration to valkey engine - #2293

Open
Kunalbehbud wants to merge 2 commits into
crossplane-contrib:masterfrom
Kunalbehbud:feat/elasticache-valkey-migration
Open

feat(elasticache): support inplace migration to valkey engine#2293
Kunalbehbud wants to merge 2 commits into
crossplane-contrib:masterfrom
Kunalbehbud:feat/elasticache-valkey-migration

Conversation

@Kunalbehbud

Copy link
Copy Markdown

Fixes #2290

This PR adds support for in-place ElastiCache ReplicationGroup migration from Redis to Valkey.

It makes spec.forProvider.engine mutable, updates the ElastiCache client logic to pass Engine through ModifyReplicationGroupInput, and extends update detection so engine changes are reconciled correctly. It also updates the AWS SDK dependency to a version that exposes the ElastiCache Valkey support needed by this flow.

Checklist

  • Added unit tests covering engine propagation and update detection
  • Added controller test verifying ModifyReplicationGroupInput includes Engine
  • Regenerated CRDs successfully
  • Ran make test successfully

@Kunalbehbud

Kunalbehbud commented Apr 15, 2026

Copy link
Copy Markdown
Author

Leaving a ping here in case this fell through the cracks. Happy to adjust the Valkey migration behavior or test coverage if there are concerns.

@ridik-il ridik-il left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Kunalbehbud — clean, well-scoped change and I appreciate the thorough test coverage (the dedicated Update test asserting Engine reaches ModifyReplicationGroup is exactly right, and updating all the cacheClusterNeedsUpdate fixtures was the correct call). I verified the whole provider builds against the bumped SDK and all elasticache tests pass, and confirmed removing +immutable is safe (it was advisory — no webhook/test enforced it — and engine is required so existing resources are unaffected). One thing to fix before merge: go.sum isn't tidy (go mod tidy drops 20 stale lines, including hashes for versions newer than what go.mod pins) — please run go mod tidy and commit. Optionally, consider an Enum=redis;valkey validation on the now-mutable engine field and a doc note that only redis→valkey migration is supported. Happy to approve once go.sum is sorted.

Signed-off-by: Kunalbehbud <b.kunal2002@gmail.com>
@Kunalbehbud
Kunalbehbud force-pushed the feat/elasticache-valkey-migration branch from 21a8256 to 1ed89a3 Compare July 14, 2026 11:42
@Kunalbehbud

Copy link
Copy Markdown
Author

Rebased onto master. The only conflict was in managed_test.go — a declaration collision with the LogDeliveryConfiguration/authTokenSecretRef helpers added in #2276, so I kept both sides and left the feature and the Engine migration test untouched. CI should be green again; ready to merge whenever convenient, and happy to rebase again if it drifts before you get to it.

- Only report an Engine change when one is explicitly requested, so an
  unset engine is not treated as drift (the resource would otherwise
  never be up to date).
- Set Engine on the cache-cluster fixtures and initialize the external
  cache in the Update test, both broken by the merge with the log
  delivery changes.
- go mod tidy to drop stale go.sum entries.

Signed-off-by: Kunalbehbud <b.kunal2002@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Inplace migrating from ReplicationGroup (ElastiCache) Engine "Redis" to "Valkey" not possible

2 participants