From ef8945268092ff4364915f5a338525c960567ed6 Mon Sep 17 00:00:00 2001
From: Pratik Jagrut <pratik.jagrut@suse.com>
Date: Wed, 5 Jun 2024 09:35:58 +0530
Subject: [PATCH] fix: add owner reference to cluster registration request

---
 .../controllers/cluster/controller.go         | 21 +---------------
 .../clusterregistration/controller.go         | 24 +++++++++++++++++++
 .../clusterregistration/controller_test.go    |  4 ++++
 3 files changed, 29 insertions(+), 20 deletions(-)

diff --git a/internal/cmd/controller/agentmanagement/controllers/cluster/controller.go b/internal/cmd/controller/agentmanagement/controllers/cluster/controller.go
index 8af63081a3..d8ccba3609 100644
--- a/internal/cmd/controller/agentmanagement/controllers/cluster/controller.go
+++ b/internal/cmd/controller/agentmanagement/controllers/cluster/controller.go
@@ -107,26 +107,7 @@ func clusterNamespace(clusterNamespace, clusterName string) string {
 }
 
 func (h *handler) OnClusterChanged(cluster *fleet.Cluster, status fleet.ClusterStatus) (fleet.ClusterStatus, error) {
-	logrus.Debugf("OnClusterChanged for cluster status %s, checking cluster registration, updating namespace in status", cluster.Name)
-	if cluster.DeletionTimestamp != nil {
-		// cluster is being deleted, clean up the cluster registrations
-		clusterRegistrations, err := h.clusterRegistrations.List(cluster.Namespace, metav1.ListOptions{})
-		if err != nil {
-			return status, err
-		}
-		for _, clusterRegistration := range clusterRegistrations.Items {
-			if clusterRegistration.Status.ClusterName == cluster.Name {
-				err := h.clusterRegistrations.Delete(clusterRegistration.Namespace, clusterRegistration.Name, &metav1.DeleteOptions{})
-				if err == nil {
-					logrus.Debugf("deleted leftover ClusterRegistration (%s) for cluster: %s", clusterRegistration.Name, cluster.Name)
-				} else if !apierrors.IsNotFound(err) {
-					return status, err
-				}
-			}
-		}
-		return status, nil
-	}
-
+	logrus.Debugf("OnClusterChanged for cluster status %s, updating namespace in status", cluster.Name)
 	if status.Namespace == "" {
 		status.Namespace = clusterNamespace(cluster.Namespace, cluster.Name)
 	}
diff --git a/internal/cmd/controller/agentmanagement/controllers/clusterregistration/controller.go b/internal/cmd/controller/agentmanagement/controllers/clusterregistration/controller.go
index 443313c5f6..9ad062d890 100644
--- a/internal/cmd/controller/agentmanagement/controllers/clusterregistration/controller.go
+++ b/internal/cmd/controller/agentmanagement/controllers/clusterregistration/controller.go
@@ -174,6 +174,30 @@ func (h *handler) OnChange(request *fleet.ClusterRegistration, status fleet.Clus
 		return nil, status, nil
 	}
 
+	// set the Cluster as owner of the cluster registration request
+	// ownerFound is used to avoid calling update on request whenever OnChange is called
+	ownerFound := false
+	for _, owner := range request.OwnerReferences {
+		if owner.Kind == "Cluster" && owner.Name == cluster.Name && owner.UID == cluster.UID {
+			ownerFound = true
+			break
+		}
+	}
+	if !ownerFound {
+		request.SetOwnerReferences([]metav1.OwnerReference{
+			{
+				APIVersion: fleet.SchemeGroupVersion.String(),
+				Kind:       "Cluster",
+				Name:       cluster.Name,
+				UID:        cluster.UID,
+			},
+		})
+		request, err = h.clusterRegistration.Update(request)
+		if err != nil {
+			return nil, status, err
+		}
+	}
+
 	saName := name.SafeConcatName(request.Name, string(request.UID))
 	sa, err := h.serviceAccountCache.Get(cluster.Status.Namespace, saName)
 	if err != nil && apierrors.IsNotFound(err) {
diff --git a/internal/cmd/controller/agentmanagement/controllers/clusterregistration/controller_test.go b/internal/cmd/controller/agentmanagement/controllers/clusterregistration/controller_test.go
index 8dd6154b18..a3163e1995 100644
--- a/internal/cmd/controller/agentmanagement/controllers/clusterregistration/controller_test.go
+++ b/internal/cmd/controller/agentmanagement/controllers/clusterregistration/controller_test.go
@@ -161,6 +161,7 @@ var _ = Describe("ClusterRegistration OnChange", func() {
 			BeforeEach(func() {
 				cluster.Status = fleet.ClusterStatus{Namespace: "fleet-default"}
 				saCache.EXPECT().Get(gomock.Any(), gomock.Any()).Return(nil, notFound)
+				clusterRegistrationController.EXPECT().Update(gomock.Any()).Return(&fleet.ClusterRegistration{}, nil)
 			})
 
 			It("creates a new service account", func() {
@@ -178,6 +179,7 @@ var _ = Describe("ClusterRegistration OnChange", func() {
 				// post k8s 1.24 service account without sa.Secrets list
 				sa = &corev1.ServiceAccount{}
 				saCache.EXPECT().Get(gomock.Any(), gomock.Any()).Return(sa, nil)
+				clusterRegistrationController.EXPECT().Update(gomock.Any()).Return(&fleet.ClusterRegistration{}, nil)
 			})
 
 			Context("cannot create secret", func() {
@@ -226,6 +228,8 @@ var _ = Describe("ClusterRegistration OnChange", func() {
 				secretController.EXPECT().Get(gomock.Any(), gomock.Any(), gomock.Any()).Return(secret, nil)
 
 				clusterRegistrationController.EXPECT().List(gomock.Any(), gomock.Any()).Return(&fleet.ClusterRegistrationList{}, nil)
+
+				clusterRegistrationController.EXPECT().Update(gomock.Any()).Return(&fleet.ClusterRegistration{}, nil)
 			})
 
 			Context("grants registration, cleans up and creates objects", func() {