Merge pull request #1990 from benluddy/subscription-constraint-strings

Bug 1921954: Clarify subscription constraint strings in resolution failures.

Author: openshift-merge-robot

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,
 					},
 				}),
 			},