Skip to content

Commit 40ee3de

Browse files
benluddybenluddy
authored
Remove vestigal cache.OperatorSet type. (#2656)
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]>
1 parent 8119718 commit 40ee3de

File tree

7 files changed

+371
-642
lines changed

7 files changed

+371
-642
lines changed

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

+1-53
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
opregistry "github.com/operator-framework/operator-registry/pkg/registry"
1515
)
1616

17+
// todo: drop fields from cache.Entry and move to pkg/controller/operators/olm
1718
type APISet map[opregistry.APIKey]struct{}
1819

1920
func EmptyAPISet() APISet {
@@ -133,59 +134,6 @@ func (s APISet) StripPlural() APISet {
133134
return set
134135
}
135136

136-
type APIOwnerSet map[opregistry.APIKey]*Entry
137-
138-
func EmptyAPIOwnerSet() APIOwnerSet {
139-
return map[opregistry.APIKey]*Entry{}
140-
}
141-
142-
type OperatorSet map[string]*Entry
143-
144-
func EmptyOperatorSet() OperatorSet {
145-
return map[string]*Entry{}
146-
}
147-
148-
// Snapshot returns a new set, pointing to the same values
149-
func (o OperatorSet) Snapshot() OperatorSet {
150-
out := make(map[string]*Entry)
151-
for key, val := range o {
152-
out[key] = val
153-
}
154-
return out
155-
}
156-
157-
type APIMultiOwnerSet map[opregistry.APIKey]OperatorSet
158-
159-
func EmptyAPIMultiOwnerSet() APIMultiOwnerSet {
160-
return map[opregistry.APIKey]OperatorSet{}
161-
}
162-
163-
func (s APIMultiOwnerSet) PopAPIKey() *opregistry.APIKey {
164-
for a := range s {
165-
api := &opregistry.APIKey{
166-
Group: a.Group,
167-
Version: a.Version,
168-
Kind: a.Kind,
169-
Plural: a.Plural,
170-
}
171-
delete(s, a)
172-
return api
173-
}
174-
return nil
175-
}
176-
177-
func (s APIMultiOwnerSet) PopAPIRequirers() OperatorSet {
178-
requirers := EmptyOperatorSet()
179-
for a := range s {
180-
for key, op := range s[a] {
181-
requirers[key] = op
182-
}
183-
delete(s, a)
184-
return requirers
185-
}
186-
return nil
187-
}
188-
189137
type OperatorSourceInfo struct {
190138
Package string
191139
Channel string

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

-126
Original file line numberDiff line numberDiff line change
@@ -716,132 +716,6 @@ func TestCatalogKey_String(t *testing.T) {
716716
}
717717
}
718718

719-
func TestAPIMultiOwnerSet_PopAPIKey(t *testing.T) {
720-
tests := []struct {
721-
name string
722-
s APIMultiOwnerSet
723-
}{
724-
{
725-
name: "Empty",
726-
s: EmptyAPIMultiOwnerSet(),
727-
},
728-
{
729-
name: "OneApi/OneOwner",
730-
s: map[opregistry.APIKey]OperatorSet{
731-
{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
732-
"owner1": {Name: "op1"},
733-
},
734-
},
735-
},
736-
{
737-
name: "OneApi/MultiOwner",
738-
s: map[opregistry.APIKey]OperatorSet{
739-
{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
740-
"owner1": {Name: "op1"},
741-
"owner2": {Name: "op2"},
742-
},
743-
},
744-
},
745-
{
746-
name: "MultipleApi/MultiOwner",
747-
s: map[opregistry.APIKey]OperatorSet{
748-
{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
749-
"owner1": {Name: "op1"},
750-
"owner2": {Name: "op2"},
751-
},
752-
{Group: "g2", Version: "v2", Kind: "k2", Plural: "p2"}: map[string]*Entry{
753-
"owner1": {Name: "op1"},
754-
"owner2": {Name: "op2"},
755-
},
756-
},
757-
},
758-
}
759-
for _, tt := range tests {
760-
t.Run(tt.name, func(t *testing.T) {
761-
startLen := len(tt.s)
762-
763-
popped := tt.s.PopAPIKey()
764-
765-
if startLen == 0 {
766-
require.Nil(t, popped, "popped key from empty MultiOwnerSet should be nil")
767-
require.Equal(t, 0, len(tt.s))
768-
} else {
769-
_, found := tt.s[*popped]
770-
require.False(t, found, "popped key should not still exist in set")
771-
require.Equal(t, startLen-1, len(tt.s))
772-
}
773-
})
774-
}
775-
}
776-
777-
func TestAPIMultiOwnerSet_PopAPIRequirers(t *testing.T) {
778-
tests := []struct {
779-
name string
780-
s APIMultiOwnerSet
781-
want OperatorSet
782-
}{
783-
{
784-
name: "Empty",
785-
s: EmptyAPIMultiOwnerSet(),
786-
want: nil,
787-
},
788-
{
789-
name: "OneApi/OneOwner",
790-
s: map[opregistry.APIKey]OperatorSet{
791-
{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
792-
"owner1": {Name: "op1"},
793-
},
794-
},
795-
want: map[string]*Entry{
796-
"owner1": {Name: "op1"},
797-
},
798-
},
799-
{
800-
name: "OneApi/MultiOwner",
801-
s: map[opregistry.APIKey]OperatorSet{
802-
{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
803-
"owner1": {Name: "op1"},
804-
"owner2": {Name: "op2"},
805-
},
806-
},
807-
want: map[string]*Entry{
808-
"owner1": {Name: "op1"},
809-
"owner2": {Name: "op2"},
810-
},
811-
},
812-
{
813-
name: "MultipleApi/MultiOwner",
814-
s: map[opregistry.APIKey]OperatorSet{
815-
{Group: "g", Version: "v", Kind: "k", Plural: "p"}: map[string]*Entry{
816-
"owner1": {Name: "op1"},
817-
"owner2": {Name: "op2"},
818-
},
819-
{Group: "g2", Version: "v2", Kind: "k2", Plural: "p2"}: map[string]*Entry{
820-
"owner1": {Name: "op1"},
821-
"owner2": {Name: "op2"},
822-
},
823-
},
824-
want: map[string]*Entry{
825-
"owner1": {Name: "op1"},
826-
"owner2": {Name: "op2"},
827-
},
828-
},
829-
}
830-
for _, tt := range tests {
831-
t.Run(tt.name, func(t *testing.T) {
832-
startLen := len(tt.s)
833-
require.Equal(t, tt.s.PopAPIRequirers(), tt.want)
834-
835-
// Verify len has decreased
836-
if startLen == 0 {
837-
require.Equal(t, 0, len(tt.s))
838-
} else {
839-
require.Equal(t, startLen-1, len(tt.s))
840-
}
841-
})
842-
}
843-
}
844-
845719
func TestOperatorSourceInfo_String(t *testing.T) {
846720
type fields struct {
847721
Package string

‎pkg/controller/registry/resolver/resolver.go

+23-30
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,15 @@ import (
1919
opregistry "github.com/operator-framework/operator-registry/pkg/registry"
2020
)
2121

22-
type OperatorResolver interface {
23-
SolveOperators(csvs []*v1alpha1.ClusterServiceVersion, add map[cache.OperatorSourceInfo]struct{}) (cache.OperatorSet, error)
24-
}
25-
26-
type SatResolver struct {
22+
type Resolver struct {
2723
cache cache.OperatorCacheProvider
2824
log logrus.FieldLogger
2925
pc *predicateConverter
3026
systemConstraintsProvider solver.ConstraintProvider
3127
}
3228

33-
func NewDefaultSatResolver(rcp cache.SourceProvider, sourcePriorityProvider cache.SourcePriorityProvider, logger logrus.FieldLogger) *SatResolver {
34-
return &SatResolver{
29+
func NewDefaultResolver(rcp cache.SourceProvider, sourcePriorityProvider cache.SourcePriorityProvider, logger logrus.FieldLogger) *Resolver {
30+
return &Resolver{
3531
cache: cache.New(rcp, cache.WithLogger(logger), cache.WithSourcePriorityProvider(sourcePriorityProvider)),
3632
log: logger,
3733
pc: &predicateConverter{
@@ -50,7 +46,7 @@ func (w *debugWriter) Write(b []byte) (int, error) {
5046
return n, nil
5147
}
5248

53-
func (r *SatResolver) SolveOperators(namespaces []string, subs []*v1alpha1.Subscription) (cache.OperatorSet, error) {
49+
func (r *Resolver) Resolve(namespaces []string, subs []*v1alpha1.Subscription) ([]*cache.Entry, error) {
5450
var errs []error
5551

5652
variables := make(map[solver.Identifier]solver.Variable)
@@ -146,7 +142,7 @@ func (r *SatResolver) SolveOperators(namespaces []string, subs []*v1alpha1.Subsc
146142
}
147143
}
148144

149-
operators := make(map[string]*cache.Entry)
145+
var operators []*cache.Entry
150146
for _, variableOperator := range operatorVariables {
151147
csvName, channel, catalog, err := variableOperator.BundleSourceInfo()
152148
if err != nil {
@@ -190,7 +186,7 @@ func (r *SatResolver) SolveOperators(namespaces []string, subs []*v1alpha1.Subsc
190186
op.SourceInfo.StartingCSV = csvName
191187
}
192188

193-
operators[csvName] = op
189+
operators = append(operators, op)
194190
}
195191

196192
if len(errs) > 0 {
@@ -202,7 +198,7 @@ func (r *SatResolver) SolveOperators(namespaces []string, subs []*v1alpha1.Subsc
202198

203199
// newBundleVariableFromEntry converts an entry into a bundle variable with
204200
// system constraints applied, if they are defined for the entry
205-
func (r *SatResolver) newBundleVariableFromEntry(entry *cache.Entry) (*BundleVariable, error) {
201+
func (r *Resolver) newBundleVariableFromEntry(entry *cache.Entry) (*BundleVariable, error) {
206202
bundleInstalleble, err := NewBundleVariableFromOperator(entry)
207203
if err != nil {
208204
return nil, err
@@ -219,7 +215,7 @@ func (r *SatResolver) newBundleVariableFromEntry(entry *cache.Entry) (*BundleVar
219215
return &bundleInstalleble, nil
220216
}
221217

222-
func (r *SatResolver) getSubscriptionVariables(sub *v1alpha1.Subscription, current *cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleVariable) (map[solver.Identifier]solver.Variable, error) {
218+
func (r *Resolver) getSubscriptionVariables(sub *v1alpha1.Subscription, current *cache.Entry, namespacedCache cache.MultiCatalogOperatorFinder, visited map[*cache.Entry]*BundleVariable) (map[solver.Identifier]solver.Variable, error) {
223219
var cachePredicates, channelPredicates []cache.Predicate
224220
variables := make(map[solver.Identifier]solver.Variable)
225221

@@ -353,7 +349,7 @@ func (r *SatResolver) getSubscriptionVariables(sub *v1alpha1.Subscription, curre
353349
return variables, nil
354350
}
355351

356-
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) {
352+
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) {
357353
errs := make([]error, 0)
358354
variables := make(map[solver.Identifier]*BundleVariable) // all variables, including dependencies
359355

@@ -461,7 +457,7 @@ func (r *SatResolver) getBundleVariables(preferredNamespace string, bundleStack
461457
return ids, variables, nil
462458
}
463459

464-
func (r *SatResolver) addInvariants(namespacedCache cache.MultiCatalogOperatorFinder, variables map[solver.Identifier]solver.Variable) {
460+
func (r *Resolver) addInvariants(namespacedCache cache.MultiCatalogOperatorFinder, variables map[solver.Identifier]solver.Variable) {
465461
// no two operators may provide the same GVK or Package in a namespace
466462
gvkConflictToVariable := make(map[opregistry.GVKProperty][]solver.Identifier)
467463
packageConflictToVariable := make(map[string][]solver.Identifier)
@@ -518,7 +514,7 @@ func (r *SatResolver) addInvariants(namespacedCache cache.MultiCatalogOperatorFi
518514
}
519515
}
520516

521-
func (r *SatResolver) sortBundles(bundles []*cache.Entry) ([]*cache.Entry, error) {
517+
func (r *Resolver) sortBundles(bundles []*cache.Entry) ([]*cache.Entry, error) {
522518
// assume bundles have been passed in sorted by catalog already
523519
catalogOrder := make([]cache.SourceKey, 0)
524520

@@ -814,8 +810,8 @@ func predicateForRequiredLabelProperty(value string) (cache.Predicate, error) {
814810
return cache.LabelPredicate(label.Label), nil
815811
}
816812

817-
func providedAPIsToProperties(apis cache.APISet) (out []*api.Property, err error) {
818-
out = make([]*api.Property, 0)
813+
func providedAPIsToProperties(apis cache.APISet) ([]*api.Property, error) {
814+
var out []*api.Property
819815
for a := range apis {
820816
val, err := json.Marshal(opregistry.GVKProperty{
821817
Group: a.Group,
@@ -830,17 +826,14 @@ func providedAPIsToProperties(apis cache.APISet) (out []*api.Property, err error
830826
Value: string(val),
831827
})
832828
}
833-
if len(out) > 0 {
834-
return
835-
}
836-
return nil, nil
829+
sort.Slice(out, func(i, j int) bool {
830+
return out[i].Value < out[j].Value
831+
})
832+
return out, nil
837833
}
838834

839-
func requiredAPIsToProperties(apis cache.APISet) (out []*api.Property, err error) {
840-
if len(apis) == 0 {
841-
return
842-
}
843-
out = make([]*api.Property, 0)
835+
func requiredAPIsToProperties(apis cache.APISet) ([]*api.Property, error) {
836+
var out []*api.Property
844837
for a := range apis {
845838
val, err := json.Marshal(struct {
846839
Group string `json:"group"`
@@ -859,8 +852,8 @@ func requiredAPIsToProperties(apis cache.APISet) (out []*api.Property, err error
859852
Value: string(val),
860853
})
861854
}
862-
if len(out) > 0 {
863-
return
864-
}
865-
return nil, nil
855+
sort.Slice(out, func(i, j int) bool {
856+
return out[i].Value < out[j].Value
857+
})
858+
return out, nil
866859
}

0 commit comments

Comments
 (0)