Skip to content

Commit 87f9927

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 87f9927

File tree

4 files changed

+530
-93
lines changed

4 files changed

+530
-93
lines changed

alpha/model/graph.go

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

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)