Remove prettyUnsatMessage (#547)

It was a workaround a Deppy issue. We no longer need it
since Deppy fixed the issue.

Signed-off-by: Mikalai Radchuk <[email protected]>

Author: m1kola

internal/controllers/operator_controller.go

@@ -18,14 +18,10 @@ 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"
@@ -142,8 +138,7 @@ func (r *OperatorReconciler) reconcile(ctx context.Context, op *operatorsv1alpha
 		op.Status.InstalledBundleResource = ""
 		setInstalledStatusConditionUnknown(&op.Status.Conditions, "installation has not been attempted as resolution is unsatisfiable", op.GetGeneration())
 		op.Status.ResolvedBundleResource = ""
-		msg := prettyUnsatMessage(err)
-		setResolvedStatusConditionFailed(&op.Status.Conditions, msg, op.GetGeneration())
+		setResolvedStatusConditionFailed(&op.Status.Conditions, err.Error(), op.GetGeneration())
 		return ctrl.Result{}, err
 	}
 
@@ -456,30 +451,3 @@ func operatorRequestsForCatalog(c client.Reader, logger logr.Logger) handler.Map
 		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(err error) string {
-	unsat := deppy.NotSatisfiable{}
-	if !errors.As(err, &unsat) {
-		// This function is specifically for formatting deppy.NotSatisfiable.
-		// Just return default format if the error is something else.
-		return err.Error()
-	}
-
-	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, "; "))
-}

internal/controllers/operator_controller_test.go

@@ -1128,7 +1128,7 @@ func TestOperatorUpgrade(t *testing.T) {
 		assert.Equal(t, metav1.ConditionFalse, cond.Status)
 		assert.Equal(t, operatorsv1alpha1.ReasonResolutionFailed, cond.Reason)
 		assert.Contains(t, cond.Message, "constraints not satisfiable")
-		assert.Contains(t, cond.Message, "installed package prometheus requires at least one of fake-catalog-prometheus-operatorhub/prometheus/beta/1.2.0, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.1, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.0;")
+		assert.Regexp(t, "installed package prometheus requires at least one of fake-catalog-prometheus-operatorhub/prometheus/beta/1.2.0, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.1, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.0$", cond.Message)
 
 		// Valid update skipping one version
 		operator.Spec.Version = "1.2.0"
@@ -1221,7 +1221,7 @@ func TestOperatorUpgrade(t *testing.T) {
 		assert.Equal(t, metav1.ConditionFalse, cond.Status)
 		assert.Equal(t, operatorsv1alpha1.ReasonResolutionFailed, cond.Reason)
 		assert.Contains(t, cond.Message, "constraints not satisfiable")
-		assert.Contains(t, cond.Message, "installed package prometheus requires at least one of fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.1, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.0;")
+		assert.Contains(t, cond.Message, "installed package prometheus requires at least one of fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.1, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.0\n")
 
 		// Valid update skipping one version
 		operator.Spec.Version = "1.0.1"
@@ -1418,7 +1418,7 @@ func TestOperatorDowngrade(t *testing.T) {
 				assert.Equal(t, metav1.ConditionFalse, cond.Status)
 				assert.Equal(t, operatorsv1alpha1.ReasonResolutionFailed, cond.Reason)
 				assert.Contains(t, cond.Message, "constraints not satisfiable")
-				assert.Contains(t, cond.Message, "installed package prometheus requires at least one of fake-catalog-prometheus-operatorhub/prometheus/beta/1.2.0, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.1;")
+				assert.Contains(t, cond.Message, "installed package prometheus requires at least one of fake-catalog-prometheus-operatorhub/prometheus/beta/1.2.0, fake-catalog-prometheus-operatorhub/prometheus/beta/1.0.1\n")
 			})
 		}
 	})

test/e2e/install_test.go

@@ -227,7 +227,7 @@ var _ = Describe("Operator Install", func() {
 					g.Expect(cond).ToNot(BeNil())
 					g.Expect(cond.Reason).To(Equal(operatorv1alpha1.ReasonResolutionFailed))
 					g.Expect(cond.Message).To(ContainSubstring("constraints not satisfiable"))
-					g.Expect(cond.Message).To(ContainSubstring("installed package prometheus requires at least one of test-catalog-prometheus-prometheus-operator.1.2.0, test-catalog-prometheus-prometheus-operator.1.0.1, test-catalog-prometheus-prometheus-operator.1.0.0;"))
+					g.Expect(cond.Message).To(MatchRegexp("installed package prometheus requires at least one of test-catalog-prometheus-prometheus-operator.1.2.0, test-catalog-prometheus-prometheus-operator.1.0.1, test-catalog-prometheus-prometheus-operator.1.0.0$"))
 					g.Expect(operator.Status.ResolvedBundleResource).To(BeEmpty())
 				}).Should(Succeed())
 			})