Check only for new bundles on add (#798)

* clarify pruned bundle message

Signed-off-by: Ankita Thomas <[email protected]>

* check only newly added bundles for pruning

Signed-off-by: Ankita Thomas <[email protected]>

* update tests, remove deprecation check on added bundles

Signed-off-by: Ankita Thomas <[email protected]>

Author: ankitathomas

pkg/lib/registry/registry.go

@@ -159,20 +159,14 @@ func populate(ctx context.Context, loader registry.Load, graphLoader registry.Gr
 		}
 	}
 
-	expectedBundles, err := expectedGraphBundles(imagesToAdd, graphLoader, overwrite)
-	if err != nil {
-
-		return err
-
-	}
 	populator := registry.NewDirectoryPopulator(loader, graphLoader, querier, unpackedImageMap, overwrittenBundles)
 
 	if err := populator.Populate(mode); err != nil {
 
 		return err
 
 	}
-	return checkForBundles(ctx, querier.(*sqlite.SQLQuerier), graphLoader, expectedBundles)
+	return checkForBundles(ctx, querier.(*sqlite.SQLQuerier), graphLoader, imagesToAdd)
 }
 
 type DeleteFromRegistryRequest struct {
@@ -399,121 +393,38 @@ func checkForBundlePaths(querier registry.GRPCQuery, bundlePaths []string) ([]st
 
 // replaces mode selects highest version as channel head and
 // prunes any bundles in the upgrade chain after the channel head.
-// check for the presence of all bundles after a replaces-mode add.
-func checkForBundles(ctx context.Context, q *sqlite.SQLQuerier, g registry.GraphLoader, required map[string]*registry.Package) error {
+// check for the presence of newly added bundles after a replaces-mode add.
+func checkForBundles(ctx context.Context, q *sqlite.SQLQuerier, g registry.GraphLoader, required []*registry.Bundle) error {
 	var errs []error
-	for _, pkg := range required {
-		graph, err := g.Generate(pkg.Name)
+	for _, bundle := range required {
+		graph, err := g.Generate(bundle.Package)
 		if err != nil {
-			errs = append(errs, fmt.Errorf("unable to verify added bundles for package %s: %v", pkg.Name, err))
+			errs = append(errs, fmt.Errorf("unable to verify added bundles for package %s: %v", bundle.Package, err))
 			continue
 		}
 
-		for channel, missing := range pkg.Channels {
-			// trace replaces chain for reachable bundles
+		for _, channel := range bundle.Channels {
+			var foundImage bool
 			for next := []registry.BundleKey{graph.Channels[channel].Head}; len(next) > 0; next = next[1:] {
-				delete(missing.Nodes, next[0])
+				if next[0].BundlePath == bundle.BundleImage {
+					foundImage = true
+					break
+				}
 				for edge := range graph.Channels[channel].Nodes[next[0]] {
 					next = append(next, edge)
 				}
 			}
 
-			for bundle := range missing.Nodes {
-				// check if bundle is deprecated. Bundles readded after deprecation should not be present in index and can be ignored.
-				deprecated, err := isDeprecated(ctx, q, bundle)
-				if err != nil {
-					errs = append(errs, fmt.Errorf("could not validate pruned bundle %s (%s) as deprecated: %v", bundle.CsvName, bundle.BundlePath, err))
-				}
-				if !deprecated {
-					errs = append(errs, fmt.Errorf("added bundle %s pruned from package %s, channel %s: this may be due to incorrect channel head (%s)", bundle.BundlePath, pkg.Name, channel, graph.Channels[channel].Head.CsvName))
-				}
+			if foundImage {
+				continue
 			}
-		}
-	}
-	return utilerrors.NewAggregate(errs)
-}
-
-func isDeprecated(ctx context.Context, q *sqlite.SQLQuerier, bundle registry.BundleKey) (bool, error) {
-	props, err := q.GetPropertiesForBundle(ctx, bundle.CsvName, bundle.Version, bundle.BundlePath)
-	if err != nil {
-		return false, err
-	}
-	for _, prop := range props {
-		if prop.Type == registry.DeprecatedType {
-			return true, nil
-		}
-	}
-	return false, nil
-}
 
-// expectedGraphBundles returns a set of package-channel-bundle tuples that MUST be present following an add.
-/* opm index add drops bundles that replace a channel head, and since channel head selection heuristics
-* choose the bundle with the greatest semver as the channel head, any bundle that replaces such a bundle
-* will be dropped from the graph following an add.
-* eg: 1.0.1 <- 1.0.1-new
-*
-* 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.
-* expectedGraphBundles gives a set of bundles (old bundles from the graphLoader and the newly added set of bundles from
-* imagesToAdd) that must be present following an add to ensure no bundle is dropped.
-*
-* Overwritten bundles will only be verified on the channels of the newly added version.
-* Any inherited channels due to addition of a new bundle on its tail bundles may not be verified
-* eg:  1.0.1 (alpha) <-[1.0.2 (alpha, stable)]
-* 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
-* the alpha channel, not on the inherited stable channel.
- */
-func expectedGraphBundles(imagesToAdd []*registry.Bundle, graphLoader registry.GraphLoader, overwrite bool) (map[string]*registry.Package, error) {
-	expectedBundles := map[string]*registry.Package{}
-	for _, bundle := range imagesToAdd {
-		version, err := bundle.Version()
-		if err != nil {
-			return nil, err
-		}
-		newBundleKey := registry.BundleKey{
-			BundlePath: bundle.BundleImage,
-			Version:    version,
-			CsvName:    bundle.Name,
-		}
-		var pkg *registry.Package
-		var ok bool
-		if pkg, ok = expectedBundles[bundle.Package]; !ok {
-			var err error
-			if pkg, err = graphLoader.Generate(bundle.Package); err != nil {
-				if err != registry.ErrPackageNotInDatabase {
-					return nil, err
-				}
-				pkg = &registry.Package{
-					Name:     bundle.Package,
-					Channels: map[string]registry.Channel{},
-				}
+			var headSkips []string
+			for b := range graph.Channels[channel].Nodes[graph.Channels[channel].Head] {
+				headSkips = append(headSkips, b.CsvName)
 			}
+			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))
 		}
-		for c, channelEntries := range pkg.Channels {
-			for oldBundle := range channelEntries.Nodes {
-				if oldBundle.CsvName == bundle.Name {
-					if overwrite {
-						delete(pkg.Channels[c].Nodes, oldBundle)
-						if len(pkg.Channels[c].Nodes) == 0 {
-							delete(pkg.Channels, c)
-						}
-					} else {
-						return nil, registry.BundleImageAlreadyAddedErr{ErrorString: fmt.Sprintf("Bundle %s already exists", bundle.BundleImage)}
-					}
-				}
-			}
-		}
-		for _, c := range bundle.Channels {
-			if _, ok := pkg.Channels[c]; !ok {
-				pkg.Channels[c] = registry.Channel{
-					Nodes: map[registry.BundleKey]map[registry.BundleKey]struct{}{},
-				}
-			}
-			// This can miss out on some channels, when a new bundle has channels that the one it replaces does not.
-			// eg: When bundle A in channel A replaces bundle B in channel B is added, bundle B is also added to channel A
-			// but it is only expected to be in channel B, presence in channel A will be ignored
-			pkg.Channels[c].Nodes[newBundleKey] = nil
-		}
-		expectedBundles[bundle.Package] = pkg
 	}
-	return expectedBundles, nil
+	return utilerrors.NewAggregate(errs)
 }