Skip to content

Commit 396ca5b

Browse files
njhalenjhale
committedNov 13, 2020
fix(indexing): order bulk add by version field
When adding more than one bundle to an index in replaces mode, sort the input by order of ascending version field (semver). This ensures a deterministic update graph in the absence of a replaces field. The replaces field takes priority.
·
v1.51.0goreleaser-all-os
1 parent ed41d85 commit 396ca5b

File tree

13 files changed

+806
-206
lines changed

13 files changed

+806
-206
lines changed
 

‎cmd/opm/index/add.go

+2
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ var (
1717
Add operator bundles to an index.
1818
1919
This command will add the given set of bundle images (specified by the --bundles option) to an index image (provided by the --from-index option).
20+
21+
If multiple bundles are given with '--mode=replaces' (the default), bundles are added to the index by order of ascending (semver) version unless the update graph specified by replaces requires a different input order; e.g. 1.0.0 replaces 1.0.1 would result in [1.0.1, 1.0.0] instead of the [1.0.0, 1.0.1] normally expected of semver. However, for most cases (e.g. 1.0.1 replaces 1.0.0) the bundle with the highest version is used to set the default channel of the related package.
2022
`)
2123

2224
addExample = templates.Examples(`

‎go.mod

+1
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ require (
5757
k8s.io/api v0.19.3
5858
k8s.io/apiextensions-apiserver v0.19.3
5959
k8s.io/apimachinery v0.19.3
60+
k8s.io/apiserver v0.19.3
6061
k8s.io/client-go v0.19.3
6162
k8s.io/klog v1.0.0
6263
k8s.io/kubectl v0.18.0

‎pkg/containertools/containertoolsfakes/fake_command_runner.go

-75
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/lib/indexer/indexerfakes/fake_index_exporter.go

+110
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/registry/input_stream.go

+162
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,162 @@
1+
package registry
2+
3+
import (
4+
"fmt"
5+
"sort"
6+
7+
"github.com/blang/semver"
8+
utilerrors "k8s.io/apimachinery/pkg/util/errors"
9+
)
10+
11+
type ReplacesInputStream struct {
12+
graph GraphLoader
13+
packages map[string][]*ImageInput
14+
}
15+
16+
func NewReplacesInputStream(graph GraphLoader, toAdd []*ImageInput) (*ReplacesInputStream, error) {
17+
stream := &ReplacesInputStream{
18+
graph: graph,
19+
packages: map[string][]*ImageInput{},
20+
}
21+
22+
// Sort the bundle images into buckets by package
23+
for _, image := range toAdd {
24+
pkg := image.Bundle.Package
25+
stream.packages[pkg] = append(stream.packages[pkg], image)
26+
}
27+
28+
// Sort each package bucket by ascending semver
29+
var errs []error
30+
for pkg, bundles := range stream.packages {
31+
var pkgErrs []error
32+
sort.SliceStable(bundles, func(i, j int) bool {
33+
less, err := bundleVersionLess(bundles[i].Bundle, bundles[j].Bundle)
34+
if err != nil {
35+
pkgErrs = append(pkgErrs, err)
36+
return false
37+
}
38+
39+
return less
40+
})
41+
42+
if len(pkgErrs) > 0 {
43+
// Ignore this package
44+
delete(stream.packages, pkg)
45+
errs = append(errs, pkgErrs...)
46+
continue
47+
}
48+
49+
stream.packages[pkg] = bundles
50+
}
51+
52+
return stream, utilerrors.NewAggregate(errs)
53+
}
54+
55+
// bundleVersionLess returns true if the version of the first bundle parameter is less than the second, otherwise it returns false.
56+
// An error is returned when either bundle doesn't specify a version or their versions are not valid semver.
57+
func bundleVersionLess(a, b *Bundle) (bool, error) {
58+
var errs []error
59+
rawVersionA, err := a.Version()
60+
if err != nil {
61+
errs = append(errs, fmt.Errorf("unable to get version for bundle %s: %s", a.Name, err))
62+
}
63+
versionA, err := semver.Parse(rawVersionA)
64+
if err != nil {
65+
errs = append(errs, fmt.Errorf("unable to parse version for bundle %s: %s", a.Name, err))
66+
}
67+
68+
rawVersionB, err := b.Version()
69+
if err != nil {
70+
errs = append(errs, fmt.Errorf("unable to get version for bundle %s: %s", b.Name, err))
71+
}
72+
versionB, err := semver.Parse(rawVersionB)
73+
if err != nil {
74+
errs = append(errs, fmt.Errorf("unable to parse version for bundle %s: %s", b.Name, err))
75+
}
76+
77+
if len(errs) > 0 {
78+
return false, utilerrors.NewAggregate(errs)
79+
}
80+
81+
return versionA.LT(versionB), nil
82+
}
83+
84+
// canAdd checks that a new bundle can be added in replaces mode (i.e. the replaces defined for the bundle already exists)
85+
func (r *ReplacesInputStream) canAdd(bundle *Bundle, packageGraph *Package) error {
86+
replaces, err := bundle.Replaces()
87+
if err != nil {
88+
return fmt.Errorf("Invalid bundle replaces: %s", err)
89+
}
90+
91+
if replaces != "" && !packageGraph.HasCsv(replaces) {
92+
// We can't add this until a replacement exists
93+
return fmt.Errorf("Invalid bundle %s, bundle specifies a non-existent replacement %s", bundle.Name, replaces)
94+
}
95+
96+
images, ok := r.packages[packageGraph.Name]
97+
if !ok || images == nil {
98+
// This shouldn't happen unless canAdd is being called without the correct setup
99+
panic(fmt.Sprintf("Programmer error: package graph %s incorrectly initialized", packageGraph.Name))
100+
}
101+
102+
// No edges to any remaining input bundles, this bundle can be added
103+
return nil
104+
}
105+
106+
// Next returns the next available bundle image from the stream, returning a nil image if the stream is exhausted.
107+
func (r *ReplacesInputStream) Next() (*ImageInput, error) {
108+
var errs []error
109+
for pkg, images := range r.packages {
110+
if len(images) < 1 {
111+
// No more images to add for this package, clean up
112+
delete(r.packages, pkg)
113+
continue
114+
}
115+
116+
packageGraph, err := r.graph.Generate(pkg)
117+
if err != nil {
118+
if err != ErrPackageNotInDatabase {
119+
// Can't parse this package any further
120+
delete(r.packages, pkg)
121+
errs = append(errs, err)
122+
continue
123+
}
124+
125+
// Adding a brand new package is a different story
126+
packageGraph = &Package{Name: pkg}
127+
}
128+
129+
// Find the next bundle in topological order
130+
var packageErrs []error
131+
for i, image := range images {
132+
if err := r.canAdd(image.Bundle, packageGraph); err != nil {
133+
// Can't parse this bundle any further right now
134+
packageErrs = append(packageErrs, err)
135+
continue
136+
}
137+
138+
// Found something we can add
139+
r.packages[pkg] = append(r.packages[pkg][:i], r.packages[pkg][i+1:]...)
140+
if len(r.packages[pkg]) < 1 {
141+
// Remove package if exhausted
142+
delete(r.packages, pkg)
143+
}
144+
145+
return image, nil
146+
}
147+
148+
// No viable bundle found in the package, can't parse it any further
149+
if len(packageErrs) > 0 {
150+
delete(r.packages, pkg)
151+
errs = append(errs, packageErrs...)
152+
}
153+
}
154+
155+
// We've exhausted all valid input bundles, any errors here indicate invalid input of some kind
156+
return nil, utilerrors.NewAggregate(errs)
157+
}
158+
159+
// Empty returns true if there are no bundles in the stream.
160+
func (r *ReplacesInputStream) Empty() bool {
161+
return len(r.packages) < 1
162+
}

‎pkg/registry/input_stream_test.go

+314
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,314 @@
1+
package registry_test
2+
3+
import (
4+
"math"
5+
"math/rand"
6+
"reflect"
7+
"testing"
8+
"testing/quick"
9+
10+
"github.com/blang/semver"
11+
"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
12+
"k8s.io/apimachinery/pkg/runtime/schema"
13+
storagenames "k8s.io/apiserver/pkg/storage/names"
14+
15+
"github.com/operator-framework/operator-registry/pkg/registry"
16+
"github.com/operator-framework/operator-registry/pkg/registry/registryfakes"
17+
)
18+
19+
func genName(base string) string {
20+
return storagenames.SimpleNameGenerator.GenerateName(base)
21+
}
22+
23+
func unstructuredCSV() *unstructured.Unstructured {
24+
csv := &unstructured.Unstructured{}
25+
csv.GetObjectKind().SetGroupVersionKind(schema.GroupVersionKind{
26+
// NOTE: For some reason, this is all bundle.Add cares about
27+
Kind: "ClusterServiceVersion",
28+
})
29+
30+
return csv
31+
}
32+
33+
type inputConfig struct {
34+
pkg string
35+
name string
36+
version string
37+
replaces string
38+
}
39+
40+
func newImageInput(config *inputConfig) *registry.ImageInput {
41+
image := &registry.ImageInput{
42+
Bundle: &registry.Bundle{
43+
Package: config.pkg,
44+
Name: config.name,
45+
},
46+
}
47+
48+
csv := unstructuredCSV()
49+
content := csv.UnstructuredContent() // We don't bother setting a name since the bundle name is used everywhere
50+
if content == nil {
51+
content = map[string]interface{}{}
52+
}
53+
content["spec"] = map[string]interface{}{
54+
"version": config.version,
55+
"replaces": config.replaces,
56+
}
57+
csv.SetUnstructuredContent(content)
58+
image.Bundle.Add(csv)
59+
60+
return image
61+
}
62+
63+
func bumpVersion(r *rand.Rand, base semver.Version) semver.Version {
64+
inc := func(v uint64) uint64 {
65+
return v + uint64(math.Max(1, float64(r.Intn(3)))) // Bump by 1 to 3 randomly
66+
}
67+
68+
return semver.Version{
69+
Major: inc(base.Major),
70+
Minor: inc(base.Minor),
71+
Patch: inc(base.Patch),
72+
}
73+
}
74+
75+
type semverSequence struct {
76+
input []*registry.ImageInput
77+
ordered []string
78+
}
79+
80+
func (semverSequence) Generate(r *rand.Rand, max int) reflect.Value {
81+
size := r.Intn(max + 1) // +1 prevents an invalid zero argument and includes max (generate should never be given a negative max)
82+
input := make([]*registry.ImageInput, size)
83+
ordered := make([]string, size)
84+
var version semver.Version
85+
for i := 0; i < size; i++ {
86+
// Each bundle is added in order of ascending semver
87+
version = bumpVersion(r, version)
88+
config := inputConfig{
89+
name: genName(""),
90+
version: version.String(),
91+
}
92+
93+
input[i] = newImageInput(&config)
94+
ordered[i] = config.name
95+
}
96+
97+
// Ensure we're not depending on any ordering
98+
r.Shuffle(len(input), func(i, j int) { input[i], input[j] = input[j], input[i] })
99+
100+
return reflect.ValueOf(semverSequence{
101+
input: input,
102+
ordered: ordered,
103+
})
104+
}
105+
106+
func TestNextReturnsSemverSequence(t *testing.T) {
107+
f := func(sequences []semverSequence) bool {
108+
if len(sequences) < 1 {
109+
return true
110+
}
111+
112+
ordered := map[string][]string{}
113+
var input []*registry.ImageInput
114+
for _, sequence := range sequences {
115+
pkg := genName("")
116+
for _, in := range sequence.input {
117+
in.Bundle.Package = pkg
118+
input = append(input, in)
119+
}
120+
ordered[pkg] = sequence.ordered
121+
}
122+
123+
loader := &registryfakes.FakeGraphLoader{
124+
GenerateStub: func(pkg string) (*registry.Package, error) {
125+
return &registry.Package{
126+
Name: pkg,
127+
}, nil
128+
},
129+
}
130+
131+
stream, err := registry.NewReplacesInputStream(loader, input)
132+
if err != nil {
133+
t.Error(err)
134+
return false
135+
}
136+
137+
for !stream.Empty() {
138+
next, err := stream.Next()
139+
if err != nil {
140+
t.Errorf("next returned unexpected error: %s", err)
141+
return false
142+
}
143+
if next == nil {
144+
t.Errorf("next returned unexpected nil")
145+
return false
146+
}
147+
148+
pkg := next.Bundle.Package
149+
ord, ok := ordered[pkg]
150+
if !ok {
151+
t.Errorf("next returned bundle for unexpected package %s", pkg)
152+
return false
153+
}
154+
155+
name := next.Bundle.Name
156+
if len(ord) < 1 {
157+
t.Errorf("next returned extra bundle for package %s: %s", pkg, name)
158+
return false
159+
}
160+
161+
if name != ord[0] {
162+
t.Errorf("next returned unexpecting bundle %s/%s, expecting %s/%s: %v", pkg, name, pkg, ord[0], ordered)
163+
return false
164+
}
165+
166+
if len(ord) > 1 {
167+
ordered[pkg] = ord[1:] // Dequeue expected order
168+
continue
169+
}
170+
171+
// We've been given all the bundles we've expected for this package
172+
delete(ordered, pkg)
173+
}
174+
175+
return true
176+
}
177+
if err := quick.Check(f, nil); err != nil {
178+
t.Error(err)
179+
}
180+
}
181+
182+
func TestVersionIgnoredForIndividualAdd(t *testing.T) {
183+
input := []*registry.ImageInput{
184+
newImageInput(&inputConfig{
185+
name: genName(""),
186+
}),
187+
}
188+
189+
if _, err := registry.NewReplacesInputStream(nil, input); err != nil {
190+
t.Error(err)
191+
}
192+
}
193+
194+
func TestVersionRequiredForBulkAdd(t *testing.T) {
195+
pkg := genName("")
196+
input := []*registry.ImageInput{
197+
newImageInput(&inputConfig{
198+
pkg: pkg,
199+
name: genName(""),
200+
}),
201+
newImageInput(&inputConfig{
202+
pkg: pkg,
203+
name: genName(""),
204+
}),
205+
}
206+
207+
if _, err := registry.NewReplacesInputStream(nil, input); err == nil {
208+
t.Errorf("input stream accepted invalid bundles with missing version fields")
209+
}
210+
}
211+
212+
func TestReplacesTakesPrecedence(t *testing.T) {
213+
// Set up semver order to be the opposite of replaces order
214+
input := []*registry.ImageInput{
215+
newImageInput(&inputConfig{
216+
name: "d",
217+
replaces: "c",
218+
version: "1.0.0",
219+
}),
220+
newImageInput(&inputConfig{
221+
name: "c",
222+
replaces: "b",
223+
version: "1.0.1",
224+
}),
225+
newImageInput(&inputConfig{
226+
name: "b",
227+
replaces: "a",
228+
version: "1.2.0",
229+
}),
230+
newImageInput(&inputConfig{
231+
name: "a",
232+
version: "1.2.5",
233+
}),
234+
}
235+
236+
added := map[registry.BundleKey]map[registry.BundleKey]struct{}{}
237+
loader := &registryfakes.FakeGraphLoader{
238+
GenerateStub: func(pkg string) (*registry.Package, error) {
239+
return &registry.Package{
240+
Name: pkg,
241+
Channels: map[string]registry.Channel{
242+
"": registry.Channel{
243+
Nodes: added,
244+
},
245+
},
246+
}, nil
247+
},
248+
}
249+
250+
stream, err := registry.NewReplacesInputStream(loader, input)
251+
if err != nil {
252+
t.Error(err)
253+
return
254+
}
255+
256+
// Expect input to be returned in replaces order
257+
for _, expected := range []string{"a", "b", "c", "d"} {
258+
next, err := stream.Next()
259+
if err != nil {
260+
t.Error(err)
261+
return
262+
}
263+
if next == nil {
264+
t.Errorf("next returned unexpected nil, expecting %s", expected)
265+
return
266+
}
267+
268+
name := next.Bundle.Name
269+
if name != expected {
270+
t.Errorf("next returned unexpected bundle %s, expecting %s", name, expected)
271+
return
272+
}
273+
274+
// Simulate an add
275+
added[registry.BundleKey{CsvName: name}] = nil // Only the key is compared in HasCsv()
276+
}
277+
278+
if !stream.Empty() {
279+
t.Errorf("stream still contains content, expected end of content")
280+
}
281+
}
282+
283+
func TestMissingReplacesReturnsError(t *testing.T) {
284+
input := []*registry.ImageInput{
285+
newImageInput(&inputConfig{
286+
name: genName(""),
287+
replaces: genName(""), // This won't exist and should cause a failure
288+
}),
289+
}
290+
291+
loader := &registryfakes.FakeGraphLoader{
292+
GenerateStub: func(pkg string) (*registry.Package, error) {
293+
return &registry.Package{
294+
Name: pkg,
295+
}, nil
296+
},
297+
}
298+
299+
stream, err := registry.NewReplacesInputStream(loader, input)
300+
if err != nil {
301+
t.Error(err)
302+
return
303+
}
304+
305+
next, err := stream.Next()
306+
if err == nil {
307+
t.Errorf("expected next to return an error")
308+
return
309+
}
310+
if next != nil {
311+
t.Errorf("expected next to return nil on error, got %v instead", next)
312+
return
313+
}
314+
}

‎pkg/registry/interface.go

+2
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,8 @@ type Query interface {
6161
GetBundlePathIfExists(ctx context.Context, csvName string) (string, error)
6262
}
6363

64+
//go:generate go run github.com/maxbrunsfeld/counterfeiter/v6 . GraphLoader
65+
6466
// GraphLoader generates a graph
6567
// GraphLoader supports multiple different loading schemes
6668
// GraphLoader from SQL, GraphLoader from old format (filesystem), GraphLoader from SQL + input bundles

‎pkg/registry/populator.go

+23-73
Original file line numberDiff line numberDiff line change
@@ -165,33 +165,42 @@ func (i *DirectoryPopulator) loadManifests(imagesToAdd []*ImageInput, imagesToRe
165165
// Additionally, it would be preferrable if there was a single database transaction
166166
// that took the updated graph as a whole as input, rather than inserting bundles of the
167167
// same package linearly.
168-
var err error
169-
var validImagesToAdd []*ImageInput
170-
171168
for pkg := range i.overwriteDirMap {
172169
// TODO: If this succeeds but the add fails there will be a disconnect between
173170
// the registry and the index. Loading the bundles in a single transactions as
174171
// described above would allow us to do the removable in that same transaction
175172
// and ensure that rollback is possible.
176-
err := i.loader.RemovePackage(pkg)
177-
if err != nil {
173+
if err := i.loader.RemovePackage(pkg); err != nil {
178174
return err
179175
}
180176
}
181177

182-
imagesToAdd = append(imagesToAdd, imagesToReAdd...)
178+
var errs []error
179+
stream, err := NewReplacesInputStream(i.graphLoader, append(imagesToAdd, imagesToReAdd...))
180+
if err != nil {
181+
errs = append(errs, fmt.Errorf("Input error: %s", err))
182+
// Don't return yet since stream may be partially initialized and still useful
183+
}
184+
if stream == nil {
185+
return utilerrors.NewAggregate(errs)
186+
}
183187

184-
for len(imagesToAdd) > 0 {
185-
validImagesToAdd, imagesToAdd, err = i.getNextReplacesImagesToAdd(imagesToAdd)
188+
for !stream.Empty() {
189+
next, err := stream.Next()
186190
if err != nil {
187-
return err
191+
errs = append(errs, err)
192+
break
188193
}
189-
for _, image := range validImagesToAdd {
190-
err := i.loadManifestsReplaces(image.Bundle, image.AnnotationsFile)
191-
if err != nil {
192-
return err
193-
}
194+
195+
if err = i.loadManifestsReplaces(next.Bundle, next.AnnotationsFile); err != nil {
196+
errs = append(errs, err)
197+
break
194198
}
199+
200+
}
201+
202+
if len(errs) > 0 {
203+
return utilerrors.NewAggregate(errs)
195204
}
196205
case SemVerMode:
197206
for _, image := range imagesToAdd {
@@ -240,65 +249,6 @@ func (i *DirectoryPopulator) loadManifestsReplaces(bundle *Bundle, annotationsFi
240249
return nil
241250
}
242251

243-
func (i *DirectoryPopulator) getNextReplacesImagesToAdd(imagesToAdd []*ImageInput) ([]*ImageInput, []*ImageInput, error) {
244-
remainingImages := make([]*ImageInput, 0)
245-
foundImages := make([]*ImageInput, 0)
246-
247-
var errs []error
248-
249-
// Separate these image sets per package, since multiple different packages have
250-
// separate graph
251-
imagesPerPackage := make(map[string][]*ImageInput, 0)
252-
for _, image := range imagesToAdd {
253-
pkg := image.Bundle.Package
254-
if _, ok := imagesPerPackage[pkg]; !ok {
255-
newPkgImages := make([]*ImageInput, 0)
256-
newPkgImages = append(newPkgImages, image)
257-
imagesPerPackage[pkg] = newPkgImages
258-
} else {
259-
imagesPerPackage[pkg] = append(imagesPerPackage[pkg], image)
260-
}
261-
}
262-
263-
for pkg, pkgImages := range imagesPerPackage {
264-
// keep a tally of valid and invalid images to ensure at least one
265-
// image per package is valid. If not, throw an error
266-
pkgRemainingImages := 0
267-
pkgFoundImages := 0
268-
269-
// first, try to pull the existing package graph from the database if it exists
270-
graph, err := i.graphLoader.Generate(pkg)
271-
if err != nil && !errors.Is(err, ErrPackageNotInDatabase) {
272-
return nil, nil, err
273-
}
274-
275-
var pkgErrs []error
276-
// then check each image to see if it can be a replacement
277-
replacesLoader := ReplacesGraphLoader{}
278-
for _, pkgImage := range pkgImages {
279-
canAdd, err := replacesLoader.CanAdd(pkgImage.Bundle, graph)
280-
if err != nil {
281-
pkgErrs = append(pkgErrs, err)
282-
}
283-
if canAdd {
284-
pkgFoundImages++
285-
foundImages = append(foundImages, pkgImage)
286-
} else {
287-
pkgRemainingImages++
288-
remainingImages = append(remainingImages, pkgImage)
289-
}
290-
}
291-
292-
// no new images can be added, the current iteration aggregates all the
293-
// errors that describe invalid bundles
294-
if pkgFoundImages == 0 && pkgRemainingImages > 0 {
295-
errs = append(errs, pkgErrs...)
296-
}
297-
}
298-
299-
return foundImages, remainingImages, utilerrors.NewAggregate(errs)
300-
}
301-
302252
func (i *DirectoryPopulator) loadManifestsSemver(bundle *Bundle, annotations *AnnotationsFile, skippatch bool) error {
303253
graph, err := i.graphLoader.Generate(bundle.Package)
304254
if err != nil && !errors.Is(err, ErrPackageNotInDatabase) {

‎pkg/registry/populator_test.go

+22-28
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import (
88
"math/rand"
99
"os"
1010
"reflect"
11-
"sort"
1211
"strings"
1312
"testing"
1413
"time"
@@ -713,11 +712,11 @@ func TestDirectoryPopulator(t *testing.T) {
713712
image.SimpleReference("quay.io/test/etcd.0.9.2"): "../../bundles/etcd.0.9.2",
714713
image.SimpleReference("quay.io/test/prometheus.0.22.2"): "../../bundles/prometheus.0.22.2",
715714
}
716-
expectedErr := errors.NewAggregate([]error{
717-
fmt.Errorf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "etcdoperator.v0.9.2", "etcdoperator.v0.9.0"),
718-
fmt.Errorf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "prometheusoperator.0.22.2", "prometheusoperator.0.15.0"),
719-
})
720-
require.ElementsMatch(t, expectedErr, populate(add))
715+
716+
err = populate(add)
717+
require.NotNil(t, err)
718+
require.Contains(t, err.Error(), fmt.Sprintf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "etcdoperator.v0.9.2", "etcdoperator.v0.9.0"))
719+
require.Contains(t, err.Error(), fmt.Sprintf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "prometheusoperator.0.22.2", "prometheusoperator.0.15.0"))
721720
}
722721

723722
func TestDeprecateBundle(t *testing.T) {
@@ -914,7 +913,7 @@ func TestOverwrite(t *testing.T) {
914913
}
915914
type pkgChannel map[string][]string
916915
type expected struct {
917-
err error
916+
errs []error
918917
remainingBundles []string
919918
remainingPkgChannels pkgChannel
920919
remainingDefaultChannels map[string]string
@@ -943,10 +942,10 @@ func TestOverwrite(t *testing.T) {
943942
overwrites: nil,
944943
},
945944
expected: expected{
946-
err: errors.NewAggregate([]error{
945+
errs: []error{
947946
fmt.Errorf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "etcdoperator.v0.9.2", "etcdoperator.v0.9.0"),
948947
fmt.Errorf("Invalid bundle %s, bundle specifies a non-existent replacement %s", "prometheusoperator.0.22.2", "prometheusoperator.0.15.0"),
949-
}),
948+
},
950949
remainingBundles: []string{
951950
"quay.io/test/prometheus.0.14.0/preview",
952951
},
@@ -971,7 +970,7 @@ func TestOverwrite(t *testing.T) {
971970
overwrites: map[string]map[image.Reference]string{"etcd": {}},
972971
},
973972
expected: expected{
974-
err: nil,
973+
errs: nil,
975974
remainingBundles: []string{
976975
"quay.io/test/new-etcd.0.9.0/alpha",
977976
"quay.io/test/new-etcd.0.9.0/beta",
@@ -1010,7 +1009,7 @@ func TestOverwrite(t *testing.T) {
10101009
overwrites: map[string]map[image.Reference]string{"etcd": getBundleRefs([]string{"etcd.0.9.0"})},
10111010
},
10121011
expected: expected{
1013-
err: nil,
1012+
errs: nil,
10141013
remainingBundles: []string{
10151014
"quay.io/test/etcd.0.9.0/alpha",
10161015
"quay.io/test/etcd.0.9.0/beta",
@@ -1051,7 +1050,7 @@ func TestOverwrite(t *testing.T) {
10511050
overwrites: map[string]map[image.Reference]string{"prometheus": getBundleRefs([]string{"prometheus.0.14.0", "prometheus.0.15.0"})},
10521051
},
10531052
expected: expected{
1054-
err: nil,
1053+
errs: nil,
10551054
remainingBundles: []string{
10561055
"quay.io/test/etcd.0.9.0/alpha",
10571056
"quay.io/test/etcd.0.9.0/beta",
@@ -1096,7 +1095,7 @@ func TestOverwrite(t *testing.T) {
10961095
overwrites: map[string]map[image.Reference]string{"prometheus": getBundleRefs([]string{"prometheus.0.14.0"})},
10971096
},
10981097
expected: expected{
1099-
err: nil,
1098+
errs: nil,
11001099
remainingBundles: []string{
11011100
"quay.io/test/etcd.0.9.0/alpha",
11021101
"quay.io/test/etcd.0.9.0/beta",
@@ -1137,7 +1136,7 @@ func TestOverwrite(t *testing.T) {
11371136
overwrites: map[string]map[image.Reference]string{"prometheus": getBundleRefs([]string{"prometheus.0.14.0"})},
11381137
},
11391138
expected: expected{
1140-
err: errors.NewAggregate([]error{registry.OverwriteErr{ErrorString: "Cannot overwrite a bundle that is not at the head of a channel using --overwrite-latest"}}),
1139+
errs: []error{registry.OverwriteErr{ErrorString: "Cannot overwrite a bundle that is not at the head of a channel using --overwrite-latest"}},
11411140
remainingBundles: []string{
11421141
"quay.io/test/prometheus.0.14.0/preview",
11431142
"quay.io/test/prometheus.0.14.0/stable",
@@ -1170,7 +1169,7 @@ func TestOverwrite(t *testing.T) {
11701169
},
11711170
},
11721171
expected: expected{
1173-
err: nil,
1172+
errs: nil,
11741173
remainingBundles: []string{
11751174
"quay.io/test/etcd.0.9.0/alpha",
11761175
"quay.io/test/etcd.0.9.0/beta",
@@ -1215,7 +1214,7 @@ func TestOverwrite(t *testing.T) {
12151214
},
12161215
},
12171216
expected: expected{
1218-
err: errors.NewAggregate([]error{registry.OverwriteErr{ErrorString: "Cannot overwrite more than one bundle at a time for a given package using --overwrite-latest"}}),
1217+
errs: []error{registry.OverwriteErr{ErrorString: "Cannot overwrite more than one bundle at a time for a given package using --overwrite-latest"}},
12191218
remainingBundles: []string{
12201219
"quay.io/test/etcd.0.9.0/alpha",
12211220
"quay.io/test/etcd.0.9.0/beta",
@@ -1272,19 +1271,14 @@ func TestOverwrite(t *testing.T) {
12721271
true).Populate(registry.ReplacesMode)
12731272
}
12741273
require.NoError(t, populate(tt.args.firstAdd, nil))
1275-
popErr := populate(tt.args.secondAdd, tt.args.overwrites)
1276-
if agg, ok := popErr.(utilerrors.Aggregate); ok {
1277-
// The order of the errors that
1278-
// comprise an aggregate error isn't
1279-
// important to the tested behaviors,
1280-
// so sort them:
1281-
errs := agg.Errors()
1282-
sort.Slice(errs, func(i, j int) bool {
1283-
return errs[i].Error() < errs[j].Error()
1284-
})
1285-
popErr = utilerrors.NewAggregate(errs)
1274+
1275+
err = populate(tt.args.secondAdd, tt.args.overwrites)
1276+
if len(tt.expected.errs) < 1 {
1277+
require.NoError(t, err)
1278+
}
1279+
for _, e := range tt.expected.errs {
1280+
require.Contains(t, err.Error(), e.Error())
12861281
}
1287-
require.Equal(t, tt.expected.err, popErr)
12881282

12891283
// Ensure remaining bundlePaths in db match
12901284
bundles, err := query.ListBundles(context.Background())

‎pkg/registry/registryfakes/fake_graph_loader.go

+115
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pkg/registry/replacesgraphloader.go

-30
This file was deleted.

‎vendor/k8s.io/apiserver/pkg/storage/names/generate.go

+54
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎vendor/modules.txt

+1
Original file line numberDiff line numberDiff line change
@@ -682,6 +682,7 @@ k8s.io/apiserver/pkg/apis/apiserver/v1alpha1
682682
k8s.io/apiserver/pkg/apis/apiserver/v1beta1
683683
k8s.io/apiserver/pkg/server/egressselector
684684
k8s.io/apiserver/pkg/server/egressselector/metrics
685+
k8s.io/apiserver/pkg/storage/names
685686
k8s.io/apiserver/pkg/util/webhook
686687
# k8s.io/client-go v0.19.3
687688
k8s.io/client-go/discovery

0 commit comments

Comments
 (0)
Please sign in to comment.