Fixes for Snapshot failures

1. Fix snapshot crud failures:
   Cause: update did not save the sync status condition
   Fix: update() needed to return latest retrieved.
2. Fix endless reconciler loop:
   Cause: ReadOne->onError() sets terminal condition to false using patchResource,
     followed by error returned from Create() leads to
     setting terminal condition to true using patchResource;
     causing reconciler loop.
   Fix: ensure patchResource is invoked once in the reconciler::sync()
3 Snapshots e2e tests fix - updated scenarios where create snapshot expects either Replication Group or Cache Cluster Id and fails if other is supplied.

Author: Kumar Gaurav Sharma

pkg/runtime/reconciler.go

@@ -156,15 +156,15 @@ func (r *reconciler) sync(
 
 	latest, err := rm.ReadOne(ctx, desired)
 	if err != nil {
-		if latest != nil {
-			// this indicates, that even though ReadOne failed
-			// there is some changes available in the latest.RuntimeObject()
-			// (example: ko.Status.Conditions) which have been
-			// updated in the resource
-			// Thus, patchResource() call should be made here
-			_ = r.patchResource(ctx, desired, latest)
-		}
 		if err != ackerr.NotFound {
+			if latest != nil {
+				// this indicates, that even though ReadOne failed
+				// there is some changes available in the latest.RuntimeObject()
+				// (example: ko.Status.Conditions) which have been
+				// updated in the resource
+				// Thus, patchResource() call should be made here
+				_ = r.patchResource(ctx, desired, latest)
+			}
 			return err
 		}
 		if isAdopted {

services/elasticache/pkg/resource/snapshot/custom_update_api.go

@@ -25,5 +25,5 @@ func (rm *resourceManager) customUpdateSnapshot(
 	latest *resource,
 	diffReporter *ackcompare.Reporter,
 ) (*resource, error) {
-	return desired, nil
+	return latest, nil
 }

test/e2e/elasticache/snapshot/e2e.sh

@@ -73,7 +73,7 @@ spec:
 EOF)
   echo "$snapshot_yaml" | kubectl apply -f -
   assert_equal "0" "$?" "Expected application of $copied_snapshot_name to succeed" || exit 1
-  k8s_wait_resource_synced "snapshots/$snapshot_name" 10
+  k8s_wait_resource_synced "snapshots/$copied_snapshot_name" 10
 
   # test deletion
   kubectl delete snapshots/"$snapshot_name"
@@ -126,7 +126,7 @@ EOF)
   kubectl delete snapshots/"$snapshot_name"
   aws_wait_snapshot_deleted "$snapshot_name"
 
-  # case 2: specify both RG and cache cluster ID (should succeed)
+  # case 2: specify cache cluster ID (should succeed)
   local snapshot_name="snapshot-cmd"
   daws elasticache delete-snapshot --snapshot-name "$snapshot_name" 1>/dev/null 2>&1
   sleep 10
@@ -138,7 +138,6 @@ metadata:
   name: $snapshot_name
 spec:
   snapshotName: $snapshot_name
-  replicationGroupID: $rg_id
   cacheClusterID: $cc_id
 EOF)
   echo "$snapshot_yaml" | kubectl apply -f -
@@ -192,24 +191,24 @@ EOF)
   kubectl delete snapshots/"$snapshot_name"
   aws_wait_snapshot_deleted "$snapshot_name"
 
-  # case 2: specify both RG and cache cluster ID
+  # case 2: specify cache cluster ID should fail with error
   local snapshot_name="snapshot-cme"
   daws elasticache delete-snapshot --snapshot-name "$snapshot_name" 1>/dev/null 2>&1
   sleep 10
-  local cc_id="$rg_id-001"
+  local cc_id="$rg_id-0001-001"   # Replication group has two node group, picking first node group.
   local snapshot_yaml=$(cat <<EOF
 apiVersion: elasticache.services.k8s.aws/v1alpha1
 kind: Snapshot
 metadata:
   name: $snapshot_name
 spec:
   snapshotName: $snapshot_name
-  replicationGroupID: $rg_id
   cacheClusterID: $cc_id
 EOF)
   echo "$snapshot_yaml" | kubectl apply -f -
   assert_equal "0" "$?" "Expected application of $snapshot_name to succeed" || exit 1
-  k8s_wait_resource_synced "snapshots/$snapshot_name" 10
+  sleep 35 # give time for server validation
+  k8s_check_resource_terminal_condition_true "snapshots/$snapshot_name" "Cannot snapshot a cache cluster with cluster-mode enabled. Please specify a replication group instead"
 
   # delete snapshot for case 2 if creation succeeded
   kubectl delete snapshots/"$snapshot_name"