Skip to content

Commit a486e54

Browse files
benluddybenluddy
committed
Give more precise reasons for Subscriptions with no candidates.
A Subscription specifies certain criteria for selecting operators from catalogs. The granularity of the Subscription criteria is, from least to most specific: catalog, package, channel, startingCSV (if applicable). If a user creates a Subscription and its criteria does not match a single operator, the failure message now indicates the highest level of granularity that excludes all available operators. For example, if a Subscription specifies a package name that doesn't exist in the referenced, catalog, the specified package name will appear in the failure text. Signed-off-by: Ben Luddy <[email protected]>
1 parent 5cd7822 commit a486e54

File tree

5 files changed

+239
-67
lines changed

5 files changed

+239
-67
lines changed

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

Lines changed: 50 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -387,7 +387,15 @@ func (s SortableSnapshots) Swap(i, j int) {
387387
s.snapshots[i], s.snapshots[j] = s.snapshots[j], s.snapshots[i]
388388
}
389389

390-
type OperatorPredicate func(*Operator) bool
390+
type OperatorPredicateFunc func(*Operator) bool
391+
392+
func (opf OperatorPredicateFunc) Test(o *Operator) bool {
393+
return opf(o)
394+
}
395+
396+
type OperatorPredicate interface {
397+
Test(*Operator) bool
398+
}
391399

392400
func (s *CatalogSnapshot) Find(p ...OperatorPredicate) []*Operator {
393401
s.m.RLock()
@@ -416,13 +424,13 @@ func (f EmptyOperatorFinder) Find(...OperatorPredicate) []*Operator {
416424
}
417425

418426
func WithCSVName(name string) OperatorPredicate {
419-
return func(o *Operator) bool {
427+
return OperatorPredicateFunc(func(o *Operator) bool {
420428
return o.name == name
421-
}
429+
})
422430
}
423431

424432
func WithChannel(channel string) OperatorPredicate {
425-
return func(o *Operator) bool {
433+
return OperatorPredicateFunc(func(o *Operator) bool {
426434
// all operators match the empty channel
427435
if channel == "" {
428436
return true
@@ -431,11 +439,11 @@ func WithChannel(channel string) OperatorPredicate {
431439
return false
432440
}
433441
return o.bundle.ChannelName == channel
434-
}
442+
})
435443
}
436444

437445
func WithPackage(pkg string) OperatorPredicate {
438-
return func(o *Operator) bool {
446+
return OperatorPredicateFunc(func(o *Operator) bool {
439447
for _, p := range o.Properties() {
440448
if p.Type != opregistry.PackageType {
441449
continue
@@ -450,22 +458,22 @@ func WithPackage(pkg string) OperatorPredicate {
450458
}
451459
}
452460
return o.Package() == pkg
453-
}
461+
})
454462
}
455463

456464
func WithoutDeprecatedProperty() OperatorPredicate {
457-
return func(o *Operator) bool {
465+
return OperatorPredicateFunc(func(o *Operator) bool {
458466
for _, p := range o.bundle.GetProperties() {
459467
if p.GetType() == opregistry.DeprecatedType {
460468
return false
461469
}
462470
}
463471
return true
464-
}
472+
})
465473
}
466474

467475
func WithVersionInRange(r semver.Range) OperatorPredicate {
468-
return func(o *Operator) bool {
476+
return OperatorPredicateFunc(func(o *Operator) bool {
469477
for _, p := range o.Properties() {
470478
if p.Type != opregistry.PackageType {
471479
continue
@@ -484,11 +492,11 @@ func WithVersionInRange(r semver.Range) OperatorPredicate {
484492
}
485493
}
486494
return o.version != nil && r(*o.version)
487-
}
495+
})
488496
}
489497

490498
func WithLabel(label string) OperatorPredicate {
491-
return func(o *Operator) bool {
499+
return OperatorPredicateFunc(func(o *Operator) bool {
492500
for _, p := range o.Properties() {
493501
if p.Type != opregistry.LabelType {
494502
continue
@@ -503,11 +511,11 @@ func WithLabel(label string) OperatorPredicate {
503511
}
504512
}
505513
return false
506-
}
514+
})
507515
}
508516

509517
func ProvidingAPI(api opregistry.APIKey) OperatorPredicate {
510-
return func(o *Operator) bool {
518+
return OperatorPredicateFunc(func(o *Operator) bool {
511519
for _, p := range o.Properties() {
512520
if p.Type != opregistry.GVKType {
513521
continue
@@ -522,19 +530,19 @@ func ProvidingAPI(api opregistry.APIKey) OperatorPredicate {
522530
}
523531
}
524532
return false
525-
}
533+
})
526534
}
527535

528536
func SkipRangeIncludes(version semver.Version) OperatorPredicate {
529-
return func(o *Operator) bool {
537+
return OperatorPredicateFunc(func(o *Operator) bool {
530538
// TODO: lift range parsing to OperatorSurface
531539
semverRange, err := semver.ParseRange(o.bundle.SkipRange)
532540
return err == nil && semverRange(version)
533-
}
541+
})
534542
}
535543

536544
func Replaces(name string) OperatorPredicate {
537-
return func(o *Operator) bool {
545+
return OperatorPredicateFunc(func(o *Operator) bool {
538546
if o.Replaces() == name {
539547
return true
540548
}
@@ -544,29 +552,29 @@ func Replaces(name string) OperatorPredicate {
544552
}
545553
}
546554
return false
547-
}
555+
})
548556
}
549557

550558
func And(p ...OperatorPredicate) OperatorPredicate {
551-
return func(o *Operator) bool {
559+
return OperatorPredicateFunc(func(o *Operator) bool {
552560
for _, l := range p {
553-
if l(o) == false {
561+
if l.Test(o) == false {
554562
return false
555563
}
556564
}
557565
return true
558-
}
566+
})
559567
}
560568

561569
func Or(p ...OperatorPredicate) OperatorPredicate {
562-
return func(o *Operator) bool {
570+
return OperatorPredicateFunc(func(o *Operator) bool {
563571
for _, l := range p {
564-
if l(o) == true {
572+
if l.Test(o) == true {
565573
return true
566574
}
567575
}
568576
return false
569-
}
577+
})
570578
}
571579

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

596604
func Matches(o *Operator, p ...OperatorPredicate) bool {
597-
return And(p...)(o)
605+
return And(p...).Test(o)
606+
}
607+
608+
func True() OperatorPredicate {
609+
return OperatorPredicateFunc(func(*Operator) bool {
610+
return true
611+
})
612+
}
613+
614+
func CountingPredicate(p OperatorPredicate, n *int) OperatorPredicate {
615+
return OperatorPredicateFunc(func(o *Operator) bool {
616+
if p.Test(o) {
617+
*n++
618+
return true
619+
}
620+
return false
621+
})
598622
}

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

Lines changed: 55 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -224,17 +224,17 @@ func TestCatalogSnapshotExpired(t *testing.T) {
224224
func TestCatalogSnapshotFind(t *testing.T) {
225225
type tc struct {
226226
Name string
227-
Predicate func(*Operator) bool
227+
Predicate OperatorPredicate
228228
Operators []*Operator
229229
Expected []*Operator
230230
}
231231

232232
for _, tt := range []tc{
233233
{
234234
Name: "nothing satisfies predicate",
235-
Predicate: func(*Operator) bool {
235+
Predicate: OperatorPredicateFunc(func(*Operator) bool {
236236
return false
237-
},
237+
}),
238238
Operators: []*Operator{
239239
{name: "a"},
240240
{name: "b"},
@@ -244,17 +244,17 @@ func TestCatalogSnapshotFind(t *testing.T) {
244244
},
245245
{
246246
Name: "no operators in snapshot",
247-
Predicate: func(*Operator) bool {
247+
Predicate: OperatorPredicateFunc(func(*Operator) bool {
248248
return true
249-
},
249+
}),
250250
Operators: nil,
251251
Expected: nil,
252252
},
253253
{
254254
Name: "everything satisfies predicate",
255-
Predicate: func(*Operator) bool {
255+
Predicate: OperatorPredicateFunc(func(*Operator) bool {
256256
return true
257-
},
257+
}),
258258
Operators: []*Operator{
259259
{name: "a"},
260260
{name: "b"},
@@ -268,9 +268,9 @@ func TestCatalogSnapshotFind(t *testing.T) {
268268
},
269269
{
270270
Name: "some satisfy predicate",
271-
Predicate: func(o *Operator) bool {
271+
Predicate: OperatorPredicateFunc(func(o *Operator) bool {
272272
return o.name != "a"
273-
},
273+
}),
274274
Operators: []*Operator{
275275
{name: "a"},
276276
{name: "b"},
@@ -339,3 +339,49 @@ func TestStripPluralRequiredAndProvidedAPIKeys(t *testing.T) {
339339
assert.Equal(t, "K.v1.g", result[0].providedAPIs.String())
340340
assert.Equal(t, "K2.v2.g2", result[0].requiredAPIs.String())
341341
}
342+
343+
func TestCountingPredicate(t *testing.T) {
344+
for _, tc := range []struct {
345+
Name string
346+
TestResults []bool
347+
Expected int
348+
}{
349+
{
350+
Name: "no increment on failure",
351+
TestResults: []bool{false},
352+
Expected: 0,
353+
},
354+
{
355+
Name: "increment on success",
356+
TestResults: []bool{true},
357+
Expected: 1,
358+
},
359+
{
360+
Name: "multiple increments",
361+
TestResults: []bool{true, true},
362+
Expected: 2,
363+
},
364+
{
365+
Name: "no increment without test",
366+
TestResults: nil,
367+
Expected: 0,
368+
},
369+
} {
370+
t.Run(tc.Name, func(t *testing.T) {
371+
var (
372+
n int
373+
result bool
374+
)
375+
376+
p := CountingPredicate(OperatorPredicateFunc(func(*Operator) bool {
377+
return result
378+
}), &n)
379+
380+
for _, result = range tc.TestResults {
381+
p.Test(nil)
382+
}
383+
384+
assert.Equal(t, tc.Expected, n)
385+
})
386+
}
387+
}

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,16 @@ func (i GenericInstallable) Constraints() []solver.Constraint {
7474
return i.constraints
7575
}
7676

77+
func NewInvalidSubscriptionInstallable(name string, reason string) solver.Installable {
78+
return GenericInstallable{
79+
identifier: solver.IdentifierFromString(fmt.Sprintf("subscription:%s", name)),
80+
constraints: []solver.Constraint{
81+
PrettyConstraint(solver.Mandatory(), fmt.Sprintf("subscription %s exists", name)),
82+
PrettyConstraint(solver.Prohibited(), reason),
83+
},
84+
}
85+
}
86+
7787
func NewSubscriptionInstallable(name string, dependencies []solver.Identifier) solver.Installable {
7888
result := GenericInstallable{
7989
identifier: solver.IdentifierFromString(fmt.Sprintf("subscription:%s", name)),

0 commit comments

Comments
 (0)