OCPBUGS-84957: Skip controller ownerReference on CRDs

pkg/controller/operconfig/operconfig_controller.go

@@ -215,8 +215,9 @@ func (r *ReconcileOperConfig) Reconcile(ctx context.Context, request reconcile.R
 			r.status.SetDegraded(statusmanager.OperatorConfig, "NoOperatorConfig",
 				fmt.Sprintf("Operator configuration %s was deleted", request.String()))
 			// Request object not found, could have been deleted after reconcile request.
-			// Owned objects are automatically garbage collected, since we set
-			// the ownerReference (see https://kubernetes.io/docs/concepts/workloads/controllers/garbage-collection/).
+			// Most owned objects are automatically garbage collected, since we
+			// set the ownerReference. CRDs are excluded from controller
+			// ownership to avoid conflicts with other operators.
 			// Return and don't requeue
 			return reconcile.Result{}, nil
 		}
@@ -477,9 +478,15 @@ func (r *ReconcileOperConfig) Reconcile(ctx context.Context, request reconcile.R
 	var degradedErr error
 	for _, obj := range objs {
 		// TODO: OwnerRef for non default clusters. For HyperShift this should probably be HostedControlPlane CR
-		if apply.GetClusterName(obj) == "" {
+		clusterName := apply.GetClusterName(obj)
+		// Skip controller ownership on CRDs: GC-deleting a CRD destroys
+		// all its CRs (data loss), and other operators (e.g. MetalLB via
+		// ClusterExtension/boxcutter) may legitimately claim controller
+		// ownership of the same CRDs, which conflicts because Kubernetes
+		// only allows a single controller ownerReference per object.
+		if clusterName == "" && !isCRD(obj) {
 			// Mark the object to be GC'd if the owner is deleted.
-			if err := controllerutil.SetControllerReference(operConfig, obj, r.client.ClientFor(apply.GetClusterName(obj)).Scheme()); err != nil {
+			if err := controllerutil.SetControllerReference(operConfig, obj, r.client.ClientFor(clusterName).Scheme()); err != nil {
 				err = errors.Wrapf(err, "could not set reference for (%s) %s/%s", obj.GroupVersionKind(), obj.GetNamespace(), obj.GetName())
 				log.Println(err)
 				r.status.SetDegraded(statusmanager.OperatorConfig, "InternalError",
@@ -563,6 +570,11 @@ func updateIPsecMetric(newOperConfigSpec *operv1.NetworkSpec) {
 	}
 }
 
+func isCRD(obj *uns.Unstructured) bool {
+	gvk := obj.GroupVersionKind()
+	return gvk.Kind == "CustomResourceDefinition" && gvk.Group == "apiextensions.k8s.io"
+}
+
 func reconcileOperConfig(ctx context.Context, obj crclient.Object) []reconcile.Request {
 	log.Printf("%s %s/%s changed, triggering operconf reconciliation", obj.GetObjectKind().GroupVersionKind().Kind, obj.GetNamespace(), obj.GetName())
 	// Update reconcile.Request object to align with unnamespaced default network,

pkg/controller/operconfig/operconfig_controller_test.go

@@ -0,0 +1,119 @@
+package operconfig
+
+import (
+	"testing"
+
+	operv1 "github.com/openshift/api/operator/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	uns "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/runtime"
+	utilruntime "k8s.io/apimachinery/pkg/util/runtime"
+	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+)
+
+func TestSetControllerReferenceSkipsCRDs(t *testing.T) {
+	scheme := runtime.NewScheme()
+	utilruntime.Must(operv1.Install(scheme))
+
+	owner := &operv1.Network{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "cluster",
+			UID:  "test-uid",
+		},
+	}
+
+	tests := []struct {
+		name           string
+		obj            *uns.Unstructured
+		expectOwnerRef bool
+	}{
+		{
+			name: "Deployment gets controller ownerReference",
+			obj: &uns.Unstructured{
+				Object: map[string]interface{}{
+					"apiVersion": "apps/v1",
+					"kind":       "Deployment",
+					"metadata": map[string]interface{}{
+						"name":      "test-deployment",
+						"namespace": "openshift-network-operator",
+					},
+				},
+			},
+			expectOwnerRef: true,
+		},
+		{
+			name: "CRD does not get controller ownerReference",
+			obj: &uns.Unstructured{
+				Object: map[string]interface{}{
+					"apiVersion": "apiextensions.k8s.io/v1",
+					"kind":       "CustomResourceDefinition",
+					"metadata": map[string]interface{}{
+						"name": "frrconfigurations.frrk8s.metallb.io",
+					},
+				},
+			},
+			expectOwnerRef: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if !isCRD(tc.obj) {
+				if err := controllerutil.SetControllerReference(owner, tc.obj, scheme); err != nil {
+					t.Fatalf("SetControllerReference failed: %v", err)
+				}
+			}
+
+			controllerRef := metav1.GetControllerOf(tc.obj)
+			if tc.expectOwnerRef && controllerRef == nil {
+				t.Error("expected controller ownerReference, got none")
+			}
+			if !tc.expectOwnerRef && controllerRef != nil {
+				t.Errorf("expected no controller ownerReference, got %v", controllerRef)
+			}
+		})
+	}
+}
+
+func TestIsCRD(t *testing.T) {
+	tests := []struct {
+		name       string
+		apiVersion string
+		kind       string
+		expected   bool
+	}{
+		{
+			name:       "apiextensions CRD",
+			apiVersion: "apiextensions.k8s.io/v1",
+			kind:       "CustomResourceDefinition",
+			expected:   true,
+		},
+		{
+			name:       "non-CRD resource with same Kind in different group",
+			apiVersion: "example.com/v1",
+			kind:       "CustomResourceDefinition",
+			expected:   false,
+		},
+		{
+			name:       "Deployment",
+			apiVersion: "apps/v1",
+			kind:       "Deployment",
+			expected:   false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			obj := &uns.Unstructured{
+				Object: map[string]interface{}{
+					"apiVersion": tc.apiVersion,
+					"kind":       tc.kind,
+					"metadata":   map[string]interface{}{"name": "test"},
+				},
+			}
+			if got := isCRD(obj); got != tc.expected {
+				t.Errorf("isCRD() = %v, want %v", got, tc.expected)
+			}
+		})
+	}
+}