From c88fa51ecad10095c2f4dcb55a78301a3c1b09d9 Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Mon, 13 Apr 2026 12:26:55 -0400 Subject: [PATCH 1/2] Add coverage for syncAvailableUpdates Follow up https://github.com/openshift/cluster-version-operator/pull/1367#discussion_r3048355757 The commit 913e324924e7f1028974c87544e1b51bea276b6d from https://github.com/openshift/cluster-version-operator/pull/1367 adds the two `upstream*` properties and use them to calculate `availableUpdates` even without fetching the updates from upstream, i.e., when `needFreshFetch==false`. It enables noticing the resolved alerts quickly. Before that change, any evaluated conditional update will stay there until `needFreshFetch==true`. This pull extends the coverage for the above behavior. --- pkg/cvo/availableupdates_test.go | 84 ++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/pkg/cvo/availableupdates_test.go b/pkg/cvo/availableupdates_test.go index aec0c29ae..6ea4e635a 100644 --- a/pkg/cvo/availableupdates_test.go +++ b/pkg/cvo/availableupdates_test.go @@ -25,6 +25,7 @@ import ( "k8s.io/client-go/util/workqueue" configv1 "github.com/openshift/api/config/v1" + "github.com/openshift/api/features" "github.com/openshift/cluster-version-operator/pkg/clusterconditions" "github.com/openshift/cluster-version-operator/pkg/clusterconditions/always" @@ -1191,3 +1192,86 @@ func Test_evaluateConditionalUpdates(t *testing.T) { }) } } + +// The setup is to satisfy needsConditionalUpdateEval == true and +// needFreshFetch == false in the targeting function optr.syncAvailableUpdates +// so that we simulate the scenario where the operator re-evaluates conditional updates +// without fetching the updates from the upstream. +func TestOperator_syncAvailableUpdates_noticeResolvedAlertsQuickly(t *testing.T) { + now := time.Now() + optr := &Operator{ + queue: workqueue.NewTypedRateLimitingQueue[any](workqueue.DefaultTypedControllerRateLimiter[any]()), + availableUpdates: &availableUpdates{ + LastSyncOrConfigChange: now, + LastAttempt: now, + AcceptRisks: sets.New[string]("RiskA"), + Architecture: "amd64", + upstreamUpdates: []configv1.Release{ + { + Version: "4.21.2", + }, + }, + // it becomes conditional because a firing alert + ConditionalUpdates: []configv1.ConditionalUpdate{ + { + Release: configv1.Release{ + Version: "4.21.2", + }, + Conditions: []metav1.Condition{ + { + Type: "Recommended", + Status: "False", + }, + }, + Risks: []configv1.ConditionalUpdateRisk{ + { + Name: "TestAlert", + Conditions: []metav1.Condition{}, + }, + }, + }, + }, + }, + } + optr.minimumUpdateCheckInterval = 10 * time.Minute + cvgGates := featuregates.CvoGatesFromFeatureGate(&configv1.FeatureGate{ + Status: configv1.FeatureGateStatus{ + FeatureGates: []configv1.FeatureGateDetails{ + { + Enabled: []configv1.FeatureGateAttributes{ + { + Name: features.FeatureGateClusterUpdateAcceptRisks, + }, + }, + }, + }, + }, + }, "") + if !cvgGates.AcceptRisks() { + t.Fatalf("accept risk feature is not enabled") + } + optr.enabledCVOFeatureGates = cvgGates + err := optr.syncAvailableUpdates(context.Background(), &configv1.ClusterVersion{ + Spec: configv1.ClusterVersionSpec{ + DesiredUpdate: &configv1.Update{ + Architecture: "amd64", + }, + }, + }) + if err != nil { + t.Fatalf("failed to sync available updates: %v", err) + } + + // The conditional update is back to be an available update after evaluation. + // There was no available updates before the sync call. + expected := &availableUpdates{ + Architecture: "amd64", + Updates: []configv1.Release{{ + Version: "4.21.2", + }}, + } + + if diff := cmp.Diff(expected, optr.availableUpdates, availableUpdatesCmpOpts...); diff != "" { + t.Errorf("syncAvailableUpdates mismatch (-want +got):\n%s", diff) + } +} From 826c556fff59f3f7b05f8c7ad4878849fc00514e Mon Sep 17 00:00:00 2001 From: Hongkai Liu Date: Fri, 29 May 2026 18:29:12 -0400 Subject: [PATCH 2/2] Test stays close to the production - Use mock risk source to trigger logic on risks [1]. - The setup such as `mock.InternalRisks=[]configv1.ConditionalUpdateRisk{}` and `availableUpdates.Condition=RetrievedUpdates` is added. They can avoid triggering warning messages. None of the above is essential to pass, but they simulate closer to the production environment and provide more coverage of the code. [1]. https://github.com/openshift/cluster-version-operator/blob/52cc8e3dce564aef0fe34659030ba48ceabce5b4/pkg/cvo/availableupdates.go#L569-L579 --- pkg/cvo/availableupdates_test.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/pkg/cvo/availableupdates_test.go b/pkg/cvo/availableupdates_test.go index 6ea4e635a..bd415fee6 100644 --- a/pkg/cvo/availableupdates_test.go +++ b/pkg/cvo/availableupdates_test.go @@ -1211,6 +1211,9 @@ func TestOperator_syncAvailableUpdates_noticeResolvedAlertsQuickly(t *testing.T) Version: "4.21.2", }, }, + Condition: configv1.ClusterOperatorStatusCondition{ + Type: configv1.RetrievedUpdates, + }, // it becomes conditional because a firing alert ConditionalUpdates: []configv1.ConditionalUpdate{ { @@ -1232,6 +1235,7 @@ func TestOperator_syncAvailableUpdates_noticeResolvedAlertsQuickly(t *testing.T) }, }, }, + risks: &riskmock.Mock{InternalName: "Mock", InternalRisks: []configv1.ConditionalUpdateRisk{}}, } optr.minimumUpdateCheckInterval = 10 * time.Minute cvgGates := featuregates.CvoGatesFromFeatureGate(&configv1.FeatureGate{ @@ -1269,6 +1273,9 @@ func TestOperator_syncAvailableUpdates_noticeResolvedAlertsQuickly(t *testing.T) Updates: []configv1.Release{{ Version: "4.21.2", }}, + Condition: configv1.ClusterOperatorStatusCondition{ + Type: configv1.RetrievedUpdates, + }, } if diff := cmp.Diff(expected, optr.availableUpdates, availableUpdatesCmpOpts...); diff != "" {