Skip to content

Commit d96879f

Browse files
committed
opm validate: channel validation improvements
1. improve detection of bundles stranded due to usage of ancestor skips 2. find and report all cycles Signed-off-by: Joe Lanford <[email protected]>
1 parent 2629279 commit d96879f

File tree

4 files changed

+527
-93
lines changed

4 files changed

+527
-93
lines changed

alpha/model/graph.go

Lines changed: 250 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,250 @@
1+
package model
2+
3+
import (
4+
"cmp"
5+
"errors"
6+
"fmt"
7+
"slices"
8+
"sort"
9+
"strings"
10+
11+
"golang.org/x/exp/maps"
12+
"k8s.io/apimachinery/pkg/util/sets"
13+
)
14+
15+
type graph struct {
16+
nodes map[string]*node
17+
}
18+
19+
func newNode(b *Bundle) *node {
20+
return &node{
21+
bundle: b,
22+
replacedBy: make(map[string]*node),
23+
skippedBy: make(map[string]*node),
24+
skips: make(map[string]*node),
25+
}
26+
}
27+
28+
func newGraph(c *Channel) *graph {
29+
nodes := map[string]*node{}
30+
31+
// Add all nodes (without edges)
32+
for _, b := range c.Bundles {
33+
nodes[b.Name] = newNode(b)
34+
}
35+
36+
// Populate edges between nodes
37+
for _, b := range c.Bundles {
38+
n := nodes[b.Name]
39+
40+
if b.Replaces != "" {
41+
replaces, ok := nodes[b.Replaces]
42+
if !ok {
43+
// the "replaces" edge points to a node outside the channel
44+
replaces = newNode(&Bundle{Name: b.Replaces})
45+
replaces.externalToChannel = true
46+
nodes[b.Replaces] = replaces
47+
}
48+
n.replaces = replaces
49+
n.replaces.replacedBy[n.bundle.Name] = n
50+
}
51+
52+
for _, skipName := range b.Skips {
53+
skip, ok := nodes[skipName]
54+
if !ok {
55+
// the "skips" edge points to a node outside the channel
56+
skip = newNode(&Bundle{Name: skipName})
57+
skip.externalToChannel = true
58+
}
59+
skip.skippedBy[b.Name] = n
60+
n.skips[skipName] = skip
61+
}
62+
}
63+
64+
return &graph{
65+
nodes: nodes,
66+
}
67+
}
68+
69+
type node struct {
70+
bundle *Bundle
71+
replacedBy map[string]*node
72+
replaces *node
73+
skippedBy map[string]*node
74+
skips map[string]*node
75+
externalToChannel bool
76+
}
77+
78+
func (n *node) pathsTo(other *node) [][]*node {
79+
var pathsToInternal func(existingPath []*node, froms map[string]*node, to *node) [][]*node
80+
pathsToInternal = func(existingPath []*node, froms map[string]*node, to *node) [][]*node {
81+
if len(froms) == 0 {
82+
// we never found a path to "to"
83+
return nil
84+
}
85+
var allPaths [][]*node
86+
for _, f := range froms {
87+
path := append(slices.Clone(existingPath), f)
88+
if f == to {
89+
// we found "to"!
90+
allPaths = append(allPaths, path)
91+
} else {
92+
// From an intermediate node, look only in replacedBy, so that we don't stray off the replaces chain.
93+
allPaths = append(allPaths, pathsToInternal(path, f.replacedBy, to)...)
94+
}
95+
}
96+
return allPaths
97+
}
98+
99+
// From the starting node, look in all ancestors (replacedBy and skippedBy).
100+
ancestors := map[string]*node{}
101+
maps.Copy(ancestors, n.replacedBy)
102+
maps.Copy(ancestors, n.skippedBy)
103+
return pathsToInternal(nil, ancestors, other)
104+
}
105+
106+
func (g *graph) validate() error {
107+
result := newValidationError("invalid upgrade graph")
108+
if err := g.validateNoCycles(); err != nil {
109+
result.subErrors = append(result.subErrors, err)
110+
}
111+
if err := g.validateNoStranded(); err != nil {
112+
result.subErrors = append(result.subErrors, err)
113+
}
114+
return result.orNil()
115+
}
116+
117+
func (g *graph) cycles() [][]*node {
118+
allCycles := [][]*node{}
119+
for _, n := range g.nodes {
120+
allCycles = append(allCycles, n.pathsTo(n)...)
121+
}
122+
dedupSameRotations(&allCycles)
123+
for i, cycle := range allCycles {
124+
allCycles[i] = append(cycle, cycle[0])
125+
}
126+
return allCycles
127+
}
128+
129+
func (g *graph) validateNoCycles() error {
130+
cycles := g.cycles()
131+
if len(cycles) == 0 {
132+
return nil
133+
}
134+
result := newValidationError("cycles found in graph")
135+
for _, cycle := range cycles {
136+
result.subErrors = append(result.subErrors, errors.New(nodeCycleString(cycle)))
137+
}
138+
return result.orNil()
139+
}
140+
141+
// dedupSameRotations removes rotations of the same cycle.
142+
// dedupSameRotations sorts the cycles so that shorter paths
143+
// and paths with lower versions appear earlier in the list.
144+
func dedupSameRotations(paths *[][]*node) {
145+
slices.SortFunc(*paths, func(a, b []*node) int {
146+
if len(a) == 0 && len(b) == 0 {
147+
return 0
148+
}
149+
if v := cmp.Compare(len(a), len(b)); v != 0 {
150+
return v
151+
}
152+
return a[0].bundle.Version.Compare(b[0].bundle.Version)
153+
})
154+
seen := map[string]struct{}{}
155+
tmp := (*paths)[:0]
156+
for _, path := range *paths {
157+
rotate(&path)
158+
k := nodeCycleString(path)
159+
if _, ok := seen[k]; ok {
160+
continue
161+
}
162+
seen[k] = struct{}{}
163+
tmp = append(tmp, path)
164+
}
165+
*paths = tmp
166+
}
167+
168+
func rotate(in *[]*node) {
169+
if len(*in) == 0 {
170+
return
171+
}
172+
maxIndex := 0
173+
for i, n := range (*in)[1:] {
174+
if n.bundle.Version.GT((*in)[maxIndex].bundle.Version) {
175+
maxIndex = i + 1
176+
}
177+
}
178+
slices.Reverse((*in)[:maxIndex])
179+
slices.Reverse((*in)[maxIndex:])
180+
slices.Reverse((*in))
181+
}
182+
183+
func nodeCycleString(nodes []*node) string {
184+
return strings.Join(mapSlice(nodes, nodeName), " -> ")
185+
}
186+
187+
func (g *graph) strandedNodes() ([]*node, error) {
188+
head, err := g.head()
189+
if err != nil {
190+
return nil, err
191+
}
192+
all := sets.New[*node](maps.Values(g.nodes)...)
193+
chain := sets.New[*node]()
194+
skipped := sets.New[*node]()
195+
196+
cur := head
197+
for cur != nil && !skipped.Has(cur) && !chain.Has(cur) {
198+
chain.Insert(cur)
199+
skipped.Insert(maps.Values(cur.skips)...)
200+
cur = cur.replaces
201+
}
202+
203+
stranded := all.Difference(chain).Difference(skipped).UnsortedList()
204+
slices.SortFunc(stranded, func(a, b *node) int {
205+
return a.bundle.Compare(b.bundle)
206+
})
207+
return stranded, nil
208+
}
209+
210+
func (g *graph) validateNoStranded() error {
211+
stranded, err := g.strandedNodes()
212+
if err != nil {
213+
return err
214+
}
215+
if len(stranded) == 0 {
216+
return nil
217+
}
218+
219+
return fmt.Errorf("channel contains one or more stranded bundles: %s", strings.Join(mapSlice(stranded, nodeName), ", "))
220+
}
221+
222+
func (g *graph) head() (*node, error) {
223+
heads := []*node{}
224+
for _, n := range g.nodes {
225+
if len(n.replacedBy) == 0 && len(n.skippedBy) == 0 {
226+
heads = append(heads, n)
227+
}
228+
}
229+
if len(heads) == 0 {
230+
return nil, fmt.Errorf("no channel head found in graph")
231+
}
232+
if len(heads) > 1 {
233+
headNames := mapSlice(heads, nodeName)
234+
sort.Strings(headNames)
235+
return nil, fmt.Errorf("multiple channel heads found in graph: %s", strings.Join(headNames, ", "))
236+
}
237+
return heads[0], nil
238+
}
239+
240+
func nodeName(n *node) string {
241+
return n.bundle.Name
242+
}
243+
244+
func mapSlice[I, O any](s []I, fn func(I) O) []O {
245+
result := make([]O, 0, len(s))
246+
for _, i := range s {
247+
result = append(result, fn(i))
248+
}
249+
return result
250+
}

alpha/model/graph_test.go

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
package model
2+
3+
import (
4+
"testing"
5+
6+
"github.com/blang/semver/v4"
7+
"github.com/stretchr/testify/assert"
8+
)
9+
10+
func Test_dedupSameRotations(t *testing.T) {
11+
type spec struct {
12+
name string
13+
paths [][]*node
14+
expect [][]*node
15+
}
16+
for _, s := range []spec{
17+
{
18+
name: "Empty",
19+
paths: [][]*node{},
20+
expect: [][]*node{},
21+
},
22+
{
23+
name: "One",
24+
paths: [][]*node{{{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}}},
25+
expect: [][]*node{{{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}}}},
26+
},
27+
{
28+
name: "Two",
29+
paths: [][]*node{
30+
{
31+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
32+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
33+
},
34+
{
35+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
36+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
37+
},
38+
},
39+
expect: [][]*node{{
40+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
41+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
42+
}},
43+
},
44+
{
45+
name: "Three",
46+
paths: [][]*node{
47+
{
48+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
49+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
50+
{bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}},
51+
},
52+
{
53+
{bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}},
54+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
55+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
56+
},
57+
{
58+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
59+
{bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}},
60+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
61+
},
62+
},
63+
expect: [][]*node{{
64+
{bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}},
65+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
66+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
67+
}},
68+
},
69+
{
70+
name: "Multiple",
71+
paths: [][]*node{
72+
{
73+
{bundle: &Bundle{Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4")}},
74+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
75+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
76+
},
77+
{
78+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
79+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
80+
{bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}},
81+
},
82+
{
83+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
84+
{bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}},
85+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
86+
},
87+
{
88+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
89+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
90+
{bundle: &Bundle{Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4")}},
91+
},
92+
{
93+
{bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}},
94+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
95+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
96+
},
97+
{
98+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
99+
{bundle: &Bundle{Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4")}},
100+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
101+
},
102+
},
103+
expect: [][]*node{
104+
{
105+
{bundle: &Bundle{Name: "anakin.v0.0.3", Version: semver.MustParse("0.0.3")}},
106+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
107+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
108+
},
109+
{
110+
{bundle: &Bundle{Name: "anakin.v0.0.4", Version: semver.MustParse("0.0.4")}},
111+
{bundle: &Bundle{Name: "anakin.v0.0.1", Version: semver.MustParse("0.0.1")}},
112+
{bundle: &Bundle{Name: "anakin.v0.0.2", Version: semver.MustParse("0.0.2")}},
113+
},
114+
},
115+
},
116+
} {
117+
t.Run(s.name, func(t *testing.T) {
118+
dedupSameRotations(&s.paths)
119+
assert.Equal(t, s.expect, s.paths)
120+
})
121+
}
122+
}

0 commit comments

Comments
 (0)