Skip to content

Commit ccc0e36

Browse files
committed
availableupdates: Unify capitalization when comparing architectures
Otherwise, the following condition in cincinnati.go in MultiArch clusters: ``` if desiredArch == string(configv1.ClusterVersionArchitectureMulti) && currentArch != desiredArch { return current, []configv1.Release{current}, nil, nil } ``` gets evaluated to: ``` if "Multi" == string(configv1.ClusterVersionArchitectureMulti) && "multi" != "Multi" { return current, []configv1.Release{current}, nil, nil } ``` This will cause MultiArch clusters with a set non-empty `ClusterVersion.Spec.DesiredUpdate.Architecture` field to indefinitely have available updates set to `[]configv1.Release{current}` because the `"multi" != "Multi"` logic will always match.
1 parent 52cc813 commit ccc0e36

File tree

2 files changed

+67
-7
lines changed

2 files changed

+67
-7
lines changed

pkg/cvo/availableupdates.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
5252
currentArch := runtime.GOARCH
5353

5454
if optr.release.Architecture == configv1.ClusterVersionArchitectureMulti {
55-
currentArch = "multi"
55+
currentArch = string(configv1.ClusterVersionArchitectureMulti)
5656
}
5757

5858
if desiredArch == "" {

pkg/cvo/availableupdates_test.go

Lines changed: 66 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -111,22 +111,27 @@ func osusWithSingleConditionalEdge() (*httptest.Server, clusterconditions.Condit
111111
return osus, mockPromql, updates, from
112112
}
113113

114-
func newOperator(url, version string, promqlMock clusterconditions.Condition) (*availableUpdates, *Operator) {
115-
currentRelease := configv1.Release{Version: version, Image: "payload/" + version}
114+
func newOperator(url, version string, promqlMock clusterconditions.Condition, arch string) (*availableUpdates, *Operator) {
115+
var currentReleaseArch configv1.ClusterVersionArchitecture
116+
if arch == string(configv1.ClusterVersionArchitectureMulti) {
117+
currentReleaseArch = configv1.ClusterVersionArchitectureMulti
118+
}
119+
currentRelease := configv1.Release{Version: version, Image: "payload/" + version, Architecture: currentReleaseArch}
120+
116121
registry := clusterconditions.NewConditionRegistry()
117122
registry.Register("Always", &always.Always{})
118123
registry.Register("PromQL", promqlMock)
119124
operator := &Operator{
120125
updateService: url,
121-
architecture: "amd64",
126+
architecture: arch,
122127
proxyLister: notFoundProxyLister{},
123128
cmConfigManagedLister: notFoundConfigMapLister{},
124129
conditionRegistry: registry,
125130
queue: workqueue.NewTypedRateLimitingQueue[any](workqueue.DefaultTypedControllerRateLimiter[any]()),
126131
release: currentRelease,
127132
}
128133
availableUpdates := &availableUpdates{
129-
Architecture: runtime.GOARCH,
134+
Architecture: arch,
130135
Current: configv1.Release{Version: version, Image: "payload/" + version},
131136
}
132137
return availableUpdates, operator
@@ -149,7 +154,7 @@ var availableUpdatesCmpOpts = []cmp.Option{
149154
func TestSyncAvailableUpdates(t *testing.T) {
150155
fakeOsus, mockPromql, expectedConditionalUpdates, version := osusWithSingleConditionalEdge()
151156
defer fakeOsus.Close()
152-
expectedAvailableUpdates, optr := newOperator(fakeOsus.URL, version, mockPromql)
157+
expectedAvailableUpdates, optr := newOperator(fakeOsus.URL, version, mockPromql, runtime.GOARCH)
153158
expectedAvailableUpdates.UpdateService = fakeOsus.URL
154159
expectedAvailableUpdates.ConditionalUpdates = expectedConditionalUpdates
155160
expectedAvailableUpdates.Channel = cvFixture.Spec.Channel
@@ -231,7 +236,7 @@ func TestSyncAvailableUpdates_ConditionalUpdateRecommendedConditions(t *testing.
231236
t.Run(tc.name, func(t *testing.T) {
232237
fakeOsus, mockPromql, conditionalUpdates, version := osusWithSingleConditionalEdge()
233238
defer fakeOsus.Close()
234-
availableUpdates, optr := newOperator(fakeOsus.URL, version, mockPromql)
239+
availableUpdates, optr := newOperator(fakeOsus.URL, version, mockPromql, runtime.GOARCH)
235240
optr.availableUpdates = availableUpdates
236241
optr.availableUpdates.ConditionalUpdates = conditionalUpdates
237242
expectedConditions := []metav1.Condition{{}}
@@ -435,3 +440,58 @@ func TestEvaluateConditionalUpdate(t *testing.T) {
435440
})
436441
}
437442
}
443+
444+
func TestSyncAvailableUpdatesMultiArchAfterMigration(t *testing.T) {
445+
fakeOsus, mockPromql, expectedConditionalUpdates, version := osusWithSingleConditionalEdge()
446+
defer fakeOsus.Close()
447+
448+
expectedAvailableUpdates, optr := newOperator(fakeOsus.URL, version, mockPromql, "Multi")
449+
450+
cv := cvFixture.DeepCopy()
451+
cv.Spec.DesiredUpdate = &configv1.Update{
452+
Architecture: configv1.ClusterVersionArchitectureMulti,
453+
Version: version,
454+
Image: optr.release.Image,
455+
Force: false,
456+
}
457+
458+
expectedAvailableUpdates.UpdateService = fakeOsus.URL
459+
expectedAvailableUpdates.ConditionalUpdates = expectedConditionalUpdates
460+
expectedAvailableUpdates.Channel = cv.Spec.Channel
461+
expectedAvailableUpdates.Condition = configv1.ClusterOperatorStatusCondition{
462+
Type: configv1.RetrievedUpdates,
463+
Status: configv1.ConditionTrue,
464+
}
465+
466+
err := optr.syncAvailableUpdates(context.Background(), cv)
467+
468+
if err != nil {
469+
t.Fatalf("syncAvailableUpdates() unexpected error: %v", err)
470+
}
471+
if diff := cmp.Diff(expectedAvailableUpdates, optr.availableUpdates, availableUpdatesCmpOpts...); diff != "" {
472+
t.Fatalf("available updates differ from expected:\n%s", diff)
473+
}
474+
}
475+
476+
func TestSyncAvailableUpdatesMultiArchAfterMigrationDesiredUpdateNil(t *testing.T) {
477+
fakeOsus, mockPromql, expectedConditionalUpdates, version := osusWithSingleConditionalEdge()
478+
defer fakeOsus.Close()
479+
480+
expectedAvailableUpdates, optr := newOperator(fakeOsus.URL, version, mockPromql, "Multi")
481+
expectedAvailableUpdates.UpdateService = fakeOsus.URL
482+
expectedAvailableUpdates.ConditionalUpdates = expectedConditionalUpdates
483+
expectedAvailableUpdates.Channel = cvFixture.Spec.Channel
484+
expectedAvailableUpdates.Condition = configv1.ClusterOperatorStatusCondition{
485+
Type: configv1.RetrievedUpdates,
486+
Status: configv1.ConditionTrue,
487+
}
488+
489+
err := optr.syncAvailableUpdates(context.Background(), cvFixture)
490+
491+
if err != nil {
492+
t.Fatalf("syncAvailableUpdates() unexpected error: %v", err)
493+
}
494+
if diff := cmp.Diff(expectedAvailableUpdates, optr.availableUpdates, availableUpdatesCmpOpts...); diff != "" {
495+
t.Fatalf("available updates differ from expected:\n%s", diff)
496+
}
497+
}

0 commit comments

Comments
 (0)