diff --git a/internal/operator-controller/resolve/catalog.go b/internal/operator-controller/resolve/catalog.go index f0d4da6fab..5e830367b2 100644 --- a/internal/operator-controller/resolve/catalog.go +++ b/internal/operator-controller/resolve/catalog.go @@ -67,6 +67,13 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio } } + // Collect successor information BEFORE the main resolution walk + // This is used for better error messages if resolution fails + var bestSuccessors []BundleRef + if installedBundle != nil && ext.Spec.Source.Catalog.UpgradeConstraintPolicy != ocv1.UpgradeConstraintPolicySelfCertified { + bestSuccessors, _ = r.GetBestSuccessors(ctx, ext, installedBundle) + } + type catStat struct { CatalogName string `json:"catalogName"` PackageFound bool `json:"packageFound"` @@ -181,12 +188,14 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio // Check for ambiguity if len(resolvedBundles) != 1 { l.Info("resolution failed", "stats", catStats) + return nil, nil, nil, resolutionError{ PackageName: packageName, Version: versionRange, Channels: channels, InstalledBundle: installedBundle, ResolvedBundles: resolvedBundles, + BestSuccessors: bestSuccessors, } } resolvedBundle := resolvedBundles[0].bundle @@ -209,12 +218,70 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio return resolvedBundle, resolvedBundleVersion, priorDeprecation, nil } +// GetBestSuccessors returns the best available successor bundles ignoring version range and channel filters. +// This provides helpful information when resolution fails. +func (r *CatalogResolver) GetBestSuccessors(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) ([]BundleRef, error) { + if installedBundle == nil { + return nil, nil + } + + packageName := ext.Spec.Source.Catalog.PackageName + + // unless overridden, default to selecting all bundles + var selector = labels.Everything() + var err error + if ext.Spec.Source.Catalog != nil { + selector, err = metav1.LabelSelectorAsSelector(ext.Spec.Source.Catalog.Selector) + if err != nil { + return nil, fmt.Errorf("desired catalog selector is invalid: %w", err) + } + if selector == labels.Nothing() { + selector = labels.Everything() + } + } + + var allSuccessors []BundleRef + + listOptions := []client.ListOption{ + client.MatchingLabelsSelector{Selector: selector}, + } + + if err := r.WalkCatalogsFunc(ctx, packageName, func(ctx context.Context, cat *ocv1.ClusterCatalog, packageFBC *declcfg.DeclarativeConfig, err error) error { + if err != nil || isFBCEmpty(packageFBC) { + return nil + } + + // Only apply successor filter, no version or channel filters + successorPredicate, err := filter.SuccessorsOf(*installedBundle, packageFBC.Channels...) + if err != nil { + return nil // Skip on error + } + + successorBundles := slices.Clone(packageFBC.Bundles) + successorBundles = filterutil.InPlace(successorBundles, successorPredicate) + + for i := range successorBundles { + allSuccessors = append(allSuccessors, BundleRef{ + Bundle: &successorBundles[i], + Catalog: cat.GetName(), + Priority: cat.Spec.Priority, + }) + } + return nil + }, listOptions...); err != nil { + return nil, err + } + + return allSuccessors, nil +} + type resolutionError struct { PackageName string Version string Channels []string InstalledBundle *ocv1.BundleMetadata ResolvedBundles []foundBundle + BestSuccessors []BundleRef // Best available successors (for better error messages) } func (rei resolutionError) Error() string { @@ -223,6 +290,23 @@ func (rei resolutionError) Error() string { sb.WriteString(fmt.Sprintf("error upgrading from currently installed version %q: ", rei.InstalledBundle.Version)) } + // Check if we have successor information for better error messages + if len(rei.ResolvedBundles) == 0 && rei.InstalledBundle != nil && len(rei.BestSuccessors) > 0 { + // We have successors available, so the version range doesn't match any successor + if rei.Version != "" { + sb.WriteString(fmt.Sprintf("desired package %q with version range %q does not match any successor of %q", rei.PackageName, rei.Version, rei.InstalledBundle.Version)) + } else { + sb.WriteString(fmt.Sprintf("no successor of %q found for package %q", rei.InstalledBundle.Version, rei.PackageName)) + } + + // Add best successor suggestions + suggestion := findBestSuccessors(rei.BestSuccessors, rei.InstalledBundle) + if suggestion != "" { + sb.WriteString(fmt.Sprintf(". %s", suggestion)) + } + return strings.TrimSpace(sb.String()) + } + if len(rei.ResolvedBundles) > 1 { sb.WriteString(fmt.Sprintf("found bundles for package %q ", rei.PackageName)) } else { @@ -249,6 +333,73 @@ func (rei resolutionError) Error() string { return strings.TrimSpace(sb.String()) } +// findBestSuccessors finds the highest version successors in different streams +func findBestSuccessors(successors []BundleRef, installedBundle *ocv1.BundleMetadata) string { + if len(successors) == 0 { + return "" + } + + // Parse installed version + installedVer, err := bsemver.Parse(installedBundle.Version) + if err != nil { + return "" + } + + var zStreamHighest *declcfg.Bundle + var yStreamHighest *declcfg.Bundle + + for _, bundleRef := range successors { + bundleVer, err := bundleutil.GetVersionAndRelease(*bundleRef.Bundle) + if err != nil { + continue + } + + // Skip the currently installed version itself + if bundleVer.Version.EQ(installedVer) { + continue + } + + // Z-stream: same major.minor, different patch + if bundleVer.Version.Major == installedVer.Major && bundleVer.Version.Minor == installedVer.Minor { + if zStreamHighest == nil { + zStreamHighest = bundleRef.Bundle + } else { + currentHighest, _ := bundleutil.GetVersionAndRelease(*zStreamHighest) + if bundleVer.Compare(*currentHighest) > 0 { + zStreamHighest = bundleRef.Bundle + } + } + } + + // Y-stream: same major, different minor + if bundleVer.Version.Major == installedVer.Major && bundleVer.Version.Minor != installedVer.Minor { + if yStreamHighest == nil { + yStreamHighest = bundleRef.Bundle + } else { + currentHighest, _ := bundleutil.GetVersionAndRelease(*yStreamHighest) + if bundleVer.Compare(*currentHighest) > 0 { + yStreamHighest = bundleRef.Bundle + } + } + } + } + + var suggestions []string + if yStreamHighest != nil { + yVer, _ := bundleutil.GetVersionAndRelease(*yStreamHighest) + suggestions = append(suggestions, fmt.Sprintf("%q (y-stream)", yVer.AsLegacyRegistryV1Version().String())) + } + if zStreamHighest != nil { + zVer, _ := bundleutil.GetVersionAndRelease(*zStreamHighest) + suggestions = append(suggestions, fmt.Sprintf("%q (z-stream)", zVer.AsLegacyRegistryV1Version().String())) + } + + if len(suggestions) > 0 { + return fmt.Sprintf("Highest version successors of %q are %s", installedBundle.Version, strings.Join(suggestions, " and ")) + } + return "" +} + func isDeprecated(bundle declcfg.Bundle, deprecation *declcfg.Deprecation) bool { if deprecation == nil { return false diff --git a/internal/operator-controller/resolve/catalog_test.go b/internal/operator-controller/resolve/catalog_test.go index 2ec3192b69..bd1f1c70c0 100644 --- a/internal/operator-controller/resolve/catalog_test.go +++ b/internal/operator-controller/resolve/catalog_test.go @@ -471,7 +471,8 @@ func TestUpgradeNotFoundLegacy(t *testing.T) { } // 0.1.0 only upgrades to 1.0.x with its legacy upgrade edges, so this fails. _, _, _, err := r.Resolve(context.Background(), ce, installedBundle) - assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "0.1.0": no bundles found for package %q matching version "<1.0.0 >=2.0.0"`, pkgName)) + // The new error message indicates that the version range doesn't match any successor + assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "0.1.0": desired package %q with version range "<1.0.0 >=2.0.0" does not match any successor of "0.1.0"`, pkgName)) } func TestDowngradeFound(t *testing.T) { @@ -526,6 +527,50 @@ func TestDowngradeNotFound(t *testing.T) { assert.EqualError(t, err, fmt.Sprintf(`error upgrading from currently installed version "1.0.2": no bundles found for package %q matching version ">0.1.0 <1.0.0"`, pkgName)) } +func TestIssue1022DowngradeNotSuccessor(t *testing.T) { + pkgName := randPkg() + + // Create a package similar to cockroachdb with versions 6.0.0, 6.0.1, etc. + fbc := &declcfg.DeclarativeConfig{ + Packages: []declcfg.Package{{Name: pkgName}}, + Channels: []declcfg.Channel{ + {Package: pkgName, Name: "stable", Entries: []declcfg.ChannelEntry{ + {Name: bundleName(pkgName, "6.0.0")}, + {Name: bundleName(pkgName, "6.0.1"), Replaces: bundleName(pkgName, "6.0.0")}, + {Name: bundleName(pkgName, "6.0.10"), Replaces: bundleName(pkgName, "6.0.1")}, + {Name: bundleName(pkgName, "6.3.11"), SkipRange: ">=6.0.0 <6.3.11"}, + }}, + }, + Bundles: []declcfg.Bundle{ + genBundle(pkgName, "6.0.0"), + genBundle(pkgName, "6.0.1"), + genBundle(pkgName, "6.0.10"), + genBundle(pkgName, "6.3.11"), + }, + } + + w := staticCatalogWalker{ + "catalog": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { + return fbc, nil, nil + }, + } + + r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + ce := buildFooClusterExtension(pkgName, []string{}, "6.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) + installedBundle := &ocv1.BundleMetadata{ + Name: bundleName(pkgName, "6.0.1"), + Version: "6.0.1", + } + + // Try to downgrade to 6.0.0, which exists but is not a successor + _, _, _, err := r.Resolve(context.Background(), ce, installedBundle) + + // The new error message should be more helpful + expectedMsg := fmt.Sprintf(`error upgrading from currently installed version "6.0.1": desired package %q with version range "6.0.0" does not match any successor of "6.0.1". Highest version successors of "6.0.1" are "6.3.11" (y-stream) and "6.0.10" (z-stream)`, pkgName) + assert.EqualError(t, err, expectedMsg) +} + + func TestCatalogWalker(t *testing.T) { t.Run("error listing catalogs", func(t *testing.T) { w := CatalogWalker( diff --git a/internal/operator-controller/resolve/provider.go b/internal/operator-controller/resolve/provider.go new file mode 100644 index 0000000000..fcb6d860a9 --- /dev/null +++ b/internal/operator-controller/resolve/provider.go @@ -0,0 +1,43 @@ +package resolve + +import ( + "context" + + "github.com/operator-framework/operator-registry/alpha/declcfg" +) + +// PackageProvider defines an API for providing bundle and deprecation info +// from a catalog for a specific package without any filtering applied. +type PackageProvider interface { + // GetPackage returns the raw package data (bundles, channels, deprecations) + // for the specified package from catalogs matching the selector. + GetPackage(ctx context.Context, packageName string, selector CatalogSelector) (*PackageData, error) +} + +// CatalogSelector defines criteria for selecting catalogs +type CatalogSelector struct { + // LabelSelector filters catalogs by labels + LabelSelector string +} + +// PackageData contains unfiltered package information from one or more catalogs +type PackageData struct { + // CatalogPackages maps catalog name to its package data + CatalogPackages map[string]*CatalogPackage +} + +// CatalogPackage represents package data from a single catalog +type CatalogPackage struct { + Name string + Priority int32 + Bundles []declcfg.Bundle + Channels []declcfg.Channel + Deprecations []declcfg.Deprecation +} + +// BundleRef references a bundle with its source catalog +type BundleRef struct { + Bundle *declcfg.Bundle + Catalog string + Priority int32 +} diff --git a/internal/operator-controller/resolve/resolver.go b/internal/operator-controller/resolve/resolver.go index 1fbde0fdea..0eaa1cb96a 100644 --- a/internal/operator-controller/resolve/resolver.go +++ b/internal/operator-controller/resolve/resolver.go @@ -9,10 +9,20 @@ import ( "github.com/operator-framework/operator-controller/internal/operator-controller/bundle" ) +// Resolver defines the interface for resolving bundles based on ClusterExtension specs type Resolver interface { + // Resolve returns a Bundle from a catalog that needs to get installed on the cluster. Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) } +// SuccessorQuerier is an optional interface that resolvers can implement to provide +// information about available successors. This is useful for generating helpful error messages. +type SuccessorQuerier interface { + // GetBestSuccessors returns the best available successor bundles for the currently + // installed bundle, ignoring version range and channel filters. + GetBestSuccessors(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) ([]BundleRef, error) +} + type Func func(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) func (f Func) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *bundle.VersionRelease, *declcfg.Deprecation, error) {