From 55bad60cefa341a2ae1ec7590ece349ae582cb4d Mon Sep 17 00:00:00 2001
From: michaelhtm <98621731+michaelhtm@users.noreply.github.com>
Date: Thu, 22 May 2025 13:50:35 -0700
Subject: [PATCH] feat: wait for resource to sync before updating

Currently, We've been setting a resource as NotSynced in ReadOne, if the
resource status is not a certain way (Eg. waiting for EKS Cluster to be
Active), and also checking the same resource Status in sdkUpdate.

This is the first attempt at addressing that issue. We are going to need
to ensure every controller/resource returns the proper NotSynced reason
and the requeue error inside the condition. This will allow us to set a
custom requeue right from ReadOne, that will be returned in preUpdate.
---
 pkg/runtime/reconciler.go |  7 +++++++
 pkg/runtime/util.go       | 22 ++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/pkg/runtime/reconciler.go b/pkg/runtime/reconciler.go
index ff02467..1d96e52 100644
--- a/pkg/runtime/reconciler.go
+++ b/pkg/runtime/reconciler.go
@@ -745,6 +745,13 @@ func (r *resourceReconciler) updateResource(
 		return latest, err
 	}
 
+	// We don't want to attempt updating the resource until it is Synced!
+	if synced, err := IsSyncedInRes(ctx, rm, latest); !synced {
+		rlog.Info(
+			"requeing until resource is synced",
+		)
+		return latest, err
+	}
 	// Check to see if the latest observed state already matches the
 	// desired state and if not, update the resource
 	delta := r.rd.Delta(desired, latest)
diff --git a/pkg/runtime/util.go b/pkg/runtime/util.go
index adc994a..229a25a 100644
--- a/pkg/runtime/util.go
+++ b/pkg/runtime/util.go
@@ -14,6 +14,7 @@
 package runtime
 
 import (
+	"context"
 	"encoding/json"
 	"fmt"
 	"strings"
@@ -70,6 +71,27 @@ func IsSynced(res acktypes.AWSResource) bool {
 	return false
 }
 
+// IsSyncedInRes is like IsSynced, but instead of returnning false
+// when the Synced condition is not there, it returns true.
+// Note(michaelhtm) We will be using this as a preUpdate check to
+// see if a resource has a NotSynced condition
+func IsSyncedInRes(ctx context.Context, rm acktypes.AWSResourceManager, res acktypes.AWSResource) (bool, error) {
+	for _, c := range res.Conditions() {
+		if c.Type == ackv1alpha1.ConditionTypeResourceSynced {
+			if c.Status == corev1.ConditionTrue {
+				return true, nil
+			}
+			// TODO(michaelhtm) we should probably start relying more and more in IsSynced below.
+			// Or maybe store the RequeueError in ackCondition.RequeueError...
+			return false, fmt.Errorf("resource is not synced")
+			
+		}
+	}
+
+	synced, err := rm.IsSynced(ctx, res)
+	return synced, err
+}
+
 // IsReadOnly returns true if the supplied AWSResource has an annotation
 // indicating that it is in read-only mode.
 func IsReadOnly(res acktypes.AWSResource) bool {