Skip to content

Give more precise reasons for Subscriptions with no candidates. #2086

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.

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
76 changes: 50 additions & 26 deletions pkg/controller/registry/resolver/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,15 @@ func (s SortableSnapshots) Swap(i, j int) {
s.snapshots[i], s.snapshots[j] = s.snapshots[j], s.snapshots[i]
}

type OperatorPredicate func(*Operator) bool
type OperatorPredicateFunc func(*Operator) bool

func (opf OperatorPredicateFunc) Test(o *Operator) bool {
return opf(o)
}

type OperatorPredicate interface {
Test(*Operator) bool
}

func (s *CatalogSnapshot) Find(p ...OperatorPredicate) []*Operator {
s.m.RLock()
Expand Down Expand Up @@ -416,13 +424,13 @@ func (f EmptyOperatorFinder) Find(...OperatorPredicate) []*Operator {
}

func WithCSVName(name string) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
return o.name == name
}
})
}

func WithChannel(channel string) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
// all operators match the empty channel
if channel == "" {
return true
Expand All @@ -431,11 +439,11 @@ func WithChannel(channel string) OperatorPredicate {
return false
}
return o.bundle.ChannelName == channel
}
})
}

func WithPackage(pkg string) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
for _, p := range o.Properties() {
if p.Type != opregistry.PackageType {
continue
Expand All @@ -450,22 +458,22 @@ func WithPackage(pkg string) OperatorPredicate {
}
}
return o.Package() == pkg
}
})
}

func WithoutDeprecatedProperty() OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
for _, p := range o.bundle.GetProperties() {
if p.GetType() == opregistry.DeprecatedType {
return false
}
}
return true
}
})
}

func WithVersionInRange(r semver.Range) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
for _, p := range o.Properties() {
if p.Type != opregistry.PackageType {
continue
Expand All @@ -484,11 +492,11 @@ func WithVersionInRange(r semver.Range) OperatorPredicate {
}
}
return o.version != nil && r(*o.version)
}
})
}

func WithLabel(label string) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
for _, p := range o.Properties() {
if p.Type != opregistry.LabelType {
continue
Expand All @@ -503,11 +511,11 @@ func WithLabel(label string) OperatorPredicate {
}
}
return false
}
})
}

func ProvidingAPI(api opregistry.APIKey) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
for _, p := range o.Properties() {
if p.Type != opregistry.GVKType {
continue
Expand All @@ -522,19 +530,19 @@ func ProvidingAPI(api opregistry.APIKey) OperatorPredicate {
}
}
return false
}
})
}

func SkipRangeIncludes(version semver.Version) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
// TODO: lift range parsing to OperatorSurface
semverRange, err := semver.ParseRange(o.bundle.SkipRange)
return err == nil && semverRange(version)
}
})
}

func Replaces(name string) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
if o.Replaces() == name {
return true
}
Expand All @@ -544,29 +552,29 @@ func Replaces(name string) OperatorPredicate {
}
}
return false
}
})
}

func And(p ...OperatorPredicate) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
for _, l := range p {
if l(o) == false {
if l.Test(o) == false {
return false
}
}
return true
}
})
}

func Or(p ...OperatorPredicate) OperatorPredicate {
return func(o *Operator) bool {
return OperatorPredicateFunc(func(o *Operator) bool {
for _, l := range p {
if l(o) == true {
if l.Test(o) == true {
return true
}
}
return false
}
})
}

func AtLeast(n int, operators []*Operator) ([]*Operator, error) {
Expand Down Expand Up @@ -594,5 +602,21 @@ func Filter(operators []*Operator, p ...OperatorPredicate) []*Operator {
}

func Matches(o *Operator, p ...OperatorPredicate) bool {
return And(p...)(o)
return And(p...).Test(o)
}

func True() OperatorPredicate {
return OperatorPredicateFunc(func(*Operator) bool {
return true
})
}

func CountingPredicate(p OperatorPredicate, n *int) OperatorPredicate {
return OperatorPredicateFunc(func(o *Operator) bool {
if p.Test(o) {
*n++
return true
}
return false
})
}
64 changes: 55 additions & 9 deletions pkg/controller/registry/resolver/cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,17 +224,17 @@ func TestCatalogSnapshotExpired(t *testing.T) {
func TestCatalogSnapshotFind(t *testing.T) {
type tc struct {
Name string
Predicate func(*Operator) bool
Predicate OperatorPredicate
Operators []*Operator
Expected []*Operator
}

for _, tt := range []tc{
{
Name: "nothing satisfies predicate",
Predicate: func(*Operator) bool {
Predicate: OperatorPredicateFunc(func(*Operator) bool {
return false
},
}),
Operators: []*Operator{
{name: "a"},
{name: "b"},
Expand All @@ -244,17 +244,17 @@ func TestCatalogSnapshotFind(t *testing.T) {
},
{
Name: "no operators in snapshot",
Predicate: func(*Operator) bool {
Predicate: OperatorPredicateFunc(func(*Operator) bool {
return true
},
}),
Operators: nil,
Expected: nil,
},
{
Name: "everything satisfies predicate",
Predicate: func(*Operator) bool {
Predicate: OperatorPredicateFunc(func(*Operator) bool {
return true
},
}),
Operators: []*Operator{
{name: "a"},
{name: "b"},
Expand All @@ -268,9 +268,9 @@ func TestCatalogSnapshotFind(t *testing.T) {
},
{
Name: "some satisfy predicate",
Predicate: func(o *Operator) bool {
Predicate: OperatorPredicateFunc(func(o *Operator) bool {
return o.name != "a"
},
}),
Operators: []*Operator{
{name: "a"},
{name: "b"},
Expand Down Expand Up @@ -339,3 +339,49 @@ func TestStripPluralRequiredAndProvidedAPIKeys(t *testing.T) {
assert.Equal(t, "K.v1.g", result[0].providedAPIs.String())
assert.Equal(t, "K2.v2.g2", result[0].requiredAPIs.String())
}

func TestCountingPredicate(t *testing.T) {
for _, tc := range []struct {
Name string
TestResults []bool
Expected int
}{
{
Name: "no increment on failure",
TestResults: []bool{false},
Expected: 0,
},
{
Name: "increment on success",
TestResults: []bool{true},
Expected: 1,
},
{
Name: "multiple increments",
TestResults: []bool{true, true},
Expected: 2,
},
{
Name: "no increment without test",
TestResults: nil,
Expected: 0,
},
} {
t.Run(tc.Name, func(t *testing.T) {
var (
n int
result bool
)

p := CountingPredicate(OperatorPredicateFunc(func(*Operator) bool {
return result
}), &n)

for _, result = range tc.TestResults {
p.Test(nil)
}

assert.Equal(t, tc.Expected, n)
})
}
}
10 changes: 10 additions & 0 deletions pkg/controller/registry/resolver/installabletypes.go
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ func (i GenericInstallable) Constraints() []solver.Constraint {
return i.constraints
}

func NewInvalidSubscriptionInstallable(name string, reason string) solver.Installable {
return GenericInstallable{
identifier: solver.IdentifierFromString(fmt.Sprintf("subscription:%s", name)),
constraints: []solver.Constraint{
PrettyConstraint(solver.Mandatory(), fmt.Sprintf("subscription %s exists", name)),
PrettyConstraint(solver.Prohibited(), reason),
},
}
}

func NewSubscriptionInstallable(name string, dependencies []solver.Identifier) solver.Installable {
result := GenericInstallable{
identifier: solver.IdentifierFromString(fmt.Sprintf("subscription:%s", name)),
Expand Down
Loading