Fix bugs in RootSyncSet controller (#3651)

porch/controllers/rootsyncsets/api/v1alpha1/rootsyncset_types.go

@@ -63,6 +63,8 @@ type SecretReference struct {
 
 // RootSyncSetStatus defines the observed state of RootSyncSet
 type RootSyncSetStatus struct {
+	ObservedGeneration int64 `json:"observedGeneration,omitempty"`
+
 	// Conditions describes the reconciliation state of the object.
 	Conditions []metav1.Condition `json:"conditions,omitempty"`
 

porch/controllers/rootsyncsets/config/rbac/role.yaml

@@ -58,6 +58,7 @@ rules:
   verbs:
   - get
   - list
+  - watch
 - apiGroups:
   - container.cnrm.cloud.google.com
   resources:

porch/controllers/rootsyncsets/pkg/controllers/rootsyncset/controller.go

@@ -100,7 +100,7 @@ type RootSyncSetReconciler struct {
 //+kubebuilder:rbac:groups=config.porch.kpt.dev,resources=rootsyncsets,verbs=get;list;watch;create;update;patch;delete
 //+kubebuilder:rbac:groups=config.porch.kpt.dev,resources=rootsyncsets/status,verbs=get;update;patch
 //+kubebuilder:rbac:groups=config.porch.kpt.dev,resources=rootsyncsets/finalizers,verbs=update
-//+kubebuilder:rbac:groups=configcontroller.cnrm.cloud.google.com,resources=configcontrollerinstances,verbs=get;list
+//+kubebuilder:rbac:groups=configcontroller.cnrm.cloud.google.com,resources=configcontrollerinstances,verbs=get;list;watch
 //+kubebuilder:rbac:groups=container.cnrm.cloud.google.com,resources=containerclusters,verbs=get;list;watch
 //+kubebuilder:rbac:groups=core.cnrm.cloud.google.com,resources=configconnectorcontexts,verbs=get;list;watch
 //+kubebuilder:rbac:groups=hub.gke.io,resources=memberships,verbs=get;list;watch
@@ -242,11 +242,13 @@ func (r *RootSyncSetReconciler) updateStatus(ctx context.Context, rss *v1alpha1.
 	}
 
 	// Don't update if there are no changes.
-	if equality.Semantic.DeepEqual(rss.Status.ClusterRefStatuses, crss) {
+	if equality.Semantic.DeepEqual(rss.Status.ClusterRefStatuses, crss) &&
+		rss.Generation == rss.Status.ObservedGeneration {
 		return nil
 	}
 
 	rss.Status.ClusterRefStatuses = crss
+	rss.Status.ObservedGeneration = rss.Generation
 	return r.Client.Status().Update(ctx, rss)
 }
 
@@ -337,8 +339,11 @@ func (r *RootSyncSetReconciler) setupWatches(ctx context.Context, client dynamic
 		cancelFunc: cancelFunc,
 		client:     client,
 		channel:    r.channel,
-		liens:      make(map[types.NamespacedName]struct{}),
+		liens: map[types.NamespacedName]struct{}{
+			nn: {},
+		},
 	}
+	klog.Infof("Creating watcher for %v", clusterRef)
 	go w.watch()
 	r.watchers[clusterRef] = w
 }
@@ -349,6 +354,7 @@ func (r *RootSyncSetReconciler) setupWatches(ctx context.Context, client dynamic
 func (r *RootSyncSetReconciler) pruneWatches(rssnn types.NamespacedName, clusterRefs []*v1alpha1.ClusterRef) {
 	r.mutex.Lock()
 	defer r.mutex.Unlock()
+	klog.Infof("Pruning watches for %s which has %v clusterRefs", rssnn.String(), clusterRefs)
 
 	// Look through all watchers to check if it used to be needed by the RootSyncSet
 	// but is no longer.
@@ -368,7 +374,6 @@ func (r *RootSyncSetReconciler) pruneWatches(rssnn types.NamespacedName, cluster
 		delete(w.liens, rssnn)
 		// If no other RootSyncSets need the watch, stop it and remove the watcher from the map.
 		if len(w.liens) == 0 {
-			klog.Infof("clusterRef %s is no longer needed, so closing watch", clusterRef.Name)
 			w.cancelFunc()
 			delete(r.watchers, clusterRef)
 		}

porch/controllers/rootsyncsets/pkg/controllers/rootsyncset/status.go

@@ -26,11 +26,26 @@ import (
 // checkSyncStatus fetches the RootSync using the provided client and computes the sync status. The rules
 // for computing status here mirrors the one used in the status command in the nomos cli.
 func checkSyncStatus(ctx context.Context, client dynamic.Interface, rssName string) (string, error) {
+	// TODO: Change this to use the RootSync type instead of Unstructured.
 	rs, err := client.Resource(rootSyncGVR).Namespace(rootSyncNamespace).Get(ctx, rssName, metav1.GetOptions{})
 	if err != nil {
 		return "", fmt.Errorf("failed to get RootSync: %w", err)
 	}
 
+	generation, _, err := unstructured.NestedInt64(rs.Object, "metadata", "generation")
+	if err != nil {
+		return "", fmt.Errorf("failed to read generation from RootSync: %w", err)
+	}
+
+	observedGeneration, _, err := unstructured.NestedInt64(rs.Object, "status", "observedGeneration")
+	if err != nil {
+		return "", fmt.Errorf("failed to read observedGeneration from RootSync: %w", err)
+	}
+
+	if generation != observedGeneration {
+		return "Pending", nil
+	}
+
 	conditions, _, err := unstructured.NestedSlice(rs.Object, "status", "conditions")
 	if err != nil {
 		return "", fmt.Errorf("failed to extract conditions from RootSync: %w", err)