Skip to content

Commit 6c7f615

Browse files
committed
fix(resolver): be smarter about the way downgrades are attempted
"downgrade" here refers to downgrading potential updates back to what has already been installed. this fixes issues where OLM would otherwise not have been able to determine a safe set of operators to update to - the case where, after tentative updates to the current generation, there are required apis missing from the set. the previous algorithm solves this by downgrading the updates that require the missing apis, which has the potential to downgrade back to the initial set of operators. in the case where the api was provided by some operator in the previous generation, it would be possible to not downgrade all requirers, but rather downgrade the operator that previously provided the api. this commit implements an exhaustive search of potential downgrades for a generation, and uses one without missing required apis if one is found if none is found, the old behavior takes place, and the set may still contract back to what is currently installed. the new resolver avoids this issue entirely, so to avoid complicating the implementation a limit of 64 updated operators per generation has been added. the implementation uses a bitmask to select either the old or new operator from the set. finding all possible combinations of old/new versions is then a simple integer count from 0 to the max.
1 parent 69478e6 commit 6c7f615

File tree

2 files changed

+89
-0
lines changed

2 files changed

+89
-0
lines changed

pkg/controller/registry/resolver/evolver.go

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ type Evolver interface {
1717
type NamespaceGenerationEvolver struct {
1818
querier SourceQuerier
1919
gen Generation
20+
replacements map[OperatorSurface]OperatorSurface
2021
}
2122

2223
func NewNamespaceGenerationEvolver(querier SourceQuerier, gen Generation) Evolver {
@@ -44,13 +45,20 @@ func (e *NamespaceGenerationEvolver) Evolve(add map[OperatorSourceInfo]struct{})
4445
return err
4546
}
4647

48+
// if apis are missing, attempt to contract back to a good set by trying all combinations of rollbacks to updates
49+
if err := e.downgradeUpdates(); err != nil {
50+
return err
51+
}
52+
4753
// for any remaining missing APIs, attempt to downgrade the operator that required them
4854
// this may contract the generation back to the original set!
4955
e.downgradeAPIs()
5056
return nil
5157
}
5258

5359
func (e *NamespaceGenerationEvolver) checkForUpdates() error {
60+
e.replacements = make(map[OperatorSurface]OperatorSurface)
61+
5462
// maps the old operator identifier to the new operator
5563
updates := EmptyOperatorSet()
5664

@@ -74,6 +82,7 @@ func (e *NamespaceGenerationEvolver) checkForUpdates() error {
7482
}
7583
o.SetReplaces(op.Identifier())
7684
updates[op.Identifier()] = o
85+
e.replacements[op] = o
7786
}
7887

7988
// remove any operators we found updates for
@@ -153,7 +162,65 @@ func (e *NamespaceGenerationEvolver) queryForRequiredAPIs() error {
153162
return nil
154163
}
155164

165+
func (e *NamespaceGenerationEvolver) downgradeUpdates() error {
166+
// no need to attempt downgrades
167+
if len(e.gen.MissingAPIs()) == 0 {
168+
return nil
169+
}
170+
171+
// smart downgrades are only supported if fewer than 64 updates are resolved at the same time
172+
// (this should be all but pathological cases)
173+
if len(e.replacements) > 64 {
174+
return nil
175+
}
176+
177+
// updates[1][x] = old
178+
// updates[0][x] = new
179+
updates := [][]OperatorSurface{make([]OperatorSurface, 0), make([]OperatorSurface, 0)}
180+
for o, n := range e.replacements {
181+
updates[1] = append(updates[1], o)
182+
updates[0] = append(updates[0], n)
183+
}
184+
flagToIndex := make(map[uint64]int)
185+
flags := make([]uint64, 0)
186+
var max uint64
187+
for i := 0; i < len(e.replacements); i++ {
188+
var f uint64 = 1 << i
189+
flags = append(flags, f)
190+
max += f
191+
flagToIndex[f] = i
192+
}
193+
194+
var i uint64
195+
var g Generation
196+
for i = 0; i <= max; i++ {
197+
g = NewEmptyGeneration()
198+
for _, f := range flags {
199+
idx := flagToIndex[f]
200+
// if toggled, pick old
201+
_ = g.AddOperator(updates[f&i][idx])
202+
}
203+
// we found a good set, update the real generation and quit
204+
if len(g.MissingAPIs()) == 0 {
205+
for _, f := range flags {
206+
idx := flagToIndex[f]
207+
// if toggled, remove new and add old
208+
if f&i != 0 {
209+
e.gen.RemoveOperator(updates[0][idx])
210+
if err := e.gen.AddOperator(updates[1][idx]); err != nil {
211+
return err
212+
}
213+
}
214+
}
215+
return nil
216+
}
217+
}
218+
219+
return nil
220+
}
221+
156222
func (e *NamespaceGenerationEvolver) downgradeAPIs() {
223+
// remove anything we can't satisfy after trying to downgrade all possible combinations
157224
e.gen.ResetUnchecked()
158225
for missingAPIs := e.gen.MissingAPIs(); len(missingAPIs) > 0; {
159226
requirers := missingAPIs.PopAPIRequirers()

pkg/controller/registry/resolver/evolver_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -451,6 +451,28 @@ func TestNamespaceGenerationEvolver(t *testing.T) {
451451
NewFakeOperatorSurface("depender.v1", "depender", "channel", "", "catsrc", "", nil,[]opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
452452
),
453453
},
454+
{
455+
name: "NoProviderInUpdateSet",
456+
fields: fields{
457+
querier: NewFakeSourceQuerier(map[CatalogKey][]*api.Bundle{
458+
CatalogKey{"catsrc", "catsrc-namespace"}: {
459+
bundle("original", "o", "c", "", APISet{opregistry.APIKey{"g", "v", "k", "ks"}: {}}, EmptyAPISet(), EmptyAPISet(), EmptyAPISet()),
460+
bundle("original.v2", "o", "c", "original", EmptyAPISet(), EmptyAPISet(), EmptyAPISet(), EmptyAPISet()),
461+
bundle("depender", "depender", "channel", "", EmptyAPISet(), APISet{opregistry.APIKey{"g", "v", "k", "ks"}: {}}, EmptyAPISet(), EmptyAPISet()),
462+
bundle("depender.v2", "depender", "channel", "depender", EmptyAPISet(), APISet{opregistry.APIKey{"g", "v", "k", "ks"}: {}}, EmptyAPISet(), EmptyAPISet()),
463+
},
464+
}),
465+
gen: NewGenerationFromOperators(
466+
NewFakeOperatorSurface("original", "o", "c", "", "catsrc", "", []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil, nil),
467+
NewFakeOperatorSurface("depender", "depender", "channel", "", "catsrc", "", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
468+
),
469+
},
470+
args: args{},
471+
wantGen: NewGenerationFromOperators(
472+
NewFakeOperatorSurface("original", "o", "c", "", "catsrc", "", []opregistry.APIKey{{"g", "v", "k", "ks"}},nil, nil, nil),
473+
NewFakeOperatorSurface("depender.v2", "depender", "channel", "depender", "catsrc", "", nil, []opregistry.APIKey{{"g", "v", "k", "ks"}}, nil, nil),
474+
),
475+
},
454476
}
455477
for _, tt := range tests {
456478
t.Run(tt.name, func(t *testing.T) {

0 commit comments

Comments
 (0)