Skip to content

Commit d16e083

Browse files
committed
check only newly added bundles for pruning
Signed-off-by: Ankita Thomas <ankithom@redhat.com>
1 parent d3be374 commit d16e083

File tree

2 files changed

+91
-305
lines changed

2 files changed

+91
-305
lines changed

pkg/lib/registry/registry.go

Lines changed: 43 additions & 100 deletions
Original file line numberDiff line numberDiff line change
@@ -159,20 +159,14 @@ func populate(ctx context.Context, loader registry.Load, graphLoader registry.Gr
159159
}
160160
}
161161

162-
expectedBundles, err := expectedGraphBundles(imagesToAdd, graphLoader, overwrite)
163-
if err != nil {
164-
165-
return err
166-
167-
}
168162
populator := registry.NewDirectoryPopulator(loader, graphLoader, querier, unpackedImageMap, overwrittenBundles)
169163

170164
if err := populator.Populate(mode); err != nil {
171165

172166
return err
173167

174168
}
175-
return checkForBundles(ctx, querier.(*sqlite.SQLQuerier), graphLoader, expectedBundles)
169+
return checkForBundles(ctx, querier.(*sqlite.SQLQuerier), graphLoader, imagesToAdd)
176170
}
177171

178172
type DeleteFromRegistryRequest struct {
@@ -399,38 +393,59 @@ func checkForBundlePaths(querier registry.GRPCQuery, bundlePaths []string) ([]st
399393

400394
// replaces mode selects highest version as channel head and
401395
// prunes any bundles in the upgrade chain after the channel head.
402-
// check for the presence of all bundles after a replaces-mode add.
403-
func checkForBundles(ctx context.Context, q *sqlite.SQLQuerier, g registry.GraphLoader, required map[string]*registry.Package) error {
396+
// check for the presence of newly added bundles after a replaces-mode add.
397+
func checkForBundles(ctx context.Context, q *sqlite.SQLQuerier, g registry.GraphLoader, required []*registry.Bundle) error {
404398
var errs []error
405-
for _, pkg := range required {
406-
graph, err := g.Generate(pkg.Name)
407-
if err != nil {
408-
errs = append(errs, fmt.Errorf("unable to verify added bundles for package %s: %v", pkg.Name, err))
409-
continue
410-
}
399+
fmt.Println("required:", required)
400+
checkRequired:
401+
for _, bundle := range required {
402+
var graph *registry.Package
403+
var deprecated *bool
404+
var err error
405+
checkForBundleInChannel:
406+
for _, channel := range bundle.Channels {
407+
if graph == nil {
408+
graph, err = g.Generate(bundle.Package)
409+
if err != nil {
410+
errs = append(errs, fmt.Errorf("unable to verify added bundles for package %s: %v", bundle.Package, err))
411+
continue
412+
}
413+
}
411414

412-
for channel, missing := range pkg.Channels {
413-
// trace replaces chain for reachable bundles
414415
for next := []registry.BundleKey{graph.Channels[channel].Head}; len(next) > 0; next = next[1:] {
415-
delete(missing.Nodes, next[0])
416+
if next[0].BundlePath == bundle.BundleImage {
417+
fmt.Println("found", bundle.Name)
418+
continue checkForBundleInChannel
419+
}
416420
for edge := range graph.Channels[channel].Nodes[next[0]] {
417421
next = append(next, edge)
418422
}
419423
}
420424

421-
for bundle := range missing.Nodes {
422-
// check if bundle is deprecated. Bundles readded after deprecation should not be present in index and can be ignored.
423-
deprecated, err := isDeprecated(ctx, q, bundle)
425+
if deprecated == nil {
426+
version, err := bundle.Version()
424427
if err != nil {
425-
errs = append(errs, fmt.Errorf("could not validate pruned bundle %s (%s) as deprecated: %v", bundle.CsvName, bundle.BundlePath, err))
428+
errs = append(errs, fmt.Errorf("could not check pruned bundle %s (%s) for deprecation: %v", bundle.Name, bundle.BundleImage, err))
429+
continue checkRequired
426430
}
427-
if !deprecated {
428-
headSkips := []string{}
429-
for b := range graph.Channels[channel].Nodes[graph.Channels[channel].Head] {
430-
headSkips = append(headSkips, b.CsvName)
431-
}
432-
errs = append(errs, fmt.Errorf("add prunes bundle %s (%s, %s) from package %s, channel %s: this may be due to incorrect channel head (%s, skips/replaces %v)", bundle.CsvName, bundle.Version, bundle.BundlePath, pkg.Name, channel, graph.Channels[channel].Head.CsvName, headSkips))
431+
depr, err := isDeprecated(ctx, q, registry.BundleKey{
432+
BundlePath: bundle.BundleImage,
433+
Version: version,
434+
CsvName: bundle.Name,
435+
})
436+
if err != nil {
437+
errs = append(errs, fmt.Errorf("could not validate pruned bundle %s (%s) as deprecated: %v", bundle.Name, bundle.BundleImage, err))
438+
continue checkRequired
439+
}
440+
deprecated = &depr
441+
}
442+
443+
if !*deprecated {
444+
headSkips := []string{}
445+
for b := range graph.Channels[channel].Nodes[graph.Channels[channel].Head] {
446+
headSkips = append(headSkips, b.CsvName)
433447
}
448+
errs = append(errs, fmt.Errorf("add prunes bundle %s (%s) from package %s, channel %s: this may be due to incorrect channel head (%s, skips/replaces %v)", bundle.Name, bundle.BundleImage, bundle.Package, channel, graph.Channels[channel].Head.CsvName, headSkips))
434449
}
435450
}
436451
}
@@ -449,75 +464,3 @@ func isDeprecated(ctx context.Context, q *sqlite.SQLQuerier, bundle registry.Bun
449464
}
450465
return false, nil
451466
}
452-
453-
// expectedGraphBundles returns a set of package-channel-bundle tuples that MUST be present following an add.
454-
/* opm index add drops bundles that replace a channel head, and since channel head selection heuristics
455-
* choose the bundle with the greatest semver as the channel head, any bundle that replaces such a bundle
456-
* will be dropped from the graph following an add.
457-
* eg: 1.0.1 <- 1.0.1-new
458-
*
459-
* 1.0.1-new replaces 1.0.1 but will not be chosen as the channel head because of its non-empty pre-release version.
460-
* expectedGraphBundles gives a set of bundles (old bundles from the graphLoader and the newly added set of bundles from
461-
* imagesToAdd) that must be present following an add to ensure no bundle is dropped.
462-
*
463-
* Overwritten bundles will only be verified on the channels of the newly added version.
464-
* Any inherited channels due to addition of a new bundle on its tail bundles may not be verified
465-
* eg: 1.0.1 (alpha) <-[1.0.2 (alpha, stable)]
466-
* When 1.0.2 in alpha and stable channels is added replacing 1.0.1, 1.0.1's presence will only be marked as expected on
467-
* the alpha channel, not on the inherited stable channel.
468-
*/
469-
func expectedGraphBundles(imagesToAdd []*registry.Bundle, graphLoader registry.GraphLoader, overwrite bool) (map[string]*registry.Package, error) {
470-
expectedBundles := map[string]*registry.Package{}
471-
for _, bundle := range imagesToAdd {
472-
version, err := bundle.Version()
473-
if err != nil {
474-
return nil, err
475-
}
476-
newBundleKey := registry.BundleKey{
477-
BundlePath: bundle.BundleImage,
478-
Version: version,
479-
CsvName: bundle.Name,
480-
}
481-
var pkg *registry.Package
482-
var ok bool
483-
if pkg, ok = expectedBundles[bundle.Package]; !ok {
484-
var err error
485-
if pkg, err = graphLoader.Generate(bundle.Package); err != nil {
486-
if err != registry.ErrPackageNotInDatabase {
487-
return nil, err
488-
}
489-
pkg = &registry.Package{
490-
Name: bundle.Package,
491-
Channels: map[string]registry.Channel{},
492-
}
493-
}
494-
}
495-
for c, channelEntries := range pkg.Channels {
496-
for oldBundle := range channelEntries.Nodes {
497-
if oldBundle.CsvName == bundle.Name {
498-
if overwrite {
499-
delete(pkg.Channels[c].Nodes, oldBundle)
500-
if len(pkg.Channels[c].Nodes) == 0 {
501-
delete(pkg.Channels, c)
502-
}
503-
} else {
504-
return nil, registry.BundleImageAlreadyAddedErr{ErrorString: fmt.Sprintf("Bundle %s already exists", bundle.BundleImage)}
505-
}
506-
}
507-
}
508-
}
509-
for _, c := range bundle.Channels {
510-
if _, ok := pkg.Channels[c]; !ok {
511-
pkg.Channels[c] = registry.Channel{
512-
Nodes: map[registry.BundleKey]map[registry.BundleKey]struct{}{},
513-
}
514-
}
515-
// This can miss out on some channels, when a new bundle has channels that the one it replaces does not.
516-
// eg: When bundle A in channel A replaces bundle B in channel B is added, bundle B is also added to channel A
517-
// but it is only expected to be in channel B, presence in channel A will be ignored
518-
pkg.Channels[c].Nodes[newBundleKey] = nil
519-
}
520-
expectedBundles[bundle.Package] = pkg
521-
}
522-
return expectedBundles, nil
523-
}

0 commit comments

Comments
 (0)