Skip to content

Commit

Permalink
Back out packagerrors
Browse files Browse the repository at this point in the history 
It's agreed that this makes debugging and reasoning about log messages and runtime errors much harder.

Signed-off-by: Brett Tofel <btofel@redhat.com>
  • Loading branch information
@bentito
bentito committed May 16, 2024
1 parent 5263633 commit 570b89f
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 51 deletions.
10 changes: 5 additions & 5 deletions internal/controllers/clusterextension_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,6 @@ import (
"github.com/operator-framework/operator-controller/internal/conditionsets"
"github.com/operator-framework/operator-controller/internal/handler"
"github.com/operator-framework/operator-controller/internal/labels"
"github.com/operator-framework/operator-controller/internal/packageerrors"
)

// ClusterExtensionReconciler reconciles a ClusterExtension object
Expand Down Expand Up @@ -450,15 +449,16 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1
if len(resultSet) == 0 {
switch {
case versionRange != "" && channelName != "":
return nil, packageerrors.GenerateVersionChannelError(packageName, versionRange, channelName)
return nil, fmt.Errorf("no package %q matching version %q in channel %q found", packageName, versionRange, channelName)
case versionRange != "":
return nil, packageerrors.GenerateVersionError(packageName, versionRange)
return nil, fmt.Errorf("no package %q matching version %q found", packageName, versionRange)
case channelName != "":
return nil, packageerrors.GenerateChannelError(packageName, channelName)
return nil, fmt.Errorf("no package %q in channel %q found", packageName, channelName)
default:
return nil, packageerrors.GenerateError(packageName)
return nil, fmt.Errorf("no package %q found", packageName)
}
}

sort.SliceStable(resultSet, func(i, j int) bool {
return catalogsort.ByVersion(resultSet[i], resultSet[j])
})
Expand Down
21 changes: 10 additions & 11 deletions internal/controllers/clusterextension_controller_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
"github.com/operator-framework/operator-controller/internal/conditionsets"
"github.com/operator-framework/operator-controller/internal/controllers"
"github.com/operator-framework/operator-controller/internal/packageerrors"
"github.com/operator-framework/operator-controller/pkg/features"
)

Expand Down Expand Up @@ -62,7 +61,7 @@ func TestClusterExtensionNonExistentPackage(t *testing.T) {
t.Log("By running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
require.Equal(t, ctrl.Result{}, res)
require.EqualError(t, err, packageerrors.GenerateError(pkgName).Error())
require.EqualError(t, err, fmt.Errorf("no package \"%v\" found", pkgName).Error())

t.Log("By fetching updated cluster extension after reconcile")
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
Expand All @@ -76,7 +75,7 @@ func TestClusterExtensionNonExistentPackage(t *testing.T) {
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
require.Equal(t, packageerrors.GenerateError(pkgName).Error(), cond.Message)
require.Equal(t, fmt.Errorf("no package \"%v\" found", pkgName).Error(), cond.Message)

verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
Expand Down Expand Up @@ -104,7 +103,7 @@ func TestClusterExtensionNonExistentVersion(t *testing.T) {
t.Log("By running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
require.Equal(t, ctrl.Result{}, res)
require.EqualError(t, err, packageerrors.GenerateVersionError(pkgName, "0.50.0").Error())
require.EqualError(t, err, fmt.Errorf("no package \"%v\" matching version \"%v\" found", pkgName, "0.50.0").Error())

t.Log("By fetching updated cluster extension after reconcile")
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
Expand All @@ -118,7 +117,7 @@ func TestClusterExtensionNonExistentVersion(t *testing.T) {
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
require.Equal(t, packageerrors.GenerateVersionError(pkgName, "0.50.0").Error(), cond.Message)
require.Equal(t, fmt.Errorf("no package \"%v\" matching version \"%v\" found", pkgName, "0.50.0").Error(), cond.Message)
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
Expand Down Expand Up @@ -334,7 +333,7 @@ func TestClusterExtensionVersionNoChannel(t *testing.T) {
t.Log("By running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
require.Equal(t, ctrl.Result{}, res)
require.EqualError(t, err, packageerrors.GenerateVersionChannelError(pkgName, pkgVer, pkgChan).Error())
require.EqualError(t, err, fmt.Errorf("no package \"%v\" matching version \"%v\" in channel \"%v\" found", pkgName, pkgVer, pkgChan).Error())

t.Log("By fetching updated cluster extension after reconcile")
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
Expand All @@ -348,7 +347,7 @@ func TestClusterExtensionVersionNoChannel(t *testing.T) {
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
require.Equal(t, packageerrors.GenerateVersionChannelError(pkgName, pkgVer, pkgChan).Error(), cond.Message)
require.Equal(t, fmt.Errorf("no package \"%v\" matching version \"%v\" in channel \"%v\" found", pkgName, pkgVer, pkgChan).Error(), cond.Message)
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)

require.NotNil(t, cond)
Expand Down Expand Up @@ -382,7 +381,7 @@ func TestClusterExtensionNoChannel(t *testing.T) {
t.Log("By running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
require.Equal(t, ctrl.Result{}, res)
require.EqualError(t, err, packageerrors.GenerateChannelError(pkgName, pkgChan).Error())
require.EqualError(t, err, fmt.Errorf("no package \"%v\" in channel \"%v\" found", pkgName, pkgChan).Error())

t.Log("By fetching updated cluster extension after reconcile")
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
Expand All @@ -396,7 +395,7 @@ func TestClusterExtensionNoChannel(t *testing.T) {
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
require.Equal(t, packageerrors.GenerateChannelError(pkgName, pkgChan).Error(), cond.Message)
require.Equal(t, fmt.Errorf("no package \"%v\" in channel \"%v\" found", pkgName, pkgChan).Error(), cond.Message)
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
Expand Down Expand Up @@ -431,7 +430,7 @@ func TestClusterExtensionNoVersion(t *testing.T) {
t.Log("By running reconcile")
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
require.Equal(t, ctrl.Result{}, res)
require.EqualError(t, err, packageerrors.GenerateVersionChannelError(pkgName, pkgVer, pkgChan).Error())
require.EqualError(t, err, fmt.Errorf("no package \"%v\" matching version \"%v\" in channel \"%v\" found", pkgName, pkgVer, pkgChan).Error())

t.Log("By fetching updated cluster extension after reconcile")
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
Expand All @@ -445,7 +444,7 @@ func TestClusterExtensionNoVersion(t *testing.T) {
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
require.Equal(t, packageerrors.GenerateVersionChannelError(pkgName, pkgVer, pkgChan).Error(), cond.Message)
require.Equal(t, fmt.Errorf("no package \"%v\" matching version \"%v\" in channel \"%v\" found", pkgName, pkgVer, pkgChan).Error(), cond.Message)
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
require.NotNil(t, cond)
require.Equal(t, metav1.ConditionFalse, cond.Status)
Expand Down
35 changes: 0 additions & 35 deletions internal/packageerrors/packageerrors.go
View file

This file was deleted.

0 comments on commit 570b89f

Please sign in to comment.