Repository: operator-framework/operator-lifecycle-manager
Pull request #3165 (2024-01-31 15:01:29)

olm,catalog: only validate the resources we label
*: don't duplicate owner references

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

---

olm,catalog: only validate the resources we label

Each controller can see (and, therefore, can label) a separate set of
GVRs. When we start up, detecting if any OLM-related resource needs
labelling means that one controller may start, detect a need for
labelling a resource it cannot itself label, detect that it's labelled
everything it can, and restart. If the other operator is stuck for
whatever reason, this leads the first controller to enter
CrashLoopBackOff and break OCP upgrade.

Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>

---



openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **NOT APPROVED**

This pull-request has been approved by: *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/3165#" title="Author self-approved">stevekuznetsov</a>*
**Once this PR has been reviewed and has the lgtm label**, please assign [grokspawn](https://github.com/grokspawn) for approval. For more information see [the Kubernetes Code Review Process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process).

The full list of commands accepted by this bot can be found [here](https://go.k8s.io/bot-commands?repo=operator-framework%2Foperator-lifecycle-manager).

<details open>
Needs approval from an approver in each of these files:

- **[OWNERS](https://github.com/operator-framework/operator-lifecycle-manager/blob/master/OWNERS)**

Approvers can indicate their approval by writing `/approve` in a comment
Approvers can cancel approval by writing `/approve cancel` in a comment
</details>
<!-- META={"approvers":["grokspawn"]} -->
openshift-ci-robot: @stevekuznetsov: This pull request references [Jira Issue OCPBUGS-28744](https://issues.redhat.com//browse/OCPBUGS-28744), which is invalid:
 - expected the bug to target the "4.16.0" version, but no target version was set

Comment <code>/jira refresh</code> to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

<details>

In response to [this](https://github.com/operator-framework/operator-lifecycle-manager/pull/3165):

>*: don't duplicate owner references
>
>Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
>
>---
>
>olm,catalog: only validate the resources we label
>
>Each controller can see (and, therefore, can label) a separate set of
>GVRs. When we start up, detecting if any OLM-related resource needs
>labelling means that one controller may start, detect a need for
>labelling a resource it cannot itself label, detect that it's labelled
>everything it can, and restart. If the other operator is stuck for
>whatever reason, this leads the first controller to enter
>CrashLoopBackOff and break OCP upgrade.
>
>Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
>
>---
>
>


Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=operator-framework%2Foperator-lifecycle-manager).  If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
</details>
openshift-ci-robot: @stevekuznetsov: No Jira issue is referenced in the title of this pull request.
To reference a jira issue, add 'XYZ-NNN:' to the title of this pull request and request another refresh with <code>/jira refresh</code>.

<details>

In response to [this](https://github.com/operator-framework/operator-lifecycle-manager/pull/3165):

>*: don't duplicate owner references
>
>Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
>
>---
>
>olm,catalog: only validate the resources we label
>
>Each controller can see (and, therefore, can label) a separate set of
>GVRs. When we start up, detecting if any OLM-related resource needs
>labelling means that one controller may start, detect a need for
>labelling a resource it cannot itself label, detect that it's labelled
>everything it can, and restart. If the other operator is stuck for
>whatever reason, this leads the first controller to enter
>CrashLoopBackOff and break OCP upgrade.
>
>Signed-off-by: Steve Kuznetsov <skuznets@redhat.com>
>
>---
>
>


Instructions for interacting with me using PR comments are available [here](https://prow.ci.openshift.org/command-help?repo=operator-framework%2Foperator-lifecycle-manager).  If you have questions or suggestions related to my behavior, please file an issue against the [openshift-eng/jira-lifecycle-plugin](https://github.com/openshift-eng/jira-lifecycle-plugin/issues/new) repository.
</details>
ncdc: @stevekuznetsov I removed the downstream jira from the title of this upstream PR
ncdc: LGTM. Probably need to investigate e2e failures to see if they're flakes or due to this PR.

@joelanford PTAL at the PR - thx
ncdc: Still LGTM. Joe?
stevekuznetsov: ```
 • [FAILED] [73.452 seconds]
Operator Condition [It] OperatorCondition Upgradeable type and overrides
/home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/operator_condition_e2e_test.go:35

  Captured StdOut/StdErr Output >>
  waiting 104.190706ms for catalog pod operator-conditions-e2e-2j9rz/catalog-2ktfc to be available (for sync) - NO_CONNECTION
  waiting 1.009379511s for catalog pod operator-conditions-e2e-2j9rz/catalog-2ktfc to be available (for sync) - IDLE
  waiting 396.888151ms for catalog pod operator-conditions-e2e-2j9rz/catalog-2ktfc to be available (for sync) - CONNECTING
  waiting 19.600061548s for catalog pod operator-conditions-e2e-2j9rz/catalog-2ktfc to be available (for sync) - TRANSIENT_FAILURE
  waiting 22.600388182s for catalog pod operator-conditions-e2e-2j9rz/catalog-2ktfc to be available (for sync) - READY
  probing catalog catalog-2ktfc pod with address catalog-2ktfc.operator-conditions-e2e-2j9rz.svc:50051
  skipping health check
  waiting 189.477209ms for catalog pod operator-conditions-e2e-2j9rz/catalog-2ktfc to be available (for sync) - READY
  probing catalog catalog-2ktfc pod with address catalog-2ktfc.operator-conditions-e2e-2j9rz.svc:50051
  skipping health check
  waited 188.344521ms for catalog pod catalog-2ktfc to be available (after catalog update) - READY
  waited 1.600263746s for catalog pod catalog-2ktfc to be available (after catalog update) - IDLE
  waited 1.200102082s for catalog pod catalog-2ktfc to be available (after catalog update) - TRANSIENT_FAILURE
  catalog updated
  waiting 199.550634ms for catalog pod operator-conditions-e2e-2j9rz/catalog-2ktfc to be available (for sync) - READY
  probing catalog catalog-2ktfc pod with address catalog-2ktfc.operator-conditions-e2e-2j9rz.svc:50051
  skipping health check
  Using the kubectl kubectl binary
  Using the artifacts/operator-conditions-e2e-2j9rz output directory
  Storing the test artifact output in the artifacts/operator-conditions-e2e-2j9rz directory
  Collecting get catalogsources -o yaml output...
  Collecting get subscriptions -o yaml output...
  Collecting get operatorgroups -o yaml output...
  Collecting get clusterserviceversions -o yaml output...
  Collecting get installplans -o yaml output...
  Collecting get pods -o wide output...
  Collecting get events --sort-by .lastTimestamp output...
  << Captured StdOut/StdErr Output

  Timeline >>
  created the operator-conditions-e2e-2j9rz testing namespace
  created the operator-conditions-e2e-2j9rz/operator-conditions-e2e-2j9rz-operatorgroup operator group
  STEP: This test proves that an operator can upgrade successfully when Upgrade condition type is set in OperatorCondition spec. Plus, an operator chooses not to use OperatorCondition, the upgrade process will proceed as expected. The overrides spec in OperatorCondition can be used to override the conditions spec. The overrides spec will remain in place until they are unset. @ 01/31/24 16:28:03.84
  STEP: Create a catalog for csvA, csvB, and csvD @ 01/31/24 16:28:03.84
  STEP: Create an nginx details deployment @ 01/31/24 16:28:03.84
  STEP: Create an nginx details deployment @ 01/31/24 16:28:03.84
  STEP: Create an nginx details deployment @ 01/31/24 16:28:03.84
  STEP: set a simple default strategy if none given @ 01/31/24 16:28:03.84
  STEP: Populate owned and required @ 01/31/24 16:28:03.84
  STEP: set a simple default strategy if none given @ 01/31/24 16:28:03.84
  STEP: Populate owned and required @ 01/31/24 16:28:03.84
  STEP: set a simple default strategy if none given @ 01/31/24 16:28:03.84
  STEP: Populate owned and required @ 01/31/24 16:28:03.84
  STEP: Create the initial catalogsources @ 01/31/24 16:28:03.84
  Creating catalog source catalog-2ktfc in namespace operator-conditions-e2e-2j9rz...
  Catalog source catalog-2ktfc created
  STEP: Await csvA's success @ 01/31/24 16:28:47.779
  waiting for CSV operator-conditions-e2e-2j9rz/a-rhlbb-stable to reach condition
  error getting csv operator-conditions-e2e-2j9rz/a-rhlbb-stable: clusterserviceversions.operators.coreos.com "a-rhlbb-stable" not found
  waited 2.998616915s for csv operator-conditions-e2e-2j9rz/a-rhlbb-stable - Installing (InstallWaiting): installing: waiting for deployment a-rhlbb-stable to become ready: deployment "a-rhlbb-stable" not available: Deployment does not have minimum availability.
  waited 3m33.783553067s for CSV operator-conditions-e2e-2j9rz/a-rhlbb-stable: to be in phases [Succeeded], in phase Installing (InstallWaiting): installing: waiting for deployment a-rhlbb-stable to become ready: deployment "a-rhlbb-stable" not available: Deployment does not have minimum availability.
  waited 1.600759552s for csv operator-conditions-e2e-2j9rz/a-rhlbb-stable - Succeeded (InstallSucceeded): install strategy completed with no errors
  waited 1.600762296s for CSV operator-conditions-e2e-2j9rz/a-rhlbb-stable: to be in phases [Succeeded], in phase Succeeded (InstallSucceeded): install strategy completed with no errors
  STEP: Get the OperatorCondition for csvA and report that it is not upgradeable @ 01/31/24 16:28:52.378
  STEP: Update the catalogsources @ 01/31/24 16:28:52.388
  STEP: Attempt to get the catalog source before creating install plan(s) @ 01/31/24 16:29:08.378
  STEP: csvB will be in Pending phase due to csvA reports Upgradeable=False condition @ 01/31/24 16:29:08.577
  waiting for CSV operator-conditions-e2e-2j9rz/b-nzcpj-stable to reach condition
  waited 201.341252ms for csv operator-conditions-e2e-2j9rz/b-nzcpj-stable - Pending (RequirementsUnknown): requirements not yet checked
  waited 201.349377ms for CSV operator-conditions-e2e-2j9rz/b-nzcpj-stable: to have reasons [OperatorConditionNotUpgradeable], in phase Pending (RequirementsUnknown): requirements not yet checked
  waited 408.946205ms for csv operator-conditions-e2e-2j9rz/b-nzcpj-stable - Pending (OperatorConditionNotUpgradeable): operator is not upgradeable: the operator is not upgradeable: test
  waited 408.950893ms for CSV operator-conditions-e2e-2j9rz/b-nzcpj-stable: to have reasons [OperatorConditionNotUpgradeable], in phase Pending (OperatorConditionNotUpgradeable): operator is not upgradeable: the operator is not upgradeable: test
  STEP: Get the OperatorCondition for csvA and report that it is upgradeable, unblocking csvB @ 01/31/24 16:29:09.188
  STEP: Await csvB's success @ 01/31/24 16:29:09.2
  waiting for CSV operator-conditions-e2e-2j9rz/b-nzcpj-stable to reach condition
  waited 178.484551ms for csv operator-conditions-e2e-2j9rz/b-nzcpj-stable - Pending (OperatorConditionNotUpgradeable): operator is not upgradeable: the operator is not upgradeable: test
  waited 17.00064196s for CSV operator-conditions-e2e-2j9rz/b-nzcpj-stable: to be in phases [Succeeded], in phase Pending (OperatorConditionNotUpgradeable): operator is not upgradeable: the operator is not upgradeable: test
  waited 435.567688ms for csv operator-conditions-e2e-2j9rz/b-nzcpj-stable - InstallReady (AllRequirementsMet): all requirements found, attempting install
  waited 435.572839ms for CSV operator-conditions-e2e-2j9rz/b-nzcpj-stable: to be in phases [Succeeded], in phase InstallReady (AllRequirementsMet): all requirements found, attempting install
  waited 563.877107ms for csv operator-conditions-e2e-2j9rz/b-nzcpj-stable - Installing (InstallSucceeded): waiting for install components to report healthy
  waited 563.87914ms for CSV operator-conditions-e2e-2j9rz/b-nzcpj-stable: to be in phases [Succeeded], in phase Installing (InstallSucceeded): waiting for install components to report healthy
  waited 400.920194ms for csv operator-conditions-e2e-2j9rz/b-nzcpj-stable - Installing (InstallWaiting): installing: waiting for deployment b-nzcpj-stable to become ready: deployment "b-nzcpj-stable" not available: Deployment does not have minimum availability.
  waited 400.922819ms for CSV operator-conditions-e2e-2j9rz/b-nzcpj-stable: to be in phases [Succeeded], in phase Installing (InstallWaiting): installing: waiting for deployment b-nzcpj-stable to become ready: deployment "b-nzcpj-stable" not available: Deployment does not have minimum availability.
  waited 1.600221574s for csv operator-conditions-e2e-2j9rz/b-nzcpj-stable - Succeeded (InstallSucceeded): install strategy completed with no errors
  waited 1.600224088s for CSV operator-conditions-e2e-2j9rz/b-nzcpj-stable: to be in phases [Succeeded], in phase Succeeded (InstallSucceeded): install strategy completed with no errors
  STEP: Get the OperatorCondition for csvB and purposedly change ObservedGeneration @ 01/31/24 16:29:12.38
  STEP: to cause mismatch generation situation @ 01/31/24 16:29:12.38
  Cleaning catalog source catalog-2ktfc
  Deleting config map catalog-2ktfc-configmap...
  Deleting catalog source catalog-2ktfc...
  waiting for the catalog source catalog-2ktfc-bjvvk pod to be deleted...
  Done cleaning catalog source catalog-2ktfc
  [FAILED] in [It] - /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/operator_condition_e2e_test.go:164 @ 01/31/24 16:29:13.79
  collecting the operator-conditions-e2e-2j9rz namespace artifacts as the 'OperatorCondition Upgradeable type and overrides' test case failed
  collecting logs in the ./artifacts/ artifacts directory
  tearing down the operator-conditions-e2e-2j9rz namespace
  resetting e2e kube client
  deleting operator-conditions-e2e-2j9rz/operator-conditions-e2e-2j9rz-operatorgroup
  deleting <global>/operator-conditions-e2e-2j9rz
  deleting package-manifest-e2epjzjc/package-manifest-e2epjzjc-operatorgroup
  deleting <global>/package-manifest-e2epjzjc
  deleting package-manifest-e2eq5fzh/package-manifest-e2eq5fzh-operatorgroup
  deleting <global>/package-manifest-e2eq5fzh
  deleting package-manifest-e2elldcm/package-manifest-e2elldcm-operatorgroup
  deleting <global>/package-manifest-e2elldcm
  deleting package-manifest-e2esf77w/package-manifest-e2esf77w-operatorgroup
  deleting <global>/package-manifest-e2esf77w
  deleting package-manifest-e2e88vf4/package-manifest-e2e88vf4-operatorgroup
  deleting <global>/package-manifest-e2e88vf4
  garbage collecting CRDs
  deleting crd a-rhlbbx7ntt.cluster.com
  << Timeline

  [FAILED] Timed out after 0.101s.
  Expected success, but got an error:
      <*errors.StatusError | 0xc0010e5a40>: 
      Operation cannot be fulfilled on operatorconditions.operators.coreos.com "b-nzcpj-stable": the object has been modified; please apply your changes to the latest version and try again
      {
          ErrStatus: {
              TypeMeta: {Kind: "", APIVersion: ""},
              ListMeta: {
                  SelfLink: "",
                  ResourceVersion: "",
                  Continue: "",
                  RemainingItemCount: nil,
              },
              Status: "Failure",
              Message: "Operation cannot be fulfilled on operatorconditions.operators.coreos.com \"b-nzcpj-stable\": the object has been modified; please apply your changes to the latest version and try again",
              Reason: "Conflict",
              Details: {
                  Name: "b-nzcpj-stable",
                  Group: "operators.coreos.com",
                  Kind: "operatorconditions",
                  UID: "",
                  Causes: nil,
                  RetryAfterSeconds: 0,
              },
              Code: 409,
          },
      }
  In [It] at: /home/runner/work/operator-lifecycle-manager/operator-lifecycle-manager/test/e2e/operator_condition_e2e_test.go:164 @ 01/31/24 16:29:13.79
```

Can't grok how that could be our fault
tmshort: /retest

Patch:
diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go
index 53b76c627..18b9d92e6 100644
--- a/pkg/controller/operators/catalog/operator.go
+++ b/pkg/controller/operators/catalog/operator.go
@@ -186,7 +186,7 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
 		return nil, err
 	}
 
-	canFilter, err := labeller.Validate(ctx, logger, metadataClient, crClient)
+	canFilter, err := labeller.Validate(ctx, logger, metadataClient, crClient, labeller.IdentityCatalogOperator)
 	if err != nil {
 		return nil, err
 	}
@@ -541,6 +541,49 @@ func NewOperator(ctx context.Context, kubeconfigPath string, clock utilclock.Clo
 		return nil, err
 	}
 
+	{
+		gvr := servicesgvk
+		informer := serviceInformer.Informer()
+
+		logger := op.logger.WithFields(logrus.Fields{"gvr": gvr.String()})
+		logger.Info("registering owner reference fixer")
+
+		queue := workqueue.NewRateLimitingQueueWithConfig(workqueue.DefaultControllerRateLimiter(), workqueue.RateLimitingQueueConfig{
+			Name: gvr.String(),
+		})
+		queueInformer, err := queueinformer.NewQueueInformer(
+			ctx,
+			queueinformer.WithQueue(queue),
+			queueinformer.WithLogger(op.logger),
+			queueinformer.WithInformer(informer),
+			queueinformer.WithSyncer(queueinformer.LegacySyncHandler(func(obj interface{}) error {
+				service, ok := obj.(*corev1.Service)
+				if !ok {
+					err := fmt.Errorf("wrong type %T, expected %T: %#v", obj, new(*corev1.Service), obj)
+					logger.WithError(err).Error("casting failed")
+					return fmt.Errorf("casting failed: %w", err)
+				}
+
+				deduped := deduplicateOwnerReferences(service.OwnerReferences)
+				if len(deduped) != len(service.OwnerReferences) {
+					localCopy := service.DeepCopy()
+					localCopy.OwnerReferences = deduped
+					if _, err := op.opClient.KubernetesInterface().CoreV1().Services(service.Namespace).Update(ctx, localCopy, metav1.UpdateOptions{}); err != nil {
+						return err
+					}
+				}
+				return nil
+			}).ToSyncer()),
+		)
+		if err != nil {
+			return nil, err
+		}
+
+		if err := op.RegisterQueueInformer(queueInformer); err != nil {
+			return nil, err
+		}
+	}
+
 	// Wire Pods for CatalogSource
 	catsrcReq, err := labels.NewRequirement(reconciler.CatalogSourceLabelKey, selection.Exists, nil)
 	if err != nil {
@@ -2860,3 +2903,7 @@ func (o *Operator) apiresourceFromGVK(gvk schema.GroupVersionKind) (metav1.APIRe
 	logger.Info("couldn't find GVK in api discovery")
 	return metav1.APIResource{}, olmerrors.GroupVersionKindNotFoundError{Group: gvk.Group, Version: gvk.Version, Kind: gvk.Kind}
 }
+
+func deduplicateOwnerReferences(refs []metav1.OwnerReference) []metav1.OwnerReference {
+	return sets.New(refs...).UnsortedList()
+}
diff --git a/pkg/controller/operators/labeller/filters.go b/pkg/controller/operators/labeller/filters.go
index 2167d3454..333bc2b9e 100644
--- a/pkg/controller/operators/labeller/filters.go
+++ b/pkg/controller/operators/labeller/filters.go
@@ -80,7 +80,39 @@ var filters = map[schema.GroupVersionResource]func(metav1.Object) bool{
 	},
 }
 
-func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadata.Interface, operatorClient operators.Interface) (bool, error) {
+type Identity string
+
+const (
+	IdentityCatalogOperator = "catalog-operator"
+	IdentityOLMOperator     = "olm-operator"
+)
+
+func gvrRelevant(identity Identity) func(schema.GroupVersionResource) bool {
+	switch identity {
+	case IdentityCatalogOperator:
+		return sets.New(
+			rbacv1.SchemeGroupVersion.WithResource("roles"),
+			rbacv1.SchemeGroupVersion.WithResource("rolebindings"),
+			corev1.SchemeGroupVersion.WithResource("serviceaccounts"),
+			corev1.SchemeGroupVersion.WithResource("services"),
+			corev1.SchemeGroupVersion.WithResource("pods"),
+			corev1.SchemeGroupVersion.WithResource("configmaps"),
+			batchv1.SchemeGroupVersion.WithResource("jobs"),
+			apiextensionsv1.SchemeGroupVersion.WithResource("customresourcedefinitions"),
+		).Has
+	case IdentityOLMOperator:
+		return sets.New(
+			appsv1.SchemeGroupVersion.WithResource("deployments"),
+			rbacv1.SchemeGroupVersion.WithResource("clusterroles"),
+			rbacv1.SchemeGroupVersion.WithResource("clusterrolebindings"),
+			apiextensionsv1.SchemeGroupVersion.WithResource("customresourcedefinitions"),
+		).Has
+	default:
+		panic("programmer error: invalid identity")
+	}
+}
+
+func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadata.Interface, operatorClient operators.Interface, identity Identity) (bool, error) {
 	okLock := sync.Mutex{}
 	ok := true
 	g, ctx := errgroup.WithContext(ctx)
@@ -125,6 +157,10 @@ func Validate(ctx context.Context, logger *logrus.Logger, metadataClient metadat
 	})
 
 	for gvr, filter := range allFilters {
+		if !gvrRelevant(identity)(gvr) {
+			logger.WithField("gvr", gvr.String()).Info("skipping irrelevant gvr")
+			continue
+		}
 		gvr, filter := gvr, filter
 		g.Go(func() error {
 			list, err := metadataClient.Resource(gvr).List(ctx, metav1.ListOptions{})
diff --git a/pkg/controller/operators/olm/operator.go b/pkg/controller/operators/olm/operator.go
index 44d3645ac..ee75f84c0 100644
--- a/pkg/controller/operators/olm/operator.go
+++ b/pkg/controller/operators/olm/operator.go
@@ -183,7 +183,7 @@ func newOperatorWithConfig(ctx context.Context, config *operatorConfig) (*Operat
 		return nil, err
 	}
 
-	canFilter, err := labeller.Validate(ctx, config.logger, config.metadataClient, config.externalClient)
+	canFilter, err := labeller.Validate(ctx, config.logger, config.metadataClient, config.externalClient, labeller.IdentityOLMOperator)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/lib/ownerutil/util.go b/pkg/lib/ownerutil/util.go
index dafaeaf8b..ebdd73a31 100644
--- a/pkg/lib/ownerutil/util.go
+++ b/pkg/lib/ownerutil/util.go
@@ -167,20 +167,13 @@ func AddNonBlockingOwner(object metav1.Object, owner Owner) {
 		ownerRefs = []metav1.OwnerReference{}
 	}
 
-	// Infer TypeMeta for the target
-	if err := InferGroupVersionKind(owner); err != nil {
-		log.Warn(err.Error())
-	}
-	gvk := owner.GetObjectKind().GroupVersionKind()
-
+	nonBlockingOwner := NonBlockingOwner(owner)
 	for _, item := range ownerRefs {
-		if item.Kind == gvk.Kind {
-			if item.Name == owner.GetName() && item.UID == owner.GetUID() {
-				return
-			}
+		if item.Kind == nonBlockingOwner.Kind && item.Name == nonBlockingOwner.Name && item.UID == nonBlockingOwner.UID {
+			return
 		}
 	}
-	ownerRefs = append(ownerRefs, NonBlockingOwner(owner))
+	ownerRefs = append(ownerRefs, nonBlockingOwner)
 	object.SetOwnerReferences(ownerRefs)
 }
 
@@ -284,14 +277,20 @@ func AddOwner(object metav1.Object, owner Owner, blockOwnerDeletion, isControlle
 	}
 	gvk := owner.GetObjectKind().GroupVersionKind()
 	apiVersion, kind := gvk.ToAPIVersionAndKind()
-	ownerRefs = append(ownerRefs, metav1.OwnerReference{
+	ownerRef := metav1.OwnerReference{
 		APIVersion:         apiVersion,
 		Kind:               kind,
 		Name:               owner.GetName(),
 		UID:                owner.GetUID(),
 		BlockOwnerDeletion: &blockOwnerDeletion,
 		Controller:         &isController,
-	})
+	}
+	for _, ref := range ownerRefs {
+		if ref.Kind == ownerRef.Kind && ref.Name == ownerRef.Name && ref.UID == ownerRef.UID {
+			return
+		}
+	}
+	ownerRefs = append(ownerRefs, ownerRef)
 	object.SetOwnerReferences(ownerRefs)
 }
 


Test patch:
diff --git a/pkg/controller/operators/catalog/ownerrefs_test.go b/pkg/controller/operators/catalog/ownerrefs_test.go
new file mode 100644
index 000000000..097a8409b
--- /dev/null
+++ b/pkg/controller/operators/catalog/ownerrefs_test.go
@@ -0,0 +1,42 @@
+package catalog
+
+import (
+	"testing"
+
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+)
+
+func TestDedupe(t *testing.T) {
+	yes := true
+	refs := []metav1.OwnerReference{
+		{
+			APIVersion:         "apiversion",
+			Kind:               "kind",
+			Name:               "name",
+			UID:                "uid",
+			Controller:         &yes,
+			BlockOwnerDeletion: &yes,
+		},
+		{
+			APIVersion:         "apiversion",
+			Kind:               "kind",
+			Name:               "name",
+			UID:                "uid",
+			Controller:         &yes,
+			BlockOwnerDeletion: &yes,
+		},
+		{
+			APIVersion:         "apiversion",
+			Kind:               "kind",
+			Name:               "name",
+			UID:                "uid",
+			Controller:         &yes,
+			BlockOwnerDeletion: &yes,
+		},
+	}
+	deduped := deduplicateOwnerReferences(refs)
+	t.Logf("got %d deduped from %d", len(deduped), len(refs))
+	if len(deduped) == len(refs) {
+		t.Errorf("didn't dedupe: %#v", deduped)
+	}
+}
diff --git a/pkg/lib/testobj/runtime.go b/pkg/lib/testobj/runtime.go
index 70da4f892..6fd7c8b72 100644
--- a/pkg/lib/testobj/runtime.go
+++ b/pkg/lib/testobj/runtime.go
@@ -135,14 +135,21 @@ func WithOwner(owner, obj RuntimeMetaObject) RuntimeMetaObject {
 	out := obj.DeepCopyObject().(RuntimeMetaObject)
 	gvk := owner.GetObjectKind().GroupVersionKind()
 	apiVersion, kind := gvk.ToAPIVersionAndKind()
-	refs := append(out.GetOwnerReferences(), metav1.OwnerReference{
+
+	nonBlockingOwner := metav1.OwnerReference{
 		APIVersion:         apiVersion,
 		Kind:               kind,
 		Name:               owner.GetName(),
 		UID:                owner.GetUID(),
 		BlockOwnerDeletion: &dontBlockOwnerDeletion,
 		Controller:         &notController,
-	})
+	}
+	for _, item := range out.GetOwnerReferences() {
+		if item.Kind == nonBlockingOwner.Kind && item.Name == nonBlockingOwner.Name && item.UID == nonBlockingOwner.UID {
+			return out
+		}
+	}
+	refs := append(out.GetOwnerReferences(), nonBlockingOwner)
 	out.SetOwnerReferences(refs)
 
 	return out