Semver lastedge error (operator-framework#1169)

* when the last edge in a bundle set is +Y the existing code does not clear the skips list and crosses Y thresholds counter to design intent missing proper last-max-z detection for replaces working at long last * re-add cumulative skips over y changes in an x stream so folks can always skip directly to channel head Signed-off-by: Jordan Keister <jordan@nimblewidget.com> --------- Signed-off-by: Jordan Keister <jordan@nimblewidget.com>
grokspawn · Mar 5, 2024 · 2096c4f · 2096c4f
1 parent a21f962
commit 2096c4f
Show file tree

Hide file tree

Showing 3 changed files with 233 additions and 119 deletions.
diff --git a/alpha/template/semver/semver.go b/alpha/template/semver/semver.go
@@ -6,14 +6,13 @@ import (
 	"io"
 	"sort"
 
-	"github.com/blang/semver/v4"
-	"k8s.io/apimachinery/pkg/util/errors"
-	"k8s.io/apimachinery/pkg/util/sets"
-	"sigs.k8s.io/yaml"
-
 	"github.com/operator-framework/operator-registry/alpha/action"
 	"github.com/operator-framework/operator-registry/alpha/declcfg"
 	"github.com/operator-framework/operator-registry/alpha/property"
+
+	"github.com/blang/semver/v4"
+	"k8s.io/apimachinery/pkg/util/errors"
+	"sigs.k8s.io/yaml"
 )
 
 func (t Template) Render(ctx context.Context) (*declcfg.DeclarativeConfig, error) {
@@ -224,7 +223,6 @@ func (sv *semverTemplate) generateChannels(semverChannels *bundleVersions) []dec
 	hwc := highwaterChannel{archetype: archetypesByPriority[0], version: semver.Version{Major: 0, Minor: 0}}
 
 	unlinkedChannels := make(map[string]*declcfg.Channel)
-	unassociatedEdges := []entryTuple{}
 
 	for _, archetype := range archetypesByPriority {
 		bundles := (*semverChannels)[archetype]
@@ -272,88 +270,84 @@ func (sv *semverTemplate) generateChannels(semverChannels *bundleVersions) []dec
 					}
 				}
 				ch.Entries = append(ch.Entries, declcfg.ChannelEntry{Name: bundleName})
-				unassociatedEdges = append(unassociatedEdges, entryTuple{arch: archetype, kind: cKey, parent: cName, name: bundleName, version: bundles[bundleName], index: len(ch.Entries) - 1})
 			}
 		}
 	}
 
 	// save off the name of the high-water-mark channel for the default for this package
 	sv.defaultChannel = hwc.name
 
-	outChannels = append(outChannels, sv.linkChannels(unlinkedChannels, unassociatedEdges)...)
+	outChannels = append(outChannels, sv.linkChannels(unlinkedChannels, semverChannels)...)
 
 	return outChannels
 }
 
-func (sv *semverTemplate) linkChannels(unlinkedChannels map[string]*declcfg.Channel, entries []entryTuple) []declcfg.Channel {
+func (sv *semverTemplate) linkChannels(unlinkedChannels map[string]*declcfg.Channel, harvestedVersions *bundleVersions) []declcfg.Channel {
 	channels := []declcfg.Channel{}
 
-	// sort to force partitioning by archetype --> kind --> semver
-	sort.Slice(entries, func(i, j int) bool {
-		if channelPriorities[entries[i].arch] != channelPriorities[entries[j].arch] {
-			return channelPriorities[entries[i].arch] < channelPriorities[entries[j].arch]
-		}
-		if streamTypePriorities[entries[i].kind] != streamTypePriorities[entries[j].kind] {
-			return streamTypePriorities[entries[i].kind] < streamTypePriorities[entries[j].kind]
-		}
-		return entries[i].version.LT(entries[j].version)
-	})
-
-	prevZMax := ""
-	var curSkips sets.String = sets.NewString()
-
-	for index := 1; index < len(entries); index++ {
-		prevTuple := entries[index-1]
-		curTuple := entries[index]
-		prevX := getMajorVersion(prevTuple.version)
-		prevY := getMinorVersion(prevTuple.version)
-		curX := getMajorVersion(curTuple.version)
-		curY := getMinorVersion(curTuple.version)
-
-		archChange := curTuple.arch != prevTuple.arch
-		kindChange := curTuple.kind != prevTuple.kind
-		xChange := !prevX.EQ(curX)
-		yChange := !prevY.EQ(curY)
-
-		if archChange || kindChange || xChange || yChange {
-			// if we passed any kind of change besides Z, then we need to set skips/replaces for previous max-Z
-			prevChannel := unlinkedChannels[prevTuple.parent]
-			finalEntry := &prevChannel.Entries[prevTuple.index]
-			finalEntry.Replaces = prevZMax
-			// don't include replaces in skips list, but they are accumulated in discrete cycles (and maybe useful for later channels) so remove here
-			if curSkips.Has(finalEntry.Replaces) {
-				finalEntry.Skips = curSkips.Difference(sets.NewString(finalEntry.Replaces)).List()
-			} else {
-				finalEntry.Skips = curSkips.List()
-			}
-		}
-
-		if archChange || kindChange || xChange {
-			// we don't maintain skips/replaces over these transitions
-			curSkips = sets.NewString()
-			prevZMax = ""
-		} else {
-			if yChange {
-				prevZMax = prevTuple.name
+	// bundle --> version lookup
+	bundleVersions := make(map[string]semver.Version)
+	for _, vs := range *harvestedVersions {
+		for b, v := range vs {
+			if _, ok := bundleVersions[b]; !ok {
+				bundleVersions[b] = v
 			}
-			curSkips.Insert(prevTuple.name)
 		}
 	}
 
-	// last entry accumulation
-	lastTuple := entries[len(entries)-1]
-	prevChannel := unlinkedChannels[lastTuple.parent]
-	finalEntry := &prevChannel.Entries[lastTuple.index]
-	finalEntry.Replaces = prevZMax
-	// don't include replaces in skips list, but they are accumulated in discrete cycles (and maybe useful for later channels) so remove here
-	if curSkips.Has(finalEntry.Replaces) {
-		finalEntry.Skips = curSkips.Difference(sets.NewString(finalEntry.Replaces)).List()
-	} else {
-		finalEntry.Skips = curSkips.List()
-	}
+	for _, channel := range unlinkedChannels {
+		entries := &channel.Entries
+		sort.Slice(*entries, func(i, j int) bool {
+			return bundleVersions[(*entries)[i].Name].LT(bundleVersions[(*entries)[j].Name])
+		})
 
-	for _, ch := range unlinkedChannels {
-		channels = append(channels, *ch)
+		// "inchworm" through the sorted entries, iterating curEdge but extending yProbe to the next Y-transition
+		// then catch up curEdge to yProbe as 'skips', and repeat until we reach the end of the entries
+		// finally, because the inchworm will always fail to pick up the last Y-transition, we test for it and link it up as a 'replaces'
+		curEdge, yProbe := 0, 0
+		zmaxQueue := ""
+		entryCount := len(*entries)
+
+		for curEdge < entryCount {
+			for yProbe < entryCount {
+				curVersion := bundleVersions[(*entries)[curEdge].Name]
+				yProbeVersion := bundleVersions[(*entries)[yProbe].Name]
+				if getMinorVersion(yProbeVersion).EQ(getMinorVersion(curVersion)) {
+					yProbe += 1
+				} else {
+					break
+				}
+			}
+			// if yProbe crossed a threshold, the previous entry is the last of the previous Y-stream
+			preChangeIndex := yProbe - 1
+
+			if curEdge != yProbe {
+				if zmaxQueue != "" {
+					// add skips edge to allow skipping over y iterations within an x stream
+					(*entries)[preChangeIndex].Skips = append((*entries)[preChangeIndex].Skips, zmaxQueue)
+					(*entries)[preChangeIndex].Replaces = zmaxQueue
+				}
+				zmaxQueue = (*entries)[preChangeIndex].Name
+			}
+			for curEdge < preChangeIndex {
+				// add skips edges to y-1 from z < y
+				(*entries)[preChangeIndex].Skips = append((*entries)[preChangeIndex].Skips, (*entries)[curEdge].Name)
+				curEdge += 1
+			}
+			curEdge += 1
+			yProbe = curEdge + 1
+		}
+		// since probe will always fail to pick up a y-change in the last item, test for it
+		if entryCount > 1 {
+			penultimateEntry := &(*entries)[len(*entries)-2]
+			ultimateEntry := &(*entries)[len(*entries)-1]
+			penultimateVersion := bundleVersions[penultimateEntry.Name]
+			ultimateVersion := bundleVersions[ultimateEntry.Name]
+			if ultimateVersion.Minor != penultimateVersion.Minor {
+				ultimateEntry.Replaces = penultimateEntry.Name
+			}
+		}
+		channels = append(channels, *channel)
 	}
 
 	return channels