feat: update RGD controller to set CRD ownership and optimize watches

Updated the ResourceGraphDefinition controller to set correct ownership references on CRDs and switched to metadata-only watches for improved performance and reduced memory consumption. Added integration tests to validate CRD ownership.

Signed-off-by: Jakob Möller <[email protected]>

Author: jakobmoellerdev

pkg/controller/resourcegraphdefinition/controller.go

@@ -20,14 +20,11 @@ import (
 
 	"github.com/go-logr/logr"
 	extv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	"k8s.io/apimachinery/pkg/api/meta"
-	"k8s.io/apimachinery/pkg/types"
 	ctrl "sigs.k8s.io/controller-runtime"
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	ctrlrtcontroller "sigs.k8s.io/controller-runtime/pkg/controller"
 	"sigs.k8s.io/controller-runtime/pkg/event"
-	"sigs.k8s.io/controller-runtime/pkg/handler"
 	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
@@ -109,50 +106,18 @@ func (r *ResourceGraphDefinitionReconciler) SetupWithManager(mgr ctrl.Manager) e
 				MaxConcurrentReconciles: r.maxConcurrentReconciles,
 			},
 		).
-		WatchesMetadata(
-			&extv1.CustomResourceDefinition{},
-			handler.EnqueueRequestsFromMapFunc(r.findRGDsForCRD),
-			builder.WithPredicates(predicate.Funcs{
-				UpdateFunc: func(e event.UpdateEvent) bool {
-					return true
-				},
-				CreateFunc: func(e event.CreateEvent) bool {
-					return false
-				},
-				DeleteFunc: func(e event.DeleteEvent) bool {
-					return false
-				},
-			}),
-		).
-		Complete(reconcile.AsReconciler[*v1alpha1.ResourceGraphDefinition](mgr.GetClient(), r))
-}
-
-// findRGDsForCRD returns a list of reconcile requests for the ResourceGraphDefinition
-// that owns the given CRD. It is used to trigger reconciliation when a CRD is updated.
-func (r *ResourceGraphDefinitionReconciler) findRGDsForCRD(ctx context.Context, obj client.Object) []reconcile.Request {
-	mobj, err := meta.Accessor(obj)
-	if err != nil {
-		return nil
-	}
-
-	// Check if the CRD is owned by a ResourceGraphDefinition
-	if !metadata.IsKROOwned(mobj) {
-		return nil
-	}
-
-	rgdName, ok := mobj.GetLabels()[metadata.ResourceGraphDefinitionNameLabel]
-	if !ok {
-		return nil
-	}
-
-	// Return a reconcile request for the corresponding RGD
-	return []reconcile.Request{
-		{
-			NamespacedName: types.NamespacedName{
-				Name: rgdName,
+		Owns(&extv1.CustomResourceDefinition{}, builder.WithPredicates(predicate.Funcs{
+			UpdateFunc: func(e event.UpdateEvent) bool {
+				return true
 			},
-		},
-	}
+			CreateFunc: func(e event.CreateEvent) bool {
+				return false
+			},
+			DeleteFunc: func(e event.DeleteEvent) bool {
+				return false
+			},
+		}), builder.OnlyMetadata).
+		Complete(reconcile.AsReconciler[*v1alpha1.ResourceGraphDefinition](mgr.GetClient(), r))
 }
 
 func (r *ResourceGraphDefinitionReconciler) Reconcile(ctx context.Context, o *v1alpha1.ResourceGraphDefinition) (ctrl.Result, error) {

pkg/controller/resourcegraphdefinition/controller_reconcile.go

@@ -19,15 +19,14 @@ import (
 	"fmt"
 	"time"
 
-	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
-	"k8s.io/apimachinery/pkg/runtime/schema"
-	ctrl "sigs.k8s.io/controller-runtime"
-
 	"github.com/kubernetes-sigs/kro/api/v1alpha1"
 	instancectrl "github.com/kubernetes-sigs/kro/pkg/controller/instance"
 	"github.com/kubernetes-sigs/kro/pkg/dynamiccontroller"
 	"github.com/kubernetes-sigs/kro/pkg/graph"
 	"github.com/kubernetes-sigs/kro/pkg/metadata"
+	v1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
+	"k8s.io/apimachinery/pkg/runtime/schema"
+	ctrl "sigs.k8s.io/controller-runtime"
 )
 
 // reconcileResourceGraphDefinition orchestrates the reconciliation of a ResourceGraphDefinition by:
@@ -59,6 +58,10 @@ func (r *ResourceGraphDefinitionReconciler) reconcileResourceGraphDefinition(
 
 	crd := processedRGD.Instance.GetCRD()
 	graphExecLabeler.ApplyLabels(&crd.ObjectMeta)
+	if err := ctrl.SetControllerReference(rgd, crd, r.Scheme()); err != nil {
+		mark.KindUnready(err.Error())
+		return nil, nil, fmt.Errorf("failed to set controller reference of CRD: %w", err)
+	}
 
 	// Ensure CRD exists and is up to date
 	log.V(1).Info("reconciling resource graph definition CRD")

test/integration/suites/core/crd_test.go

@@ -18,6 +18,9 @@ import (
 	"fmt"
 	"time"
 
+	krov1alpha1 "github.com/kubernetes-sigs/kro/api/v1alpha1"
+	"github.com/kubernetes-sigs/kro/pkg/metadata"
+	"github.com/kubernetes-sigs/kro/pkg/testutil/generator"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 	corev1 "k8s.io/api/core/v1"
@@ -26,10 +29,6 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
-
-	krov1alpha1 "github.com/kubernetes-sigs/kro/api/v1alpha1"
-	"github.com/kubernetes-sigs/kro/pkg/metadata"
-	"github.com/kubernetes-sigs/kro/pkg/testutil/generator"
 )
 
 var _ = Describe("CRD", func() {
@@ -104,6 +103,25 @@ var _ = Describe("CRD", func() {
 				g.Expect(props["spec"].Properties["field2"].Default.Raw).To(Equal([]byte("42")))
 			}, 10*time.Second, time.Second).WithContext(ctx).Should(Succeed())
 
+			// Check ownership relationship
+			owners := crd.GetOwnerReferences()
+			ownerRefExists := false
+			for _, owner := range owners {
+				if owner.Controller != nil && *owner.Controller {
+					ownerRefExists = true
+					Expect(owner.Name).To(Equal(rgd.Name))
+					Expect(owner.Kind).To(Equal("ResourceGraphDefinition"))
+					Expect(owner.APIVersion).To(Equal(krov1alpha1.GroupVersion.String()))
+					Expect(owner.UID).To(Equal(rgd.UID))
+					Expect(owner.BlockOwnerDeletion).ToNot(BeNil())
+					Expect(*owner.BlockOwnerDeletion).To(BeTrue())
+					Expect(owner.Controller).ToNot(BeNil())
+					Expect(*owner.Controller).To(BeTrue())
+					break
+				}
+			}
+			Expect(ownerRefExists).To(BeTrue(), "CRD should be owned by ResourceGraphDefinition")
+
 			Expect(env.Client.Delete(ctx, rgd)).To(Succeed())
 		})