Remove vestigal cache.OperatorSet type.

Nothing of consequence has used OperatorSet in ages, but it's been
lingering in tests and as the result type of the main resolution
method.

Signed-off-by: Ben Luddy <[email protected]>

Author: benluddy

pkg/controller/registry/resolver/cache/operators.go

@@ -14,6 +14,7 @@ import (
 	opregistry "github.com/operator-framework/operator-registry/pkg/registry"
 )
 
+// todo: drop fields from cache.Entry and move to pkg/controller/operators/olm
 type APISet map[opregistry.APIKey]struct{}
 
 func EmptyAPISet() APISet {
@@ -133,59 +134,6 @@ func (s APISet) StripPlural() APISet {
 	return set
 }
 
-type APIOwnerSet map[opregistry.APIKey]*Entry
-
-func EmptyAPIOwnerSet() APIOwnerSet {
-	return map[opregistry.APIKey]*Entry{}
-}
-
-type OperatorSet map[string]*Entry
-
-func EmptyOperatorSet() OperatorSet {
-	return map[string]*Entry{}
-}
-
-// Snapshot returns a new set, pointing to the same values
-func (o OperatorSet) Snapshot() OperatorSet {
-	out := make(map[string]*Entry)
-	for key, val := range o {
-		out[key] = val
-	}
-	return out
-}
-
-type APIMultiOwnerSet map[opregistry.APIKey]OperatorSet
-
-func EmptyAPIMultiOwnerSet() APIMultiOwnerSet {
-	return map[opregistry.APIKey]OperatorSet{}
-}
-
-func (s APIMultiOwnerSet) PopAPIKey() *opregistry.APIKey {
-	for a := range s {
-		api := &opregistry.APIKey{
-			Group:   a.Group,
-			Version: a.Version,
-			Kind:    a.Kind,
-			Plural:  a.Plural,
-		}
-		delete(s, a)
-		return api
-	}
-	return nil
-}
-
-func (s APIMultiOwnerSet) PopAPIRequirers() OperatorSet {
-	requirers := EmptyOperatorSet()
-	for a := range s {
-		for key, op := range s[a] {
-			requirers[key] = op
-		}
-		delete(s, a)
-		return requirers
-	}
-	return nil
-}
-
 type OperatorSourceInfo struct {
 	Package        string
 	Channel        string

pkg/controller/registry/resolver/cache/operators_test.go

@@ -716,132 +716,6 @@ func TestCatalogKey_String(t *testing.T) {
 	}
 }
 
-func TestAPIMultiOwnerSet_PopAPIKey(t *testing.T) {
-	tests := []struct {
-		name string
-		s    APIMultiOwnerSet
-	}{
-		{
-			name: "Empty",
-			s:    EmptyAPIMultiOwnerSet(),
-		},
-		{
-			name: "OneApi/OneOwner",
-			s: map[opregistry.APIKey]OperatorSet{
-				{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
-					"owner1": {Name: "op1"},
-				},
-			},
-		},
-		{
-			name: "OneApi/MultiOwner",
-			s: map[opregistry.APIKey]OperatorSet{
-				{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
-					"owner1": {Name: "op1"},
-					"owner2": {Name: "op2"},
-				},
-			},
-		},
-		{
-			name: "MultipleApi/MultiOwner",
-			s: map[opregistry.APIKey]OperatorSet{
-				{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
-					"owner1": {Name: "op1"},
-					"owner2": {Name: "op2"},
-				},
-				{Group: "g2", Version: "v2", Kind: "k2", Plural: "p2"}: map[string]*Entry{
-					"owner1": {Name: "op1"},
-					"owner2": {Name: "op2"},
-				},
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			startLen := len(tt.s)
-
-			popped := tt.s.PopAPIKey()
-
-			if startLen == 0 {
-				require.Nil(t, popped, "popped key from empty MultiOwnerSet should be nil")
-				require.Equal(t, 0, len(tt.s))
-			} else {
-				_, found := tt.s[*popped]
-				require.False(t, found, "popped key should not still exist in set")
-				require.Equal(t, startLen-1, len(tt.s))
-			}
-		})
-	}
-}
-
-func TestAPIMultiOwnerSet_PopAPIRequirers(t *testing.T) {
-	tests := []struct {
-		name string
-		s    APIMultiOwnerSet
-		want OperatorSet
-	}{
-		{
-			name: "Empty",
-			s:    EmptyAPIMultiOwnerSet(),
-			want: nil,
-		},
-		{
-			name: "OneApi/OneOwner",
-			s: map[opregistry.APIKey]OperatorSet{
-				{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
-					"owner1": {Name: "op1"},
-				},
-			},
-			want: map[string]*Entry{
-				"owner1": {Name: "op1"},
-			},
-		},
-		{
-			name: "OneApi/MultiOwner",
-			s: map[opregistry.APIKey]OperatorSet{
-				{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
-					"owner1": {Name: "op1"},
-					"owner2": {Name: "op2"},
-				},
-			},
-			want: map[string]*Entry{
-				"owner1": {Name: "op1"},
-				"owner2": {Name: "op2"},
-			},
-		},
-		{
-			name: "MultipleApi/MultiOwner",
-			s: map[opregistry.APIKey]OperatorSet{
-				{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
-					"owner1": {Name: "op1"},
-					"owner2": {Name: "op2"},
-				},
-				{Group: "g2", Version: "v2", Kind: "k2", Plural: "p2"}: map[string]*Entry{
-					"owner1": {Name: "op1"},
-					"owner2": {Name: "op2"},
-				},
-			},
-			want: map[string]*Entry{
-				"owner1": {Name: "op1"},
-				"owner2": {Name: "op2"},
-			},
-		},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			startLen := len(tt.s)
-			require.Equal(t, tt.s.PopAPIRequirers(), tt.want)
-
-			// Verify len has decreased
-			if startLen == 0 {
-				require.Equal(t, 0, len(tt.s))
-			} else {
-				require.Equal(t, startLen-1, len(tt.s))
-			}
-		})
-	}
-}
-
 func TestOperatorSourceInfo_String(t *testing.T) {
 	type fields struct {
 		Package                string

pkg/controller/registry/resolver/resolver.go

@@ -19,19 +19,15 @@ import (
 	opregistry "github.com/operator-framework/operator-registry/pkg/registry"
 )
 
-type OperatorResolver interface {
-	SolveOperators(csvs []*v1alpha1.ClusterServiceVersion, add map[cache.OperatorSourceInfo]struct{}) (cache.OperatorSet, error)
-}
-
-type SatResolver struct {
+type Resolver struct {
 	cache                     cache.OperatorCacheProvider
 	log                       logrus.FieldLogger
 	pc                        *predicateConverter
 	systemConstraintsProvider solver.ConstraintProvider
 }
 
-func NewDefaultSatResolver(rcp cache.SourceProvider, sourcePriorityProvider cache.SourcePriorityProvider, logger logrus.FieldLogger) *SatResolver {
-	return &SatResolver{
+func NewDefaultResolver(rcp cache.SourceProvider, sourcePriorityProvider cache.SourcePriorityProvider, logger logrus.FieldLogger) *Resolver {
+	return &Resolver{
 		cache: cache.New(rcp, cache.WithLogger(logger), cache.WithSourcePriorityProvider(sourcePriorityProvider)),
 		log:   logger,
 		pc: &predicateConverter{
@@ -50,7 +46,7 @@ func (w *debugWriter) Write(b []byte) (int, error) {
 	return n, nil
 }
 
-func (r *SatResolver) SolveOperators(namespaces []string, subs []*v1alpha1.Subscription) (cache.OperatorSet, error) {
+func (r *Resolver) Resolve(namespaces []string, subs []*v1alpha1.Subscription) ([]*cache.Entry, error) {
 	var errs []error
 
 	variables := make(map[solver.Identifier]solver.Variable)
@@ -146,7 +142,7 @@ func (r *SatResolver) SolveOperators(namespaces []string, subs []*v1alpha1.Subsc
 		}
 	}
 
-	operators := make(map[string]*cache.Entry)
+	var operators []*cache.Entry
 	for _, variableOperator := range operatorVariables {
 		csvName, channel, catalog, err := variableOperator.BundleSourceInfo()
 		if err != nil {
@@ -190,7 +186,7 @@ func (r *SatResolver) SolveOperators(namespaces []string, subs []*v1alpha1.Subsc
 			op.SourceInfo.StartingCSV = csvName
 		}
 
-		operators[csvName] = op
+		operators = append(operators, op)
 	}
 
 	if len(errs) > 0 {
@@ -202,7 +198,7 @@ func (r *SatResolver) SolveOperators(namespaces []string, subs []*v1alpha1.Subsc
 
 // newBundleVariableFromEntry converts an entry into a bundle variable with
 // system constraints applied, if they are defined for the entry
-func (r *SatResolver) newBundleVariableFromEntry(entry *cache.Entry) (*BundleVariable, error) {
+func (r *Resolver) newBundleVariableFromEntry(entry *cache.Entry) (*BundleVariable, error) {
 	bundleInstalleble, err := NewBundleVariableFromOperator(entry)
 	if err != nil {
 		return nil, err
@@ -219,7 +215,7 @@ func (r *SatResolver) newBundleVariableFromEntry(entry *cache.Entry) (*BundleVar
 	return &bundleInstalleble, nil
 }
 
-func (r *SatResolver) getSubscriptionVariables(sub *v1alpha1.Subscription, current *cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleVariable) (map[solver.Identifier]solver.Variable, error) {
+func (r *Resolver) getSubscriptionVariables(sub *v1alpha1.Subscription, current *cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleVariable) (map[solver.Identifier]solver.Variable, error) {
 	var cachePredicates, channelPredicates []cache.Predicate
 	variables := make(map[solver.Identifier]solver.Variable)
 
@@ -353,7 +349,7 @@ func (r *SatResolver) getSubscriptionVariables(sub *v1alpha1.Subscription, curre
 	return variables, nil
 }
 
-func (r *SatResolver) getBundleVariables(preferredNamespace string, bundleStack []*cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleVariable) (map[solver.Identifier]struct{}, map[solver.Identifier]*BundleVariable, error) {
+func (r *Resolver) getBundleVariables(preferredNamespace string, bundleStack []*cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleVariable) (map[solver.Identifier]struct{}, map[solver.Identifier]*BundleVariable, error) {
 	errs := make([]error, 0)
 	variables := make(map[solver.Identifier]*BundleVariable) // all variables, including dependencies
 
@@ -461,7 +457,7 @@ func (r *SatResolver) getBundleVariables(preferredNamespace string, bundleStack
 	return ids, variables, nil
 }
 
-func (r *SatResolver) addInvariants(namespacedCache cache.MultiCatalogOperatorFinder, variables map[solver.Identifier]solver.Variable) {
+func (r *Resolver) addInvariants(namespacedCache cache.MultiCatalogOperatorFinder, variables map[solver.Identifier]solver.Variable) {
 	// no two operators may provide the same GVK or Package in a namespace
 	gvkConflictToVariable := make(map[opregistry.GVKProperty][]solver.Identifier)
 	packageConflictToVariable := make(map[string][]solver.Identifier)
@@ -518,7 +514,7 @@ func (r *SatResolver) addInvariants(namespacedCache cache.MultiCatalogOperatorFi
 	}
 }
 
-func (r *SatResolver) sortBundles(bundles []*cache.Entry) ([]*cache.Entry, error) {
+func (r *Resolver) sortBundles(bundles []*cache.Entry) ([]*cache.Entry, error) {
 	// assume bundles have been passed in sorted by catalog already
 	catalogOrder := make([]cache.SourceKey, 0)
 
@@ -814,8 +810,8 @@ func predicateForRequiredLabelProperty(value string) (cache.Predicate, error) {
 	return cache.LabelPredicate(label.Label), nil
 }
 
-func providedAPIsToProperties(apis cache.APISet) (out []*api.Property, err error) {
-	out = make([]*api.Property, 0)
+func providedAPIsToProperties(apis cache.APISet) ([]*api.Property, error) {
+	var out []*api.Property
 	for a := range apis {
 		val, err := json.Marshal(opregistry.GVKProperty{
 			Group:   a.Group,
@@ -830,17 +826,14 @@ func providedAPIsToProperties(apis cache.APISet) (out []*api.Property, err error
 			Value: string(val),
 		})
 	}
-	if len(out) > 0 {
-		return
-	}
-	return nil, nil
+	sort.Slice(out, func(i, j int) bool {
+		return out[i].Value < out[j].Value
+	})
+	return out, nil
 }
 
-func requiredAPIsToProperties(apis cache.APISet) (out []*api.Property, err error) {
-	if len(apis) == 0 {
-		return
-	}
-	out = make([]*api.Property, 0)
+func requiredAPIsToProperties(apis cache.APISet) ([]*api.Property, error) {
+	var out []*api.Property
 	for a := range apis {
 		val, err := json.Marshal(struct {
 			Group   string `json:"group"`
@@ -859,8 +852,8 @@ func requiredAPIsToProperties(apis cache.APISet) (out []*api.Property, err error
 			Value: string(val),
 		})
 	}
-	if len(out) > 0 {
-		return
-	}
-	return nil, nil
+	sort.Slice(out, func(i, j int) bool {
+		return out[i].Value < out[j].Value
+	})
+	return out, nil
 }