Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove vestigal cache.OperatorSet type. #2656

Merged
merged 1 commit into from
Feb 18, 2022
Merged

Remove vestigal cache.OperatorSet type. #2656

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 1 addition & 53 deletions pkg/controller/registry/resolver/cache/operators.go
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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