Ensure CRS controller always add ownerReference to resources

kubernetes-sigs · Jun 14, 2024 · b145e9d · b145e9d
1 parent 77a4db2
commit b145e9d
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 19 deletions.
diff --git a/exp/addons/internal/controllers/clusterresourceset_controller.go b/exp/addons/internal/controllers/clusterresourceset_controller.go
@@ -266,6 +266,40 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
 	log := ctrl.LoggerFrom(ctx, "Cluster", klog.KObj(cluster))
 	ctx = ctrl.LoggerInto(ctx, log)
 
+	// Iterate all resources and ensure an ownerReference to the clusterResourceSet is on the resource.
+	// NOTE: we have to do this before getting a remote client, otherwise owner reference won't be created until it is
+	// possible to connect to the remote cluster.
+	errList := []error{}
+	objList := make([]*unstructured.Unstructured, len(clusterResourceSet.Spec.Resources))
+	for i, resource := range clusterResourceSet.Spec.Resources {
+		unstructuredObj, err := r.getResource(ctx, resource, cluster.GetNamespace())
+		if err != nil {
+			if err == ErrSecretTypeNotSupported {
+				conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.WrongSecretTypeReason, clusterv1.ConditionSeverityWarning, err.Error())
+			} else {
+				conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.RetrievingResourceFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
+
+				// Continue without adding the error to the aggregate if we can't find the resource.
+				if apierrors.IsNotFound(err) {
+					continue
+				}
+			}
+			errList = append(errList, err)
+			continue
+		}
+
+		// Ensure an ownerReference to the clusterResourceSet is on the resource.
+		if err := r.ensureResourceOwnerRef(ctx, clusterResourceSet, unstructuredObj); err != nil {
+			log.Error(err, "Failed to add ClusterResourceSet as resource owner reference",
+				"Resource type", unstructuredObj.GetKind(), "Resource name", unstructuredObj.GetName())
+			errList = append(errList, err)
+		}
+		objList[i] = unstructuredObj
+	}
+	if len(errList) > 0 {
+		return kerrors.NewAggregate(errList)
+	}
+
 	remoteClient, err := r.Tracker.GetClient(ctx, util.ObjectKey(cluster))
 	if err != nil {
 		conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.RemoteClusterClientFailedReason, clusterv1.ConditionSeverityError, err.Error())
@@ -305,24 +339,13 @@ func (r *ClusterResourceSetReconciler) ApplyClusterResourceSet(ctx context.Conte
 		Name:       clusterResourceSet.Name,
 		UID:        clusterResourceSet.UID,
 	}))
-	errList := []error{}
 	resourceSetBinding := clusterResourceSetBinding.GetOrCreateBinding(clusterResourceSet)
 
 	// Iterate all resources and apply them to the cluster and update the resource status in the ClusterResourceSetBinding object.
-	for _, resource := range clusterResourceSet.Spec.Resources {
-		unstructuredObj, err := r.getResource(ctx, resource, cluster.GetNamespace())
-		if err != nil {
-			if err == ErrSecretTypeNotSupported {
-				conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.WrongSecretTypeReason, clusterv1.ConditionSeverityWarning, err.Error())
-			} else {
-				conditions.MarkFalse(clusterResourceSet, addonsv1.ResourcesAppliedCondition, addonsv1.RetrievingResourceFailedReason, clusterv1.ConditionSeverityWarning, err.Error())
-
-				// Continue without adding the error to the aggregate if we can't find the resource.
-				if apierrors.IsNotFound(err) {
-					continue
-				}
-			}
-			errList = append(errList, err)
+	for i, resource := range clusterResourceSet.Spec.Resources {
+		unstructuredObj := objList[i]
+		if unstructuredObj == nil {
+			// Continue without adding the error to the aggregate if we can't find the resource.
 			continue
 		}
 

diff --git a/exp/addons/internal/controllers/clusterresourceset_controller_test.go b/exp/addons/internal/controllers/clusterresourceset_controller_test.go
@@ -345,8 +345,13 @@ metadata:
 		}
 		g.Eventually(func() bool {
 			m := &corev1.ConfigMap{}
-			err := env.Get(ctx, cmKey, m)
-			return err == nil
+			if err := env.Get(ctx, cmKey, m); err != nil {
+				return false
+			}
+			if len(m.OwnerReferences) != 1 || m.OwnerReferences[0].Name != crsInstance.Name {
+				return false
+			}
+			return true
 		}, timeout).Should(BeTrue())
 
 		// When the ConfigMap resource is created, CRS should get reconciled immediately.
@@ -445,8 +450,13 @@ metadata:
 		}
 		g.Eventually(func() bool {
 			m := &corev1.Secret{}
-			err := env.Get(ctx, cmKey, m)
-			return err == nil
+			if err := env.Get(ctx, cmKey, m); err != nil {
+				return false
+			}
+			if len(m.OwnerReferences) != 1 || m.OwnerReferences[0].Name != crsInstance.Name {
+				return false
+			}
+			return true
 		}, timeout).Should(BeTrue())
 
 		// When the Secret resource is created, CRS should get reconciled immediately.