diff --git a/alpha/model/graph.go b/alpha/model/graph.go new file mode 100644 index 000000000..1983d69be --- /dev/null +++ b/alpha/model/graph.go @@ -0,0 +1,250 @@ +package model + +import ( + "cmp" + "errors" + "fmt" + "slices" + "sort" + "strings" + + "golang.org/x/exp/maps" + "k8s.io/apimachinery/pkg/util/sets" +) + +type graph struct { + nodes map[string]*node +} + +func newNode(b *Bundle) *node { + return &node{ + bundle: b, + replacedBy: make(map[string]*node), + skippedBy: make(map[string]*node), + skips: make(map[string]*node), + } +} + +func newGraph(c *Channel) *graph { + nodes := map[string]*node{} + + // Add all nodes (without edges) + for _, b := range c.Bundles { + nodes[b.Name] = newNode(b) + } + + // Populate edges between nodes + for _, b := range c.Bundles { + n := nodes[b.Name] + + if b.Replaces != "" { + replaces, ok := nodes[b.Replaces] + if !ok { + // the "replaces" edge points to a node outside the channel + replaces = newNode(&Bundle{Name: b.Replaces}) + replaces.externalToChannel = true + nodes[b.Replaces] = replaces + } + n.replaces = replaces + n.replaces.replacedBy[n.bundle.Name] = n + } + + for _, skipName := range b.Skips { + skip, ok := nodes[skipName] + if !ok { + // the "skips" edge points to a node outside the channel + skip = newNode(&Bundle{Name: skipName}) + skip.externalToChannel = true + } + skip.skippedBy[b.Name] = n + n.skips[skipName] = skip + } + } + + return &graph{ + nodes: nodes, + } +} + +type node struct { + bundle *Bundle + replacedBy map[string]*node + replaces *node + skippedBy map[string]*node + skips map[string]*node + externalToChannel bool +} + +func (n *node) pathsTo(other *node) [][]*node { + var pathsToInternal func(existingPath []*node, froms map[string]*node, to *node) [][]*node + pathsToInternal = func(existingPath []*node, froms map[string]*node, to *node) [][]*node { + if len(froms) == 0 { + // we never found a path to "to" + return nil + } + var allPaths [][]*node + for _, f := range froms { + path := append(slices.Clone(existingPath), f) + if f == to { + // we found "to"! + allPaths = append(allPaths, path) + } else { + // From an intermediate node, look only in replacedBy, so that we don't stray off the replaces chain. + allPaths = append(allPaths, pathsToInternal(path, f.replacedBy, to)...) + } + } + return allPaths + } + + // From the starting node, look in all ancestors (replacedBy and skippedBy). + ancestors := map[string]*node{} + maps.Copy(ancestors, n.replacedBy) + maps.Copy(ancestors, n.skippedBy) + return pathsToInternal(nil, ancestors, other) +} + +func (g *graph) validate() error { + result := newValidationError("invalid upgrade graph") + if err := g.validateNoCycles(); err != nil { + result.subErrors = append(result.subErrors, err) + } + if err := g.validateNoStranded(); err != nil { + result.subErrors = append(result.subErrors, err) + } + return result.orNil() +} + +func (g *graph) cycles() [][]*node { + allCycles := [][]*node{} + for _, n := range g.nodes { + allCycles = append(allCycles, n.pathsTo(n)...) + } + dedupSameRotations(&allCycles) + for i, cycle := range allCycles { + allCycles[i] = append(cycle, cycle[0]) + } + return allCycles +} + +func (g *graph) validateNoCycles() error { + cycles := g.cycles() + if len(cycles) == 0 { + return nil + } + result := newValidationError("cycles found in graph") + for _, cycle := range cycles { + result.subErrors = append(result.subErrors, errors.New(nodeCycleString(cycle))) + } + return result.orNil() +} + +// dedupSameRotations removes rotations of the same cycle. +// dedupSameRotations sorts the cycles so that shorter paths +// and paths with lower versions appear earlier in the list. +func dedupSameRotations(paths *[][]*node) { + slices.SortFunc(*paths, func(a, b []*node) int { + if len(a) == 0 && len(b) == 0 { + return 0 + } + if v := cmp.Compare(len(a), len(b)); v != 0 { + return v + } + return a[0].bundle.Version.Compare(b[0].bundle.Version) + }) + seen := map[string]struct{}{} + tmp := (*paths)[:0] + for _, path := range *paths { + rotate(&path) + k := nodeCycleString(path) + if _, ok := seen[k]; ok { + continue + } + seen[k] = struct{}{} + tmp = append(tmp, path) + } + *paths = tmp +} + +func rotate(in *[]*node) { + if len(*in) == 0 { + return + } + maxIndex := 0 + for i, n := range (*in)[1:] { + if n.bundle.Version.GT((*in)[maxIndex].bundle.Version) { + maxIndex = i + 1 + } + } + slices.Reverse((*in)[:maxIndex]) + slices.Reverse((*in)[maxIndex:]) + slices.Reverse((*in)) +} + +func nodeCycleString(nodes []*node) string { + return strings.Join(mapSlice(nodes, nodeName), " -> ") +} + +func (g *graph) strandedNodes() ([]*node, error) { + head, err := g.head() + if err != nil { + return nil, err + } + all := sets.New[*node](maps.Values(g.nodes)...) + chain := sets.New[*node]() + skipped := sets.New[*node]() + + cur := head + for cur != nil && !skipped.Has(cur) && !chain.Has(cur) { + chain.Insert(cur) + skipped.Insert(maps.Values(cur.skips)...) + cur = cur.replaces + } + + stranded := all.Difference(chain).Difference(skipped).UnsortedList() + slices.SortFunc(stranded, func(a, b *node) int { + return a.bundle.Compare(b.bundle) + }) + return stranded, nil +} + +func (g *graph) validateNoStranded() error { + stranded, err := g.strandedNodes() + if err != nil { + return err + } + if len(stranded) == 0 { + return nil + } + + return fmt.Errorf("channel contains one or more stranded bundles: %s", strings.Join(mapSlice(stranded, nodeName), ", ")) +} + +func (g *graph) head() (*node, error) { + heads := []*node{} + for _, n := range g.nodes { + if len(n.replacedBy) == 0 && len(n.skippedBy) == 0 { + heads = append(heads, n) + } + } + if len(heads) == 0 { + return nil, fmt.Errorf("no channel head found in graph") + } + if len(heads) > 1 { + headNames := mapSlice(heads, nodeName) + sort.Strings(headNames) + return nil, fmt.Errorf("multiple channel heads found in graph: %s", strings.Join(headNames, ", ")) + } + return heads[0], nil +} + +func nodeName(n *node) string { + return n.bundle.Name +} + +func mapSlice[I, O any](s []I, fn func(I) O) []O { + result := make([]O, 0, len(s)) + for _, i := range s { + result = append(result, fn(i)) + } + return result +} diff --git a/alpha/model/graph_test.go b/alpha/model/graph_test.go new file mode 100644 index 000000000..911b9cf4c --- /dev/null +++ b/alpha/model/graph_test.go @@ -0,0 +1,122 @@ +package model + +import ( + "testing" + + "github.com/blang/semver/v4" + "github.com/stretchr/testify/assert" +) + +func Test_dedupSameRotations(t *testing.T) { + type spec struct { + name string + paths [][]*node + expect [][]*node + } + for _, s := range []spec{ + { + name: "Empty", + paths: [][]*node{}, + expect: [][]*node{}, + }, + { + name: "One", + paths: [][]*node{{{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}}}, + expect: [][]*node{{{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}}}, + }, + { + name: "Two", + paths: [][]*node{ + { + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + }, + { + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + }, + }, + expect: [][]*node{{ + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + }}, + }, + { + name: "Three", + paths: [][]*node{ + { + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + {bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}}, + }, + { + {bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + }, + { + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + {bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + }, + }, + expect: [][]*node{{ + {bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + }}, + }, + { + name: "Multiple", + paths: [][]*node{ + { + {bundle: &Bundle{Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + }, + { + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + {bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}}, + }, + { + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + {bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + }, + { + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + {bundle: &Bundle{Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4")}}, + }, + { + {bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + }, + { + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + {bundle: &Bundle{Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + }, + }, + expect: [][]*node{ + { + {bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + }, + { + {bundle: &Bundle{Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4")}}, + {bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}, + {bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}}, + }, + }, + }, + } { + t.Run(s.name, func(t *testing.T) { + dedupSameRotations(&s.paths) + assert.Equal(t, s.expect, s.paths) + }) + } +} diff --git a/alpha/model/model.go b/alpha/model/model.go index 9b4e3ae85..a0104e220 100644 --- a/alpha/model/model.go +++ b/alpha/model/model.go @@ -1,9 +1,9 @@ package model import ( + "cmp" "errors" "fmt" - "sort" "strings" "github.com/blang/semver/v4" @@ -15,6 +15,7 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "github.com/operator-framework/operator-registry/alpha/property" + libsemver "github.com/operator-framework/operator-registry/pkg/lib/semver" ) type Deprecation struct { @@ -187,38 +188,15 @@ type Channel struct { Properties []property.Property } -// TODO(joelanford): This function determines the channel head by finding the bundle that has 0 -// -// incoming edges, based on replaces and skips. It also expects to find exactly one such bundle. -// Is this the correct algorithm? +// Head determines the channel head by finding the bundle that has 0 incoming edges, +// based on replaces and skips. It also expects to find exactly one such bundle. func (c Channel) Head() (*Bundle, error) { - incoming := map[string]int{} - for _, b := range c.Bundles { - if b.Replaces != "" { - incoming[b.Replaces]++ - } - for _, skip := range b.Skips { - incoming[skip]++ - } - } - var heads []*Bundle - for _, b := range c.Bundles { - if _, ok := incoming[b.Name]; !ok { - heads = append(heads, b) - } - } - if len(heads) == 0 { - return nil, fmt.Errorf("no channel head found in graph") - } - if len(heads) > 1 { - var headNames []string - for _, head := range heads { - headNames = append(headNames, head.Name) - } - sort.Strings(headNames) - return nil, fmt.Errorf("multiple channel heads found in graph: %s", strings.Join(headNames, ", ")) + g := newGraph(&c) + head, err := g.head() + if err != nil { + return nil, err } - return heads[0], nil + return c.Bundles[head.bundle.Name], nil } func (c *Channel) Validate() error { @@ -237,7 +215,7 @@ func (c *Channel) Validate() error { } if len(c.Bundles) > 0 { - if err := c.validateReplacesChain(); err != nil { + if err := c.validateUpgradeGraph(); err != nil { result.subErrors = append(result.subErrors, err) } } @@ -261,49 +239,18 @@ func (c *Channel) Validate() error { return result.orNil() } -// validateReplacesChain checks the replaces chain of a channel. +// validateUpgradeGraph checks the replaces chain of a channel. // Specifically the following rules must be followed: // 1. There must be exactly 1 channel head. -// 2. Beginning at the head, the replaces chain must reach all non-skipped entries. -// Non-skipped entries are defined as entries that are not skipped by any other entry in the channel. -// 3. There must be no cycles in the replaces chain. -// 4. The tail entry in the replaces chain is permitted to replace a non-existent entry. -func (c *Channel) validateReplacesChain() error { - head, err := c.Head() - if err != nil { - return err - } - - allBundles := sets.NewString() - skippedBundles := sets.NewString() - for _, b := range c.Bundles { - allBundles = allBundles.Insert(b.Name) - skippedBundles = skippedBundles.Insert(b.Skips...) - } - - chainFrom := map[string][]string{} - replacesChainFromHead := sets.NewString(head.Name) - cur := head - for cur != nil { - if _, ok := chainFrom[cur.Name]; !ok { - chainFrom[cur.Name] = []string{cur.Name} - } - for k := range chainFrom { - chainFrom[k] = append(chainFrom[k], cur.Replaces) - } - if replacesChainFromHead.Has(cur.Replaces) { - return fmt.Errorf("detected cycle in replaces chain of upgrade graph: %s", strings.Join(chainFrom[cur.Replaces], " -> ")) - } - replacesChainFromHead = replacesChainFromHead.Insert(cur.Replaces) - cur = c.Bundles[cur.Replaces] - } - - strandedBundles := allBundles.Difference(replacesChainFromHead).Difference(skippedBundles).List() - if len(strandedBundles) > 0 { - return fmt.Errorf("channel contains one or more stranded bundles: %s", strings.Join(strandedBundles, ", ")) - } - - return nil +// 2. Beginning at the head, the replaces chain traversal must reach all entries. +// Unreached entries are considered "stranded" and cause a channel to be invalid. +// 3. Skipped entries are always leaf nodes. We never follow replaces or skips edges +// of skipped entries during replaces chain traversal. +// 4. There must be no cycles in the replaces chain. +// 5. The tail entry in the replaces chain is permitted to replace a non-existent entry. +func (c *Channel) validateUpgradeGraph() error { + g := newGraph(c) + return g.validate() } type Bundle struct { @@ -378,6 +325,26 @@ func (b *Bundle) Validate() error { return result.orNil() } +// Compare compares bundles by their "registry+v1" version. That is, +// it compares their semver versions, but if the semver versions are +// the same, it uses compares their build metadata as if the build +// metadata was actually a pre-release. +// +// If there is an error trying to compare build metadata as a pre-release, +// then a simple string comparison is made on the build metadata instead. +func (b *Bundle) Compare(other *Bundle) int { + if v := b.Version.Compare(other.Version); v != 0 { + return v + } + if v, err := libsemver.BuildIdCompare(b.Version, other.Version); err == nil { + return v + } + return cmp.Compare( + strings.Join(b.Version.Build, "."), + strings.Join(other.Version.Build, "."), + ) +} + type RelatedImage struct { Name string Image string diff --git a/alpha/model/model_test.go b/alpha/model/model_test.go index 11391b74c..b74e604f7 100644 --- a/alpha/model/model_test.go +++ b/alpha/model/model_test.go @@ -130,44 +130,139 @@ func TestValidReplacesChain(t *testing.T) { { name: "Success/Valid", ch: Channel{Bundles: map[string]*Bundle{ - "anakin.v0.0.1": {Name: "anakin.v0.0.1"}, - "anakin.v0.0.2": {Name: "anakin.v0.0.2", Skips: []string{"anakin.v0.0.1"}}, - "anakin.v0.0.3": {Name: "anakin.v0.0.3", Skips: []string{"anakin.v0.0.2"}}, - "anakin.v0.0.4": {Name: "anakin.v0.0.4", Replaces: "anakin.v0.0.3"}, + "anakin.v0.0.1": {Name: "anakin.v0.0.1", Replaces: "anakin.v0.0.1-alpha1"}, + "anakin.v0.0.2-rc1": {Name: "anakin.v0.0.2-rc1"}, + "anakin.v0.0.2": {Name: "anakin.v0.0.2", Replaces: "anakin.v0.0.1", Skips: []string{"anakin.v0.0.2-beta1", "anakin.v0.0.2-rc1"}}, + "anakin.v0.0.3": {Name: "anakin.v0.0.3", Replaces: "anakin.v0.0.2"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Replaces: "anakin.v0.0.3"}, }}, assertion: require.NoError, }, { - name: "Error/CycleNoHops", + name: "Error/CycleNoHopsReplaces", ch: Channel{Bundles: map[string]*Bundle{ - "anakin.v0.0.4": {Name: "anakin.v0.0.4", Replaces: "anakin.v0.0.4"}, - "anakin.v0.0.5": {Name: "anakin.v0.0.5", Replaces: "anakin.v0.0.4"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4"), Replaces: "anakin.v0.0.4"}, + "anakin.v0.0.5": {Name: "anakin.v0.0.5", Version: semver.MustParse("0.0.5"), Replaces: "anakin.v0.0.4"}, }}, - assertion: hasError(`detected cycle in replaces chain of upgrade graph: anakin.v0.0.4 -> anakin.v0.0.4`), + assertion: hasError(`anakin.v0.0.4 -> anakin.v0.0.4`), }, { - name: "Error/CycleMultipleHops", + name: "Error/CycleNoHopsSkips", ch: Channel{Bundles: map[string]*Bundle{ - "anakin.v0.0.1": {Name: "anakin.v0.0.1", Replaces: "anakin.v0.0.3"}, - "anakin.v0.0.2": {Name: "anakin.v0.0.2", Replaces: "anakin.v0.0.1"}, - "anakin.v0.0.3": {Name: "anakin.v0.0.3", Replaces: "anakin.v0.0.2"}, - "anakin.v0.0.4": {Name: "anakin.v0.0.4", Replaces: "anakin.v0.0.3"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4"), Skips: []string{"anakin.v0.0.4"}}, + "anakin.v0.0.5": {Name: "anakin.v0.0.5", Version: semver.MustParse("0.0.5"), Replaces: "anakin.v0.0.4"}, + }}, + assertion: hasError(`anakin.v0.0.4 -> anakin.v0.0.4`), + }, + { + name: "Error/CycleMultipleHopsAllReplaces", + ch: Channel{Bundles: map[string]*Bundle{ + "anakin.v0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1"), Replaces: "anakin.v0.0.4"}, + "anakin.v0.0.2": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2"), Replaces: "anakin.v0.0.1"}, + "anakin.v0.0.3": {Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3"), Replaces: "anakin.v0.0.2"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4"), Replaces: "anakin.v0.0.3"}, + }}, + assertion: hasError(`anakin.v0.0.4 -> anakin.v0.0.1 -> anakin.v0.0.2 -> anakin.v0.0.3 -> anakin.v0.0.4`), + }, + { + name: "Error/CycleMultipleHopsTailSkips", + ch: Channel{Bundles: map[string]*Bundle{ + "anakin.v0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1"), Skips: []string{"anakin.v0.0.3"}}, + "anakin.v0.0.2": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2"), Replaces: "anakin.v0.0.1"}, + "anakin.v0.0.3": {Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3"), Replaces: "anakin.v0.0.2"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4"), Replaces: "anakin.v0.0.3"}, + "anakin.v0.0.5": {Name: "anakin.v0.0.5", Version: semver.MustParse("0.0.5"), Replaces: "anakin.v0.0.4"}, + }}, + assertion: hasError(`anakin.v0.0.3 -> anakin.v0.0.1 -> anakin.v0.0.2 -> anakin.v0.0.3`), + }, + { + name: "Error/CycleMultipleHopsAllSkipsNotCyclic", + ch: Channel{Bundles: map[string]*Bundle{ + "anakin.v0.0.5": {Name: "anakin.v0.0.5", Version: semver.MustParse("0.0.5"), Skips: []string{"anakin.v0.0.7"}}, + "anakin.v0.0.6": {Name: "anakin.v0.0.6", Version: semver.MustParse("0.0.6"), Skips: []string{"anakin.v0.0.5"}}, + "anakin.v0.0.7": {Name: "anakin.v0.0.7", Version: semver.MustParse("0.0.7"), Skips: []string{"anakin.v0.0.6"}}, + "anakin.v0.0.8": {Name: "anakin.v0.0.8", Version: semver.MustParse("0.0.8"), Skips: []string{"anakin.v0.0.7"}}, + }}, + assertion: hasError(`channel contains one or more stranded bundles: anakin.v0.0.5, anakin.v0.0.6`), + }, + { + name: "Error/MultipleSeparateCycles", + ch: Channel{Bundles: map[string]*Bundle{ + "anakin.v0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}, + "anakin.v0.0.2": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2"), Replaces: "anakin.v0.0.1", Skips: []string{"anakin.v0.0.4"}}, + "anakin.v0.0.3": {Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3"), Replaces: "anakin.v0.0.2"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4"), Replaces: "anakin.v0.0.3"}, + "anakin.v0.0.5": {Name: "anakin.v0.0.5", Version: semver.MustParse("0.0.5"), Replaces: "anakin.v0.0.4", Skips: []string{"anakin.v0.0.7"}}, + "anakin.v0.0.6": {Name: "anakin.v0.0.6", Version: semver.MustParse("0.0.6"), Replaces: "anakin.v0.0.5"}, + "anakin.v0.0.7": {Name: "anakin.v0.0.7", Version: semver.MustParse("0.0.7"), Replaces: "anakin.v0.0.6"}, + "anakin.v0.0.8": {Name: "anakin.v0.0.8", Version: semver.MustParse("0.0.8"), Replaces: "anakin.v0.0.7"}, }}, - assertion: hasError(`detected cycle in replaces chain of upgrade graph: anakin.v0.0.3 -> anakin.v0.0.2 -> anakin.v0.0.1 -> anakin.v0.0.3`), + assertion: func(t require.TestingT, err error, _ ...interface{}) { + hasError(`anakin.v0.0.4 -> anakin.v0.0.2 -> anakin.v0.0.3 -> anakin.v0.0.4`)(t, err) + hasError(`anakin.v0.0.7 -> anakin.v0.0.5 -> anakin.v0.0.6 -> anakin.v0.0.7`)(t, err) + }, + }, + { + name: "Error/MultipleCyclesFromTheSameNode", + ch: Channel{Bundles: map[string]*Bundle{ + "anakin.v0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1"), Replaces: "anakin.v0.0.3", Skips: []string{"anakin.v0.0.2"}}, + "anakin.v0.0.2": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2"), Replaces: "anakin.v0.0.1"}, + "anakin.v0.0.3": {Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3"), Replaces: "anakin.v0.0.2"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4"), Replaces: "anakin.v0.0.3"}, + }}, + assertion: func(t require.TestingT, err error, _ ...interface{}) { + hasError(`anakin.v0.0.3 -> anakin.v0.0.1 -> anakin.v0.0.2 -> anakin.v0.0.3`)(t, err) + hasError(`anakin.v0.0.2 -> anakin.v0.0.1 -> anakin.v0.0.2`)(t, err) + }, + }, + { + name: "Error/StrandedSkipsBeforeReplaces", + ch: Channel{Bundles: map[string]*Bundle{ + "anakin.v0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}, + "anakin.v0.0.2": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2"), Replaces: "anakin.v0.0.1"}, + "anakin.v0.0.3": {Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3"), Replaces: "anakin.v0.0.2"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4"), Replaces: "anakin.v0.0.3"}, + "anakin.v0.0.5": {Name: "anakin.v0.0.5", Version: semver.MustParse("0.0.5"), Replaces: "anakin.v0.0.4", Skips: []string{"anakin.v0.0.3"}}, + }}, + assertion: hasError(`channel contains one or more stranded bundles: anakin.v0.0.1, anakin.v0.0.2`), + }, + { + name: "Error/StrandedSkipsWithReplaces", + ch: Channel{Bundles: map[string]*Bundle{ + "anakin.v0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}, + "anakin.v0.0.2": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2"), Replaces: "anakin.v0.0.1"}, + "anakin.v0.0.3": {Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3"), Replaces: "anakin.v0.0.2"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4"), Replaces: "anakin.v0.0.3", Skips: []string{"anakin.v0.0.3"}}, + "anakin.v0.0.5": {Name: "anakin.v0.0.5", Version: semver.MustParse("0.0.5"), Replaces: "anakin.v0.0.4"}, + }}, + assertion: hasError(`channel contains one or more stranded bundles: anakin.v0.0.1, anakin.v0.0.2`), }, { - name: "Error/Stranded", + name: "Error/StrandingIsCommutative", + ch: Channel{Bundles: map[string]*Bundle{ + "anakin.v0.0.1": {Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}, + "anakin.v0.0.2": {Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2"), Replaces: "anakin.v0.0.1"}, + "anakin.v0.0.3": {Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3"), Replaces: "anakin.v0.0.2"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4"), Replaces: "anakin.v0.0.3", Skips: []string{"anakin.v0.0.2"}}, + "anakin.v0.0.5": {Name: "anakin.v0.0.5", Version: semver.MustParse("0.0.5"), Skips: []string{"anakin.v0.0.4"}}, + }}, + assertion: hasError(`channel contains one or more stranded bundles: anakin.v0.0.1, anakin.v0.0.2, anakin.v0.0.3`), + }, + { + name: "Success/StrandedSkipsTheTail", ch: Channel{Bundles: map[string]*Bundle{ "anakin.v0.0.1": {Name: "anakin.v0.0.1"}, - "anakin.v0.0.2": {Name: "anakin.v0.0.2", Replaces: "anakin.v0.0.1"}, - "anakin.v0.0.3": {Name: "anakin.v0.0.3", Skips: []string{"anakin.v0.0.2"}}, + "anakin.v0.0.2": {Name: "anakin.v0.0.2", Replaces: "anakin.v0.0.1", Skips: []string{"anakin.v0.0.1"}}, + "anakin.v0.0.3": {Name: "anakin.v0.0.3", Replaces: "anakin.v0.0.2"}, + "anakin.v0.0.4": {Name: "anakin.v0.0.4", Replaces: "anakin.v0.0.3"}, + "anakin.v0.0.5": {Name: "anakin.v0.0.5", Replaces: "anakin.v0.0.4"}, }}, - assertion: hasError(`channel contains one or more stranded bundles: anakin.v0.0.1`), + assertion: require.NoError, }, } for _, s := range specs { t.Run(s.name, func(t *testing.T) { - err := s.ch.validateReplacesChain() + err := s.ch.validateUpgradeGraph() s.assertion(t, err) }) } @@ -196,7 +291,7 @@ func hasError(expectedError string) require.ErrorAssertionFunc { } } t.Errorf("expected error to be or contain suberror `%s`, got `%s`", expectedError, actualError) - t.FailNow() + //t.FailNow() } }