Repository: operator-framework/operator-lifecycle-manager
Pull request #2262 (2021-07-16 20:29:11)

feat(sub): Include IP failure condition message in sub status condition
**Description of the change:**

Currently when an InstallPlan fails, due to bundle unpacking error
is pending due to errors like invalid operatorgroup, or pending bundle unpacking job
the reason is propagated to the Subscription that owns it. The message for the failure is
however missing from the Subscription condition.

eg

```
kind: Subscription
status:
conditions:
    - lastTransitionTime: "2021-07-07T17:55:20Z"
      reason: Installing
      status: "True"
      type: InstallPlanPending
```

This PR propagates the message assosiated with the reason in the InstallPlan
condition when an InstallPlan is either pending, or has failed permanantly.

eg

```
kind: Subscription
status:
conditions:
    - lastTransitionTime: "2021-07-07T17:55:20Z"
      message: no operator group found that is managing this namespace
      reason: Installing
      status: "True"
      type: InstallPlanPending
```


**Motivation for the change:**

**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/2262#" title="Author self-approved">anik120</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **njhale** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @njhale` 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":["njhale"]} -->
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/2262#" title="Author self-approved">anik120</a>*, *<a href="https://github.com/operator-framework/operator-lifecycle-manager/pull/2262#pullrequestreview-708748492" title="LGTM">ankitathomas</a>*
To complete the [pull request process](https://git.k8s.io/community/contributors/guide/owners.md#the-code-review-process), please assign **njhale** after the PR has been reviewed.
You can assign the PR to them by writing `/assign @njhale` 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":["njhale"]} -->
anik120: /hold 
have to review the test case. 

Patch:
diff --git a/pkg/controller/operators/catalog/subscription/state.go b/pkg/controller/operators/catalog/subscription/state.go
index f5da96614..e7280e39b 100644
--- a/pkg/controller/operators/catalog/subscription/state.go
+++ b/pkg/controller/operators/catalog/subscription/state.go
@@ -1,6 +1,7 @@
 package subscription
 
 import (
+	"bytes"
 	"context"
 	"fmt"
 
@@ -452,7 +453,7 @@ func (i *installPlanReferencedState) CheckInstallPlanStatus(now *metav1.Time, cl
 		if cond.Reason == "" {
 			cond.Reason = string(phase)
 		}
-
+		cond.Message = extractMessage(status)
 		cond.Type = v1alpha1.SubscriptionInstallPlanPending
 		cond.Status = corev1.ConditionTrue
 		out.Status.SetCondition(cond)
@@ -472,6 +473,7 @@ func (i *installPlanReferencedState) CheckInstallPlanStatus(now *metav1.Time, cl
 		}
 
 		cond.Type = v1alpha1.SubscriptionInstallPlanFailed
+		cond.Message = extractMessage(status)
 		cond.Status = corev1.ConditionTrue
 		out.Status.SetCondition(cond)
 
@@ -508,6 +510,26 @@ func (i *installPlanReferencedState) CheckInstallPlanStatus(now *metav1.Time, cl
 	return known, nil
 }
 
+func extractMessage(status *v1alpha1.InstallPlanStatus) string {
+	str := ""
+	if len(status.BundleLookups) > 0 {
+		var b bytes.Buffer
+		for _, lookup := range status.BundleLookups {
+			if cond := lookup.GetCondition(v1alpha1.BundleLookupPending); cond.Status != corev1.ConditionUnknown {
+				b.WriteString(cond.Message)
+				b.WriteString(".")
+			}
+		}
+		str = b.String()
+	}
+	if cond := status.GetCondition(v1alpha1.InstallPlanInstalled); cond.Status != corev1.ConditionUnknown {
+		if cond.Message != "" {
+			str = cond.Message
+		}
+	}
+	return str
+}
+
 type installPlanKnownState struct {
 	InstallPlanReferencedState
 }
diff --git a/pkg/controller/registry/resolver/resolver.go b/pkg/controller/registry/resolver/resolver.go
index 668f9be04..a4473f36b 100644
--- a/pkg/controller/registry/resolver/resolver.go
+++ b/pkg/controller/registry/resolver/resolver.go
@@ -5,7 +5,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"sort"
-	"strings"
 
 	"github.com/blang/semver/v4"
 	"github.com/sirupsen/logrus"
@@ -238,7 +237,7 @@ func (r *SatResolver) getSubscriptionInstallables(sub *v1alpha1.Subscription, cu
 		if i != len(bundles) && bundles[i].bundle.ChannelName == lastChannel {
 			continue
 		}
-		channel, err := sortChannel(bundles[lastIndex:i])
+		channel, err := r.sortChannel(bundles[lastIndex:i])
 		if err != nil {
 			return nil, err
 		}
@@ -611,7 +610,7 @@ func (r *SatResolver) sortBundles(bundles []*Operator) ([]*Operator, error) {
 			return pi.Channel < pj.Channel
 		})
 		for channel := range partitionedBundles[catalog] {
-			sorted, err := sortChannel(partitionedBundles[catalog][channel])
+			sorted, err := r.sortChannel(partitionedBundles[catalog][channel])
 			if err != nil {
 				return nil, err
 			}
@@ -627,15 +626,13 @@ func (r *SatResolver) sortBundles(bundles []*Operator) ([]*Operator, error) {
 	return all, nil
 }
 
-// Sorts bundle in a channel by replaces. All entries in the argument
-// are assumed to have the same Package and Channel.
-func sortChannel(bundles []*Operator) ([]*Operator, error) {
-	if len(bundles) < 1 {
+// sorts bundle in a channel by replaces
+func (r *SatResolver) sortChannel(bundles []*Operator) ([]*Operator, error) {
+	if len(bundles) <= 1 {
 		return bundles, nil
 	}
 
-	packageName := bundles[0].Package()
-	channelName := bundles[0].Channel()
+	channel := []*Operator{}
 
 	bundleLookup := map[string]*Operator{}
 
@@ -663,60 +660,44 @@ func sortChannel(bundles []*Operator) ([]*Operator, error) {
 		}
 	}
 
-	// a bundle without a replacement is a channel head, but if we
-	// find more than one of those something is weird
+	// a bundle without a replacement is a channel head, but if we find more than one of those something is weird
 	headCandidates := []*Operator{}
 	for _, b := range bundles {
 		if _, ok := replacedBy[b]; !ok {
 			headCandidates = append(headCandidates, b)
 		}
 	}
-	if len(headCandidates) == 0 {
-		return nil, fmt.Errorf("no channel heads (entries not replaced by another entry) found in channel %q of package %q", channelName, packageName)
-	}
 
-	var chains [][]*Operator
-	for _, head := range headCandidates {
-		var chain []*Operator
-		visited := make(map[*Operator]struct{})
-		current := head
-		for {
-			visited[current] = struct{}{}
-			if _, ok := skipped[current.Identifier()]; !ok {
-				chain = append(chain, current)
-			}
-			next, ok := replaces[current]
-			if !ok {
-				break
-			}
-			if _, ok := visited[next]; ok {
-				return nil, fmt.Errorf("a cycle exists in the chain of replacement beginning with %q in channel %q of package %q", head.Identifier(), channelName, packageName)
-			}
-			current = next
+	if len(headCandidates) > 1 {
+		var names []string
+		for _, v := range headCandidates {
+			names = append(names, v.Identifier())
 		}
-		chains = append(chains, chain)
-	}
+		return nil, fmt.Errorf("found multiple channel heads: %v, please check the `replaces`/`skipRange` fields of the operator bundles", names)
 
-	if len(chains) > 1 {
-		schains := make([]string, len(chains))
-		for i, chain := range chains {
-			switch len(chain) {
-			case 0:
-				schains[i] = "[]" // Bug?
-			case 1:
-				schains[i] = chain[0].Identifier()
-			default:
-				schains[i] = fmt.Sprintf("%s...%s", chain[0].Identifier(), chain[len(chain)-1].Identifier())
-			}
-		}
-		return nil, fmt.Errorf("a unique replacement chain within a channel is required to determine the relative order between channel entries, but %d replacement chains were found in channel %q of package %q: %s", len(schains), channelName, packageName, strings.Join(schains, ", "))
+	} else if len(headCandidates) < 1 {
+		return nil, fmt.Errorf("head of channel not found")
 	}
 
-	if len(chains) == 0 {
-		// Bug?
-		return nil, fmt.Errorf("found no replacement chains in channel %q of package %q", channelName, packageName)
+	head := headCandidates[0]
+	current := head
+	skip := false
+	for {
+		if skip == false {
+			channel = append(channel, current)
+		}
+		skip = false
+		next, ok := replaces[current]
+		if !ok {
+			break
+		}
+		if _, ok := skipped[current.Identifier()]; ok {
+			skip = true
+		}
+		current = next
 	}
 
 	// TODO: do we care if the channel doesn't include every bundle in the input?
-	return chains[0], nil
+
+	return channel, nil
 }


Test patch:
diff --git a/pkg/controller/operators/catalog/subscription/state_test.go b/pkg/controller/operators/catalog/subscription/state_test.go
index 3ba23f4fd..422821649 100644
--- a/pkg/controller/operators/catalog/subscription/state_test.go
+++ b/pkg/controller/operators/catalog/subscription/state_test.go
@@ -1336,6 +1336,14 @@ func TestCheckInstallPlanStatus(t *testing.T) {
 				now: &now,
 				status: &v1alpha1.InstallPlanStatus{
 					Phase: v1alpha1.InstallPlanPhaseInstalling,
+					Conditions: []v1alpha1.InstallPlanCondition{
+						{
+							Type:    v1alpha1.InstallPlanInstalled,
+							Message: "no operatorgroup found that is managing this namespace",
+							Reason:  v1alpha1.InstallPlanConditionReason("Installing"),
+							Status:  corev1.ConditionFalse,
+						},
+					},
 				},
 			},
 			want: want{
@@ -1346,7 +1354,7 @@ func TestCheckInstallPlanStatus(t *testing.T) {
 					},
 					Status: v1alpha1.SubscriptionStatus{
 						Conditions: []v1alpha1.SubscriptionCondition{
-							planPendingCondition(corev1.ConditionTrue, string(v1alpha1.InstallPlanPhaseInstalling), "", &now),
+							planPendingCondition(corev1.ConditionTrue, string(v1alpha1.InstallPlanPhaseInstalling), "no operatorgroup found that is managing this namespace", &now),
 						},
 						LastUpdated: now,
 					},
@@ -1535,6 +1543,17 @@ func TestCheckInstallPlanStatus(t *testing.T) {
 							Reason: v1alpha1.InstallPlanReasonComponentFailed,
 						},
 					},
+					BundleLookups: []v1alpha1.BundleLookup{
+						{
+							Conditions: []v1alpha1.BundleLookupCondition{
+								{
+									Type:    v1alpha1.BundleLookupPending,
+									Status:  corev1.ConditionTrue,
+									Message: "unpack job not completed: Unpack pod(olm/c5a4) container(pull) is pending. Reason: ImagePullBackOff, Message: Back-off pulling image",
+								},
+							},
+						},
+					},
 				},
 			},
 			want: want{
@@ -1545,7 +1564,7 @@ func TestCheckInstallPlanStatus(t *testing.T) {
 					},
 					Status: v1alpha1.SubscriptionStatus{
 						Conditions: []v1alpha1.SubscriptionCondition{
-							planFailedCondition(corev1.ConditionTrue, string(v1alpha1.InstallPlanReasonComponentFailed), "", &now),
+							planFailedCondition(corev1.ConditionTrue, string(v1alpha1.InstallPlanReasonComponentFailed), "unpack job not completed: Unpack pod(olm/c5a4) container(pull) is pending. Reason: ImagePullBackOff, Message: Back-off pulling image.", &now),
 						},
 						LastUpdated: now,
 					},
diff --git a/pkg/controller/registry/resolver/resolver_test.go b/pkg/controller/registry/resolver/resolver_test.go
index aa288d094..22106e155 100644
--- a/pkg/controller/registry/resolver/resolver_test.go
+++ b/pkg/controller/registry/resolver/resolver_test.go
@@ -1,7 +1,6 @@
 package resolver
 
 import (
-	"errors"
 	"fmt"
 	"testing"
 
@@ -22,11 +21,11 @@ import (
 )
 
 func TestSolveOperators(t *testing.T) {
-	APISet := APISet{opregistry.APIKey{Group: "g", Version: "v", Kind: "k", Plural: "ks"}: struct{}{}}
+	APISet := APISet{opregistry.APIKey{"g", "v", "k", "ks"}: struct{}{}}
 	Provides := APISet
 
-	const namespace = "test-namespace"
-	catalog := registry.CatalogKey{Name: "test-catalog", Namespace: namespace}
+	namespace := "olm"
+	catalog := registry.CatalogKey{"community", namespace}
 
 	csv := existingOperator(namespace, "packageA.v1", "packageA", "alpha", "", Provides, nil, nil, nil)
 	csvs := []*v1alpha1.ClusterServiceVersion{csv}
@@ -36,11 +35,17 @@ func TestSolveOperators(t *testing.T) {
 
 	fakeNamespacedOperatorCache := NamespacedOperatorCache{
 		snapshots: map[registry.CatalogKey]*CatalogSnapshot{
-			catalog: {
-				key: catalog,
+			registry.CatalogKey{
+				Namespace: "olm",
+				Name:      "community",
+			}: {
+				key: registry.CatalogKey{
+					Namespace: "olm",
+					Name:      "community",
+				},
 				operators: []*Operator{
-					genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false),
-					genOperator("packageB.v1", "1.0.1", "", "packageB", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false),
+					genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", "community", "olm", nil, nil, nil, "", false),
+					genOperator("packageB.v1", "1.0.1", "", "packageB", "alpha", "community", "olm", nil, nil, nil, "", false),
 				},
 			},
 		},
@@ -50,11 +55,12 @@ func TestSolveOperators(t *testing.T) {
 		log:   logrus.New(),
 	}
 
-	operators, err := satResolver.SolveOperators([]string{namespace}, csvs, subs)
+	operators, err := satResolver.SolveOperators([]string{"olm"}, csvs, subs)
 	assert.NoError(t, err)
 
 	expected := OperatorSet{
-		"packageB.v1": genOperator("packageB.v1", "1.0.1", "", "packageB", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false),
+		"packageA.v1": genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", "community", "olm", nil, nil, nil, "", false),
+		"packageB.v1": genOperator("packageB.v1", "1.0.1", "", "packageB", "alpha", "community", "olm", nil, nil, nil, "", false),
 	}
 	require.EqualValues(t, expected, operators)
 }
@@ -111,10 +117,16 @@ func TestSolveOperators_MultipleChannels(t *testing.T) {
 
 	fakeNamespacedOperatorCache := NamespacedOperatorCache{
 		snapshots: map[registry.CatalogKey]*CatalogSnapshot{
-			catalog: {
-				key: catalog,
+			registry.CatalogKey{
+				Namespace: "olm",
+				Name:      "community",
+			}: {
+				key: registry.CatalogKey{
+					Namespace: "olm",
+					Name:      "community",
+				},
 				operators: []*Operator{
-					genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", "community", "olm", nil, nil, nil, "", false),
+					genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", "community", "olm", nil, nil, nil, "", false),
 					genOperator("packageB.v1", "1.0.0", "", "packageB", "alpha", "community", "olm", nil, nil, nil, "", false),
 					genOperator("packageB.v1", "1.0.0", "", "packageB", "beta", "community", "olm", nil, nil, nil, "", false),
 				},
@@ -129,9 +141,10 @@ func TestSolveOperators_MultipleChannels(t *testing.T) {
 	operators, err := satResolver.SolveOperators([]string{"olm"}, csvs, subs)
 	assert.NoError(t, err)
 	expected := OperatorSet{
+		"packageA.v1": genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", "community", "olm", nil, nil, nil, "", false),
 		"packageB.v1": genOperator("packageB.v1", "1.0.0", "", "packageB", "alpha", "community", "olm", nil, nil, nil, "", false),
 	}
-	assert.Len(t, operators, 1)
+	assert.Equal(t, 2, len(operators))
 	for k, e := range expected {
 		assert.EqualValues(t, e, operators[k])
 	}
@@ -360,7 +373,7 @@ func TestSolveOperators_CatsrcPrioritySorting(t *testing.T) {
 					Name:      "community",
 				},
 				operators: []*Operator{
-					genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", "community", namespace, nil,
+					genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", "community", namespace, nil,
 						nil, opToAddVersionDeps, "", false),
 				},
 			},
@@ -402,7 +415,7 @@ func TestSolveOperators_CatsrcPrioritySorting(t *testing.T) {
 	operators, err := satResolver.SolveOperators([]string{"olm"}, []*v1alpha1.ClusterServiceVersion{}, subs)
 	assert.NoError(t, err)
 	expected := OperatorSet{
-		"packageA.v1": genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", "community", "olm",
+		"packageA.v1": genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", "community", "olm",
 			nil, nil, opToAddVersionDeps, "", false),
 		"packageB.v1": genOperator("packageB.v1", "0.0.1", "", "packageB", "alpha", "high-priority-operator", "olm",
 			nil, nil, nil, "", false),
@@ -435,7 +448,7 @@ func TestSolveOperators_CatsrcPrioritySorting(t *testing.T) {
 	operators, err = satResolver.SolveOperators([]string{"olm"}, []*v1alpha1.ClusterServiceVersion{}, subs)
 	assert.NoError(t, err)
 	expected = OperatorSet{
-		"packageA.v1": genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", "community", "olm",
+		"packageA.v1": genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", "community", "olm",
 			nil, nil, opToAddVersionDeps, "", false),
 		"packageB.v1": genOperator("packageB.v1", "0.0.1", "", "packageB", "alpha", "community-operator", "olm",
 			nil, nil, nil, "", false),
@@ -455,7 +468,7 @@ func TestSolveOperators_CatsrcPrioritySorting(t *testing.T) {
 			Name:      "community",
 		},
 		operators: []*Operator{
-			genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", "community", namespace, nil,
+			genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", "community", namespace, nil,
 				nil, opToAddVersionDeps, "", false),
 			genOperator("packageB.v1", "0.0.1", "", "packageB", "alpha", "community",
 				namespace, nil, nil, nil, "", false),
@@ -469,7 +482,7 @@ func TestSolveOperators_CatsrcPrioritySorting(t *testing.T) {
 	operators, err = satResolver.SolveOperators([]string{"olm"}, []*v1alpha1.ClusterServiceVersion{}, subs)
 	assert.NoError(t, err)
 	expected = OperatorSet{
-		"packageA.v1": genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", "community", "olm",
+		"packageA.v1": genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", "community", "olm",
 			nil, nil, opToAddVersionDeps, "", false),
 		"packageB.v1": genOperator("packageB.v1", "0.0.1", "", "packageB", "alpha", "community", "olm",
 			nil, nil, nil, "", false),
@@ -1343,7 +1356,7 @@ func TestSolveOperators_WithoutDeprecated(t *testing.T) {
 			catalog: {
 				key: catalog,
 				operators: []*Operator{
-					genOperator("packageA.v1", "0.0.1", "", "packageA", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", true),
+					genOperator("packageA.v1", "0.0.1", "packageA.v1", "packageA", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", true),
 				},
 			},
 		},
@@ -1410,9 +1423,9 @@ func TestSolveOperators_WithSkipsAndStartingCSV(t *testing.T) {
 	op1 := genOperator("packageA.v1", "1.0.0", "", "packageA", "alpha", "community", "olm", nil, Provides, nil, "", false)
 	op2 := genOperator("packageA.v2", "2.0.0", "packageA.v1", "packageA", "alpha", "community", "olm", nil, Provides, nil, "", false)
 	op3 := genOperator("packageA.v3", "3.0.0", "packageA.v2", "packageA", "alpha", "community", "olm", nil, Provides, nil, "", false)
-	op4 := genOperator("packageA.v4", "4.0.0", "packageA.v3", "packageA", "alpha", "community", "olm", nil, Provides, nil, "", false)
+	op4 := genOperator("packageA.v4", "4.0.0", "packageA.v2", "packageA", "alpha", "community", "olm", nil, Provides, nil, "", false)
 	op4.skips = []string{"packageA.v3"}
-	op5 := genOperator("packageA.v5", "5.0.0", "packageA.v4", "packageA", "alpha", "community", "olm", nil, Provides, nil, "", false)
+	op5 := genOperator("packageA.v5", "5.0.0", "packageA.v1", "packageA", "alpha", "community", "olm", nil, Provides, nil, "", false)
 	op5.skips = []string{"packageA.v2", "packageA.v3", "packageA.v4"}
 	op6 := genOperator("packageA.v6", "6.0.0", "packageA.v5", "packageA", "alpha", "community", "olm", nil, Provides, nil, "", false)
 
@@ -1448,20 +1461,26 @@ func TestSolveOperators_WithSkipsAndStartingCSV(t *testing.T) {
 }
 
 func TestSolveOperators_WithSkips(t *testing.T) {
-	const namespace = "test-namespace"
-	catalog := registry.CatalogKey{Name: "test-catalog", Namespace: namespace}
+	namespace := "olm"
+	catalog := registry.CatalogKey{"community", namespace}
 
 	newSub := newSub(namespace, "packageB", "alpha", catalog)
 	subs := []*v1alpha1.Subscription{newSub}
 
-	opB := genOperator("packageB.v1", "1.0.0", "", "packageB", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false)
-	opB2 := genOperator("packageB.v2", "2.0.0", "", "packageB", "alpha", catalog.Name, catalog.Namespace, nil, nil, nil, "", false)
+	opB := genOperator("packageB.v1", "1.0.0", "", "packageB", "alpha", "community", "olm", nil, nil, nil, "", false)
+	opB2 := genOperator("packageB.v2", "2.0.0", "", "packageB", "alpha", "community", "olm", nil, nil, nil, "", false)
 	opB2.skips = []string{"packageB.v1"}
 
 	fakeNamespacedOperatorCache := NamespacedOperatorCache{
 		snapshots: map[registry.CatalogKey]*CatalogSnapshot{
-			catalog: {
-				key: catalog,
+			registry.CatalogKey{
+				Namespace: "olm",
+				Name:      "community",
+			}: {
+				key: registry.CatalogKey{
+					Namespace: "olm",
+					Name:      "community",
+				},
 				operators: []*Operator{
 					opB, opB2,
 				},
@@ -1473,7 +1492,7 @@ func TestSolveOperators_WithSkips(t *testing.T) {
 		log:   logrus.New(),
 	}
 
-	operators, err := satResolver.SolveOperators([]string{namespace}, nil, subs)
+	operators, err := satResolver.SolveOperators([]string{"olm"}, nil, subs)
 	assert.NoError(t, err)
 	expected := OperatorSet{
 		"packageB.v2": opB2,
@@ -1481,38 +1500,6 @@ func TestSolveOperators_WithSkips(t *testing.T) {
 	require.EqualValues(t, expected, operators)
 }
 
-func TestSolveOperatorsWithSkipsPreventingSelection(t *testing.T) {
-	const namespace = "test-namespace"
-	catalog := registry.CatalogKey{Name: "test-catalog", Namespace: namespace}
-	gvks := APISet{opregistry.APIKey{Group: "g", Version: "v", Kind: "k", Plural: "ks"}: struct{}{}}
-
-	// Subscription candidate a-1 requires a GVK provided
-	// exclusively by b-1, but b-1 is skipped by b-3 and can't be
-	// chosen.
-	subs := []*v1alpha1.Subscription{newSub(namespace, "a", "channel", catalog)}
-	a1 := genOperator("a-1", "1.0.0", "", "a", "channel", catalog.Name, catalog.Namespace, gvks, nil, nil, "", false)
-	b3 := genOperator("b-3", "3.0.0", "b-2", "b", "channel", catalog.Name, catalog.Namespace, nil, nil, nil, "", false)
-	b3.skips = []string{"b-1"}
-	b2 := genOperator("b-2", "2.0.0", "b-1", "b", "channel", catalog.Name, catalog.Namespace, nil, nil, nil, "", false)
-	b1 := genOperator("b-1", "1.0.0", "", "b", "channel", catalog.Name, catalog.Namespace, nil, gvks, nil, "", false)
-
-	logger, _ := test.NewNullLogger()
-	satResolver := SatResolver{
-		cache: getFakeOperatorCache(NamespacedOperatorCache{
-			snapshots: map[registry.CatalogKey]*CatalogSnapshot{
-				catalog: {
-					key:       catalog,
-					operators: []*Operator{a1, b3, b2, b1},
-				},
-			},
-		}),
-		log: logger,
-	}
-
-	_, err := satResolver.SolveOperators([]string{namespace}, nil, subs)
-	assert.IsType(t, solver.NotSatisfiable{}, err)
-}
-
 func TestSolveOperatorsWithClusterServiceVersionHavingDependency(t *testing.T) {
 	const namespace = "test-namespace"
 	catalog := registry.CatalogKey{Name: "test-catalog", Namespace: namespace}
@@ -1549,6 +1536,7 @@ func TestSolveOperatorsWithClusterServiceVersionHavingDependency(t *testing.T) {
 
 	operators, err := r.SolveOperators([]string{namespace}, csvs, subs)
 	assert.NoError(t, err)
+	//expected := OperatorSet{}
 	require.Empty(t, operators)
 }
 
@@ -1754,194 +1742,3 @@ func TestInferProperties(t *testing.T) {
 		})
 	}
 }
-
-func TestSortChannel(t *testing.T) {
-	for _, tc := range []struct {
-		Name string
-		In   []*Operator
-		Out  []*Operator
-		Err  error
-	}{
-		{
-			Name: "wrinkle-free",
-			In: []*Operator{
-				{
-					name: "b",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-				{
-					name:     "a",
-					replaces: "b",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-			},
-			Out: []*Operator{
-				{
-					name:     "a",
-					replaces: "b",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-				{
-					name: "b",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-			},
-		},
-		{
-			Name: "empty",
-			In:   nil,
-			Out:  nil,
-		},
-		{
-			Name: "replacement cycle",
-			In: []*Operator{
-				{
-					name:     "a",
-					replaces: "b",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-				{
-					name:     "b",
-					replaces: "a",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-			},
-			Err: errors.New(`no channel heads (entries not replaced by another entry) found in channel "channel" of package "package"`),
-		},
-		{
-			Name: "replacement cycle",
-			In: []*Operator{
-				{
-					name:     "a",
-					replaces: "b",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-				{
-					name:     "b",
-					replaces: "c",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-				{
-					name:     "c",
-					replaces: "b",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-			},
-			Err: errors.New(`a cycle exists in the chain of replacement beginning with "a" in channel "channel" of package "package"`),
-		},
-		{
-			Name: "skipped and replaced entry omitted",
-			In: []*Operator{
-				{
-					name:     "a",
-					replaces: "b",
-					skips:    []string{"b"},
-				},
-				{
-					name: "b",
-				},
-			},
-			Out: []*Operator{
-				{
-					name:     "a",
-					replaces: "b",
-					skips:    []string{"b"},
-				},
-			},
-		},
-		{
-			Name: "skipped entry omitted",
-			In: []*Operator{
-				{
-					name:     "a",
-					replaces: "b",
-					skips:    []string{"c"},
-				},
-				{
-					name:     "b",
-					replaces: "c",
-				},
-				{
-					name: "c",
-				},
-			},
-			Out: []*Operator{
-				{
-					name:     "a",
-					replaces: "b",
-					skips:    []string{"c"},
-				},
-				{
-					name:     "b",
-					replaces: "c",
-				},
-			},
-		},
-		{
-			Name: "two replaces chains",
-			In: []*Operator{
-				{
-					name: "a",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-				{
-					name:     "b",
-					replaces: "c",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-				{
-					name: "c",
-					bundle: &api.Bundle{
-						PackageName: "package",
-						ChannelName: "channel",
-					},
-				},
-			},
-			Err: errors.New(`a unique replacement chain within a channel is required to determine the relative order between channel entries, but 2 replacement chains were found in channel "channel" of package "package": a, b...c`),
-		},
-	} {
-		t.Run(tc.Name, func(t *testing.T) {
-			assert := assert.New(t)
-			actual, err := sortChannel(tc.In)
-			if tc.Err == nil {
-				assert.NoError(err)
-			} else {
-				assert.EqualError(err, tc.Err.Error())
-			}
-			assert.Equal(tc.Out, actual)
-		})
-	}
-}