diff --git a/alpha/template/semver/semver.go b/alpha/template/semver/semver.go index 77b713737..8a4b837b4 100644 --- a/alpha/template/semver/semver.go +++ b/alpha/template/semver/semver.go @@ -10,6 +10,7 @@ import ( "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/declcfg" @@ -225,6 +226,7 @@ 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,6 +274,7 @@ 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}) } } } @@ -279,76 +282,95 @@ func (sv *semverTemplate) generateChannels(semverChannels *bundleVersions) []dec // 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, semverChannels)...) + outChannels = append(outChannels, sv.linkChannels(unlinkedChannels, unassociatedEdges)...) return outChannels } -func (sv *semverTemplate) linkChannels(unlinkedChannels map[string]*declcfg.Channel, harvestedVersions *bundleVersions) []declcfg.Channel { - // 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 - } - } - } +func (sv *semverTemplate) linkChannels(unlinkedChannels map[string]*declcfg.Channel, entries []entryTuple) []declcfg.Channel { + channels := []declcfg.Channel{} - channels := make([]declcfg.Channel, 0, len(unlinkedChannels)) - 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]) - }) + // 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) + }) - // "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 - } + prevZMax := "" + var curSkips = sets.Set[string]{} + + // iterate over the entries, starting from the second + // write any skips/replaces for the previous entry to the current entry + // then accumulate the skips/replaces for the current entry to be used in subsequent iterations + 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 + skips := sets.List(curSkips.Difference(sets.New(finalEntry.Replaces))) + if len(skips) > 0 { + finalEntry.Skips = skips } - // if yProbe crossed a threshold, the previous entry is the last of the previous Y-stream - preChangeIndex := yProbe - 1 + } - if curEdge != yProbe { - if zmaxQueue != "" { - (*entries)[preChangeIndex].Replaces = zmaxQueue - } - zmaxQueue = (*entries)[preChangeIndex].Name - } - for curEdge < preChangeIndex { - // add skips edges to y-1 from z < y - if (*entries)[preChangeIndex].Replaces != (*entries)[curEdge].Name { - (*entries)[preChangeIndex].Skips = append((*entries)[preChangeIndex].Skips, (*entries)[curEdge].Name) - } - curEdge += 1 + if archChange || kindChange || xChange { + // we don't maintain skips/replaces over these transitions + curSkips = sets.Set[string]{} + prevZMax = "" + } else { + if yChange { + prevZMax = prevTuple.name } - curEdge += 1 - yProbe = curEdge + 1 + curSkips.Insert(prevTuple.name) } - // 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 + } + + if len(entries) > 1 { + // add edges for the last entry + // note: this is substantially similar to the main iteration, but there are some subtle differences since the main loop mode + // design is to write the edges and then accumulate new info for subsequent edges (and this is the last edge): + // - we only need to watch for arch/kind/x change + // - we don't need to accumulate skips/replaces, since we're not writing edges for subsequent entries + lastTuple := entries[len(entries)-1] + penultimateTuple := entries[len(entries)-2] + prevX := getMajorVersion(penultimateTuple.version) + curX := getMajorVersion(lastTuple.version) + + archChange := penultimateTuple.arch != lastTuple.arch + kindChange := penultimateTuple.kind != lastTuple.kind + xChange := !prevX.EQ(curX) + // for arch / kind / x changes, we don't maintain skips/replaces + if !archChange && !kindChange && !xChange { + prevChannel := unlinkedChannels[lastTuple.parent] + finalEntry := &prevChannel.Entries[lastTuple.index] + finalEntry.Replaces = prevZMax + skips := sets.List(curSkips.Difference(sets.New(finalEntry.Replaces))) + if len(skips) > 0 { + finalEntry.Skips = skips } } - channels = append(channels, *channel) + } + + for _, ch := range unlinkedChannels { + channels = append(channels, *ch) } slices.SortFunc(channels, func(a, b declcfg.Channel) int { diff --git a/alpha/template/semver/semver_test.go b/alpha/template/semver/semver_test.go index 700843039..d85522ba4 100644 --- a/alpha/template/semver/semver_test.go +++ b/alpha/template/semver/semver_test.go @@ -1,12 +1,14 @@ package semver import ( + "cmp" "fmt" + "slices" "strings" "testing" "github.com/blang/semver/v4" - "github.com/google/go-cmp/cmp" + gocmp "github.com/google/go-cmp/cmp" "github.com/stretchr/testify/require" "github.com/operator-framework/operator-registry/alpha/declcfg" @@ -14,26 +16,48 @@ import ( ) func TestLinkChannels(t *testing.T) { - // type bundleVersions map[string]map[string]semver.Version // e.g. d["stable"]["example-operator.v1.0.0"] = 1.0.0 - channelOperatorVersions := bundleVersions{ - "stable": { - "a-v0.1.0": semver.MustParse("0.1.0"), - "a-v0.1.1": semver.MustParse("0.1.1"), - "a-v1.1.0": semver.MustParse("1.1.0"), - "a-v1.2.1": semver.MustParse("1.2.1"), - "a-v1.3.1": semver.MustParse("1.3.1"), - "a-v2.1.0": semver.MustParse("2.1.0"), - "a-v1.3.1-beta": semver.MustParse(("1.3.1-beta")), - "a-v2.1.1": semver.MustParse("2.1.1"), - "a-v2.3.1": semver.MustParse("2.3.1"), - "a-v2.3.2": semver.MustParse("2.3.2"), - "a-v3.1.0": semver.MustParse("3.1.0"), - "a-v3.1.1": semver.MustParse("3.1.1"), - "a-v1.3.1-alpha": semver.MustParse("1.3.1-alpha"), - "a-v1.4.1": semver.MustParse("1.4.1"), - "a-v1.4.1-beta1": semver.MustParse("1.4.1-beta1"), - "a-v1.4.1-beta2": semver.MustParse("1.4.1-beta2"), - }, + // type entryTuple struct { + // arch channelArchetype + // kind streamType + // name string + // parent string + // index int + // version semver.Version + // } + + minimumChannelEntries := []entryTuple{ + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v0.1.0", parent: "stable-v0.1", index: 0, version: semver.MustParse("0.1.0")}, + } + + majorChannelEntries := []entryTuple{ + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v0.1.0", parent: "stable-v0", index: 0, version: semver.MustParse("0.1.0")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v0.1.1", parent: "stable-v0", index: 1, version: semver.MustParse("0.1.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v1.1.0", parent: "stable-v1", index: 0, version: semver.MustParse("1.1.0")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v1.2.1", parent: "stable-v1", index: 1, version: semver.MustParse("1.2.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v1.3.1", parent: "stable-v1", index: 2, version: semver.MustParse("1.3.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v2.1.0", parent: "stable-v2", index: 0, version: semver.MustParse("2.1.0")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v2.1.1", parent: "stable-v2", index: 1, version: semver.MustParse("2.1.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v2.3.1", parent: "stable-v2", index: 2, version: semver.MustParse("2.3.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v2.3.2", parent: "stable-v2", index: 3, version: semver.MustParse("2.3.2")}, + } + + majorChannelEntriesLastXChange := []entryTuple{ + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v0.1.0", parent: "stable-v0", index: 0, version: semver.MustParse("0.1.0")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v0.1.1", parent: "stable-v0", index: 1, version: semver.MustParse("0.1.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v1.1.0", parent: "stable-v1", index: 0, version: semver.MustParse("1.1.0")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v1.2.1", parent: "stable-v1", index: 1, version: semver.MustParse("1.2.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v1.3.1", parent: "stable-v1", index: 2, version: semver.MustParse("1.3.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v2.1.0", parent: "stable-v2", index: 0, version: semver.MustParse("2.1.0")}, + } + + majorChannelEntriesLastArchChange := []entryTuple{ + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v1.1.0", parent: "stable-v1", index: 0, version: semver.MustParse("1.1.0")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v1.2.1", parent: "stable-v1", index: 1, version: semver.MustParse("1.2.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v1.3.1", parent: "stable-v1", index: 2, version: semver.MustParse("1.3.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v2.1.0", parent: "stable-v2", index: 0, version: semver.MustParse("2.1.0")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v2.1.1", parent: "stable-v2", index: 1, version: semver.MustParse("2.1.1")}, + {arch: stableChannelArchetype, kind: majorStreamType, name: "a-v2.3.1", parent: "stable-v2", index: 2, version: semver.MustParse("2.3.1")}, + {arch: candidateChannelArchetype, kind: majorStreamType, name: "a-v2.3.2", parent: "candidate-v2", index: 0, version: semver.MustParse("2.3.2")}, } majorGeneratedUnlinkedChannels := map[string]*declcfg.Channel{ @@ -130,9 +154,21 @@ func TestLinkChannels(t *testing.T) { }, } + minimumGeneratedUnlinkedChannels := map[string]*declcfg.Channel{ + "stable-v0": { + Schema: "olm.channel", + Name: "stable-v0", + Package: "a", + Entries: []declcfg.ChannelEntry{ + {Name: "a-v0.1.0"}, + }, + }, + } + tests := []struct { name string unlinkedChannels map[string]*declcfg.Channel + channelEntries []entryTuple generateMinorChannels bool generateMajorChannels bool out []declcfg.Channel @@ -140,6 +176,7 @@ func TestLinkChannels(t *testing.T) { { name: "No edges between successive major channels", unlinkedChannels: majorGeneratedUnlinkedChannels, + channelEntries: majorChannelEntries, generateMinorChannels: false, generateMajorChannels: true, out: []declcfg.Channel{ @@ -159,7 +196,7 @@ func TestLinkChannels(t *testing.T) { Entries: []declcfg.ChannelEntry{ {Name: "a-v1.1.0", Replaces: ""}, {Name: "a-v1.2.1", Replaces: "a-v1.1.0"}, - {Name: "a-v1.3.1", Replaces: "a-v1.2.1"}, + {Name: "a-v1.3.1", Replaces: "a-v1.2.1", Skips: []string{"a-v1.1.0"}}, }, }, { @@ -170,7 +207,7 @@ func TestLinkChannels(t *testing.T) { {Name: "a-v2.1.0", Replaces: ""}, {Name: "a-v2.1.1", Replaces: "", Skips: []string{"a-v2.1.0"}}, {Name: "a-v2.3.1", Replaces: ""}, - {Name: "a-v2.3.2", Replaces: "a-v2.1.1", Skips: []string{"a-v2.3.1"}}, + {Name: "a-v2.3.2", Replaces: "a-v2.1.1", Skips: []string{"a-v2.1.0", "a-v2.3.1"}}, }, }, }, @@ -178,6 +215,7 @@ func TestLinkChannels(t *testing.T) { { name: "No edges between successive major channels where last edge is X change", unlinkedChannels: majorGeneratedUnlinkedChannelsLastXChange, + channelEntries: majorChannelEntriesLastXChange, generateMinorChannels: false, generateMajorChannels: true, out: []declcfg.Channel{ @@ -197,7 +235,7 @@ func TestLinkChannels(t *testing.T) { Entries: []declcfg.ChannelEntry{ {Name: "a-v1.1.0", Replaces: ""}, {Name: "a-v1.2.1", Replaces: "a-v1.1.0"}, - {Name: "a-v1.3.1", Replaces: "a-v1.2.1"}, + {Name: "a-v1.3.1", Replaces: "a-v1.2.1", Skips: []string{"a-v1.1.0"}}, }, }, { @@ -213,6 +251,7 @@ func TestLinkChannels(t *testing.T) { { name: "No edges between successive major channels where last edge is archetype change", unlinkedChannels: majorGeneratedUnlinkedChannelsLastArchChange, + channelEntries: majorChannelEntriesLastArchChange, generateMinorChannels: false, generateMajorChannels: true, out: []declcfg.Channel{ @@ -231,7 +270,7 @@ func TestLinkChannels(t *testing.T) { Entries: []declcfg.ChannelEntry{ {Name: "a-v1.1.0", Replaces: ""}, {Name: "a-v1.2.1", Replaces: "a-v1.1.0"}, - {Name: "a-v1.3.1", Replaces: "a-v1.2.1"}, + {Name: "a-v1.3.1", Replaces: "a-v1.2.1", Skips: []string{"a-v1.1.0"}}, }, }, { @@ -241,7 +280,24 @@ func TestLinkChannels(t *testing.T) { Entries: []declcfg.ChannelEntry{ {Name: "a-v2.1.0", Replaces: ""}, {Name: "a-v2.1.1", Replaces: "", Skips: []string{"a-v2.1.0"}}, - {Name: "a-v2.3.1", Replaces: "a-v2.1.1"}, + {Name: "a-v2.3.1", Replaces: "a-v2.1.1", Skips: []string{"a-v2.1.0"}}, + }, + }, + }, + }, + { + name: "Minimum viable major channel", + unlinkedChannels: minimumGeneratedUnlinkedChannels, + channelEntries: minimumChannelEntries, + generateMinorChannels: false, + generateMajorChannels: true, + out: []declcfg.Channel{ + { + Schema: "olm.channel", + Name: "stable-v0", + Package: "a", + Entries: []declcfg.ChannelEntry{ + {Name: "a-v0.1.0", Replaces: ""}, }, }, }, @@ -251,7 +307,7 @@ func TestLinkChannels(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { sv := &semverTemplate{pkg: "a", GenerateMajorChannels: tt.generateMajorChannels, GenerateMinorChannels: tt.generateMinorChannels} - diff := cmp.Diff(tt.out, sv.linkChannels(tt.unlinkedChannels, &channelOperatorVersions)) + diff := gocmp.Diff(tt.out, sv.linkChannels(tt.unlinkedChannels, tt.channelEntries)) if diff != "" { t.Errorf("unexpected channel diff (-expected +received):\n%s", diff) } @@ -301,10 +357,10 @@ func TestGenerateChannels(t *testing.T) { {Name: "a-v1.2.1", Replaces: "a-v1.1.0"}, {Name: "a-v1.3.1-alpha", Replaces: ""}, {Name: "a-v1.3.1-beta", Replaces: ""}, - {Name: "a-v1.3.1", Replaces: "a-v1.2.1", Skips: []string{"a-v1.3.1-alpha", "a-v1.3.1-beta"}}, + {Name: "a-v1.3.1", Replaces: "a-v1.2.1", Skips: []string{"a-v1.1.0", "a-v1.3.1-alpha", "a-v1.3.1-beta"}}, {Name: "a-v1.4.1-beta1", Replaces: ""}, {Name: "a-v1.4.1-beta2", Replaces: ""}, - {Name: "a-v1.4.1", Replaces: "a-v1.3.1", Skips: []string{"a-v1.4.1-beta1", "a-v1.4.1-beta2"}}, + {Name: "a-v1.4.1", Replaces: "a-v1.3.1", Skips: []string{"a-v1.1.0", "a-v1.2.1", "a-v1.3.1-alpha", "a-v1.3.1-beta", "a-v1.4.1-beta1", "a-v1.4.1-beta2"}}, }, }, { @@ -315,7 +371,7 @@ func TestGenerateChannels(t *testing.T) { {Name: "a-v2.1.0", Replaces: ""}, {Name: "a-v2.1.1", Replaces: "", Skips: []string{"a-v2.1.0"}}, {Name: "a-v2.3.1", Replaces: ""}, - {Name: "a-v2.3.2", Replaces: "a-v2.1.1", Skips: []string{"a-v2.3.1"}}, + {Name: "a-v2.3.2", Replaces: "a-v2.1.1", Skips: []string{"a-v2.1.0", "a-v2.3.1"}}, }, }, { @@ -352,7 +408,7 @@ func TestGenerateChannels(t *testing.T) { Name: "stable-v1.2", Package: "a", Entries: []declcfg.ChannelEntry{ - {Name: "a-v1.2.1", Replaces: ""}, + {Name: "a-v1.2.1", Replaces: "a-v1.1.0"}, }, }, { @@ -362,7 +418,7 @@ func TestGenerateChannels(t *testing.T) { Entries: []declcfg.ChannelEntry{ {Name: "a-v1.3.1-alpha", Replaces: ""}, {Name: "a-v1.3.1-beta", Replaces: ""}, - {Name: "a-v1.3.1", Replaces: "", Skips: []string{"a-v1.3.1-alpha", "a-v1.3.1-beta"}}, + {Name: "a-v1.3.1", Replaces: "a-v1.2.1", Skips: []string{"a-v1.1.0", "a-v1.3.1-alpha", "a-v1.3.1-beta"}}, }, }, { @@ -372,7 +428,7 @@ func TestGenerateChannels(t *testing.T) { Entries: []declcfg.ChannelEntry{ {Name: "a-v1.4.1-beta1", Replaces: ""}, {Name: "a-v1.4.1-beta2", Replaces: ""}, - {Name: "a-v1.4.1", Replaces: "", Skips: []string{"a-v1.4.1-beta1", "a-v1.4.1-beta2"}}, + {Name: "a-v1.4.1", Replaces: "a-v1.3.1", Skips: []string{"a-v1.1.0", "a-v1.2.1", "a-v1.3.1-alpha", "a-v1.3.1-beta", "a-v1.4.1-beta1", "a-v1.4.1-beta2"}}, }, }, { @@ -390,7 +446,7 @@ func TestGenerateChannels(t *testing.T) { Package: "a", Entries: []declcfg.ChannelEntry{ {Name: "a-v2.3.1", Replaces: ""}, - {Name: "a-v2.3.2", Replaces: "", Skips: []string{"a-v2.3.1"}}, + {Name: "a-v2.3.2", Replaces: "a-v2.1.1", Skips: []string{"a-v2.1.0", "a-v2.3.1"}}, }, }, { @@ -407,6 +463,9 @@ func TestGenerateChannels(t *testing.T) { var combinedLinkedChannels []declcfg.Channel combinedLinkedChannels = append(combinedLinkedChannels, minorLinkedChannels...) combinedLinkedChannels = append(combinedLinkedChannels, majorLinkedChannels...) + slices.SortFunc(combinedLinkedChannels, func(a, b declcfg.Channel) int { + return cmp.Compare(a.Name, b.Name) + }) tests := []struct { name string @@ -469,8 +528,10 @@ func TestGenerateChannels(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { sv := &semverTemplate{GenerateMajorChannels: tt.generateMajorChannels, GenerateMinorChannels: tt.generateMinorChannels, pkg: "a", DefaultChannelTypePreference: tt.channelTypePreference} - out := sv.generateChannels(&channelOperatorVersions) - require.ElementsMatch(t, tt.out, out) + diff := gocmp.Diff(tt.out, sv.generateChannels(&channelOperatorVersions)) + if diff != "" { + t.Errorf("unexpected channel diff (-expected +received):\n%s", diff) + } require.Equal(t, tt.defaultChannel, sv.defaultChannel) }) } diff --git a/alpha/template/semver/types.go b/alpha/template/semver/types.go index b0846a64f..fda01139a 100644 --- a/alpha/template/semver/types.go +++ b/alpha/template/semver/types.go @@ -82,3 +82,12 @@ type highwaterChannel struct { version semver.Version name string } + +type entryTuple struct { + arch channelArchetype + kind streamType + name string + parent string + index int + version semver.Version +}