Add serverlesscache to elasticache #175

rushmash91 · 2025-07-17T22:03:10Z

fixes aws-controllers-k8s/community#2522

Description of changes:

Add support serverlesscache resource

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

ack-prow · 2025-07-17T22:03:20Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: rushmash91

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

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [rushmash91]

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

rushmash91 · 2025-07-18T00:12:20Z

/test elasticache-kind-e2e

rushmash91 · 2025-07-18T18:10:47Z

unrelated test..
/test elasticache-kind-e2e

apis/v1alpha1/generator.yaml

michaelhtm · 2025-07-21T21:29:00Z

apis/v1alpha1/generator.yaml

+    synced:
+      when:
+      - path: Status.Status
+        in:
+        - available


The status here should be capitalized: https://github.com/aws/aws-sdk-go-v2/blob/0475b822f1f48ca746369818d51272f989c06b91/service/elasticache/types/types.go#L2019

Also, should CREATE_FAILED be a synced/terminal condition?

michaelhtm · 2025-07-21T21:32:31Z

apis/v1alpha1/generator.yaml

+        - ServerlessCacheAlreadyExistsFault
+        - ServerlessCacheQuotaForCustomerExceededFault
+        - InvalidParameterValue
+        - InvalidParameterCombination
+        - InvalidVPCNetworkStateFault
+        - TagQuotaPerResourceExceeded
+        - InvalidKMSKeyFault


maybe we should ensure these are truly terminal

Should we change for all resources? havent had any customer issues reported yet.

michaelhtm · 2025-07-21T21:33:48Z

apis/v1alpha1/generator.yaml

+    print:
+      add_age_column: true
+      add_synced_column: true


these are true by default i believe

No they are not, trying to have serverless cache as consistent with cache cluster as possible..

apis/v1alpha1/generator.yaml

michaelhtm · 2025-07-21T21:37:01Z

pkg/resource/serverless_cache/hooks.go

+	if r.ko.Status.Status == nil {
+		return false
+	}
+	return *r.ko.Status.Status == "creating"


Maybe make the states Constants?
Also this one should also be capitalized

michaelhtm · 2025-07-21T21:37:06Z

pkg/resource/serverless_cache/hooks.go

+	if r.ko.Status.Status == nil {
+		return false
+	}
+	return *r.ko.Status.Status == "deleting"


michaelhtm · 2025-07-21T21:37:11Z

pkg/resource/serverless_cache/hooks.go

+	if r.ko.Status.Status == nil {
+		return false
+	}
+	return *r.ko.Status.Status == "modifying"


michaelhtm · 2025-07-21T21:39:24Z

pkg/resource/serverless_cache/hooks.go

+	case delta.DifferentAt("Spec.Description"):
+		if err := rm.syncDescription(ctx, desired); err != nil {
+			return nil, fmt.Errorf("cannot update serverless cache: %v", err)
+		}
+
+	case delta.DifferentAt("Spec.Tags"):
+		if err := rm.syncTags(ctx, desired, latest); err != nil {
+			return &resource{ko}, err
+		}
+
+	case delta.DifferentAt("Spec.DailySnapshotTime"):
+		if err := rm.syncDailySnapshotTime(ctx, desired); err != nil {
+			return nil, fmt.Errorf("cannot update serverless cache: %v", err)
+		}
+
+	case delta.DifferentAt("Spec.SnapshotRetentionLimit"):
+		if err := rm.syncSnapshotRetentionLimit(ctx, desired); err != nil {
+			return nil, fmt.Errorf("cannot update serverless cache: %v", err)
+		}
+
+	case delta.DifferentAt("Spec.SecurityGroupIDs"):
+		if err := rm.syncSecurityGroupIDs(ctx, desired); err != nil {
+			return nil, fmt.Errorf("cannot update serverless cache: %v", err)
+		}
+
+	case delta.DifferentAt("Spec.UserGroupID"):
+		if err := rm.syncUserGroupID(ctx, desired); err != nil {
+			return nil, fmt.Errorf("cannot update serverless cache: %v", err)
+		}
+
+	case delta.DifferentAt("Spec.CacheUsageLimits"):
+		if err := rm.syncCacheUsageLimits(ctx, desired); err != nil {
+			return nil, fmt.Errorf("cannot update serverless cache: %v", err)
+		}
+
+	case delta.DifferentAt("Spec.Engine") || delta.DifferentAt("Spec.MajorEngineVersion"):
+		if err := rm.syncEngineAndVersion(ctx, desired); err != nil {


can we not set all values in the same call (except Tags)? with this code we will be updating a single field at a time

That is the point 🙂

hashicorp/terraform-provider-aws#35897

ack-prow · 2025-07-22T20:17:58Z

@rushmash91: The following test failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
elasticache-verify-attribution	`c18af55`	link	false	`/test elasticache-verify-attribution`

Full PR test history. Your PR dashboard.

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/test-infra repository. I understand the commands that are listed here.

ack-test-agent and others added 2 commits July 14, 2025 23:21

Add serverlesscache to elasticache

d16f722

add serverless cache support

22fbe51

ack-prow bot requested review from a-hilaly and jlbutler July 17, 2025 22:03

ack-prow bot added the approved label Jul 17, 2025

rushmash91 force-pushed the elasticache-add-serverlesscache branch from 4e8c424 to 23bb928 Compare July 17, 2025 22:57

temp disable other tests

667b5c5

rushmash91 force-pushed the elasticache-add-serverlesscache branch from fc9cbc6 to 667b5c5 Compare July 17, 2025 23:51

Merge branch 'main' into elasticache-add-serverlesscache

3d10da0

rushmash91 force-pushed the elasticache-add-serverlesscache branch 4 times, most recently from 1f5cf20 to 61ea0e8 Compare July 18, 2025 07:48

rushmash91 force-pushed the elasticache-add-serverlesscache branch from 61ea0e8 to 767653a Compare July 21, 2025 17:27

michaelhtm reviewed Jul 21, 2025

View reviewed changes

enable all tests

c18af55

rushmash91 force-pushed the elasticache-add-serverlesscache branch from 767653a to c18af55 Compare July 22, 2025 20:17

Add serverlesscache to elasticache #175

Are you sure you want to change the base?

Add serverlesscache to elasticache #175

Conversation

rushmash91 commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ack-prow bot commented Jul 17, 2025

Uh oh!

rushmash91 commented Jul 18, 2025

Uh oh!

rushmash91 commented Jul 18, 2025

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ack-prow bot commented Jul 22, 2025

Uh oh!

Uh oh!

rushmash91 commented Jul 17, 2025 •

edited

Loading