Clarify subscription constraint strings in resolution failures.

ResolutionFailed events exposed internal terminology (i.e. "mandatory"
and "dependency") directly to users. Common constraints applied to
subscriptions appeared as "PACKAGE is mandatory" or "PACKAGE has a
dependency without any candidates to satisfy it", which in practice
meant "a subscription to PACKAGE exists in the namespace" and "no
operators found matching the criteria of the subscription to PACKAGE",
respectively. The language of these Subscription-related constraint
strings now better reflects what they mean to users.

Signed-off-by: Ben Luddy <[email protected]>

Author: benluddy

pkg/controller/registry/resolver/installabletypes.go

@@ -64,8 +64,10 @@ func NewBundleInstallable(bundle, channel string, catalog registry.CatalogKey, c
 
 func NewSubscriptionInstallable(pkg string) SubscriptionInstallable {
 	return SubscriptionInstallable{
-		identifier:  solver.Identifier(pkg),
-		constraints: []solver.Constraint{solver.Mandatory()},
+		identifier: solver.Identifier(pkg),
+		constraints: []solver.Constraint{
+			ConstraintSubscriptionExists,
+		},
 	}
 }
 
@@ -83,5 +85,35 @@ func (r SubscriptionInstallable) Constraints() []solver.Constraint {
 }
 
 func (r *SubscriptionInstallable) AddDependency(dependencies []solver.Identifier) {
-	r.constraints = append(r.constraints, solver.Dependency(dependencies...))
+	if len(dependencies) == 0 {
+		r.constraints = append(r.constraints, ConstraintSubscriptionWithoutCandidates)
+		return
+	}
+
+	s := make([]string, len(dependencies))
+	for i, each := range dependencies {
+		s[i] = string(each)
+	}
+	r.constraints = append(r.constraints, PrettyConstraint(solver.Dependency(dependencies...), fmt.Sprintf("subscription to %%s requires at least one of %s", strings.Join(s, ", "))))
+}
+
+func PrettyConstraint(c solver.Constraint, format string) solver.Constraint {
+	return prettyConstraint{
+		Constraint: c,
+		format:     format,
+	}
 }
+
+type prettyConstraint struct {
+	solver.Constraint
+	format string
+}
+
+func (pc prettyConstraint) String(subject solver.Identifier) string {
+	return fmt.Sprintf(pc.format, subject)
+}
+
+var (
+	ConstraintSubscriptionExists            = PrettyConstraint(solver.Mandatory(), "a subscription to package %s exists in the namespace")
+	ConstraintSubscriptionWithoutCandidates = PrettyConstraint(solver.Dependency(), "no operators found matching the subscription criteria for %s")
+)

pkg/controller/registry/resolver/solver/constraints.go

@@ -14,6 +14,7 @@ type Constraint interface {
 	String(subject Identifier) string
 	apply(c *logic.C, lm *litMapping, subject Identifier) z.Lit
 	order() []Identifier
+	anchor() bool
 }
 
 // zeroConstraint is returned by ConstraintOf in error cases.
@@ -33,6 +34,10 @@ func (zeroConstraint) order() []Identifier {
 	return nil
 }
 
+func (zeroConstraint) anchor() bool {
+	return false
+}
+
 // AppliedConstraint values compose a single Constraint with the
 // Installable it applies to.
 type AppliedConstraint struct {
@@ -60,6 +65,10 @@ func (constraint mandatory) order() []Identifier {
 	return nil
 }
 
+func (constraint mandatory) anchor() bool {
+	return true
+}
+
 // Mandatory returns a Constraint that will permit only solutions that
 // contain a particular Installable.
 func Mandatory() Constraint {
@@ -80,6 +89,10 @@ func (constraint prohibited) order() []Identifier {
 	return nil
 }
 
+func (constraint prohibited) anchor() bool {
+	return false
+}
+
 // Prohibited returns a Constraint that will reject any solution that
 // contains a particular Installable. Callers may also decide to omit
 // an Installable from input to Solve rather than apply such a
@@ -113,6 +126,10 @@ func (constraint dependency) order() []Identifier {
 	return constraint
 }
 
+func (constraint dependency) anchor() bool {
+	return false
+}
+
 // Dependency returns a Constraint that will only permit solutions
 // containing a given Installable on the condition that at least one
 // of the Installables identified by the given Identifiers also
@@ -136,6 +153,10 @@ func (constraint conflict) order() []Identifier {
 	return nil
 }
 
+func (constraint conflict) anchor() bool {
+	return false
+}
+
 // Conflict returns a Constraint that will permit solutions containing
 // either the constrained Installable, the Installable identified by
 // the given Identifier, or neither, but not both.
@@ -168,6 +189,10 @@ func (constraint leq) order() []Identifier {
 	return nil
 }
 
+func (constraint leq) anchor() bool {
+	return false
+}
+
 // AtMost returns a Constraint that forbids solutions that contain
 // more than n of the Installables identified by the given
 // Identifiers.

pkg/controller/registry/resolver/solver/lit_mapping.go

@@ -158,14 +158,14 @@ func (d *litMapping) CardinalityConstrainer(g inter.Adder, ms []z.Lit) *logic.Ca
 	return cs
 }
 
-// MandatoryIdentifiers returns a slice containing the Identifiers of
-// every Installable with at least one "Mandatory" constraint, in the
+// AnchorIdentifiers returns a slice containing the Identifiers of
+// every Installable with at least one "anchor" constraint, in the
 // order they appear in the input.
-func (d *litMapping) MandatoryIdentifiers() []Identifier {
+func (d *litMapping) AnchorIdentifiers() []Identifier {
 	var ids []Identifier
 	for _, installable := range d.inorder {
 		for _, constraint := range installable.Constraints() {
-			if _, ok := constraint.(mandatory); ok {
+			if constraint.anchor() {
 				ids = append(ids, installable.Identifier())
 				break
 			}

pkg/controller/registry/resolver/solver/search_test.go

@@ -88,7 +88,7 @@ func TestSearch(t *testing.T) {
 			}
 
 			var anchors []z.Lit
-			for _, id := range h.lits.MandatoryIdentifiers() {
+			for _, id := range h.lits.AnchorIdentifiers() {
 				anchors = append(anchors, h.lits.LitOf(id))
 			}
 

pkg/controller/registry/resolver/solver/solve.go

@@ -65,7 +65,7 @@ func (s *solver) Solve(ctx context.Context) (result []Installable, err error) {
 
 	// collect literals of all mandatory installables to assume as a baseline
 	var assumptions []z.Lit
-	for _, anchor := range s.litMap.MandatoryIdentifiers() {
+	for _, anchor := range s.litMap.AnchorIdentifiers() {
 		assumptions = append(assumptions, s.litMap.LitOf(anchor))
 	}
 

pkg/controller/registry/resolver/step_resolver_test.go

@@ -105,7 +105,7 @@ func SharedResolverSpecs() []resolverTest {
 								solver.Dependency(),
 							},
 						},
-						Constraint: solver.Mandatory(),
+						Constraint: ConstraintSubscriptionExists,
 					},
 					{
 						Installable: &SubscriptionInstallable{
@@ -115,7 +115,7 @@ func SharedResolverSpecs() []resolverTest {
 								solver.Dependency(),
 							},
 						},
-						Constraint: solver.Dependency(),
+						Constraint: ConstraintSubscriptionWithoutCandidates,
 					},
 				},
 			},
@@ -270,12 +270,11 @@ func SharedResolverSpecs() []resolverTest {
 						Installable: &SubscriptionInstallable{
 							identifier: "a",
 							constraints: []solver.Constraint{
-								solver.Mandatory(),
-								solver.Dependency("catsrc/catsrc-namespace/alpha/a.v1"),
-								solver.AtMost(1, "catsrc/catsrc-namespace/alpha/a.v1"),
+								ConstraintSubscriptionExists,
+								PrettyConstraint(solver.Dependency("catsrc/catsrc-namespace/alpha/a.v1"), "subscription to %s requires at least one of catsrc/catsrc-namespace/alpha/a.v1"),
 							},
 						},
-						Constraint: solver.Dependency("catsrc/catsrc-namespace/alpha/a.v1"),
+						Constraint: PrettyConstraint(solver.Dependency("catsrc/catsrc-namespace/alpha/a.v1"), "subscription to %s requires at least one of catsrc/catsrc-namespace/alpha/a.v1"),
 					},
 					{
 						Installable: &BundleInstallable{
@@ -288,12 +287,11 @@ func SharedResolverSpecs() []resolverTest {
 						Installable: &SubscriptionInstallable{
 							identifier: "a",
 							constraints: []solver.Constraint{
-								solver.Mandatory(),
-								solver.Dependency("catsrc/catsrc-namespace/alpha/a.v1"),
-								solver.AtMost(1, "catsrc/catsrc-namespace/alpha/a.v1"),
+								ConstraintSubscriptionExists,
+								PrettyConstraint(solver.Dependency("catsrc/catsrc-namespace/alpha/a.v1"), "subscription to %s requires at least one of catsrc/catsrc-namespace/alpha/a.v1"),
 							},
 						},
-						Constraint: solver.Mandatory(),
+						Constraint: ConstraintSubscriptionExists,
 					},
 				}),
 			},