Skip to content

Commit 8afcb15

Browse files
tmshorttmshort
authored
Consolidate error message generation (#807)
Signed-off-by: Todd Short <[email protected]>
1 parent 9464455 commit 8afcb15

File tree

5 files changed

+55
-35
lines changed

5 files changed

+55
-35
lines changed

‎internal/controllers/clusterextension_controller.go

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ import (
6666
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
6767
catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
6868
catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
69+
"github.com/operator-framework/operator-controller/internal/packageerrors"
6970
rukpakapi "github.com/operator-framework/operator-controller/internal/rukpak/api"
7071
"github.com/operator-framework/operator-controller/internal/rukpak/handler"
7172
helmpredicate "github.com/operator-framework/operator-controller/internal/rukpak/helm-operator-plugins/predicate"
@@ -610,17 +611,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, clusterExtensi
610611
resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...))
611612

612613
if len(resultSet) == 0 {
613-
var versionError, channelError, existingVersionError string
614-
if versionRange != "" {
615-
versionError = fmt.Sprintf(" matching version %q", versionRange)
616-
}
617-
if channelName != "" {
618-
channelError = fmt.Sprintf(" in channel %q", channelName)
619-
}
620-
if installedVersion != "" {
621-
existingVersionError = fmt.Sprintf(" which upgrades currently installed version %q", installedVersion)
622-
}
623-
return nil, fmt.Errorf("no package %q%s%s%s found", packageName, versionError, channelError, existingVersionError)
614+
return nil, packageerrors.GenerateFullError(packageName, versionRange, channelName, installedVersion)
624615
}
625616

626617
sort.SliceStable(resultSet, func(i, j int) bool {

‎internal/controllers/clusterextension_controller_test.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ import (
2929
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
3030
"github.com/operator-framework/operator-controller/internal/conditionsets"
3131
"github.com/operator-framework/operator-controller/internal/controllers"
32+
"github.com/operator-framework/operator-controller/internal/packageerrors"
3233
"github.com/operator-framework/operator-controller/pkg/features"
3334
)
3435

@@ -61,7 +62,7 @@ func TestClusterExtensionNonExistentPackage(t *testing.T) {
6162
t.Log("By running reconcile")
6263
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
6364
require.Equal(t, ctrl.Result{}, res)
64-
require.EqualError(t, err, fmt.Sprintf("no package %q found", pkgName))
65+
require.EqualError(t, err, packageerrors.GenerateError(pkgName).Error())
6566

6667
t.Log("By fetching updated cluster extension after reconcile")
6768
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
@@ -75,7 +76,7 @@ func TestClusterExtensionNonExistentPackage(t *testing.T) {
7576
require.NotNil(t, cond)
7677
require.Equal(t, metav1.ConditionFalse, cond.Status)
7778
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
78-
require.Equal(t, fmt.Sprintf("no package %q found", pkgName), cond.Message)
79+
require.Equal(t, packageerrors.GenerateError(pkgName).Error(), cond.Message)
7980

8081
verifyInvariants(ctx, t, reconciler.Client, clusterExtension)
8182
require.NoError(t, cl.DeleteAllOf(ctx, &ocv1alpha1.ClusterExtension{}))
@@ -103,7 +104,7 @@ func TestClusterExtensionNonExistentVersion(t *testing.T) {
103104
t.Log("By running reconcile")
104105
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
105106
require.Equal(t, ctrl.Result{}, res)
106-
require.EqualError(t, err, fmt.Sprintf(`no package %q matching version "0.50.0" found`, pkgName))
107+
require.EqualError(t, err, packageerrors.GenerateVersionError(pkgName, "0.50.0").Error())
107108

108109
t.Log("By fetching updated cluster extension after reconcile")
109110
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
@@ -117,7 +118,7 @@ func TestClusterExtensionNonExistentVersion(t *testing.T) {
117118
require.NotNil(t, cond)
118119
require.Equal(t, metav1.ConditionFalse, cond.Status)
119120
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
120-
require.Equal(t, fmt.Sprintf(`no package %q matching version "0.50.0" found`, pkgName), cond.Message)
121+
require.Equal(t, packageerrors.GenerateVersionError(pkgName, "0.50.0").Error(), cond.Message)
121122
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
122123
require.NotNil(t, cond)
123124
require.Equal(t, metav1.ConditionUnknown, cond.Status)
@@ -799,7 +800,7 @@ func TestClusterExtensionVersionNoChannel(t *testing.T) {
799800
t.Log("By running reconcile")
800801
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
801802
require.Equal(t, ctrl.Result{}, res)
802-
require.EqualError(t, err, fmt.Sprintf("no package %q matching version %q found in channel %q", pkgName, pkgVer, pkgChan))
803+
require.EqualError(t, err, packageerrors.GenerateVersionChannelError(pkgName, pkgVer, pkgChan).Error())
803804

804805
t.Log("By fetching updated cluster extension after reconcile")
805806
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
@@ -813,7 +814,7 @@ func TestClusterExtensionVersionNoChannel(t *testing.T) {
813814
require.NotNil(t, cond)
814815
require.Equal(t, metav1.ConditionFalse, cond.Status)
815816
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
816-
require.Equal(t, fmt.Sprintf("no package %q matching version %q found in channel %q", pkgName, pkgVer, pkgChan), cond.Message)
817+
require.Equal(t, packageerrors.GenerateVersionChannelError(pkgName, pkgVer, pkgChan).Error(), cond.Message)
817818
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
818819

819820
require.NotNil(t, cond)
@@ -848,7 +849,7 @@ func TestClusterExtensionNoChannel(t *testing.T) {
848849
t.Log("By running reconcile")
849850
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
850851
require.Equal(t, ctrl.Result{}, res)
851-
require.EqualError(t, err, fmt.Sprintf("no package %q found in channel %q", pkgName, pkgChan))
852+
require.EqualError(t, err, packageerrors.GenerateChannelError(pkgName, pkgChan).Error())
852853

853854
t.Log("By fetching updated cluster extension after reconcile")
854855
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
@@ -862,7 +863,7 @@ func TestClusterExtensionNoChannel(t *testing.T) {
862863
require.NotNil(t, cond)
863864
require.Equal(t, metav1.ConditionFalse, cond.Status)
864865
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
865-
require.Equal(t, fmt.Sprintf("no package %q found in channel %q", pkgName, pkgChan), cond.Message)
866+
require.Equal(t, packageerrors.GenerateChannelError(pkgName, pkgChan).Error(), cond.Message)
866867
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
867868
require.NotNil(t, cond)
868869
require.Equal(t, metav1.ConditionUnknown, cond.Status)
@@ -898,7 +899,7 @@ func TestClusterExtensionNoVersion(t *testing.T) {
898899
t.Log("By running reconcile")
899900
res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey})
900901
require.Equal(t, ctrl.Result{}, res)
901-
require.EqualError(t, err, fmt.Sprintf("no package %q matching version %q found in channel %q", pkgName, pkgVer, pkgChan))
902+
require.EqualError(t, err, packageerrors.GenerateVersionChannelError(pkgName, pkgVer, pkgChan).Error())
902903

903904
t.Log("By fetching updated cluster extension after reconcile")
904905
require.NoError(t, cl.Get(ctx, extKey, clusterExtension))
@@ -912,7 +913,7 @@ func TestClusterExtensionNoVersion(t *testing.T) {
912913
require.NotNil(t, cond)
913914
require.Equal(t, metav1.ConditionFalse, cond.Status)
914915
require.Equal(t, ocv1alpha1.ReasonResolutionFailed, cond.Reason)
915-
require.Equal(t, fmt.Sprintf("no package %q matching version %q found in channel %q", pkgName, pkgVer, pkgChan), cond.Message)
916+
require.Equal(t, packageerrors.GenerateVersionChannelError(pkgName, pkgVer, pkgChan).Error(), cond.Message)
916917
cond = apimeta.FindStatusCondition(clusterExtension.Status.Conditions, ocv1alpha1.TypeInstalled)
917918
require.NotNil(t, cond)
918919
require.Equal(t, metav1.ConditionUnknown, cond.Status)

‎internal/packageerrors/packageerrors.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package packageerrors
2+
3+
import (
4+
"fmt"
5+
)
6+
7+
func GenerateError(packageName string) error {
8+
return GenerateFullError(packageName, "", "", "")
9+
}
10+
11+
func GenerateVersionError(packageName, versionRange string) error {
12+
return GenerateFullError(packageName, versionRange, "", "")
13+
}
14+
15+
func GenerateChannelError(packageName, channelName string) error {
16+
return GenerateFullError(packageName, "", channelName, "")
17+
}
18+
19+
func GenerateVersionChannelError(packageName, versionRange, channelName string) error {
20+
return GenerateFullError(packageName, versionRange, channelName, "")
21+
}
22+
23+
func GenerateFullError(packageName, versionRange, channelName, installedVersion string) error {
24+
var versionError, channelError, existingVersionError string
25+
if versionRange != "" {
26+
versionError = fmt.Sprintf(" matching version %q", versionRange)
27+
}
28+
if channelName != "" {
29+
channelError = fmt.Sprintf(" in channel %q", channelName)
30+
}
31+
if installedVersion != "" {
32+
existingVersionError = fmt.Sprintf(" which upgrades currently installed version %q", installedVersion)
33+
}
34+
return fmt.Errorf("no package %q%s%s%s found", packageName, versionError, channelError, existingVersionError)
35+
}

‎internal/resolution/variablesources/required_package.go

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
1111
catalogfilter "github.com/operator-framework/operator-controller/internal/catalogmetadata/filter"
1212
catalogsort "github.com/operator-framework/operator-controller/internal/catalogmetadata/sort"
13+
"github.com/operator-framework/operator-controller/internal/packageerrors"
1314
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
1415
)
1516

@@ -42,16 +43,7 @@ func MakeRequiredPackageVariables(allBundles []*catalogmetadata.Bundle, clusterE
4243

4344
resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...))
4445
if len(resultSet) == 0 {
45-
if versionRange != "" && channelName != "" {
46-
return nil, fmt.Errorf("no package %q matching version %q found in channel %q", packageName, versionRange, channelName)
47-
}
48-
if versionRange != "" {
49-
return nil, fmt.Errorf("no package %q matching version %q found", packageName, versionRange)
50-
}
51-
if channelName != "" {
52-
return nil, fmt.Errorf("no package %q found in channel %q", packageName, channelName)
53-
}
54-
return nil, fmt.Errorf("no package %q found", packageName)
46+
return nil, packageerrors.GenerateVersionChannelError(packageName, versionRange, channelName)
5547
}
5648
sort.SliceStable(resultSet, func(i, j int) bool {
5749
return catalogsort.ByVersion(resultSet[i], resultSet[j])

‎internal/resolution/variablesources/required_package_test.go

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818

1919
ocv1alpha1 "github.com/operator-framework/operator-controller/api/v1alpha1"
2020
"github.com/operator-framework/operator-controller/internal/catalogmetadata"
21+
"github.com/operator-framework/operator-controller/internal/packageerrors"
2122
olmvariables "github.com/operator-framework/operator-controller/internal/resolution/variables"
2223
"github.com/operator-framework/operator-controller/internal/resolution/variablesources"
2324
)
@@ -208,28 +209,28 @@ func TestMakeRequiredPackageVariables(t *testing.T) {
208209
clusterExtensions: []ocv1alpha1.ClusterExtension{
209210
fakeClusterExtension("non-existent-test-package", "", ""),
210211
},
211-
expectedError: `no package "non-existent-test-package" found`,
212+
expectedError: packageerrors.GenerateError("non-existent-test-package").Error(),
212213
},
213214
{
214215
name: "not found: package name and channel",
215216
clusterExtensions: []ocv1alpha1.ClusterExtension{
216217
fakeClusterExtension("non-existent-test-package", "stable", ""),
217218
},
218-
expectedError: `no package "non-existent-test-package" found in channel "stable"`,
219+
expectedError: packageerrors.GenerateChannelError("non-existent-test-package", "stable").Error(),
219220
},
220221
{
221222
name: "not found: package name and version range",
222223
clusterExtensions: []ocv1alpha1.ClusterExtension{
223224
fakeClusterExtension("non-existent-test-package", "", "1.0.0"),
224225
},
225-
expectedError: `no package "non-existent-test-package" matching version "1.0.0" found`,
226+
expectedError: packageerrors.GenerateVersionError("non-existent-test-package", "1.0.0").Error(),
226227
},
227228
{
228229
name: "not found: package name with channel and version range",
229230
clusterExtensions: []ocv1alpha1.ClusterExtension{
230231
fakeClusterExtension("non-existent-test-package", "stable", "1.0.0"),
231232
},
232-
expectedError: `no package "non-existent-test-package" matching version "1.0.0" found in channel "stable"`,
233+
expectedError: packageerrors.GenerateVersionChannelError("non-existent-test-package", "1.0.0", "stable").Error(),
233234
},
234235
} {
235236
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)