Skip to content

Commit ed15e26

Browse files
Copilotjoelanford
andcommitted
Improve error messages for upgrade constraint failures (issue #1022)
- Enhanced resolutionError to track bundles before upgrade constraints - Added logic to distinguish between no bundles and no successors - Implemented best successor suggestions (y-stream and z-stream) - Added test case for issue #1022 scenario Co-authored-by: joelanford <580047+joelanford@users.noreply.github.com>
1 parent 1ccc6dc commit ed15e26

2 files changed

Lines changed: 177 additions & 15 deletions

File tree

internal/operator-controller/resolve/catalog.go

Lines changed: 133 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,8 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio
7777
var catStats []*catStat
7878

7979
var resolvedBundles []foundBundle
80+
var bundlesBeforeUpgrade []foundBundle // Track bundles before upgrade constraints
81+
var allSuccessors []foundBundle // Track all successors for suggestions
8082
var priorDeprecation *declcfg.Deprecation
8183

8284
listOptions := []client.ListOption{
@@ -97,25 +99,59 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio
9799
cs.PackageFound = true
98100
cs.TotalBundles = len(packageFBC.Bundles)
99101

100-
var predicates []filterutil.Predicate[declcfg.Bundle]
102+
// Build predicates for channel and version filtering
103+
var preUpgradePredicates []filterutil.Predicate[declcfg.Bundle]
101104
if len(channels) > 0 {
102105
channelSet := sets.New(channels...)
103106
filteredChannels := slices.DeleteFunc(packageFBC.Channels, func(c declcfg.Channel) bool {
104107
return !channelSet.Has(c.Name)
105108
})
106-
predicates = append(predicates, filter.InAnyChannel(filteredChannels...))
109+
preUpgradePredicates = append(preUpgradePredicates, filter.InAnyChannel(filteredChannels...))
107110
}
108111

109112
if versionRangeConstraints != nil {
110-
predicates = append(predicates, filter.InSemverRange(versionRangeConstraints))
113+
preUpgradePredicates = append(preUpgradePredicates, filter.InSemverRange(versionRangeConstraints))
114+
}
115+
116+
// Apply pre-upgrade predicates to track bundles before upgrade constraints
117+
bundlesBeforeUpgradeConstraints := slices.Clone(packageFBC.Bundles)
118+
bundlesBeforeUpgradeConstraints = filterutil.InPlace(bundlesBeforeUpgradeConstraints, filterutil.And(preUpgradePredicates...))
119+
120+
// Track bundles before upgrade constraints for better error messages
121+
if len(bundlesBeforeUpgradeConstraints) > 0 && ext.Spec.Source.Catalog.UpgradeConstraintPolicy != ocv1.UpgradeConstraintPolicySelfCertified && installedBundle != nil {
122+
// Sort to get the best bundle
123+
slices.SortStableFunc(bundlesBeforeUpgradeConstraints, compare.ByVersionAndRelease)
124+
if len(bundlesBeforeUpgradeConstraints) > 0 {
125+
bundlesBeforeUpgrade = append(bundlesBeforeUpgrade, foundBundle{&bundlesBeforeUpgradeConstraints[0], cat.GetName(), cat.Spec.Priority})
126+
}
111127
}
112128

129+
// Now apply upgrade constraints
130+
var predicates []filterutil.Predicate[declcfg.Bundle]
131+
predicates = append(predicates, preUpgradePredicates...)
132+
113133
if ext.Spec.Source.Catalog.UpgradeConstraintPolicy != ocv1.UpgradeConstraintPolicySelfCertified && installedBundle != nil {
114134
successorPredicate, err := filter.SuccessorsOf(*installedBundle, packageFBC.Channels...)
115135
if err != nil {
116136
return fmt.Errorf("error finding upgrade edges: %w", err)
117137
}
118138
predicates = append(predicates, successorPredicate)
139+
140+
// Also collect all successors (without version constraints) for suggestions
141+
allSuccessorBundles := slices.Clone(packageFBC.Bundles)
142+
var successorOnlyPredicates []filterutil.Predicate[declcfg.Bundle]
143+
if len(channels) > 0 {
144+
channelSet := sets.New(channels...)
145+
filteredChannels := slices.DeleteFunc(packageFBC.Channels, func(c declcfg.Channel) bool {
146+
return !channelSet.Has(c.Name)
147+
})
148+
successorOnlyPredicates = append(successorOnlyPredicates, filter.InAnyChannel(filteredChannels...))
149+
}
150+
successorOnlyPredicates = append(successorOnlyPredicates, successorPredicate)
151+
allSuccessorBundles = filterutil.InPlace(allSuccessorBundles, filterutil.And(successorOnlyPredicates...))
152+
for i := range allSuccessorBundles {
153+
allSuccessors = append(allSuccessors, foundBundle{&allSuccessorBundles[i], cat.GetName(), cat.Spec.Priority})
154+
}
119155
}
120156

121157
// Apply the predicates to get the candidate bundles
@@ -182,11 +218,13 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio
182218
if len(resolvedBundles) != 1 {
183219
l.Info("resolution failed", "stats", catStats)
184220
return nil, nil, nil, resolutionError{
185-
PackageName: packageName,
186-
Version: versionRange,
187-
Channels: channels,
188-
InstalledBundle: installedBundle,
189-
ResolvedBundles: resolvedBundles,
221+
PackageName: packageName,
222+
Version: versionRange,
223+
Channels: channels,
224+
InstalledBundle: installedBundle,
225+
ResolvedBundles: resolvedBundles,
226+
BundlesBeforeUpgrade: bundlesBeforeUpgrade,
227+
AllSuccessors: allSuccessors,
190228
}
191229
}
192230
resolvedBundle := resolvedBundles[0].bundle
@@ -210,11 +248,13 @@ func (r *CatalogResolver) Resolve(ctx context.Context, ext *ocv1.ClusterExtensio
210248
}
211249

212250
type resolutionError struct {
213-
PackageName string
214-
Version string
215-
Channels []string
216-
InstalledBundle *ocv1.BundleMetadata
217-
ResolvedBundles []foundBundle
251+
PackageName string
252+
Version string
253+
Channels []string
254+
InstalledBundle *ocv1.BundleMetadata
255+
ResolvedBundles []foundBundle
256+
BundlesBeforeUpgrade []foundBundle // Bundles that matched before applying upgrade constraints
257+
AllSuccessors []foundBundle // All successor bundles found (for suggestions)
218258
}
219259

220260
func (rei resolutionError) Error() string {
@@ -223,13 +263,29 @@ func (rei resolutionError) Error() string {
223263
sb.WriteString(fmt.Sprintf("error upgrading from currently installed version %q: ", rei.InstalledBundle.Version))
224264
}
225265

226-
if len(rei.ResolvedBundles) > 1 {
266+
// Check if we have bundles that matched before upgrade constraints
267+
if len(rei.ResolvedBundles) == 0 && len(rei.BundlesBeforeUpgrade) > 0 {
268+
// Bundles exist matching the version range, but none are successors
269+
if rei.Version != "" {
270+
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))
271+
} else {
272+
sb.WriteString(fmt.Sprintf("no successor of %q found for package %q", rei.InstalledBundle.Version, rei.PackageName))
273+
}
274+
275+
// Add suggestions for best successors if available
276+
if len(rei.AllSuccessors) > 0 {
277+
suggestions := rei.findBestSuccessors()
278+
if suggestions != "" {
279+
sb.WriteString(fmt.Sprintf(". %s", suggestions))
280+
}
281+
}
282+
} else if len(rei.ResolvedBundles) > 1 {
227283
sb.WriteString(fmt.Sprintf("found bundles for package %q ", rei.PackageName))
228284
} else {
229285
sb.WriteString(fmt.Sprintf("no bundles found for package %q ", rei.PackageName))
230286
}
231287

232-
if rei.Version != "" {
288+
if rei.Version != "" && !(len(rei.ResolvedBundles) == 0 && len(rei.BundlesBeforeUpgrade) > 0) {
233289
sb.WriteString(fmt.Sprintf("matching version %q ", rei.Version))
234290
}
235291

@@ -249,6 +305,68 @@ func (rei resolutionError) Error() string {
249305
return strings.TrimSpace(sb.String())
250306
}
251307

308+
// findBestSuccessors finds the highest version successors in different streams
309+
func (rei resolutionError) findBestSuccessors() string {
310+
if len(rei.AllSuccessors) == 0 {
311+
return ""
312+
}
313+
314+
// Parse installed version
315+
installedVer, err := bsemver.Parse(rei.InstalledBundle.Version)
316+
if err != nil {
317+
return ""
318+
}
319+
320+
var zStreamHighest *declcfg.Bundle
321+
var yStreamHighest *declcfg.Bundle
322+
323+
for _, fb := range rei.AllSuccessors {
324+
bundleVer, err := bundleutil.GetVersionAndRelease(*fb.bundle)
325+
if err != nil {
326+
continue
327+
}
328+
329+
// Z-stream: same major.minor, different patch
330+
if bundleVer.Version.Major == installedVer.Major && bundleVer.Version.Minor == installedVer.Minor {
331+
if zStreamHighest == nil {
332+
zStreamHighest = fb.bundle
333+
} else {
334+
currentHighest, _ := bundleutil.GetVersionAndRelease(*zStreamHighest)
335+
if bundleVer.Compare(*currentHighest) > 0 {
336+
zStreamHighest = fb.bundle
337+
}
338+
}
339+
}
340+
341+
// Y-stream: same major, different minor
342+
if bundleVer.Version.Major == installedVer.Major && bundleVer.Version.Minor != installedVer.Minor {
343+
if yStreamHighest == nil {
344+
yStreamHighest = fb.bundle
345+
} else {
346+
currentHighest, _ := bundleutil.GetVersionAndRelease(*yStreamHighest)
347+
if bundleVer.Compare(*currentHighest) > 0 {
348+
yStreamHighest = fb.bundle
349+
}
350+
}
351+
}
352+
}
353+
354+
var suggestions []string
355+
if yStreamHighest != nil {
356+
yVer, _ := bundleutil.GetVersionAndRelease(*yStreamHighest)
357+
suggestions = append(suggestions, fmt.Sprintf("%q (y-stream)", yVer.AsLegacyRegistryV1Version().String()))
358+
}
359+
if zStreamHighest != nil {
360+
zVer, _ := bundleutil.GetVersionAndRelease(*zStreamHighest)
361+
suggestions = append(suggestions, fmt.Sprintf("%q (z-stream)", zVer.AsLegacyRegistryV1Version().String()))
362+
}
363+
364+
if len(suggestions) > 0 {
365+
return fmt.Sprintf("Highest version successors of %q are %s", rei.InstalledBundle.Version, strings.Join(suggestions, " and "))
366+
}
367+
return ""
368+
}
369+
252370
func isDeprecated(bundle declcfg.Bundle, deprecation *declcfg.Deprecation) bool {
253371
if deprecation == nil {
254372
return false

internal/operator-controller/resolve/catalog_test.go

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -526,6 +526,50 @@ func TestDowngradeNotFound(t *testing.T) {
526526
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))
527527
}
528528

529+
func TestIssue1022DowngradeNotSuccessor(t *testing.T) {
530+
pkgName := randPkg()
531+
532+
// Create a package similar to cockroachdb with versions 6.0.0, 6.0.1, etc.
533+
fbc := &declcfg.DeclarativeConfig{
534+
Packages: []declcfg.Package{{Name: pkgName}},
535+
Channels: []declcfg.Channel{
536+
{Package: pkgName, Name: "stable", Entries: []declcfg.ChannelEntry{
537+
{Name: bundleName(pkgName, "6.0.0")},
538+
{Name: bundleName(pkgName, "6.0.1"), Replaces: bundleName(pkgName, "6.0.0")},
539+
{Name: bundleName(pkgName, "6.0.10"), Replaces: bundleName(pkgName, "6.0.1")},
540+
{Name: bundleName(pkgName, "6.3.11"), SkipRange: ">=6.0.0 <6.3.11"},
541+
}},
542+
},
543+
Bundles: []declcfg.Bundle{
544+
genBundle(pkgName, "6.0.0"),
545+
genBundle(pkgName, "6.0.1"),
546+
genBundle(pkgName, "6.0.10"),
547+
genBundle(pkgName, "6.3.11"),
548+
},
549+
}
550+
551+
w := staticCatalogWalker{
552+
"catalog": func() (*declcfg.DeclarativeConfig, *ocv1.ClusterCatalogSpec, error) {
553+
return fbc, nil, nil
554+
},
555+
}
556+
557+
r := CatalogResolver{WalkCatalogsFunc: w.WalkCatalogs}
558+
ce := buildFooClusterExtension(pkgName, []string{}, "6.0.0", ocv1.UpgradeConstraintPolicyCatalogProvided)
559+
installedBundle := &ocv1.BundleMetadata{
560+
Name: bundleName(pkgName, "6.0.1"),
561+
Version: "6.0.1",
562+
}
563+
564+
// Try to downgrade to 6.0.0, which exists but is not a successor
565+
_, _, _, err := r.Resolve(context.Background(), ce, installedBundle)
566+
567+
// The new error message should be more helpful
568+
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)
569+
assert.EqualError(t, err, expectedMsg)
570+
}
571+
572+
529573
func TestCatalogWalker(t *testing.T) {
530574
t.Run("error listing catalogs", func(t *testing.T) {
531575
w := CatalogWalker(

0 commit comments

Comments
 (0)