From dc7f86eab8ae52f06fe8bd51fa9f9aceae481fd5 Mon Sep 17 00:00:00 2001 From: Jordan Keister Date: Fri, 14 Jun 2024 15:50:23 -0500 Subject: [PATCH] predicate error tracking sketch Signed-off-by: Jordan Keister --- .../filter/bundle_predicates.go | 72 +++++++++++++------ .../filter/bundle_predicates_test.go | 2 +- internal/catalogmetadata/filter/filter.go | 39 ++++++---- .../clusterextension_controller.go | 23 +++--- 4 files changed, 88 insertions(+), 48 deletions(-) diff --git a/internal/catalogmetadata/filter/bundle_predicates.go b/internal/catalogmetadata/filter/bundle_predicates.go index 8f16c6c5e..2ee4838c2 100644 --- a/internal/catalogmetadata/filter/bundle_predicates.go +++ b/internal/catalogmetadata/filter/bundle_predicates.go @@ -1,6 +1,8 @@ package filter import ( + "fmt" + mmsemver "github.com/Masterminds/semver/v3" bsemver "github.com/blang/semver/v4" @@ -11,16 +13,20 @@ import ( ) func WithPackageName(packageName string) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - return bundle.Package == packageName + return func(bundle *catalogmetadata.Bundle) (bool, []string) { + value := bundle.Package == packageName + if !value { + return false, []string{packageName} + } + return value, nil } } func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { + return func(bundle *catalogmetadata.Bundle) (bool, []string) { bVersion, err := bundle.Version() if err != nil { - return false + return false, []string{err.Error()} } // No error should occur here because the simple version was successfully parsed by blang // We are unaware of any tests cases that would cause one to fail but not the other @@ -28,42 +34,59 @@ func InMastermindsSemverRange(semverRange *mmsemver.Constraints) Predicate[catal // there might be that one extreme edge case that might cause one to fail but not the other mVersion, err := mmsemver.NewVersion(bVersion.String()) if err != nil { - return false + return false, []string{err.Error()} + } + res := semverRange.Check(mVersion) + if !res { + return false, []string{fmt.Sprintf("no package %q matching version %q found", bundle.Package, semverRange.String())} } - return semverRange.Check(mVersion) + return true, nil } } -func InBlangSemverRange(semverRange bsemver.Range) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { +func InBlangSemverRange(semverRange bsemver.Range, semverRangeString string) Predicate[catalogmetadata.Bundle] { + return func(bundle *catalogmetadata.Bundle) (bool, []string) { bundleVersion, err := bundle.Version() if err != nil { - return false + return false, []string{err.Error()} } - return semverRange(*bundleVersion) + if !semverRange(*bundleVersion) { + return false, []string{fmt.Sprintf("no package %q matching version %q found", bundle.Package, semverRangeString)} + } + return true, nil } } func InChannel(channelName string) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { + return func(bundle *catalogmetadata.Bundle) (bool, []string) { for _, ch := range bundle.InChannels { if ch.Name == channelName { - return true + return true, nil } } - return false + return false, []string{fmt.Sprintf("no package %q found in channel %q", bundle.Package, channelName)} } } func WithBundleImage(bundleImage string) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - return bundle.Image == bundleImage + return func(bundle *catalogmetadata.Bundle) (bool, []string) { + res := bundle.Image == bundleImage + if !res { + return false, []string{fmt.Sprintf("no matching bundle image %q found for package %s", bundleImage, bundle.Package)} + } else { + return true, nil + } } } func WithBundleName(bundleName string) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - return bundle.Name == bundleName + return func(bundle *catalogmetadata.Bundle) (bool, []string) { + res := bundle.Name == bundleName + if !res { + return false, []string{fmt.Sprintf("no matching bundle name %q found for package %s", bundleName, bundle.Package)} + } else { + return true, nil + } } } @@ -87,20 +110,25 @@ func LegacySuccessor(installedBundle *ocv1alpha1.BundleMetadata) Predicate[catal return false } - return func(candidateBundle *catalogmetadata.Bundle) bool { + return func(candidateBundle *catalogmetadata.Bundle) (bool, []string) { for _, ch := range candidateBundle.InChannels { for _, chEntry := range ch.Entries { if candidateBundle.Name == chEntry.Name && isSuccessor(chEntry) { - return true + return true, nil } } } - return false + return false, []string{fmt.Sprintf("no legacy successor found for bundle name %q", candidateBundle.Name)} } } func WithDeprecation(deprecated bool) Predicate[catalogmetadata.Bundle] { - return func(bundle *catalogmetadata.Bundle) bool { - return bundle.HasDeprecation() == deprecated + return func(bundle *catalogmetadata.Bundle) (bool, []string) { + res := bundle.HasDeprecation() == deprecated + if !res { + return false, []string{fmt.Sprintf("no bundle %q found with desired deprecation status %t", bundle.Name, deprecated)} + } else { + return true, nil + } } } diff --git a/internal/catalogmetadata/filter/bundle_predicates_test.go b/internal/catalogmetadata/filter/bundle_predicates_test.go index 51bb5746a..6b743306a 100644 --- a/internal/catalogmetadata/filter/bundle_predicates_test.go +++ b/internal/catalogmetadata/filter/bundle_predicates_test.go @@ -92,7 +92,7 @@ func TestInBlangSemverRange(t *testing.T) { vRange := bsemver.MustParseRange(">=1.0.0") - f := filter.InBlangSemverRange(vRange) + f := filter.InBlangSemverRange(vRange, ">=1.0.0") assert.True(t, f(b1)) assert.False(t, f(b2)) diff --git a/internal/catalogmetadata/filter/filter.go b/internal/catalogmetadata/filter/filter.go index 36af20661..935011abe 100644 --- a/internal/catalogmetadata/filter/filter.go +++ b/internal/catalogmetadata/filter/filter.go @@ -5,43 +5,54 @@ import ( ) // Predicate returns true if the object should be kept when filtering -type Predicate[T catalogmetadata.Schemas] func(entity *T) bool +type Predicate[T catalogmetadata.Schemas] func(entity *T) (bool, []string) // Filter filters a slice accordingly to -func Filter[T catalogmetadata.Schemas](in []*T, test Predicate[T]) []*T { +func Filter[T catalogmetadata.Schemas](in []*T, test Predicate[T]) ([]*T, []string) { out := []*T{} + var errs []string for i := range in { - if test(in[i]) { + res, e := test(in[i]) + if res { out = append(out, in[i]) + } else { + errs = append(errs, e...) } } - return out + return out, errs } func And[T catalogmetadata.Schemas](predicates ...Predicate[T]) Predicate[T] { - return func(obj *T) bool { + return func(obj *T) (bool, []string) { for _, predicate := range predicates { - if !predicate(obj) { - return false + if res, errs := predicate(obj); !res { + return false, errs } } - return true + return true, nil } } func Or[T catalogmetadata.Schemas](predicates ...Predicate[T]) Predicate[T] { - return func(obj *T) bool { + return func(obj *T) (bool, []string) { + var errs []string for _, predicate := range predicates { - if predicate(obj) { - return true + if res, e := predicate(obj); !res { + errs = append(errs, e...) + } else { + return true, nil } } - return false + return false, errs } } func Not[T catalogmetadata.Schemas](predicate Predicate[T]) Predicate[T] { - return func(obj *T) bool { - return !predicate(obj) + return func(obj *T) (bool, []string) { + predicateTrue, errs := predicate(obj) + if predicateTrue { + return false, errs + } + return true, nil } } diff --git a/internal/controllers/clusterextension_controller.go b/internal/controllers/clusterextension_controller.go index 9a791fef5..bf49c6572 100644 --- a/internal/controllers/clusterextension_controller.go +++ b/internal/controllers/clusterextension_controller.go @@ -402,7 +402,7 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1 predicates = append(predicates, upgradePredicate) } - resultSet := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...)) + resultSet, errs := catalogfilter.Filter(allBundles, catalogfilter.And(predicates...)) var upgradeErrorPrefix string if installedBundle != nil { @@ -413,16 +413,17 @@ func (r *ClusterExtensionReconciler) resolve(ctx context.Context, ext ocv1alpha1 upgradeErrorPrefix = fmt.Sprintf("error upgrading from currently installed version %q: ", installedBundleVersion.String()) } if len(resultSet) == 0 { - switch { - case versionRange != "" && channelName != "": - return nil, fmt.Errorf("%sno package %q matching version %q in channel %q found", upgradeErrorPrefix, packageName, versionRange, channelName) - case versionRange != "": - return nil, fmt.Errorf("%sno package %q matching version %q found", upgradeErrorPrefix, packageName, versionRange) - case channelName != "": - return nil, fmt.Errorf("%sno package %q in channel %q found", upgradeErrorPrefix, packageName, channelName) - default: - return nil, fmt.Errorf("%sno package %q found", upgradeErrorPrefix, packageName) - } + return nil, fmt.Errorf("%s %s", upgradeErrorPrefix, errs) + // switch { + // case versionRange != "" && channelName != "": + // return nil, fmt.Errorf("%sno package %q matching version %q in channel %q found", upgradeErrorPrefix, packageName, versionRange, channelName) + // case versionRange != "": + // return nil, fmt.Errorf("%sno package %q matching version %q found", upgradeErrorPrefix, packageName, versionRange) + // case channelName != "": + // return nil, fmt.Errorf("%sno package %q in channel %q found", upgradeErrorPrefix, packageName, channelName) + // default: + // return nil, fmt.Errorf("%sno package %q found", upgradeErrorPrefix, packageName) + // } } sort.SliceStable(resultSet, func(i, j int) bool {