fix: check for unsat resolutions in Reconcile (#352)

Signed-off-by: Joe Lanford <[email protected]>

Author: joelanford

internal/controllers/operator_controller.go

@@ -18,10 +18,14 @@ package controllers
 
 import (
 	"context"
+	"errors"
 	"fmt"
+	"sort"
+	"strings"
 
 	"github.com/go-logr/logr"
 	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
+	"github.com/operator-framework/deppy/pkg/deppy"
 	"github.com/operator-framework/deppy/pkg/deppy/solver"
 	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
 	"k8s.io/apimachinery/pkg/api/equality"
@@ -136,6 +140,21 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
 		return ctrl.Result{}, err
 	}
 
+	// TODO: Checking for unsat is awkward using the current version of deppy.
+	//    This awkwardness has been fixed in an unreleased version of deppy.
+	//    When there is a new minor release of deppy, we can revisit this and
+	//    simplify this to a normal error check.
+	//    See https://github.com/operator-framework/deppy/issues/139.
+	unsat := deppy.NotSatisfiable{}
+	if ok := errors.As(solution.Error(), &unsat); ok && len(unsat) > 0 {
+		op.Status.InstalledBundleResource = ""
+		setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted as resolution is unsatisfiable", op.GetGeneration())
+		op.Status.ResolvedBundleResource = ""
+		msg := prettyUnsatMessage(unsat)
+		setResolvedStatusConditionFailed(&op.Status.Conditions, msg, op.GetGeneration())
+		return ctrl.Result{}, unsat
+	}
+
 	// lookup the bundle entity in the solution that corresponds to the
 	// Operator's desired package name.
 	bundleEntity, err := r.getBundleEntityFromSolution(solution, op.Spec.PackageName)
@@ -459,3 +478,23 @@ func operatorRequestsForCatalog(ctx context.Context, c client.Reader, logger log
 		return requests
 	}
 }
+
+// TODO: This can be removed when operator controller bumps to a
+//    version of deppy that contains a fix for this issue:
+//    https://github.com/operator-framework/deppy/issues/142
+
+// prettyUnsatMessage ensures that the unsat message is deterministic and
+// human-readable. It sorts the individual constraint strings lexicographically
+// and joins them with a semicolon (rather than a comma, which the unsat.Error()
+// function does). This function also has the side effect of sorting the items
+// in the unsat slice.
+func prettyUnsatMessage(unsat deppy.NotSatisfiable) string {
+	sort.Slice(unsat, func(i, j int) bool {
+		return unsat[i].String() < unsat[j].String()
+	})
+	msgs := make([]string, 0, len(unsat))
+	for _, c := range unsat {
+		msgs = append(msgs, c.String())
+	}
+	return fmt.Sprintf("constraints not satisfiable: %s", strings.Join(msgs, "; "))
+}

test/e2e/install_test.go

@@ -6,16 +6,14 @@ import (
 
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
-
+	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
+	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
 	"k8s.io/apimachinery/pkg/api/errors"
 	apimeta "k8s.io/apimachinery/pkg/api/meta"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
 
-	catalogd "github.com/operator-framework/catalogd/api/core/v1alpha1"
-	rukpakv1alpha1 "github.com/operator-framework/rukpak/api/v1alpha1"
-
 	operatorv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
 )
 
@@ -185,16 +183,16 @@ var _ = Describe("Operator Install", func() {
 				g.Expect(operator.Status.ResolvedBundleResource).ToNot(BeEmpty())
 			}).Should(Succeed())
 
-			By("updating the Operator resource to an invalid version")
-			operator.Spec.Version = "0.65.1" // the correct one should be 0.47.0
+			By("updating the Operator resource to a non-successor version")
+			operator.Spec.Version = "0.65.1" // current (0.37.0) and successor (0.47.0) are the only values that would be SAT.
 			Expect(c.Update(ctx, operator)).To(Succeed())
-			By("eventually reporting a failed resolution")
+			By("eventually reporting an unsatisfiable resolution")
 			Eventually(func(g Gomega) {
 				g.Expect(c.Get(ctx, types.NamespacedName{Name: operator.Name}, operator)).To(Succeed())
 				cond := apimeta.FindStatusCondition(operator.Status.Conditions, operatorv1alpha1.TypeResolved)
 				g.Expect(cond).ToNot(BeNil())
 				g.Expect(cond.Reason).To(Equal(operatorv1alpha1.ReasonResolutionFailed))
-				g.Expect(cond.Message).To(ContainSubstring("entity for package \"prometheus\" not found in solutio"))
+				g.Expect(cond.Message).To(MatchRegexp(`^constraints not satisfiable:.*; installed package prometheus requires at least one of.*0.47.0[^,]*,[^,]*0.37.0[^;]*;.*`))
 				g.Expect(operator.Status.ResolvedBundleResource).To(BeEmpty())
 			}).Should(Succeed())