diff --git a/cmd/operator-controller/main.go b/cmd/operator-controller/main.go index 0192d06579..a77aa20ee5 100644 --- a/cmd/operator-controller/main.go +++ b/cmd/operator-controller/main.go @@ -436,16 +436,14 @@ func run() error { }) resolver := &resolve.CatalogResolver{ - WalkCatalogsFunc: resolve.CatalogWalker( - func(ctx context.Context, option ...client.ListOption) ([]ocv1.ClusterCatalog, error) { - var catalogs ocv1.ClusterCatalogList - if err := cl.List(ctx, &catalogs, option...); err != nil { - return nil, err - } - return catalogs.Items, nil - }, - catalogClient.GetPackage, - ), + ListCatalogsFunc: func(ctx context.Context, option ...client.ListOption) ([]ocv1.ClusterCatalog, error) { + var catalogs ocv1.ClusterCatalogList + if err := cl.List(ctx, &catalogs, option...); err != nil { + return nil, err + } + return catalogs.Items, nil + }, + GetPackageFunc: catalogClient.GetPackage, Validations: []resolve.ValidationFunc{ resolve.NoDependencyValidation, }, diff --git a/docs/concepts/controlling-catalog-selection.md b/docs/concepts/controlling-catalog-selection.md index e4d77ae7d5..f75977cc37 100644 --- a/docs/concepts/controlling-catalog-selection.md +++ b/docs/concepts/controlling-catalog-selection.md @@ -133,7 +133,7 @@ This selects catalogs that do not have the `example.com/support` label. ### Setting Catalog Priority -When multiple catalogs provide the same package, you can set priorities to resolve ambiguities. Higher priority catalogs are preferred. +When multiple available catalogs selected by a `ClusterExtension` provide the same package, you can set priorities to choose which catalog supplies it. Higher priority catalogs are considered first. #### Defining Catalog Priority @@ -152,23 +152,26 @@ spec: ref: quay.io/example/high-priority-content-management:latest ``` -Catalogs have a default priority of `0`. The priority can be any 32-bit integer. Catalogs with higher priority values are preferred during bundle resolution. +Catalogs have a default priority of `0`. The priority can be any 32-bit integer. Catalogs with higher priority values are considered first when selecting a catalog for a package. #### How Priority Resolves Ambiguity -When multiple bundles match your criteria: +Catalogs are grouped by priority, from highest to lowest. For each group: -1. **Bundles from catalogs with higher priority are selected.** -2. **If multiple bundles are from catalogs with the same highest priority, and there is still ambiguity, an error is generated.** -3. **Deprecated bundles are deprioritized.** If non-deprecated bundles are available, deprecated ones are ignored. +1. If no catalog in the group contains the requested package, consider the next lower-priority group. +2. If exactly one catalog contains the package, select that catalog. Only its bundles are filtered by the requested version, channels, and upgrade constraints. If none qualifies, resolution fails rather than switching to a lower-priority catalog. +3. If multiple catalogs in the group contain the package, resolution fails due to ambiguity, even if only one has a bundle matching the requested criteria. + +Within the selected catalog, non-deprecated bundles are preferred over deprecated bundles, then the highest eligible version and release is chosen. Deprecation in another catalog does not affect catalog selection. ### Handling Ambiguity Errors -If the system cannot resolve to a single bundle due to ambiguity, it will generate an error. You can resolve this by: +If multiple catalogs with the same priority contain the package, resolution fails. You can resolve this by: + +- **Selecting only one catalog by name or label.** +- **Assigning different catalog priorities.** -- **Refining your catalog selection criteria.** -- **Adjusting catalog priorities.** -- **Ensuring that only one bundle matches your package name and version requirements.** +Changing the requested version or channel does not resolve package-level catalog ambiguity. If the selected catalog has no matching bundle, adjust the version, channel, or upgrade constraints, or explicitly select another catalog. ## End to End Example @@ -195,7 +198,7 @@ If the system cannot resolve to a single bundle due to ambiguity, it will genera metadata: name: catalog-b labels: - example.com/support: "false" + example.com/support: "true" spec: priority: 500 source: @@ -241,13 +244,13 @@ If the system cannot resolve to a single bundle due to ambiguity, it will genera kubectl get clusterextension install-my-operator -o yaml ``` - The status should indicate that the bundle was resolved from `catalog-a` due to the higher priority and matching label. + Both catalogs match the selector. If both contain `my-operator`, the higher-priority `catalog-a` is selected. Check `.status.install.bundle.name` and `.status.install.bundle.version` against the bundle content published in `catalog-a`. The `ClusterExtension` status does not directly report the name of the catalog used for resolution. ## Important Notes -- **Default Behavior**: If you do not specify any catalog selection criteria, the system may select any available catalog that provides the requested package, and the choice is undefined. +- **Default Behavior**: Without a selector, every available catalog is considered. The package is resolved from the highest-priority catalog that contains it; if multiple catalogs at that priority contain it, resolution fails due to ambiguity. - **Logical AND of Selectors**: When using both `matchLabels` and `matchExpressions`, catalogs must satisfy all criteria. -- **Deprecation Status**: Non-deprecated bundles are preferred over deprecated ones during resolution. +- **Deprecation Status**: Within the selected catalog, non-deprecated bundles are preferred over deprecated ones. Deprecation conditions reflect that catalog's package metadata even if no bundle matches the requested criteria. - **Error Messages**: The system will update the `.status.conditions` of the `ClusterExtension` with meaningful messages if resolution fails due to ambiguity or no catalogs being selected. ## References diff --git a/internal/operator-controller/controllers/clusterextension_controller_test.go b/internal/operator-controller/controllers/clusterextension_controller_test.go index 8275b4f828..eeaf5261fe 100644 --- a/internal/operator-controller/controllers/clusterextension_controller_test.go +++ b/internal/operator-controller/controllers/clusterextension_controller_test.go @@ -177,7 +177,7 @@ func TestClusterExtensionResolutionFails(t *testing.T) { // TestClusterExtensionResolutionFailsWithDeprecationData verifies that deprecation warnings are shown even when resolution fails. // // Scenario: -// - Resolution fails (package not found or version not available) +// - Bundle resolution fails after a catalog package was selected // - Resolver returns deprecation data along with the error // - Catalog has marked the package as deprecated // - PackageDeprecated and Deprecated conditions show True with the deprecation message @@ -195,7 +195,7 @@ func TestClusterExtensionResolutionFailsWithDeprecationData(t *testing.T) { Reference: declcfg.PackageScopedReference{Schema: declcfg.SchemaPackage}, Message: deprecationMessage, }}, - }, fmt.Errorf("no package %q found", pkgName) + }, fmt.Errorf("no bundles found for package %q", pkgName) }) }) @@ -214,7 +214,7 @@ func TestClusterExtensionResolutionFailsWithDeprecationData(t *testing.T) { res, err := reconciler.Reconcile(ctx, ctrl.Request{NamespacedName: extKey}) require.Equal(t, ctrl.Result{}, res) - require.EqualError(t, err, fmt.Sprintf("no package %q found", pkgName)) + require.EqualError(t, err, fmt.Sprintf("no bundles found for package %q", pkgName)) require.NoError(t, cl.Get(ctx, extKey, clusterExtension)) diff --git a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go index 9f6dea4ea7..8341c034e8 100644 --- a/internal/operator-controller/controllers/clusterextension_reconcile_steps.go +++ b/internal/operator-controller/controllers/clusterextension_reconcile_steps.go @@ -163,19 +163,10 @@ func ResolveBundle(r resolve.Resolver, c client.Client) ReconcileStepFunc { installedBundleName = state.revisionStates.Installed.Name } - // Set deprecation status based on resolution results: - // - If resolution succeeds: hasCatalogData=true, deprecation shows catalog data (nil=not deprecated) - // - If resolution fails but returns deprecation: hasCatalogData=true, show package/channel deprecation warnings - // - If resolution fails with nil deprecation: hasCatalogData=false, all conditions go Unknown - // - // Note: We DO check for deprecation data even when resolution fails (hasCatalogData = err == nil || resolvedDeprecation != nil). - // This allows us to show package/channel deprecation warnings even when we can't resolve a specific bundle. - // - // TODO: Open question - what if different catalogs have different opinions of what's deprecated? - // If we can't resolve a bundle, how do we know which catalog to trust for deprecation information? - // Perhaps if the package shows up in multiple catalogs and deprecations don't match, we can set - // the deprecation status to unknown? Or perhaps we somehow combine the deprecation information from - // all catalogs? This needs a follow-up discussion and PR. + // Once the catalog resolver selects one package, it returns that catalog's deprecation + // data even when no bundle matches. An empty, non-nil deprecation means the selected + // catalog has no deprecation entries; nil on failure means no catalog was selected. + // Keep the success check for other resolver implementations that may return nil. hasCatalogData := err == nil || resolvedDeprecation != nil state.resolvedDeprecation = resolvedDeprecation state.hasCatalogData = hasCatalogData diff --git a/internal/operator-controller/resolve/catalog.go b/internal/operator-controller/resolve/catalog.go index 98a34b5ca4..b6b44186cd 100644 --- a/internal/operator-controller/resolve/catalog.go +++ b/internal/operator-controller/resolve/catalog.go @@ -1,10 +1,10 @@ package resolve import ( + "cmp" "context" "fmt" "slices" - "sort" "strings" bsemver "github.com/blang/semver/v4" @@ -27,16 +27,11 @@ import ( type ValidationFunc func(*declcfg.Bundle) error type CatalogResolver struct { - WalkCatalogsFunc func(context.Context, string, CatalogWalkFunc, ...client.ListOption) error + ListCatalogsFunc func(context.Context, ...client.ListOption) ([]ocv1.ClusterCatalog, error) + GetPackageFunc func(context.Context, *ocv1.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error) Validations []ValidationFunc } -type foundBundle struct { - bundle *declcfg.Bundle - catalog string - priority int32 -} - // Resolve returns a Bundle from a catalog that needs to get installed on the cluster. func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtension, installedBundle *ocv1.BundleMetadata) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { l := log.FromContext(ctx) @@ -44,7 +39,7 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio versionRange := ext.Spec.Source.Catalog.Version channels := ext.Spec.Source.Catalog.Channels - // unless overridden, default to selecting all bundles + // Unless overridden, select all catalogs. var selector = labels.Everything() var err error if ext.Spec.Source.Catalog != nil { @@ -66,132 +61,62 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio } } - type catStat struct { - CatalogName string `json:"catalogName"` - PackageFound bool `json:"packageFound"` - TotalBundles int `json:"totalBundles"` - MatchedBundles int `json:"matchedBundles"` + cat, packageFBC, err := r.selectPackage(ctx, packageName, selector) + if err != nil { + return nil, nil, nil, err } - - var catStats []*catStat - - var resolvedBundles []foundBundle - var priorDeprecation *declcfg.Deprecation - - listOptions := []client.ListOption{ - client.MatchingLabelsSelector{Selector: selector}, + if cat == nil { + return nil, nil, nil, fmt.Errorf("package %q not found in any eligible catalog", packageName) } - if err := r.WalkCatalogsFunc(ctx, packageName, func(ctx context.Context, cat *ocv1.ClusterCatalog, packageFBC *declcfg.DeclarativeConfig, err error) error { - if err != nil { - return fmt.Errorf("error getting package %q from catalog %q: %w", packageName, cat.Name, err) - } - - cs := catStat{CatalogName: cat.Name} - catStats = append(catStats, &cs) - - if isFBCEmpty(packageFBC) { - return nil - } - - cs.PackageFound = true - cs.TotalBundles = len(packageFBC.Bundles) - - var predicates []filterutil.Predicate[declcfg.Bundle] - if len(channels) > 0 { - channelSet := sets.New(channels...) - filteredChannels := slices.DeleteFunc(packageFBC.Channels, func(c declcfg.Channel) bool { - return !channelSet.Has(c.Name) - }) - predicates = append(predicates, filter.InAnyChannel(filteredChannels...)) - } - - if versionRangeConstraints != nil { - predicates = append(predicates, filter.InSemverRange(versionRangeConstraints)) - } - - if ext.Spec.Source.Catalog.UpgradeConstraintPolicy != ocv1.UpgradeConstraintPolicySelfCertified && installedBundle != nil { - successorPredicate, err := filter.SuccessorsOf(*installedBundle, packageFBC.Channels...) - if err != nil { - return fmt.Errorf("error finding upgrade edges: %w", err) - } - predicates = append(predicates, successorPredicate) - } - - // Apply the predicates to get the candidate bundles - packageFBC.Bundles = filterutil.InPlace(packageFBC.Bundles, filterutil.And(predicates...)) - cs.MatchedBundles = len(packageFBC.Bundles) - if len(packageFBC.Bundles) == 0 { - return nil - } - // If this package has a deprecation, we: - // 1. Want to sort deprecated bundles to the end of the list - // 2. Want to keep track of it so that we can return it if we end - // up resolving a bundle from this package. - byDeprecation := func(a, b declcfg.Bundle) int { return 0 } - var thisDeprecation *declcfg.Deprecation - if len(packageFBC.Deprecations) > 0 { - thisDeprecation = &packageFBC.Deprecations[0] - byDeprecation = compare.ByDeprecationFunc(*thisDeprecation) - } + // A non-nil deprecation means that one catalog's package data was read, + // even when it contains no deprecation entries or bundle resolution fails. + deprecation := &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: packageName} + if len(packageFBC.Deprecations) > 0 { + deprecation = &packageFBC.Deprecations[0] + } + failed := func(err error) (*declcfg.Bundle, *declcfg.VersionRelease, *declcfg.Deprecation, error) { + l.Info("resolution failed", "catalog", cat.Name, "error", err) + return nil, nil, deprecation, fmt.Errorf("resolving package %q from catalog %q: %w", packageName, cat.Name, err) + } - // Sort the bundles by deprecation and then by version - slices.SortStableFunc(packageFBC.Bundles, func(a, b declcfg.Bundle) int { - if lessDep := byDeprecation(a, b); lessDep != 0 { - return lessDep - } - return compare.ByVersionAndRelease(a, b) + var predicates []filterutil.Predicate[declcfg.Bundle] + if len(channels) > 0 { + channelSet := sets.New(channels...) + filteredChannels := slices.DeleteFunc(packageFBC.Channels, func(c declcfg.Channel) bool { + return !channelSet.Has(c.Name) }) - - thisBundle := packageFBC.Bundles[0] - - if len(resolvedBundles) != 0 { - // We've already found one or more package candidates - currentIsDeprecated := isDeprecated(thisBundle, thisDeprecation) - priorIsDeprecated := isDeprecated(*resolvedBundles[len(resolvedBundles)-1].bundle, priorDeprecation) - if currentIsDeprecated && !priorIsDeprecated { - // Skip this deprecated package and retain the non-deprecated package(s) - return nil - } else if !currentIsDeprecated && priorIsDeprecated { - // Our package candidates so far were deprecated and this one is not; clear the lists - resolvedBundles = []foundBundle{} - } + predicates = append(predicates, filter.InAnyChannel(filteredChannels...)) + } + if versionRangeConstraints != nil { + predicates = append(predicates, filter.InSemverRange(versionRangeConstraints)) + } + if ext.Spec.Source.Catalog.UpgradeConstraintPolicy != ocv1.UpgradeConstraintPolicySelfCertified && installedBundle != nil { + successorPredicate, err := filter.SuccessorsOf(*installedBundle, packageFBC.Channels...) + if err != nil { + return failed(fmt.Errorf("error finding upgrade edges: %w", err)) } - // The current bundle shares deprecation status with prior bundles or - // there are no prior bundles. Add it to the list. - resolvedBundles = append(resolvedBundles, foundBundle{&thisBundle, cat.GetName(), cat.Spec.Priority}) - priorDeprecation = thisDeprecation - return nil - }, listOptions...); err != nil { - return nil, nil, nil, fmt.Errorf("error walking catalogs: %w", err) + predicates = append(predicates, successorPredicate) } - // Resolve for priority - if len(resolvedBundles) > 1 { - // Want highest first (reverse sort) - sort.Slice(resolvedBundles, func(i, j int) bool { return resolvedBundles[i].priority > resolvedBundles[j].priority }) - // If the top two bundles do not have the same priority, then priority breaks the tie - // Reduce resolvedBundles to just the first item (highest priority) - if resolvedBundles[0].priority != resolvedBundles[1].priority { - resolvedBundles = []foundBundle{resolvedBundles[0]} - } + // Apply bundle filters only after selecting a catalog for the package. + packageFBC.Bundles = filterutil.InPlace(packageFBC.Bundles, filterutil.And(predicates...)) + if len(packageFBC.Bundles) == 0 { + return failed(resolutionError{PackageName: packageName, Version: versionRange, Channels: channels, InstalledBundle: installedBundle}) } - // 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, + // Within the selected catalog, prefer non-deprecated bundles, then the highest version and release. + byDeprecation := compare.ByDeprecationFunc(*deprecation) + slices.SortStableFunc(packageFBC.Bundles, func(a, b declcfg.Bundle) int { + if lessDep := byDeprecation(a, b); lessDep != 0 { + return lessDep } - } - resolvedBundle := resolvedBundles[0].bundle + return compare.ByVersionAndRelease(a, b) + }) + resolvedBundle := &packageFBC.Bundles[0] resolvedBundleVersion, err := bundleutil.GetVersionAndRelease(*resolvedBundle) if err != nil { - return nil, nil, nil, fmt.Errorf("error getting resolved bundle version for bundle %q: %w", resolvedBundle.Name, err) + return failed(fmt.Errorf("error getting resolved bundle version for bundle %q: %w", resolvedBundle.Name, err)) } // Run validations against the resolved bundle to ensure only valid resolved bundles are being returned @@ -200,12 +125,12 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio // constrained in order to eliminate the invalid bundle from the resolution. for _, validation := range r.Validations { if err := validation(resolvedBundle); err != nil { - return nil, nil, nil, fmt.Errorf("validating bundle %q: %w", resolvedBundle.Name, err) + return failed(fmt.Errorf("validating bundle %q: %w", resolvedBundle.Name, err)) } } - l.V(4).Info("resolution succeeded", "stats", catStats) - return resolvedBundle, resolvedBundleVersion, priorDeprecation, nil + l.V(4).Info("resolution succeeded", "catalog", cat.Name, "bundle", resolvedBundle.Name) + return resolvedBundle, resolvedBundleVersion, deprecation, nil } type resolutionError struct { @@ -213,7 +138,6 @@ type resolutionError struct { Version string Channels []string InstalledBundle *ocv1.BundleMetadata - ResolvedBundles []foundBundle } func (rei resolutionError) Error() string { @@ -222,11 +146,7 @@ func (rei resolutionError) Error() string { sb.WriteString(fmt.Sprintf("error upgrading from currently installed version %q: ", rei.InstalledBundle.Version)) } - if len(rei.ResolvedBundles) > 1 { - sb.WriteString(fmt.Sprintf("found bundles for package %q ", rei.PackageName)) - } else { - sb.WriteString(fmt.Sprintf("no bundles found for package %q ", rei.PackageName)) - } + sb.WriteString(fmt.Sprintf("no bundles found for package %q ", rei.PackageName)) if rei.Version != "" { sb.WriteString(fmt.Sprintf("matching version %q ", rei.Version)) @@ -236,68 +156,71 @@ func (rei resolutionError) Error() string { sb.WriteString(fmt.Sprintf("in channels %v ", rei.Channels)) } - matchedCatalogs := make([]string, 0, len(rei.ResolvedBundles)) - for _, r := range rei.ResolvedBundles { - matchedCatalogs = append(matchedCatalogs, r.catalog) - } - slices.Sort(matchedCatalogs) // sort for consistent error message - if len(matchedCatalogs) > 1 { - sb.WriteString(fmt.Sprintf("in multiple catalogs with the same priority %v ", matchedCatalogs)) - } - return strings.TrimSpace(sb.String()) } -func isDeprecated(bundle declcfg.Bundle, deprecation *declcfg.Deprecation) bool { - if deprecation == nil { - return false +func (r *CatalogResolver) selectPackage(ctx context.Context, packageName string, selector labels.Selector) (*ocv1.ClusterCatalog, *declcfg.DeclarativeConfig, error) { + l := log.FromContext(ctx) + catalogs, err := r.ListCatalogsFunc(ctx, client.MatchingLabelsSelector{Selector: selector}) + if err != nil { + return nil, nil, fmt.Errorf("error listing catalogs: %w", err) } - for _, entry := range deprecation.Entries { - if entry.Reference.Schema == declcfg.SchemaBundle && entry.Reference.Name == bundle.Name { + // Exclude unavailable catalogs before sorting and grouping by priority. + catalogs = slices.DeleteFunc(catalogs, func(c ocv1.ClusterCatalog) bool { + if c.Spec.AvailabilityMode == ocv1.AvailabilityModeUnavailable { + l.Info("excluding ClusterCatalog from resolution process since it is disabled", "catalog", c.Name) return true } - } - return false -} - -type CatalogWalkFunc func(context.Context, *ocv1.ClusterCatalog, *declcfg.DeclarativeConfig, error) error - -func CatalogWalker( - listCatalogs func(context.Context, ...client.ListOption) ([]ocv1.ClusterCatalog, error), - getPackage func(context.Context, *ocv1.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error), -) func(ctx context.Context, packageName string, f CatalogWalkFunc, catalogListOpts ...client.ListOption) error { - return func(ctx context.Context, packageName string, f CatalogWalkFunc, catalogListOpts ...client.ListOption) error { - l := log.FromContext(ctx) - catalogs, err := listCatalogs(ctx, catalogListOpts...) - if err != nil { - return fmt.Errorf("error listing catalogs: %w", err) - } - - // Remove disabled catalogs from consideration - catalogs = slices.DeleteFunc(catalogs, func(c ocv1.ClusterCatalog) bool { - if c.Spec.AvailabilityMode == ocv1.AvailabilityModeUnavailable { - l.Info("excluding ClusterCatalog from resolution process since it is disabled", "catalog", c.Name) - return true + return false + }) + groups := groupCatalogsByPriority(catalogs) + l.Info("using ClusterCatalogs for resolution", "catalogs", slicesutil.Map(catalogs, func(c ocv1.ClusterCatalog) string { return c.Name })) + + for _, group := range groups { + var selected *ocv1.ClusterCatalog + var packageFBC *declcfg.DeclarativeConfig + var foundIn []string + for i := range group { + cat := &group[i] + fbc, err := r.GetPackageFunc(ctx, cat, packageName) + if err != nil { + return nil, nil, fmt.Errorf("error getting package %q from catalog %q: %w", packageName, cat.Name, err) } - return false - }) - - availableCatalogNames := slicesutil.Map(catalogs, func(c ocv1.ClusterCatalog) string { return c.Name }) - l.Info("using ClusterCatalogs for resolution", "catalogs", availableCatalogNames) - - for i := range catalogs { - cat := &catalogs[i] - - // process enabled catalogs - fbc, fbcErr := getPackage(ctx, cat, packageName) - - if walkErr := f(ctx, cat, fbc, fbcErr); walkErr != nil { - return walkErr + if isFBCEmpty(fbc) { + continue } + selected, packageFBC = cat, fbc + foundIn = append(foundIn, cat.Name) + } + if len(foundIn) > 1 { + return nil, nil, fmt.Errorf("package %q found in multiple catalogs with the same priority %d: %v", packageName, group[0].Spec.Priority, foundIn) + } + if selected != nil { + return selected, packageFBC, nil } - - return nil } + return nil, nil, nil +} + +// groupCatalogsByPriority sorts catalogs by descending priority and ascending name, +// then returns priority groups that share the input slice's storage. +func groupCatalogsByPriority(catalogs []ocv1.ClusterCatalog) [][]ocv1.ClusterCatalog { + slices.SortFunc(catalogs, func(a, b ocv1.ClusterCatalog) int { + if priority := cmp.Compare(b.Spec.Priority, a.Spec.Priority); priority != 0 { + return priority + } + return cmp.Compare(a.Name, b.Name) + }) + groups := make([][]ocv1.ClusterCatalog, 0, len(catalogs)) + for len(catalogs) > 0 { + end := 1 + for end < len(catalogs) && catalogs[end].Spec.Priority == catalogs[0].Spec.Priority { + end++ + } + groups = append(groups, catalogs[:end]) + catalogs = catalogs[end:] + } + return groups } func isFBCEmpty(fbc *declcfg.DeclarativeConfig) bool { diff --git a/internal/operator-controller/resolve/catalog_test.go b/internal/operator-controller/resolve/catalog_test.go index 2c6ab5c5b6..52c2663316 100644 --- a/internal/operator-controller/resolve/catalog_test.go +++ b/internal/operator-controller/resolve/catalog_test.go @@ -30,30 +30,42 @@ func TestInvalidClusterExtensionVersionRange(t *testing.T) { } func TestErrorWalkingCatalogs(t *testing.T) { - r := CatalogResolver{WalkCatalogsFunc: func(context.Context, string, CatalogWalkFunc, ...client.ListOption) error { - return fmt.Errorf("fake error") + r := CatalogResolver{ListCatalogsFunc: func(context.Context, ...client.ListOption) ([]ocv1.ClusterCatalog, error) { + return nil, fmt.Errorf("fake error") }} pkgName := randPkg() ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) _, _, _, err := r.Resolve(context.Background(), ce, nil) - assert.EqualError(t, err, "error walking catalogs: fake error") + assert.EqualError(t, err, "error listing catalogs: fake error") } func TestErrorGettingPackage(t *testing.T) { - w := staticCatalogWalker{ + pkgName := randPkg() + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { - return nil, nil, fmt.Errorf("fake error") + return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 1}, nil + }, + "b": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { + return nil, &ocv1.ClusterCatalogSpec{Priority: 1}, fmt.Errorf("fake error") + }, + "c": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { + return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 0}, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} - pkgName := randPkg() + var consulted []string + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: func(ctx context.Context, cat *ocv1.ClusterCatalog, name string) (*declcfg.DeclarativeConfig, error) { + consulted = append(consulted, cat.Name) + return w.GetPackage(ctx, cat, name) + }} ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) - assert.EqualError(t, err, fmt.Sprintf(`error walking catalogs: error getting package %q from catalog "a": fake error`, pkgName)) + _, _, deprecation, err := r.Resolve(context.Background(), ce, nil) + require.EqualError(t, err, fmt.Sprintf(`error getting package %q from catalog "b": fake error`, pkgName)) + assert.Equal(t, []string{"a", "b"}, consulted) + assert.Nil(t, deprecation) } func TestPackageDoesNotExist(t *testing.T) { - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -64,16 +76,16 @@ func TestPackageDoesNotExist(t *testing.T) { return &declcfg.DeclarativeConfig{}, nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} pkgName := randPkg() ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) _, _, _, err := r.Resolve(context.Background(), ce, nil) - assert.EqualError(t, err, fmt.Sprintf(`no bundles found for package %q`, pkgName)) + assert.EqualError(t, err, fmt.Sprintf(`package %q not found in any eligible catalog`, pkgName)) } func TestPackageExists(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -84,7 +96,7 @@ func TestPackageExists(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) @@ -95,19 +107,24 @@ func TestPackageExists(t *testing.T) { func TestValidationFailed(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, "b": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { - return &declcfg.DeclarativeConfig{}, nil, nil + return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 0}, nil }, "c": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { - return genPackage(pkgName), nil, nil + return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 1}, nil }, } + var consulted []string r := CatalogResolver{ - WalkCatalogsFunc: w.WalkCatalogs, + ListCatalogsFunc: w.ListCatalogs, + GetPackageFunc: func(ctx context.Context, cat *ocv1.ClusterCatalog, name string) (*declcfg.DeclarativeConfig, error) { + consulted = append(consulted, cat.Name) + return w.GetPackage(ctx, cat, name) + }, Validations: []ValidationFunc{ func(b *declcfg.Bundle) error { return errors.New("fail") @@ -115,13 +132,15 @@ func TestValidationFailed(t *testing.T) { }, } ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) - require.Error(t, err) + _, _, deprecation, err := r.Resolve(context.Background(), ce, nil) + require.ErrorContains(t, err, `from catalog "c": validating bundle`) + assert.Equal(t, []string{"c"}, consulted) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), deprecation) } func TestVersionDoesNotExist(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -132,15 +151,16 @@ func TestVersionDoesNotExist(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, "4.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, _, _, err := r.Resolve(context.Background(), ce, nil) - assert.EqualError(t, err, fmt.Sprintf(`no bundles found for package %q matching version "4.0.0"`, pkgName)) + _, _, deprecation, err := r.Resolve(context.Background(), ce, nil) + require.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": no bundles found for package %q matching version "4.0.0"`, pkgName, pkgName)) + assert.Equal(t, ptr.To(packageDeprecation(pkgName)), deprecation) } func TestVersionExists(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -151,7 +171,7 @@ func TestVersionExists(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <2.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) @@ -162,7 +182,7 @@ func TestVersionExists(t *testing.T) { func TestChannelDoesNotExist(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -173,15 +193,15 @@ func TestChannelDoesNotExist(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"stable"}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) _, _, _, err := r.Resolve(context.Background(), ce, nil) - assert.EqualError(t, err, fmt.Sprintf(`no bundles found for package %q in channels [stable]`, pkgName)) + assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": no bundles found for package %q in channels [stable]`, pkgName, pkgName)) } func TestChannelExists(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -192,7 +212,7 @@ func TestChannelExists(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"beta"}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) @@ -203,7 +223,7 @@ func TestChannelExists(t *testing.T) { func TestChannelExistsButNotVersion(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -214,15 +234,15 @@ func TestChannelExistsButNotVersion(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"beta"}, "3.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) _, _, _, err := r.Resolve(context.Background(), ce, nil) - assert.EqualError(t, err, fmt.Sprintf(`no bundles found for package %q matching version "3.0.0" in channels [beta]`, pkgName)) + assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": no bundles found for package %q matching version "3.0.0" in channels [beta]`, pkgName, pkgName)) } func TestVersionExistsButNotChannel(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -233,15 +253,15 @@ func TestVersionExistsButNotChannel(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"stable"}, "1.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) _, _, _, err := r.Resolve(context.Background(), ce, nil) - assert.EqualError(t, err, fmt.Sprintf(`no bundles found for package %q matching version "1.0.0" in channels [stable]`, pkgName)) + assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": no bundles found for package %q matching version "1.0.0" in channels [stable]`, pkgName, pkgName)) } func TestChannelAndVersionExist(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -252,7 +272,7 @@ func TestChannelAndVersionExist(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"alpha"}, "0.1.0", ocv1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) @@ -263,7 +283,7 @@ func TestChannelAndVersionExist(t *testing.T) { func TestPreferNonDeprecated(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -274,7 +294,7 @@ func TestPreferNonDeprecated(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, ">=0.1.0 <=1.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) @@ -285,7 +305,7 @@ func TestPreferNonDeprecated(t *testing.T) { func TestAcceptDeprecated(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -296,7 +316,7 @@ func TestAcceptDeprecated(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <=1.0.1", ocv1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) @@ -307,7 +327,7 @@ func TestAcceptDeprecated(t *testing.T) { func TestPackageVariationsBetweenCatalogs(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -377,35 +397,12 @@ func TestPackageVariationsBetweenCatalogs(t *testing.T) { return fbc, nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} - t.Run("when bundle candidates for a package are deprecated in all but one catalog", func(t *testing.T) { + t.Run("same-priority packages are ambiguous despite non-deprecated candidates", func(t *testing.T) { ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <=1.0.3", ocv1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) - require.NoError(t, err) - // We choose the only non-deprecated package - assert.Equal(t, genBundle(pkgName, "1.0.2").Name, gotBundle.Name) - assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.0.2")}, *gotVersion) - assert.Equal(t, (*declcfg.Deprecation)(nil), gotDeprecation) - }) - - t.Run("when bundle candidates are found and deprecated in multiple catalogs", func(t *testing.T) { - ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <=1.0.1", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) - require.Error(t, err) - // We will not make a decision on which catalog to use - require.ErrorContains(t, err, "in multiple catalogs with the same priority [b c]") - assert.Nil(t, gotBundle) - assert.Nil(t, gotVersion) - assert.Nil(t, gotDeprecation) - }) - - t.Run("when bundle candidates are found and not deprecated in multiple catalogs", func(t *testing.T) { - ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <=1.0.4", ocv1.UpgradeConstraintPolicyCatalogProvided) - gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) - require.Error(t, err) - // We will not make a decision on which catalog to use - require.ErrorContains(t, err, "in multiple catalogs with the same priority [d f]") + require.ErrorContains(t, err, "in multiple catalogs with the same priority 0: [b c d e f]") assert.Nil(t, gotBundle) assert.Nil(t, gotVersion) assert.Nil(t, gotDeprecation) @@ -413,18 +410,19 @@ func TestPackageVariationsBetweenCatalogs(t *testing.T) { t.Run("highest semver bundle is chosen when candidates are all from the same catalog", func(t *testing.T) { ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.4 <=1.0.5", ocv1.UpgradeConstraintPolicyCatalogProvided) + ce.Spec.Source.Catalog.Selector = &metav1.LabelSelector{MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": "f"}} gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) // Bundles within one catalog for a package will be sorted by semver and deprecation and the best is returned assert.Equal(t, genBundle(pkgName, "1.0.5").Name, gotBundle.Name) assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.0.5")}, *gotVersion) - assert.Equal(t, (*declcfg.Deprecation)(nil), gotDeprecation) + assert.Equal(t, &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: pkgName}, gotDeprecation) }) } func TestUpgradeFoundLegacy(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -435,7 +433,7 @@ func TestUpgradeFoundLegacy(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) installedBundle := &ocv1.BundleMetadata{ Name: bundleName(pkgName, "0.1.0"), @@ -451,7 +449,7 @@ func TestUpgradeFoundLegacy(t *testing.T) { func TestUpgradeNotFoundLegacy(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -462,7 +460,7 @@ func TestUpgradeNotFoundLegacy(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, "<1.0.0 >=2.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) installedBundle := &ocv1.BundleMetadata{ Name: bundleName(pkgName, "0.1.0"), @@ -470,12 +468,12 @@ 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)) + assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": error upgrading from currently installed version "0.1.0": no bundles found for package %q matching version "<1.0.0 >=2.0.0"`, pkgName, pkgName)) } func TestDowngradeFound(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -486,7 +484,7 @@ func TestDowngradeFound(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, "<1.0.2", ocv1.UpgradeConstraintPolicySelfCertified) installedBundle := &ocv1.BundleMetadata{ Name: bundleName(pkgName, "1.0.2"), @@ -503,7 +501,7 @@ func TestDowngradeFound(t *testing.T) { func TestDowngradeNotFound(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -514,7 +512,7 @@ func TestDowngradeNotFound(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, ">0.1.0 <1.0.0", ocv1.UpgradeConstraintPolicySelfCertified) installedBundle := &ocv1.BundleMetadata{ Name: bundleName(pkgName, "1.0.2"), @@ -522,61 +520,7 @@ func TestDowngradeNotFound(t *testing.T) { } // Downgrades are allowed via the upgrade constraint policy, but there is no bundle in the specified range. _, _, _, err := r.Resolve(context.Background(), ce, installedBundle) - 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 TestCatalogWalker(t *testing.T) { - t.Run("error listing catalogs", func(t *testing.T) { - w := CatalogWalker( - func(ctx context.Context, option ...client.ListOption) ([]ocv1.ClusterCatalog, error) { - return nil, fmt.Errorf("fake error") - }, - func(context.Context, *ocv1.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error) { - return nil, nil - }, - ) - walkFunc := func(ctx context.Context, cat *ocv1.ClusterCatalog, fbc *declcfg.DeclarativeConfig, err error) error { - return nil - } - assert.EqualError(t, w(context.Background(), "", walkFunc), "error listing catalogs: fake error") - }) - - t.Run("error getting package", func(t *testing.T) { - w := CatalogWalker( - func(ctx context.Context, option ...client.ListOption) ([]ocv1.ClusterCatalog, error) { - return []ocv1.ClusterCatalog{{ObjectMeta: metav1.ObjectMeta{Name: "a"}}}, nil - }, - func(context.Context, *ocv1.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error) { - return nil, fmt.Errorf("fake error getting package FBC") - }, - ) - walkFunc := func(ctx context.Context, cat *ocv1.ClusterCatalog, fbc *declcfg.DeclarativeConfig, err error) error { - return err - } - assert.EqualError(t, w(context.Background(), "", walkFunc), "fake error getting package FBC") - }) - - t.Run("success", func(t *testing.T) { - w := CatalogWalker( - func(ctx context.Context, option ...client.ListOption) ([]ocv1.ClusterCatalog, error) { - return []ocv1.ClusterCatalog{ - {ObjectMeta: metav1.ObjectMeta{Name: "a"}}, - {ObjectMeta: metav1.ObjectMeta{Name: "b"}}, - }, nil - }, - func(context.Context, *ocv1.ClusterCatalog, string) (*declcfg.DeclarativeConfig, error) { - return &declcfg.DeclarativeConfig{}, nil - }, - ) - - seenCatalogs := []string{} - walkFunc := func(ctx context.Context, cat *ocv1.ClusterCatalog, fbc *declcfg.DeclarativeConfig, err error) error { - seenCatalogs = append(seenCatalogs, cat.Name) - return nil - } - require.NoError(t, w(context.Background(), "", walkFunc)) - assert.Equal(t, []string{"a", "b"}, seenCatalogs) - }) + assert.EqualError(t, err, fmt.Sprintf(`resolving package %q from catalog "c": error upgrading from currently installed version "1.0.2": no bundles found for package %q matching version ">0.1.0 <1.0.0"`, pkgName, pkgName)) } func buildFooClusterExtension(pkg string, channels []string, version string, upgradeConstraintPolicy ocv1.UpgradeConstraintPolicy) *ocv1.ClusterExtension { @@ -601,11 +545,12 @@ func buildFooClusterExtension(pkg string, channels []string, version string, upg type getPackageFunc func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) -type staticCatalogWalker map[string]getPackageFunc +type staticCatalogs map[string]getPackageFunc -func (w staticCatalogWalker) WalkCatalogs(ctx context.Context, _ string, f CatalogWalkFunc, opts ...client.ListOption) error { +func (w staticCatalogs) ListCatalogs(_ context.Context, opts ...client.ListOption) ([]ocv1.ClusterCatalog, error) { + var catalogs []ocv1.ClusterCatalog for k, v := range w { - cat := &ocv1.ClusterCatalog{ + cat := ocv1.ClusterCatalog{ ObjectMeta: metav1.ObjectMeta{ Name: k, Labels: map[string]string{ @@ -620,15 +565,18 @@ func (w staticCatalogWalker) WalkCatalogs(ctx context.Context, _ string, f Catal if !options.LabelSelector.Matches(labels.Set(cat.Labels)) { continue } - fbc, catCfg, fbcErr := v() + _, catCfg, _ := v() if catCfg != nil { catCfg.DeepCopyInto(&cat.Spec) } - if err := f(ctx, cat, fbc, fbcErr); err != nil { - return err - } + catalogs = append(catalogs, cat) } - return nil + return catalogs, nil +} + +func (w staticCatalogs) GetPackage(_ context.Context, cat *ocv1.ClusterCatalog, _ string) (*declcfg.DeclarativeConfig, error) { + fbc, _, err := w[cat.Name]() + return fbc, err } func randPkg() string { @@ -724,12 +672,12 @@ func TestInvalidClusterExtensionCatalogMatchExpressions(t *testing.T) { } func TestInvalidClusterExtensionCatalogMatchLabelsName(t *testing.T) { - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return genPackage("foo"), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", @@ -750,12 +698,12 @@ func TestInvalidClusterExtensionCatalogMatchLabelsName(t *testing.T) { } func TestInvalidClusterExtensionCatalogMatchLabelsValue(t *testing.T) { - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return genPackage("foo"), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := &ocv1.ClusterExtension{ ObjectMeta: metav1.ObjectMeta{ Name: "foo", @@ -777,7 +725,7 @@ func TestInvalidClusterExtensionCatalogMatchLabelsValue(t *testing.T) { func TestClusterExtensionMatchLabel(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return genPackage(pkgName), nil, nil }, @@ -785,7 +733,7 @@ func TestClusterExtensionMatchLabel(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) ce.Spec.Source.Catalog.Selector = &metav1.LabelSelector{ MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": "b"}, @@ -797,7 +745,7 @@ func TestClusterExtensionMatchLabel(t *testing.T) { func TestClusterExtensionNoMatchLabel(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -805,7 +753,7 @@ func TestClusterExtensionNoMatchLabel(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) ce.Spec.Source.Catalog.Selector = &metav1.LabelSelector{ MatchLabels: map[string]string{"olm.operatorframework.io/metadata.name": "a"}, @@ -813,12 +761,12 @@ func TestClusterExtensionNoMatchLabel(t *testing.T) { _, _, _, err := r.Resolve(context.Background(), ce, nil) require.Error(t, err) - require.ErrorContains(t, err, fmt.Sprintf("no bundles found for package %q", pkgName)) + require.ErrorContains(t, err, fmt.Sprintf("package %q not found in any eligible catalog", pkgName)) } func TestUnequalPriority(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{ Packages: []declcfg.Package{{Name: pkgName}}, @@ -848,17 +796,51 @@ func TestUnequalPriority(t *testing.T) { }, &ocv1.ClusterCatalogSpec{Priority: 0}, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + var consulted []string + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: func(ctx context.Context, cat *ocv1.ClusterCatalog, name string) (*declcfg.DeclarativeConfig, error) { + consulted = append(consulted, cat.Name) + return w.GetPackage(ctx, cat, name) + }} ce := buildFooClusterExtension(pkgName, []string{}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) - _, gotVersion, _, err := r.Resolve(context.Background(), ce, nil) + _, gotVersion, deprecation, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) require.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.0.0")}, *gotVersion) + assert.Equal(t, []string{"a"}, consulted) + assert.Equal(t, &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: pkgName}, deprecation) + + // The lower-priority catalog has 1.1.0, but the selected catalog cannot satisfy it. + consulted = nil + ce.Spec.Source.Catalog.Version = "1.1.0" + _, _, deprecation, err = r.Resolve(context.Background(), ce, nil) + require.ErrorContains(t, err, `from catalog "a": no bundles found`) + assert.Equal(t, []string{"a"}, consulted) + assert.Equal(t, &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: pkgName}, deprecation) + + // Only a genuinely absent package permits a lower-priority catalog. + w["a"] = func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { + return &declcfg.DeclarativeConfig{}, &ocv1.ClusterCatalogSpec{Priority: 1}, nil + } + consulted = nil + _, gotVersion, _, err = r.Resolve(context.Background(), ce, nil) + require.NoError(t, err) + assert.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("1.1.0")}, *gotVersion) + assert.Equal(t, []string{"a", "b"}, consulted) + + // Preserve the existing definition: nonempty config means package found. + w["a"] = func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { + return &declcfg.DeclarativeConfig{Channels: []declcfg.Channel{{Package: pkgName, Name: "alpha"}}}, &ocv1.ClusterCatalogSpec{Priority: 1}, nil + } + consulted = nil + _, _, deprecation, err = r.Resolve(context.Background(), ce, nil) + require.ErrorContains(t, err, `from catalog "a": no bundles found`) + assert.Equal(t, []string{"a"}, consulted) + assert.Equal(t, &declcfg.Deprecation{Schema: declcfg.SchemaDeprecation, Package: pkgName}, deprecation) } func TestMultiplePriority(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 1}, nil }, @@ -866,23 +848,48 @@ func TestMultiplePriority(t *testing.T) { return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 0}, nil }, "c": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { - return genPackage(pkgName), &ocv1.ClusterCatalogSpec{Priority: 1}, nil + return &declcfg.DeclarativeConfig{Packages: []declcfg.Package{{Name: pkgName}}}, &ocv1.ClusterCatalogSpec{Priority: 1}, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} - ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0 <=1.0.1", ocv1.UpgradeConstraintPolicyCatalogProvided) + // "a" has an eligible bundle; "c" has no bundles but still creates an ambiguity. + ce := buildFooClusterExtension(pkgName, nil, "3.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.Error(t, err) - require.ErrorContains(t, err, "in multiple catalogs with the same priority [a b c]") + require.ErrorContains(t, err, "in multiple catalogs with the same priority 1: [a c]") assert.Nil(t, gotBundle) assert.Nil(t, gotVersion) assert.Nil(t, gotDeprecation) } +func priorityCatalog(name string, priority int32) ocv1.ClusterCatalog { + return ocv1.ClusterCatalog{ObjectMeta: metav1.ObjectMeta{Name: name}, Spec: ocv1.ClusterCatalogSpec{Priority: priority}} +} + +func TestGroupCatalogsByPriority(t *testing.T) { + catalogs := []ocv1.ClusterCatalog{ + priorityCatalog("lowest", -2147483648), + priorityCatalog("z", 0), + priorityCatalog("highest", 2147483647), + priorityCatalog("a", 0), + } + groups := groupCatalogsByPriority(catalogs) + names := make([][]string, 0, len(groups)) + for _, group := range groups { + groupNames := make([]string, 0, len(group)) + for _, cat := range group { + groupNames = append(groupNames, cat.Name) + } + names = append(names, groupNames) + } + assert.Equal(t, [][]string{{"highest"}, {"a", "z"}, {"lowest"}}, names) + assert.Empty(t, groupCatalogsByPriority(nil)) +} + func TestMultipleChannels(t *testing.T) { pkgName := randPkg() - w := staticCatalogWalker{ + w := staticCatalogs{ "a": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) { return &declcfg.DeclarativeConfig{}, nil, nil }, @@ -893,7 +900,7 @@ func TestMultipleChannels(t *testing.T) { return genPackage(pkgName), nil, nil }, } - r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs} + r := CatalogResolver{ListCatalogsFunc: w.ListCatalogs, GetPackageFunc: w.GetPackage} ce := buildFooClusterExtension(pkgName, []string{"beta", "alpha"}, "", ocv1.UpgradeConstraintPolicyCatalogProvided) gotBundle, gotVersion, gotDeprecation, err := r.Resolve(context.Background(), ce, nil) require.NoError(t, err) @@ -924,17 +931,18 @@ func TestAllCatalogsDisabled(t *testing.T) { } r := CatalogResolver{ - WalkCatalogsFunc: CatalogWalker(listCatalogs, getPackage), + ListCatalogsFunc: listCatalogs, GetPackageFunc: getPackage, } ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) _, _, _, err := r.Resolve(context.Background(), ce, nil) require.Error(t, err) - assert.Contains(t, err.Error(), "no bundles found for package") + assert.Contains(t, err.Error(), "not found in any eligible catalog") } func TestSomeCatalogsDisabled(t *testing.T) { pkgName := randPkg() + var consulted []string listCatalogs := func(ctx context.Context, options ...client.ListOption) ([]ocv1.ClusterCatalog, error) { return []ocv1.ClusterCatalog{ { @@ -959,12 +967,12 @@ func TestSomeCatalogsDisabled(t *testing.T) { } getPackage := func(ctx context.Context, cat *ocv1.ClusterCatalog, packageName string) (*declcfg.DeclarativeConfig, error) { - // Only enabled catalog should be processed + consulted = append(consulted, cat.Name) return genPackage(pkgName), nil } r := CatalogResolver{ - WalkCatalogsFunc: CatalogWalker(listCatalogs, getPackage), + ListCatalogsFunc: listCatalogs, GetPackageFunc: getPackage, } ce := buildFooClusterExtension(pkgName, []string{}, ">=1.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided) @@ -972,4 +980,5 @@ func TestSomeCatalogsDisabled(t *testing.T) { require.NoError(t, err) require.NotNil(t, gotBundle) require.Equal(t, declcfg.VersionRelease{Version: bsemver.MustParse("3.0.0")}, *gotVersion) + require.Equal(t, []string{"enabledCatalog"}, consulted) } diff --git a/test/e2e/features/install.feature b/test/e2e/features/install.feature index 3a9ce02224..8e96f53cf8 100644 --- a/test/e2e/features/install.feature +++ b/test/e2e/features/install.feature @@ -64,7 +64,7 @@ Feature: Install ClusterExtension And resource "deployment/test-operator-${SCENARIO_ID}" is installed - Scenario: Report that bundle cannot be installed when it exists in multiple catalogs with same priority + Scenario: Report package ambiguity when multiple catalogs have the same priority Given a catalog "test" with packages: | package | version | channel | replaces | contents | | test | 1.2.0 | beta | | CRD, Deployment, ConfigMap | @@ -86,7 +86,7 @@ Feature: Install ClusterExtension """ Then ClusterExtension reports Progressing as True with Reason Retrying and Message includes: """ - found bundles for package "${PACKAGE:test}" in multiple catalogs with the same priority + package "${PACKAGE:test}" found in multiple catalogs with the same priority """ @SingleOwnNamespaceInstallSupport diff --git a/test/e2e/features/update.feature b/test/e2e/features/update.feature index 3c39d34cbc..e3d0a3ae27 100644 --- a/test/e2e/features/update.feature +++ b/test/e2e/features/update.feature @@ -80,7 +80,7 @@ Feature: Update ClusterExtension """ Then ClusterExtension reports Progressing as True with Reason Retrying and Message: """ - error upgrading from currently installed version "1.0.0": no bundles found for package "${PACKAGE:test}" matching version "1.2.0" + resolving package "${PACKAGE:test}" from catalog "${CATALOG:test}": error upgrading from currently installed version "1.0.0": no bundles found for package "${PACKAGE:test}" matching version "1.2.0" """ Scenario: Force update to non successor version