Repository: operator-framework/operator-lifecycle-manager
Pull request #2523 (2021-12-13 02:55:43)

Add system constraint provider logic to resolver
Signed-off-by: Per G. da Silva <perdasilva@redhat.com>

**Description of the change:**
This PR adds a `systemConstraintsProvider` to the `resolver`. It also modifies the resolver to apply any constraints comming from the `systemContraintsProvider`. Secondly, it add `initHooks` to the `OperatorStepResolver`. This allows the downstream to modify resolver behaviour.

**Motivation for the change:**
For 4.10 we'd like to have a way for the resolver to respect the `maxOpenShiftVersion` property that can be provided by bundles to ensure that operator supports the cluster onto which it is being installed. This is done by injecting an OpenShift `systemConstraintsProvider` into the `satResolver` using an `initHook` on the downstream.

The init hooks provide a lightweight way of doing this from downstream without affecting the upstream or providing (in the upstream) a way for users to reach the `systemConstraintsProvider`. At least until we want to open this more generally.

*Note:* This PR must be merged sync'ed with the downstream before the downstream PR can be cut


**Reviewer Checklist**
- [ ] Implementation matches the proposed design, or proposal is updated to match implementation
- [ ] Sufficient unit test coverage
- [ ] Sufficient end-to-end test coverage
- [ ] Docs updated or added to `/doc`
- [ ] Commit messages sensible and descriptive


<!--

Note: If this PR is fixing an issue make sure to add a note saying:
Closes #<ISSUE_NUMBER>

-->


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/2523#" title="Author self-approved">perdasilva</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **kevinrizza** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @kevinrizza` in a comment when ready.

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":["kevinrizza"]} -->
perdasilva: /hold until #2418 is merged and this had been rebased
perdasilva: > @perdasilva I can see the commit here that modifies the kubeconfig flag. Should that be a separate PR if it's not related to what the PR title/summary talks about, unless it's absolutely necessary for that commit to be in this PR?
> 
> Also, is the downstream PR ready? It's hard to grasp how the `init` hook will be used or what code changes here are motivated by the eventual downstream code changes without looking at that code beside this.

@anik120 
I've removed the commit with the flag fixes. Here's a PR: https://github.com/perdasilva/operator-framework-olm/pull/1 that contains both up and downs stream changes. (the upstream part may slightly differ due to changes in response to reviewer comments - but should be fine).
perdasilva: /unhold
openshift-ci[bot]: [APPROVALNOTIFIER] This PR is **APPROVED**

This pull-request has been approved by: *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/2523#pullrequestreview-831898382" title="Approved">njhale</a>*, *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/2523#" title="Author self-approved">perdasilva</a>*

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).

The pull request process is described [here](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process)

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

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

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":[]} -->
openshift-ci[bot]: New changes are detected. LGTM label has been removed.
estroz: /lgtm
openshift-bot: /retest-required

Please review the [full test history](https://prow.ci.openshift.org/pr-history/?org=operator-framework&repo=operator-lifecycle-manager&pr=2523) for this PR and help us cut down flakes.
openshift-bot: /retest-required

Please review the [full test history](https://prow.ci.openshift.org/pr-history/?org=operator-framework&repo=operator-lifecycle-manager&pr=2523) for this PR and help us cut down flakes.
openshift-bot: /retest-required

Please review the [full test history](https://prow.ci.openshift.org/pr-history/?org=operator-framework&repo=operator-lifecycle-manager&pr=2523) for this PR and help us cut down flakes.
openshift-bot: /retest-required

Please review the [full test history](https://prow.ci.openshift.org/pr-history/?org=operator-framework&repo=operator-lifecycle-manager&pr=2523) for this PR and help us cut down flakes.
perdasilva: /retest
openshift-ci[bot]: New changes are detected. LGTM label has been removed.
timflannagan: /lgtm
perdasilva: /retest

openshift-bot: /retest-required

Please review the [full test history](https://prow.ci.openshift.org/pr-history/?org=operator-framework&repo=operator-lifecycle-manager&pr=2523) for this PR and help us cut down flakes.
openshift-bot: /retest-required

Please review the [full test history](https://prow.ci.openshift.org/pr-history/?org=operator-framework&repo=operator-lifecycle-manager&pr=2523) for this PR and help us cut down flakes.
openshift-bot: /retest-required

Please review the [full test history](https://prow.ci.openshift.org/pr-history/?org=operator-framework&repo=operator-lifecycle-manager&pr=2523) for this PR and help us cut down flakes.
openshift-bot: /retest-required

Please review the [full test history](https://prow.ci.openshift.org/pr-history/?org=operator-framework&repo=operator-lifecycle-manager&pr=2523) for this PR and help us cut down flakes.
perdasilva: /retest

openshift-bot: /retest-required

Please review the [full test history](https://prow.ci.openshift.org/pr-history/?org=operator-framework&repo=operator-lifecycle-manager&pr=2523) for this PR and help us cut down flakes.

Patch:
diff --git a/pkg/controller/operators/catalog/operator.go b/pkg/controller/operators/catalog/operator.go
index d152b399a..a749a9c11 100644
--- a/pkg/controller/operators/catalog/operator.go
+++ b/pkg/controller/operators/catalog/operator.go
@@ -803,6 +803,7 @@ func (o *Operator) syncConnection(logger *logrus.Entry, in *v1alpha1.CatalogSour
 		// Set connection status and return.
 		out.Status.GRPCConnectionState.LastConnectTime = now
 		out.Status.GRPCConnectionState.LastObservedState = source.ConnectionState.String()
+		out.Status.GRPCConnectionState.Address = source.Address
 	}
 
 	return
diff --git a/pkg/controller/registry/reconciler/grpc.go b/pkg/controller/registry/reconciler/grpc.go
index 8406740be..145d29bce 100644
--- a/pkg/controller/registry/reconciler/grpc.go
+++ b/pkg/controller/registry/reconciler/grpc.go
@@ -191,7 +191,7 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(catalogSource *v1alpha1.Ca
 	source := grpcCatalogSourceDecorator{catalogSource}
 
 	// if service status is nil, we force create every object to ensure they're created the first time
-	overwrite := source.Status.RegistryServiceStatus == nil
+	overwrite := source.Status.RegistryServiceStatus == nil || !isRegistryServiceStatusValid(&source)
 
 	//TODO: if any of these error out, we should write a status back (possibly set RegistryServiceStatus to nil so they get recreated)
 	sa, err := c.ensureSA(source)
@@ -216,17 +216,33 @@ func (c *GrpcRegistryReconciler) EnsureRegistryServer(catalogSource *v1alpha1.Ca
 
 	if overwritePod {
 		now := c.now()
+		service := source.Service()
 		catalogSource.Status.RegistryServiceStatus = &v1alpha1.RegistryServiceStatus{
 			CreatedAt:        now,
 			Protocol:         "grpc",
-			ServiceName:      source.Service().GetName(),
+			ServiceName:      service.GetName(),
 			ServiceNamespace: source.GetNamespace(),
-			Port:             fmt.Sprintf("%d", source.Service().Spec.Ports[0].Port),
+			Port:             getPort(service),
 		}
 	}
 	return nil
 }
 
+func getPort(service *corev1.Service) string {
+	return fmt.Sprintf("%d", service.Spec.Ports[0].Port)
+}
+
+func isRegistryServiceStatusValid(source *grpcCatalogSourceDecorator) bool {
+	service := source.Service()
+	if source.Status.RegistryServiceStatus.ServiceName != service.GetName() ||
+		source.Status.RegistryServiceStatus.ServiceNamespace != service.GetNamespace() ||
+		source.Status.RegistryServiceStatus.Port != getPort(service) ||
+		source.Status.RegistryServiceStatus.Protocol != "grpc" {
+		return false
+	}
+	return true
+}
+
 func (c *GrpcRegistryReconciler) ensurePod(source grpcCatalogSourceDecorator, saName string, overwrite bool) error {
 	// currentLivePods refers to the currently live instances of the catalog source
 	currentLivePods := c.currentPods(source)
diff --git a/pkg/controller/registry/resolver/hooks.go b/pkg/controller/registry/resolver/hooks.go
new file mode 100644
index 000000000..aaa98e64c
--- /dev/null
+++ b/pkg/controller/registry/resolver/hooks.go
@@ -0,0 +1,9 @@
+package resolver
+
+// stepResolverInitHook provides a way for the downstream
+// to modify the step resolver at creation time.
+// This is a bit of a hack to enable system constraints downstream
+// without affecting the upstream. We may want to clean this up when
+// either we have a more pluggable architecture; or system constraints
+// come to the upstream
+type stepResolverInitHook func(*OperatorStepResolver) error
diff --git a/pkg/controller/registry/resolver/resolver.go b/pkg/controller/registry/resolver/resolver.go
index f1b5d42a0..427bbfaed 100644
--- a/pkg/controller/registry/resolver/resolver.go
+++ b/pkg/controller/registry/resolver/resolver.go
@@ -26,9 +26,10 @@ type OperatorResolver interface {
 }
 
 type SatResolver struct {
-	cache cache.OperatorCacheProvider
-	log   logrus.FieldLogger
-	pc    *predicateConverter
+	cache                     cache.OperatorCacheProvider
+	log                       logrus.FieldLogger
+	pc                        *predicateConverter
+	systemConstraintsProvider solver.ConstraintProvider
 }
 
 func NewDefaultSatResolver(rcp cache.SourceProvider, catsrcLister v1alpha1listers.CatalogSourceLister, logger logrus.FieldLogger) *SatResolver {
@@ -177,6 +178,25 @@ func (r *SatResolver) SolveOperators(namespaces []string, csvs []*v1alpha1.Clust
 	return operators, nil
 }
 
+// newBundleInstallableFromEntry converts an entry into a bundle installable with
+// system constraints applied, if they are defined for the entry
+func (r *SatResolver) newBundleInstallableFromEntry(entry *cache.Entry) (*BundleInstallable, error) {
+	bundleInstalleble, err := NewBundleInstallableFromOperator(entry)
+	if err != nil {
+		return nil, err
+	}
+
+	// apply system constraints if necessary
+	if r.systemConstraintsProvider != nil && !(entry.SourceInfo.Catalog.Virtual()) {
+		systemConstraints, err := r.systemConstraintsProvider.Constraints(entry)
+		if err != nil {
+			return nil, err
+		}
+		bundleInstalleble.constraints = append(bundleInstalleble.constraints, systemConstraints...)
+	}
+	return &bundleInstalleble, nil
+}
+
 func (r *SatResolver) getSubscriptionInstallables(sub *v1alpha1.Subscription, current *cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleInstallable) (map[solver.Identifier]solver.Installable, error) {
 	var cachePredicates, channelPredicates []cache.Predicate
 	installables := make(map[solver.Identifier]solver.Installable, 0)
@@ -334,13 +354,13 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
 			continue
 		}
 
-		bundleInstallable, err := NewBundleInstallableFromOperator(bundle)
+		bundleInstallable, err := r.newBundleInstallableFromEntry(bundle)
 		if err != nil {
 			errs = append(errs, err)
 			continue
 		}
 
-		visited[bundle] = &bundleInstallable
+		visited[bundle] = bundleInstallable
 
 		dependencyPredicates, err := r.pc.convertDependencyProperties(bundle.Properties)
 		if err != nil {
@@ -389,12 +409,12 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
 			// (after sorting) to remove all bundles that
 			// don't satisfy the dependency.
 			for _, b := range cache.Filter(sortedBundles, d) {
-				i, err := NewBundleInstallableFromOperator(b)
+				i, err := r.newBundleInstallableFromEntry(b)
 				if err != nil {
 					errs = append(errs, err)
 					continue
 				}
-				installables[i.Identifier()] = &i
+				installables[i.Identifier()] = i
 				bundleDependencies = append(bundleDependencies, i.Identifier())
 				bundleStack = append(bundleStack, b)
 			}
@@ -404,7 +424,7 @@ func (r *SatResolver) getBundleInstallables(preferredNamespace string, bundleSta
 			))
 		}
 
-		installables[bundleInstallable.Identifier()] = &bundleInstallable
+		installables[bundleInstallable.Identifier()] = bundleInstallable
 	}
 
 	if len(errs) > 0 {
diff --git a/pkg/controller/registry/resolver/solver/constraint_provider.go b/pkg/controller/registry/resolver/solver/constraint_provider.go
new file mode 100644
index 000000000..ce119abed
--- /dev/null
+++ b/pkg/controller/registry/resolver/solver/constraint_provider.go
@@ -0,0 +1,19 @@
+package solver
+
+import "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver/cache"
+
+// ConstraintProvider knows how to provide solver constraints for a given cache entry.
+// For instance, it could be used to surface additional constraints against an entry given some
+// properties it may expose. E.g. olm.maxOpenShiftVersion could be checked against the cluster version
+// and prohibit any entry that doesn't meet the requirement
+type ConstraintProvider interface {
+	// Constraints returns a set of solver constraints for a cache entry.
+	Constraints(e *cache.Entry) ([]Constraint, error)
+}
+
+// ConstraintProviderFunc is a simple implementation of ConstraintProvider
+type ConstraintProviderFunc func(e *cache.Entry) ([]Constraint, error)
+
+func (c ConstraintProviderFunc) Constraints(e *cache.Entry) ([]Constraint, error) {
+	return c(e)
+}
diff --git a/pkg/controller/registry/resolver/step_resolver.go b/pkg/controller/registry/resolver/step_resolver.go
index 079b92806..224cc277b 100644
--- a/pkg/controller/registry/resolver/step_resolver.go
+++ b/pkg/controller/registry/resolver/step_resolver.go
@@ -26,6 +26,9 @@ const (
 	BundleLookupConditionPacked v1alpha1.BundleLookupConditionType = "BundleLookupNotPersisted"
 )
 
+// init hooks provides the downstream a way to modify the upstream behavior
+var initHooks []stepResolverInitHook
+
 var timeNow = func() metav1.Time { return metav1.NewTime(time.Now().UTC()) }
 
 type StepResolver interface {
@@ -48,7 +51,7 @@ var _ StepResolver = &OperatorStepResolver{}
 
 func NewOperatorStepResolver(lister operatorlister.OperatorLister, client versioned.Interface, kubeclient kubernetes.Interface,
 	globalCatalogNamespace string, provider RegistryClientProvider, log logrus.FieldLogger) *OperatorStepResolver {
-	return &OperatorStepResolver{
+	stepResolver := &OperatorStepResolver{
 		subLister:              lister.OperatorsV1alpha1().SubscriptionLister(),
 		csvLister:              lister.OperatorsV1alpha1().ClusterServiceVersionLister(),
 		ipLister:               lister.OperatorsV1alpha1().InstallPlanLister(),
@@ -58,6 +61,15 @@ func NewOperatorStepResolver(lister operatorlister.OperatorLister, client versio
 		satResolver:            NewDefaultSatResolver(SourceProviderFromRegistryClientProvider(provider, log), lister.OperatorsV1alpha1().CatalogSourceLister(), log),
 		log:                    log,
 	}
+
+	// init hooks can be added to the downstream to
+	// modify resolver behaviour
+	for _, initHook := range initHooks {
+		if err := initHook(stepResolver); err != nil {
+			panic(err)
+		}
+	}
+	return stepResolver
 }
 
 func (r *OperatorStepResolver) Expire(key cache.SourceKey) {


Test patch:
diff --git a/pkg/controller/operators/catalog/operator_test.go b/pkg/controller/operators/catalog/operator_test.go
index f6aceb8bc..8abd022d9 100644
--- a/pkg/controller/operators/catalog/operator_test.go
+++ b/pkg/controller/operators/catalog/operator_test.go
@@ -49,6 +49,7 @@ import (
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/clientset/versioned/fake"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/api/client/informers/externalversions"
 	olmerrors "github.com/operator-framework/operator-lifecycle-manager/pkg/controller/errors"
+	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/grpc"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/reconciler"
 	"github.com/operator-framework/operator-lifecycle-manager/pkg/controller/registry/resolver"
@@ -758,6 +759,12 @@ func TestExecutePlanDynamicResources(t *testing.T) {
 	}
 }
 
+func withStatus(catalogSource v1alpha1.CatalogSource, status v1alpha1.CatalogSourceStatus) *v1alpha1.CatalogSource {
+	copy := catalogSource.DeepCopy()
+	copy.Status = status
+	return copy
+}
+
 func TestSyncCatalogSources(t *testing.T) {
 	clockFake := utilclock.NewFakeClock(time.Date(2018, time.January, 26, 20, 40, 0, 0, time.UTC))
 	now := metav1.NewTime(clockFake.Now())
@@ -786,14 +793,15 @@ func TestSyncCatalogSources(t *testing.T) {
 		},
 	}
 	tests := []struct {
-		testName       string
-		namespace      string
-		catalogSource  *v1alpha1.CatalogSource
-		k8sObjs        []runtime.Object
-		configMap      *corev1.ConfigMap
-		expectedStatus *v1alpha1.CatalogSourceStatus
-		expectedObjs   []runtime.Object
-		expectedError  error
+		testName        string
+		namespace       string
+		catalogSource   *v1alpha1.CatalogSource
+		k8sObjs         []runtime.Object
+		configMap       *corev1.ConfigMap
+		expectedStatus  *v1alpha1.CatalogSourceStatus
+		expectedObjs    []runtime.Object
+		expectedError   error
+		existingSources []sourceAddress
 	}{
 		{
 			testName:  "CatalogSourceWithInvalidSourceType",
@@ -1013,6 +1021,47 @@ func TestSyncCatalogSources(t *testing.T) {
 			},
 			expectedError: nil,
 		},
+		{
+			testName:  "GRPCConnectionStateAddressIsUpdated",
+			namespace: "cool-namespace",
+			catalogSource: withStatus(*grpcCatalog, v1alpha1.CatalogSourceStatus{
+				RegistryServiceStatus: &v1alpha1.RegistryServiceStatus{
+					Protocol:         "grpc",
+					ServiceName:      "cool-catalog",
+					ServiceNamespace: "cool-namespace",
+					Port:             "50051",
+					CreatedAt:        now,
+				},
+				GRPCConnectionState: &v1alpha1.GRPCConnectionState{
+					Address: "..svc:", // Needs to be updated to cool-catalog.cool-namespace.svc:50051
+				},
+			}),
+			k8sObjs: []runtime.Object{
+				pod(*grpcCatalog),
+				service(grpcCatalog.GetName(), grpcCatalog.GetNamespace()),
+			},
+			existingSources: []sourceAddress{
+				{
+					sourceKey: registry.CatalogKey{Name: "cool-catalog", Namespace: "cool-namespace"},
+					address:   "cool-catalog.cool-namespace.svc:50051",
+				},
+			},
+			expectedStatus: &v1alpha1.CatalogSourceStatus{
+				RegistryServiceStatus: &v1alpha1.RegistryServiceStatus{
+					Protocol:         "grpc",
+					ServiceName:      "cool-catalog",
+					ServiceNamespace: "cool-namespace",
+					Port:             "50051",
+					CreatedAt:        now,
+				},
+				GRPCConnectionState: &v1alpha1.GRPCConnectionState{
+					Address:           "cool-catalog.cool-namespace.svc:50051",
+					LastObservedState: "",
+					LastConnectTime:   now,
+				},
+			},
+			expectedError: nil,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.testName, func(t *testing.T) {
@@ -1023,7 +1072,7 @@ func TestSyncCatalogSources(t *testing.T) {
 			ctx, cancel := context.WithCancel(context.TODO())
 			defer cancel()
 
-			op, err := NewFakeOperator(ctx, tt.namespace, []string{tt.namespace}, withClock(clockFake), withClientObjs(clientObjs...), withK8sObjs(tt.k8sObjs...))
+			op, err := NewFakeOperator(ctx, tt.namespace, []string{tt.namespace}, withClock(clockFake), withClientObjs(clientObjs...), withK8sObjs(tt.k8sObjs...), withSources(tt.existingSources...))
 			require.NoError(t, err)
 
 			// Run sync
@@ -1040,6 +1089,13 @@ func TestSyncCatalogSources(t *testing.T) {
 			require.NotEmpty(t, updated)
 
 			if tt.expectedStatus != nil {
+				if tt.expectedStatus.GRPCConnectionState != nil {
+					updated.Status.GRPCConnectionState.LastConnectTime = now
+					// Ignore LastObservedState difference if an expected LastObservedState is no provided
+					if tt.expectedStatus.GRPCConnectionState.LastObservedState == "" {
+						updated.Status.GRPCConnectionState.LastObservedState = ""
+					}
+				}
 				require.NotEmpty(t, updated.Status)
 				require.Equal(t, *tt.expectedStatus, updated.Status)
 
@@ -1384,6 +1440,7 @@ type fakeOperatorConfig struct {
 	resolver      resolver.StepResolver
 	recorder      record.EventRecorder
 	reconciler    reconciler.RegistryReconcilerFactory
+	sources       []sourceAddress
 }
 
 // fakeOperatorOption applies an option to the given fake operator configuration.
@@ -1395,6 +1452,12 @@ func withResolver(res resolver.StepResolver) fakeOperatorOption {
 	}
 }
 
+func withSources(sources ...sourceAddress) fakeOperatorOption {
+	return func(config *fakeOperatorConfig) {
+		config.sources = sources
+	}
+}
+
 func withReconciler(rec reconciler.RegistryReconcilerFactory) fakeOperatorOption {
 	return func(config *fakeOperatorConfig) {
 		config.reconciler = rec
@@ -1431,6 +1494,11 @@ func withFakeClientOptions(options ...clientfake.Option) fakeOperatorOption {
 	}
 }
 
+type sourceAddress struct {
+	address   string
+	sourceKey registry.CatalogKey
+}
+
 // NewFakeOperator creates a new operator using fake clients.
 func NewFakeOperator(ctx context.Context, namespace string, namespaces []string, fakeOptions ...fakeOperatorOption) (*Operator, error) {
 	// Apply options to default config
@@ -1548,6 +1616,9 @@ func NewFakeOperator(ctx context.Context, namespace string, namespaces []string,
 
 	op.RunInformers(ctx)
 	op.sources.Start(ctx)
+	for _, source := range config.sources {
+		op.sources.Add(source.sourceKey, source.address)
+	}
 
 	if ok := cache.WaitForCacheSync(ctx.Done(), op.HasSynced); !ok {
 		return nil, fmt.Errorf("failed to wait for caches to sync")
diff --git a/pkg/controller/registry/reconciler/grpc_test.go b/pkg/controller/registry/reconciler/grpc_test.go
index 882728621..6ac340648 100644
--- a/pkg/controller/registry/reconciler/grpc_test.go
+++ b/pkg/controller/registry/reconciler/grpc_test.go
@@ -49,6 +49,11 @@ func grpcCatalogSourceWithSecret(secretNames []string) *v1alpha1.CatalogSource {
 		},
 	}
 }
+func grpcCatalogSourceWithStatus(status v1alpha1.CatalogSourceStatus) *v1alpha1.CatalogSource {
+	catsrc := validGrpcCatalogSource("image", "")
+	catsrc.Status = status
+	return catsrc
+}
 
 func grpcCatalogSourceWithAnnotations(annotations map[string]string) *v1alpha1.CatalogSource {
 	catsrc := validGrpcCatalogSource("image", "")
@@ -284,6 +289,29 @@ func TestGrpcRegistryReconciler(t *testing.T) {
 				},
 			},
 		},
+		{
+			testName: "Grpc/ExistingRegistry/UpdateInvalidRegistryServiceStatus",
+			in: in{
+				cluster: cluster{
+					k8sObjs: objectsForCatalogSource(validGrpcCatalogSource("image", "")),
+				},
+				catsrc: grpcCatalogSourceWithStatus(v1alpha1.CatalogSourceStatus{
+					RegistryServiceStatus: &v1alpha1.RegistryServiceStatus{
+						CreatedAt: now(),
+						Protocol:  "grpc",
+					},
+				}),
+			},
+			out: out{
+				status: &v1alpha1.RegistryServiceStatus{
+					CreatedAt:        now(),
+					Protocol:         "grpc",
+					ServiceName:      "img-catalog",
+					ServiceNamespace: testNamespace,
+					Port:             "50051",
+				},
+			},
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.testName, func(t *testing.T) {
diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go
index 776f88e6d..6e0fc6aef 100644
--- a/pkg/controller/registry/resolver/resolver_test.go
+++ b/pkg/controller/registry/resolver/resolver_test.go
@@ -66,6 +66,109 @@ func TestSolveOperators(t *testing.T) {
 	require.EqualValues(t, expected, operators)
 }
 
+func TestSolveOperators_WithSystemConstraints(t *testing.T) {
+	const namespace = "test-namespace"
+	catalog := cache.SourceKey{Name: "test-catalog", Namespace: namespace}
+
+	packageASub := newSub(namespace, "packageA", "alpha", catalog)
+	packageDSub := existingSub(namespace, "packageD.v1", "packageD", "alpha", catalog)
+
+	APISet := cache.APISet{opregistry.APIKey{Group: "g", Version: "v", Kind: "k", Plural: "ks"}: struct{}{}}
+
+	// packageA requires an API that can be provided by B or C
+	packageA := genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", catalog.Name, catalog.Namespace, APISet, nil, nil, "", false)
+	packageB := genOperator("packageB.v1", "1.0.0", "", "packageB", "alpha", catalog.Name, catalog.Namespace, nil, APISet, nil, "", false)
+	packageC := genOperator("packageC.v1", "1.0.0", "", "packageC", "alpha", catalog.Name, catalog.Namespace, nil, APISet, nil, "", false)
+
+	// Existing operators
+	packageD := genOperator("packageD.v1", "1.0.0", "", "packageD", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false)
+	existingPackageD := existingOperator(namespace, "packageD.v1", "packageD", "alpha", "", nil, nil, nil, nil)
+	existingPackageD.Annotations = map[string]string{"operatorframework.io/properties": `{"properties":[{"type":"olm.package","value":{"packageName":"packageD","version":"1.0.0"}}]}`}
+
+	whiteListConstraintProvider := func(whiteList ...*cache.Entry) solver.ConstraintProviderFunc {
+		return func(entry *cache.Entry) ([]solver.Constraint, error) {
+			for _, whiteListedEntry := range whiteList {
+				if whiteListedEntry.Package() == entry.Package() &&
+					whiteListedEntry.Name == entry.Name &&
+					whiteListedEntry.Version == entry.Version {
+					return nil, nil
+				}
+			}
+			return []solver.Constraint{PrettyConstraint(
+				solver.Prohibited(),
+				fmt.Sprintf("package: %s is not white listed", entry.Package()),
+			)}, nil
+		}
+	}
+
+	testCases := []struct {
+		title                     string
+		systemConstraintsProvider solver.ConstraintProvider
+		expectedOperators         cache.OperatorSet
+		csvs                      []*v1alpha1.ClusterServiceVersion
+		subs                      []*v1alpha1.Subscription
+		snapshotEntries           []*cache.Entry
+		err                       string
+	}{
+		{
+			title:                     "No runtime constraints",
+			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
+			systemConstraintsProvider: nil,
+			expectedOperators:         cache.OperatorSet{"packageA.v1": packageA, "packageB.v1": packageB},
+			csvs:                      nil,
+			subs:                      []*v1alpha1.Subscription{packageASub},
+			err:                       "",
+		},
+		{
+			title:                     "Runtime constraints only accept packages A and C",
+			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
+			systemConstraintsProvider: whiteListConstraintProvider(packageA, packageC),
+			expectedOperators:         cache.OperatorSet{"packageA.v1": packageA, "packageC.v1": packageC},
+			csvs:                      nil,
+			subs:                      []*v1alpha1.Subscription{packageASub},
+			err:                       "",
+		},
+		{
+			title:                     "Existing packages are ignored",
+			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
+			systemConstraintsProvider: whiteListConstraintProvider(packageA, packageC),
+			expectedOperators:         cache.OperatorSet{"packageA.v1": packageA, "packageC.v1": packageC},
+			csvs:                      []*v1alpha1.ClusterServiceVersion{existingPackageD},
+			subs:                      []*v1alpha1.Subscription{packageASub, packageDSub},
+			err:                       "",
+		},
+		{
+			title:                     "Runtime constraints don't allow A",
+			snapshotEntries:           []*cache.Entry{packageA, packageB, packageC, packageD},
+			systemConstraintsProvider: whiteListConstraintProvider(),
+			expectedOperators:         nil,
+			csvs:                      nil,
+			subs:                      []*v1alpha1.Subscription{packageASub},
+			err:                       "packageA is not white listed",
+		},
+	}
+
+	for _, testCase := range testCases {
+		satResolver := SatResolver{
+			cache: cache.New(cache.StaticSourceProvider{
+				catalog: &cache.Snapshot{
+					Entries: testCase.snapshotEntries,
+				},
+			}),
+			log:                       logrus.New(),
+			systemConstraintsProvider: testCase.systemConstraintsProvider,
+		}
+		operators, err := satResolver.SolveOperators([]string{namespace}, testCase.csvs, testCase.subs)
+
+		if testCase.err != "" {
+			require.Containsf(t, err.Error(), testCase.err, "Test %s failed", testCase.title)
+		} else {
+			require.NoErrorf(t, err, "Test %s failed", testCase.title)
+		}
+		require.EqualValuesf(t, testCase.expectedOperators, operators, "Test %s failed", testCase.title)
+	}
+}
+
 func TestDisjointChannelGraph(t *testing.T) {
 	const namespace = "test-namespace"
 	catalog := cache.SourceKey{Name: "test-catalog", Namespace: namespace}
diff --git a/pkg/controller/registry/resolver/step_resolver_test.go b/pkg/controller/registry/resolver/step_resolver_test.go
index 6c29c127e..1839bccbd 100644
--- a/pkg/controller/registry/resolver/step_resolver_test.go
+++ b/pkg/controller/registry/resolver/step_resolver_test.go
@@ -47,6 +47,33 @@ var (
 	Requires4 = APISet4
 )
 
+func TestInitHooks(t *testing.T) {
+	clientFake := fake.NewSimpleClientset()
+	lister := operatorlister.NewLister()
+	kClientFake := k8sfake.NewSimpleClientset()
+	log := logrus.New()
+
+	// no init hooks
+	resolver := NewOperatorStepResolver(lister, clientFake, kClientFake, "", nil, log)
+	require.NotNil(t, resolver.satResolver)
+
+	// with init hook
+	var testHook stepResolverInitHook = func(resolver *OperatorStepResolver) error {
+		resolver.satResolver = nil
+		return nil
+	}
+
+	// defined in step_resolver.go
+	initHooks = append(initHooks, testHook)
+	defer func() {
+		// reset initHooks
+		initHooks = nil
+	}()
+
+	resolver = NewOperatorStepResolver(lister, clientFake, kClientFake, "", nil, log)
+	require.Nil(t, resolver.satResolver)
+}
+
 func TestResolver(t *testing.T) {
 	const namespace = "catsrc-namespace"
 	catalog := resolvercache.SourceKey{Name: "catsrc", Namespace: namespace}