ClusterSync: Warn on resource conflicts

Add a new ClusterSync.Status field, ResourceConflicts, and a
corresponding printColumn visible when `-o wide` is used.

When a single resource (GVK + name [+ namespace if ns-scoped]) is
mentioned more than once in the `resources` sections (not `patches` or
`secretMappings`) of the [Selector]SyncSets matching a
ClusterDeployment, we will add a human-readable message to this status
field indicating the resource and the [Selector]SyncSets in which it is
mentioned.

Example message:

```
"Resource {v1 Namespace openshift-logging } is mentioned by 2 [Selector]SyncSets: mygroup, osd-logging1"
```

HIVE-2634

apis/hiveinternal/v1alpha1/clustersync_types.go

@@ -16,6 +16,7 @@ import (
 // +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[0].reason`
 // +kubebuilder:printcolumn:name="ControllerReplica",type=string,JSONPath=`.status.controlledByReplica`
 // +kubebuilder:printcolumn:name="Message",type=string,priority=1,JSONPath=`.status.conditions[?(@.type=="Failed")].message`
+// +kubebuilder:printcolumn:name="ResourceConflicts",type=string,priority=1,JSONPath=`.status.resourceConflicts`
 type ClusterSync struct {
 	metav1.TypeMeta   `json:",inline"`
 	metav1.ObjectMeta `json:"metadata,omitempty"`
@@ -50,6 +51,12 @@ type ClusterSyncStatus struct {
 	// recently handled the ClusterSync. If the hive-clustersync statefulset is scaled up or down, the
 	// controlling replica can change, potentially causing logs to be spread across multiple pods.
 	ControlledByReplica *int64 `json:"controlledByReplica,omitempty"`
+
+	// ResourceConflicts is a list of human-readable messages warning when a resource is mentioned
+	// more than once across all [Selector]SyncSets affecting this ClusterSync. Note that this only
+	// counts Spec.Resources, not Patches or SecretMappings.
+	// +optional
+	ResourceConflicts []string `json:"resourceConflicts,omitempty"`
 }
 
 // SyncStatus is the status of applying a specific SyncSet or SelectorSyncSet to the cluster.

config/crds/hiveinternal.openshift.io_clustersyncs.yaml

@@ -26,6 +26,10 @@ spec:
       name: Message
       priority: 1
       type: string
+    - jsonPath: .status.resourceConflicts
+      name: ResourceConflicts
+      priority: 1
+      type: string
     name: v1alpha1
     schema:
       openAPIV3Schema:
@@ -103,6 +107,14 @@ spec:
                   all (selector)syncsets to a cluster.
                 format: date-time
                 type: string
+              resourceConflicts:
+                description: |-
+                  ResourceConflicts is a list of human-readable messages warning when a resource is mentioned
+                  more than once across all [Selector]SyncSets affecting this ClusterSync. Note that this only
+                  counts Spec.Resources, not Patches or SecretMappings.
+                items:
+                  type: string
+                type: array
               selectorSyncSets:
                 description: SelectorSyncSets is the sync status of all of the SelectorSyncSets
                   for the cluster.

hack/app-sre/saas-template.yaml

@@ -4547,6 +4547,10 @@ objects:
         name: Message
         priority: 1
         type: string
+      - jsonPath: .status.resourceConflicts
+        name: ResourceConflicts
+        priority: 1
+        type: string
       name: v1alpha1
       schema:
         openAPIV3Schema:
@@ -4640,6 +4644,17 @@ objects:
                     applied all (selector)syncsets to a cluster.
                   format: date-time
                   type: string
+                resourceConflicts:
+                  description: 'ResourceConflicts is a list of human-readable messages
+                    warning when a resource is mentioned
+
+                    more than once across all [Selector]SyncSets affecting this ClusterSync.
+                    Note that this only
+
+                    counts Spec.Resources, not Patches or SecretMappings.'
+                  items:
+                    type: string
+                  type: array
                 selectorSyncSets:
                   description: SelectorSyncSets is the sync status of all of the SelectorSyncSets
                     for the cluster.

pkg/controller/clustersync/clustersync_controller.go

@@ -41,6 +41,8 @@ import (
 	"github.com/openshift/hive/pkg/resource"
 )
 
+type ResourceToSyncSetTracker map[hiveintv1alpha1.SyncResourceReference][]string
+
 const (
 	ControllerName         = hivev1.ClustersyncControllerName
 	defaultReapplyInterval = 2 * time.Hour
@@ -391,12 +393,15 @@ func (r *ReconcileClusterSync) Reconcile(ctx context.Context, request reconcile.
 	needToDoFullReapply := needToCreateLease || needToRenew
 	recobsrv.SetOutcome(hivemetrics.ReconcileOutcomeFullSync)
 
+	r2sTracker := ResourceToSyncSetTracker{}
+
 	// Apply SyncSets
 	syncStatusesForSyncSets, syncSetsNeedRequeue := r.applySyncSets(
 		cd,
 		"SyncSet",
 		syncSets,
 		clusterSync.Status.SyncSets,
+		r2sTracker,
 		needToDoFullReapply,
 		false, // no need to report SelectorSyncSet metrics if we're reconciling non-selector SyncSets
 		resourceHelper,
@@ -410,6 +415,7 @@ func (r *ReconcileClusterSync) Reconcile(ctx context.Context, request reconcile.
 		"SelectorSyncSet",
 		selectorSyncSets,
 		clusterSync.Status.SelectorSyncSets,
+		r2sTracker,
 		needToDoFullReapply,
 		clusterSync.Status.FirstSuccessTime == nil, // only report SelectorSyncSet metrics if we haven't reached first success
 		resourceHelper,
@@ -418,6 +424,7 @@ func (r *ReconcileClusterSync) Reconcile(ctx context.Context, request reconcile.
 	clusterSync.Status.SelectorSyncSets = syncStatusesForSelectorSyncSets
 
 	setFailedCondition(clusterSync)
+	setResourceConflicts(clusterSync, r2sTracker)
 
 	// Set clusterSync.Status.FirstSyncSetsSuccessTime
 	syncStatuses := append(syncStatusesForSyncSets, syncStatusesForSelectorSyncSets...)
@@ -469,6 +476,7 @@ func (r *ReconcileClusterSync) applySyncSets(
 	syncSetType string,
 	syncSets []CommonSyncSet,
 	syncStatuses []hiveintv1alpha1.SyncStatus,
+	r2sTracker ResourceToSyncSetTracker,
 	needToDoFullReapply bool,
 	reportSelectorSyncSetMetrics bool,
 	resourceHelper resource.Helper,
@@ -538,7 +546,7 @@ func (r *ReconcileClusterSync) applySyncSets(
 		}
 
 		// Apply the syncset
-		resourcesApplied, resourcesInSyncSet, syncSetNeedsRequeue, err := r.applySyncSet(syncSet, cd, resourceHelper, logger)
+		resourcesApplied, resourcesInSyncSet, syncSetNeedsRequeue, err := r.applySyncSet(syncSet, cd, r2sTracker, resourceHelper, logger)
 		newSyncStatus := hiveintv1alpha1.SyncStatus{
 			Name:               syncSet.AsMetaObject().GetName(),
 			ObservedGeneration: syncSet.AsMetaObject().GetGeneration(),
@@ -649,6 +657,7 @@ func getOldSyncStatus(syncSet CommonSyncSet, syncSetStatuses []hiveintv1alpha1.S
 func (r *ReconcileClusterSync) applySyncSet(
 	syncSet CommonSyncSet,
 	cd *hivev1.ClusterDeployment,
+	r2sTracker ResourceToSyncSetTracker,
 	resourceHelper resource.Helper,
 	logger log.FieldLogger,
 ) (
@@ -657,7 +666,7 @@ func (r *ReconcileClusterSync) applySyncSet(
 	requeue bool,
 	returnErr error,
 ) {
-	resources, referencesToResources, decodeErr := decodeResources(syncSet, cd, logger)
+	resources, referencesToResources, decodeErr := decodeResources(syncSet, cd, r2sTracker, logger)
 	referencesToSecrets := referencesToSecrets(syncSet)
 	resourcesInSyncSet = append(referencesToResources, referencesToSecrets...)
 	if decodeErr != nil {
@@ -708,7 +717,7 @@ func (r *ReconcileClusterSync) applySyncSet(
 	return
 }
 
-func decodeResources(syncSet CommonSyncSet, cd *hivev1.ClusterDeployment, logger log.FieldLogger) (
+func decodeResources(syncSet CommonSyncSet, cd *hivev1.ClusterDeployment, r2sTracker ResourceToSyncSetTracker, logger log.FieldLogger) (
 	resources []*unstructured.Unstructured, references []hiveintv1alpha1.SyncResourceReference, returnErr error,
 ) {
 	var decodeErrors []error
@@ -728,12 +737,15 @@ func decodeResources(syncSet CommonSyncSet, cd *hivev1.ClusterDeployment, logger
 			}
 		}
 		resources = append(resources, u)
-		references = append(references, hiveintv1alpha1.SyncResourceReference{
+		ref := hiveintv1alpha1.SyncResourceReference{
 			APIVersion: u.GetAPIVersion(),
 			Kind:       u.GetKind(),
 			Namespace:  u.GetNamespace(),
 			Name:       u.GetName(),
-		})
+		}
+		references = append(references, ref)
+		r2sTracker[ref] = append(r2sTracker[ref], syncSet.AsMetaObject().GetName())
+		logger.Debugf("Tracking resource %v from syncset %s", ref, syncSet.AsMetaObject().GetName())
 	}
 	returnErr = utilerrors.NewAggregate(decodeErrors)
 	return
@@ -997,6 +1009,18 @@ func setFailedCondition(clusterSync *hiveintv1alpha1.ClusterSync) {
 	}}
 }
 
+func setResourceConflicts(clusterSync *hiveintv1alpha1.ClusterSync, r2sTracker ResourceToSyncSetTracker) {
+	clusterSync.Status.ResourceConflicts = []string{}
+	tpl := "Resource %v is mentioned by %d [Selector]SyncSets: %v"
+	for resource, syncsets := range r2sTracker {
+		if count := len(syncsets); count > 1 {
+			clusterSync.Status.ResourceConflicts = append(
+				clusterSync.Status.ResourceConflicts,
+				fmt.Sprintf(tpl, resource, count, strings.Join(syncsets, ", ")))
+		}
+	}
+}
+
 func getFailingSyncSets(syncStatuses []hiveintv1alpha1.SyncStatus) []string {
 	var failures []string
 	for _, status := range syncStatuses {