support multiple subscriptions resolving the same bundles

this better aligns the on-cluster status with the output of the resolver

we may want to prevent this case from happening at a higher layer (i.e.
a webhook that prevents two subscriptions to the same package), but
for now this behavior ensures that the actions the resolver takes are
clearly visible in the subscription status.

this also adds test cases for multiple subscriptions to the same package

Signed-off-by: Evan <[email protected]>

Author: ecordell

pkg/controller/registry/resolver/step_resolver.go

@@ -99,11 +99,8 @@ func (r *OperatorStepResolver) ResolveSteps(namespace string, _ SourceQuerier) (
 	updatedSubs := []*v1alpha1.Subscription{}
 	bundleLookups := []v1alpha1.BundleLookup{}
 	for name, op := range operators {
-		// Find an existing subscription that resolves to this operator.
-		var (
-			existingSubscription *v1alpha1.Subscription
-			alreadyExists        bool
-		)
+		// Find any existing subscriptions that resolve to this operator.
+		existingSubscriptions := make(map[*v1alpha1.Subscription]bool)
 		sourceInfo := *op.SourceInfo()
 		for _, sub := range subs {
 			if sub.Spec.Package != sourceInfo.Package {
@@ -119,17 +116,24 @@ func (r *OperatorStepResolver) ResolveSteps(namespace string, _ SourceQuerier) (
 			if !subCatalogKey.Empty() && !subCatalogKey.Equal(sourceInfo.Catalog) {
 				continue
 			}
-			alreadyExists, err = r.hasExistingCurrentCSV(sub)
+			alreadyExists, err := r.hasExistingCurrentCSV(sub)
 			if err != nil {
-				return nil, nil, nil, fmt.Errorf("unable to determine whether subscription has a preexisting CSV")
+				return nil, nil, nil, fmt.Errorf("unable to determine whether subscription %s has a preexisting CSV", sub.GetName())
 			}
-			existingSubscription = sub
-			break
+			existingSubscriptions[sub] = alreadyExists
 		}
 
-		// subscription exists and is up to date
-		if existingSubscription != nil && existingSubscription.Status.CurrentCSV == op.Identifier() && alreadyExists {
-			continue
+		if len(existingSubscriptions) > 0 {
+			upToDate := true
+			for sub, exists := range existingSubscriptions {
+				if !exists || sub.Status.CurrentCSV != op.Identifier() {
+					upToDate = false
+				}
+			}
+			// all matching subscriptions are up to date
+			if upToDate {
+				continue
+			}
 		}
 
 		// add steps for any new bundle
@@ -172,7 +176,7 @@ func (r *OperatorStepResolver) ResolveSteps(namespace string, _ SourceQuerier) (
 				bundleLookups = append(bundleLookups, lookup)
 			}
 
-			if existingSubscription == nil {
+			if len(existingSubscriptions) == 0 {
 				// explicitly track the resolved CSV as the starting CSV on the resolved subscriptions
 				op.SourceInfo().StartingCSV = op.Identifier()
 				subStep, err := NewSubscriptionStepResource(namespace, *op.SourceInfo())
@@ -188,10 +192,13 @@ func (r *OperatorStepResolver) ResolveSteps(namespace string, _ SourceQuerier) (
 		}
 
 		// add steps for subscriptions for bundles that were added through resolution
-		if existingSubscription != nil && existingSubscription.Status.CurrentCSV != op.Identifier() {
+		for sub := range existingSubscriptions {
+			if sub.Status.CurrentCSV == op.Identifier() {
+				continue
+			}
 			// update existing subscription status
-			existingSubscription.Status.CurrentCSV = op.Identifier()
-			updatedSubs = append(updatedSubs, existingSubscription)
+			sub.Status.CurrentCSV = op.Identifier()
+			updatedSubs = append(updatedSubs, sub)
 		}
 	}
 

pkg/controller/registry/resolver/step_resolver_test.go

@@ -305,6 +305,57 @@ func TestResolver(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "ConflictingSubscriptionsToSamePackage",
+			clusterState: []runtime.Object{
+				newSub(namespace, "a", "alpha", catalog),
+				newSub(namespace, "a", "beta", catalog),
+			},
+			bundlesByCatalog: map[registry.CatalogKey][]*api.Bundle{
+				catalog: {
+					bundle("a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
+					bundle("a.v2", "a", "beta", "", Provides1, nil, nil, nil),
+				},
+			},
+			out: resolverTestOut{
+				errAssert: func(t *testing.T, err error) {
+					fmt.Println(err)
+					assert.IsType(t, solver.NotSatisfiable{}, err)
+				},
+			},
+		},
+		{
+			// No two operators from the same package may run at the same time, but it's possible to have two
+			// subscriptions to the same package as long as it's possible to find a bundle that satisfies both
+			// constraints
+			name: "SatisfiableSubscriptionsToSamePackage",
+			clusterState: []runtime.Object{
+				newSub(namespace, "a", "alpha", catalog),
+				func() (s *v1alpha1.Subscription) {
+					s = newSub(namespace, "a", "alpha", catalog)
+					s.Name = s.Name+"-2"
+					return
+				}(),
+			},
+			bundlesByCatalog: map[registry.CatalogKey][]*api.Bundle{
+				catalog: {
+					bundle("a.v1", "a", "alpha", "", Provides1, nil, nil, nil),
+				},
+			},
+			out: resolverTestOut{
+				steps: [][]*v1alpha1.Step{
+					bundleSteps(bundle("a.v1", "a", "alpha", "", Provides1, nil, nil, nil), namespace, "", catalog),
+				},
+				subs: []*v1alpha1.Subscription{
+					updatedSub(namespace, "a.v1", "", "a", "alpha", catalog),
+					func() (s *v1alpha1.Subscription) {
+						s = updatedSub(namespace, "a.v1", "", "a", "alpha", catalog)
+						s.Name = s.Name+"-2"
+						return
+					}(),
+				},
+			},
+		},
 		{
 			name: "TwoExistingOperatorsWithSameName/NoError",
 			clusterState: []runtime.Object{