Skip to content

Commit

Permalink
Remove prettyUnsatMessage
Browse files Browse the repository at this point in the history
It was a workaround a Deppy issue. We no longer need it
since Deppy fixed the issue.

Signed-off-by: Mikalai Radchuk <mradchuk@redhat.com>
  • Loading branch information
m1kola committed Nov 17, 2023
1 parent 8b2bfc0 commit 2419537
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 37 deletions.
34 changes: 1 addition & 33 deletions internal/controllers/operator_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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, "; "))
}
6 changes: 3 additions & 3 deletions internal/controllers/operator_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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")
})
}
})
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/install_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
})
Expand Down

0 comments on commit 2419537

Please sign in to comment.