Fix Redis to Valkey in-place upgrade by handling Engine in ModifyReplicationGroup (#188)

Fixes: aws-controllers-k8s/community#2299

Description of changes:
fixes the issue where in-place upgrades from ElastiCache Redis to Valkey clusters failed with `InvalidParameterCombination: valkey parameter group is not applicable to engine redis`. The problem occurred because the ElastiCache controller's custom `ModifyReplicationGroup` implementation was missing the `Engine` and `CacheParameterGroupName` fields in the API request payload, causing AWS to reject Valkey parameter groups being applied to what it perceived as a Redis engine. Additionally, the `ModifyReplicationGroup` API returns stale field values that don't immediately reflect requested changes, leading the controller to detect false differences and trigger terminal conditions. The solution adds both missing fields to the modify request payload, and normalizes API responses by overriding stale Engine field values with the user's intended values before processing.


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

Author: rushmash91

apis/v1alpha1/ack-generate-metadata.yaml

@@ -1,8 +1,8 @@
 ack_generate_info:
-  build_date: "2025-09-05T22:54:05Z"
-  build_hash: 1d9076d0211773ff8ab8682b28b912c7ece10676
+  build_date: "2025-09-10T22:13:23Z"
+  build_hash: 9e29f017d9e942548af133d2f31aecae248a8816
   go_version: go1.25.0
-  version: v0.51.0-2-g1d9076d
+  version: v0.51.0-3-g9e29f01
 api_directory_checksum: 65127f2f0a24a801fad4e043be37857f0e6bcfb9
 api_version: v1alpha1
 aws_sdk_go_version: v1.32.6

helm/templates/deployment.yaml

@@ -10,6 +10,9 @@ metadata:
     app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
     k8s-app: {{ include "ack-elasticache-controller.app.name" . }}
     helm.sh/chart: {{ include "ack-elasticache-controller.chart.name-version" . }}
+{{- range $key, $value := .Values.deployment.labels }}
+    {{ $key }}: {{ $value | quote }}
+{{- end }}
 spec:
   replicas: {{ .Values.deployment.replicas }}
   selector:

pkg/resource/replication_group/hooks.go

@@ -613,7 +613,7 @@ func (rm *resourceManager) modifyReplicationGroup(
 
 	// SecurityGroupIds, EngineVersion
 	if rm.securityGroupIdsDiffer(desired, latest, latestCacheCluster) ||
-		delta.DifferentAt("Spec.EngineVersion") {
+		delta.DifferentAt("Spec.EngineVersion") || delta.DifferentAt("Spec.Engine") || delta.DifferentAt("Spec.CacheParameterGroupName") {
 		input := rm.newModifyReplicationGroupRequestPayload(desired, latest, latestCacheCluster, delta)
 		resp, respErr := rm.sdkapi.ModifyReplicationGroup(ctx, input)
 		rm.metrics.RecordAPICall("UPDATE", "ModifyReplicationGroup", respErr)
@@ -622,7 +622,16 @@ func (rm *resourceManager) modifyReplicationGroup(
 			return nil, respErr
 		}
 
-		return rm.setReplicationGroupOutput(ctx, desired, resp.ReplicationGroup)
+		// The ModifyReplicationGroup API returns stale field Engine that don't
+		// immediately reflect the requested changes, causing the controller to detect false
+		// differences and trigger terminal conditions. Override these fields with the user's
+		// intended values before passing to the generated setReplicationGroupOutput function.
+		normalizedRG := *resp.ReplicationGroup
+		if desired.ko.Spec.Engine != nil {
+			normalizedRG.Engine = desired.ko.Spec.Engine
+		}
+
+		return rm.setReplicationGroupOutput(ctx, desired, &normalizedRG)
 	}
 
 	// no updates done
@@ -1169,6 +1178,16 @@ func (rm *resourceManager) newModifyReplicationGroupRequestPayload(
 		input.EngineVersion = desired.ko.Spec.EngineVersion
 	}
 
+	if delta.DifferentAt("Spec.Engine") &&
+		desired.ko.Spec.Engine != nil {
+		input.Engine = desired.ko.Spec.Engine
+	}
+
+	if delta.DifferentAt("Spec.CacheParameterGroupName") &&
+		desired.ko.Spec.CacheParameterGroupName != nil {
+		input.CacheParameterGroupName = desired.ko.Spec.CacheParameterGroupName
+	}
+
 	return input
 }