Skip to content

Commit

Permalink
Remove vestigal cache.OperatorSet type.
Browse files Browse the repository at this point in the history 
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 <bluddy@redhat.com>
  • Loading branch information
@benluddy
benluddy committed Feb 17, 2022
1 parent 8119718 commit d95495d
Show file tree
Hide file tree
Showing 7 changed files with 371 additions and 642 deletions.
54 changes: 1 addition & 53 deletions pkg/controller/registry/resolver/cache/operators.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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
Expand Down
126 changes: 0 additions & 126 deletions pkg/controller/registry/resolver/cache/operators_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 23 additions & 30 deletions pkg/controller/registry/resolver/resolver.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand All @@ -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)
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand All @@ -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)

Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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,
Expand All @@ -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"`
Expand All @@ -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
}
Loading

0 comments on commit d95495d

Please sign in to comment.