handle deprecating branch points (#767)

* do not add skips/replaces entries for deprecated bundles

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

* remove overwritten channel head before add in overwrite-latest mode - preserve upgrade graph after add

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

* handle branchpoint deprecation

Remove deprecated bundles from channels of the deprecated bundles,
truncating only when a bundle is not on any channel. Branchpoints (i.e,
branches that have two or more different replacements on different
channels) are removed from any channel that the deprecated bundle was
on, while everything till the first branch point is truncated

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

* fix unit tests, add function documentation

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

* remove deprecated channel heads on package delete, simplify populator signature

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

* optional interface for RemoveOverwrittenChannelHead

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

* comment changes

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

Upstream-repository: operator-registry
Upstream-commit: 38994ece779c79a6b238cff667810892c6df0d4e

Author: ankitathomas

staging/operator-registry/alpha/action/render_test.go

@@ -792,7 +792,7 @@ func generateSqliteFile(path string, imageMap map[image.Reference]string) error
 		return err
 	}
 
-	populator := registry.NewDirectoryPopulator(loader, graphLoader, dbQuerier, imageMap, nil, false)
+	populator := registry.NewDirectoryPopulator(loader, graphLoader, dbQuerier, imageMap, nil)
 	if err := populator.Populate(registry.ReplacesMode); err != nil {
 		return err
 	}

staging/operator-registry/pkg/lib/registry/registry.go

@@ -128,23 +128,23 @@ func unpackImage(ctx context.Context, reg image.Registry, ref image.Reference) (
 
 func populate(ctx context.Context, loader registry.Load, graphLoader registry.GraphLoader, querier registry.Query, reg image.Registry, refs []image.Reference, mode registry.Mode, overwrite bool) error {
 	unpackedImageMap := make(map[image.Reference]string, 0)
+	overwrittenBundles := map[string][]string{}
+	var imagesToAdd []*registry.Bundle
 	for _, ref := range refs {
 		to, from, cleanup, err := unpackImage(ctx, reg, ref)
 		if err != nil {
 			return err
 		}
 		unpackedImageMap[to] = from
 		defer cleanup()
-	}
 
-	overwriteImageMap := make(map[string]map[image.Reference]string, 0)
-	if overwrite {
-		// find all bundles that are attempting to overwrite
-		for to, from := range unpackedImageMap {
-			img, err := registry.NewImageInput(to, from)
-			if err != nil {
-				return err
-			}
+		img, err := registry.NewImageInput(to, from)
+		if err != nil {
+			return err
+		}
+		imagesToAdd = append(imagesToAdd, img.Bundle)
+
+		if overwrite {
 			overwritten, err := querier.GetBundlePathIfExists(ctx, img.Bundle.Name)
 			if err != nil {
 				if err == registry.ErrBundleImageNotInDatabase {
@@ -155,57 +155,24 @@ func populate(ctx context.Context, loader registry.Load, graphLoader registry.Gr
 			if overwritten == "" {
 				return fmt.Errorf("index add --overwrite-latest is only supported when using bundle images")
 			}
-			// get all bundle paths for that package - we will re-add these to regenerate the graph
-			bundles, err := querier.GetBundlesForPackage(ctx, img.Bundle.Package)
-			if err != nil {
-				return err
-			}
-			type unpackedImage struct {
-				to      image.Reference
-				from    string
-				cleanup func()
-				err     error
-			}
-			unpacked := make(chan unpackedImage)
-			for bundle := range bundles {
-				// parallelize image pulls
-				go func(bundle registry.BundleKey, img *registry.ImageInput) {
-					if bundle.CsvName != img.Bundle.Name {
-						to, from, cleanup, err := unpackImage(ctx, reg, image.SimpleReference(bundle.BundlePath))
-						unpacked <- unpackedImage{to: to, from: from, cleanup: cleanup, err: err}
-					} else {
-						unpacked <- unpackedImage{to: to, from: from, cleanup: func() { return }, err: nil}
-					}
-				}(bundle, img)
-			}
-			if _, ok := overwriteImageMap[img.Bundle.Package]; !ok {
-				overwriteImageMap[img.Bundle.Package] = make(map[image.Reference]string, 0)
-			}
-			for i := 0; i < len(bundles); i++ {
-				unpack := <-unpacked
-				if unpack.err != nil {
-					return unpack.err
-				}
-				overwriteImageMap[img.Bundle.Package][unpack.to] = unpack.from
-				if _, ok := unpackedImageMap[unpack.to]; ok {
-					delete(unpackedImageMap, unpack.to)
-				}
-				defer unpack.cleanup()
-			}
+			overwrittenBundles[img.Bundle.Package] = append(overwrittenBundles[img.Bundle.Package], img.Bundle.Name)
 		}
 	}
 
-	populator := registry.NewDirectoryPopulator(loader, graphLoader, querier, unpackedImageMap, overwriteImageMap, overwrite)
-	if err := populator.Populate(mode); err != nil {
+	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
 
-	for _, imgMap := range overwriteImageMap {
-		for to, from := range imgMap {
-			unpackedImageMap[to] = from
-		}
 	}
-	return checkForBundles(ctx, querier.(*sqlite.SQLQuerier), graphLoader, unpackedImageMap)
+	return checkForBundles(ctx, querier.(*sqlite.SQLQuerier), graphLoader, expectedBundles)
 }
 
 type DeleteFromRegistryRequest struct {
@@ -291,7 +258,7 @@ func (r RegistryUpdater) PruneFromRegistry(request PruneFromRegistryRequest) err
 	}
 	defer db.Close()
 
-	dbLoader, err := sqlite.NewSQLLiteLoader(db)
+	dbLoader, err := sqlite.NewDeprecationAwareLoader(db)
 	if err != nil {
 		return err
 	}
@@ -344,7 +311,7 @@ func (r RegistryUpdater) DeprecateFromRegistry(request DeprecateFromRegistryRequ
 	}
 	defer db.Close()
 
-	dbLoader, err := sqlite.NewSQLLiteLoader(db)
+	dbLoader, err := sqlite.NewDeprecationAwareLoader(db)
 	if err != nil {
 		return err
 	}
@@ -430,55 +397,10 @@ func checkForBundlePaths(querier registry.GRPCQuery, bundlePaths []string) ([]st
 	return found, missing, nil
 }
 
-// packagesFromUnpackedRefs creates packages from a set of unpacked ref dirs without their upgrade edges.
-func packagesFromUnpackedRefs(bundles map[image.Reference]string) (map[string]registry.Package, error) {
-	graph := map[string]registry.Package{}
-	for to, from := range bundles {
-		b, err := registry.NewImageInput(to, from)
-		if err != nil {
-			return nil, fmt.Errorf("failed to parse unpacked bundle image %s: %v", to, err)
-		}
-		v, err := b.Bundle.Version()
-		if err != nil {
-			return nil, fmt.Errorf("failed to parse version for %s (%s): %v", b.Bundle.Name, b.Bundle.BundleImage, err)
-		}
-		key := registry.BundleKey{
-			CsvName:    b.Bundle.Name,
-			Version:    v,
-			BundlePath: b.Bundle.BundleImage,
-		}
-		if _, ok := graph[b.Bundle.Package]; !ok {
-			graph[b.Bundle.Package] = registry.Package{
-				Name:     b.Bundle.Package,
-				Channels: map[string]registry.Channel{},
-			}
-		}
-		for _, c := range b.Bundle.Channels {
-			if _, ok := graph[b.Bundle.Package].Channels[c]; !ok {
-				graph[b.Bundle.Package].Channels[c] = registry.Channel{
-					Nodes: map[registry.BundleKey]map[registry.BundleKey]struct{}{},
-				}
-			}
-			graph[b.Bundle.Package].Channels[c].Nodes[key] = nil
-		}
-	}
-
-	return graph, nil
-}
-
 // 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, bundles map[image.Reference]string) error {
-	if len(bundles) == 0 {
-		return nil
-	}
-
-	required, err := packagesFromUnpackedRefs(bundles)
-	if err != nil {
-		return err
-	}
-
+func checkForBundles(ctx context.Context, q *sqlite.SQLQuerier, g registry.GraphLoader, required map[string]*registry.Package) error {
 	var errs []error
 	for _, pkg := range required {
 		graph, err := g.Generate(pkg.Name)
@@ -523,3 +445,75 @@ func isDeprecated(ctx context.Context, q *sqlite.SQLQuerier, bundle registry.Bun
 	}
 	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{},
+				}
+			}
+		}
+		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
+}