Skip to content

Add GlobalReplicationGroup resource support - #237

Open
backlog-burndown-bot wants to merge 1 commit into
aws-controllers-k8s:mainfrom
backlog-burndown-bot:feat/1048-global-replication-group
Open

backlog-burndown-bot wants to merge 1 commit into
aws-controllers-k8s:mainfrom
backlog-burndown-bot:feat/1048-global-replication-group

Conversation

@backlog-burndown-bot

Copy link
Copy Markdown
Contributor

Issue #, if available: fixes aws-controllers-k8s/community#1048

Description of changes:

Adds the ElastiCache GlobalReplicationGroup resource, which backs the Global Datastore feature for cross-region replication. The configuration is declarative apart from what the API forces into hooks: ModifyGlobalReplicationGroup accepts only one logical change per request, deleting a datastore requires disassociating every secondary member first, and the Describe response carries no top-level automatic-failover field, so that value is derived from live member state (which is why ShowMemberInfo is set).

This supersedes #228, rebuilt on current main and incorporating that PR's review feedback. It is co-authored with @NicholasBlaskey, whose live-AWS testing established the API's one-change-per-request and engine-upgrade grouping rules that this implementation relies on.

Points a reviewer may want to weigh, called out rather than buried:

  • Shard-shrink policy needs your buy-in. DecreaseNodeGroupsInGlobalReplicationGroup requires naming which shards survive, and the issue does not specify a policy. This retains the lowest-ordered shard IDs, sorted so the choice does not depend on the order AWS returns them in. Deterministic and safe for data, but users cannot pick specific shards.
  • spec.cacheParameterGroupName is an engine-upgrade input, not observable state. ElastiCache copies it onto members and never reports it back on the datastore, so it is sent only alongside an engine-family or major-version upgrade and produces no delta of its own. A major upgrade without it is rejected terminally.
  • Shard-scaling coverage is unit tests, not e2e. The scaling paths are the slowest Global Datastore operations, and elasticache-kind-e2e is a required presubmit on every elasticache PR, so the scaling decisions are tested over the delta classifier instead.
  • Two paths are not covered by e2e. Secondary disassociation on delete needs a cross-region setup CI cannot provide. The major-upgrade-plus-parameter-group branch is unit-tested only, as it could not be validated against live AWS; the e2e engine-upgrade case is a minor upgrade.

Excluded on purpose, as pre-existing and outside this resource's scope: util.EngineVersionsMatch panics on an empty version string and is reached the same way from cache_cluster and replication_group, so it is guarded locally here rather than changed for all three; and primaryReplicationGroupRef does not inherit the CEL immutability rule that guards primaryReplicationGroupID, which is general ACK reference behaviour.

Agent-assisted: authored by an AI agent (backlog-burndown-bot) under human supervision — @gobos12

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Adds the ElastiCache Global Datastore resource, which backs cross-region
replication. The configuration is declarative apart from what the API forces
into hooks: ModifyGlobalReplicationGroup accepts only one logical change per
request, deleting a datastore requires disassociating every secondary member
first, and the Describe response carries no top-level automatic-failover
field, so that value is derived from member state.

Rebuilds the approach prototyped in aws-controllers-k8s#228
onto current main and incorporates the review findings from that PR.

Co-authored-by: NicholasBlaskey <31451191+NicholasBlaskey@users.noreply.github.com>
@ack-prow
ack-prow Bot requested review from jlbutler and sapphirew September 18, 2026 16:45
@ack-prow

ack-prow Bot commented Sep 18, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: backlog-burndown-bot
Once this PR has been reviewed and has the lgtm label, please assign knottnt for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ack-prow ack-prow Bot added the needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. label Sep 18, 2026
@ack-prow

ack-prow Bot commented Sep 18, 2026

Copy link
Copy Markdown

Hi @backlog-burndown-bot. Thanks for your PR.

I'm waiting for a aws-controllers-k8s member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work.

Tip

We noticed you've done this a few times! Consider joining the org to skip this step and gain /lgtm and other bot rights. We recommend asking approvers on your previous PRs to sponsor you.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Support GlobalReplicationGroup in elasticache-controller

1 participant