Skip to content

Commit 38ddd31

Browse files
committed
pkg/cvo/availableupdates: Log target version when explaining risk-acceptance
Because [1]: 1. create a cluster which has 1 recommend version and 1 non-recommend version 2. create alert PodDisruptionBudgetAtLimit with warning severity 3. oc adm upgrade accept PodDisruptionBudgetAtLimit had been creating fairly noisy output like: I0408 03:58:45.443626 1 alerts.go:87] refreshed: 10 alerts I0408 03:58:45.443753 1 availableupdates.go:534] Found 10 alerts I0408 03:58:45.443818 1 availableupdates.go:631] Got 0 risks I0408 03:58:45.445403 1 availableupdates.go:868] Risk with name "PodDisruptionBudgetAtLimit" is accepted by the cluster admin and thus not in the evaluation of conditional update I0408 03:58:45.445421 1 availableupdates.go:868] Risk with name "PodDisruptionBudgetAtLimit" is accepted by the cluster admin and thus not in the evaluation of conditional update I0408 03:58:45.445429 1 availableupdates.go:868] Risk with name "PodDisruptionBudgetAtLimit" is accepted by the cluster admin and thus not in the evaluation of conditional update ... I0408 03:58:45.446455 1 availableupdates.go:868] Risk with name "PodDisruptionBudgetAtLimit" is accepted by the cluster admin and thus not in the evaluation of conditional update I0408 03:58:45.446460 1 availableupdates.go:868] Risk with name "PodDisruptionBudgetAtLimit" is accepted by the cluster admin and thus not in the evaluation of conditional update I0408 03:58:45.446475 1 availableupdates.go:177] Requeue available-update evaluation, because "4.22.0-ec.4" is Recommended=Unknown: EvaluationFailed: Could not evaluate exposure to update risk SomeInvokerThing (invalid PromQL result length must be one, but is 0) SomeInvokerThing description: On clusters on default invoker user, this imaginary bug can happen. SomeInvokerThing URL: https://bug.example.com/a I0408 03:58:45.449976 1 cvo.go:816] Finished syncing available updates "openshift-cluster-version/version" (159.68958ms) Logging the version should help distinguish confusedly looping on a single risk for a single target version, from looping over that same risks for multiple different target versions. [1]: https://redhat.atlassian.net/browse/OTA-1813?focusedCommentId=16643738
1 parent 3c4250d commit 38ddd31

2 files changed

Lines changed: 6 additions & 5 deletions

File tree

pkg/cvo/availableupdates.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ func (u *availableUpdates) evaluateConditionalUpdates(ctx context.Context) {
580580
u.RiskConditions = loadRiskConditions(ctx, risks, riskVersions, u.ConditionRegistry)
581581

582582
for i, conditionalUpdate := range u.ConditionalUpdates {
583-
condition := evaluateConditionalUpdate(conditionalUpdate.Risks, u.AcceptRisks, u.ShouldReconcileAcceptRisks, u.RiskConditions)
583+
condition := evaluateConditionalUpdate(conditionalUpdate, u.AcceptRisks, u.ShouldReconcileAcceptRisks, u.RiskConditions)
584584

585585
if condition.Status == metav1.ConditionTrue {
586586
u.addUpdate(conditionalUpdate.Release)
@@ -666,7 +666,7 @@ func newRecommendedReason(now, want string) string {
666666
}
667667

668668
func evaluateConditionalUpdate(
669-
risks []configv1.ConditionalUpdateRisk,
669+
conditionalUpdate configv1.ConditionalUpdate,
670670
acceptRisks sets.Set[string],
671671
shouldReconcileAcceptRisks func() bool,
672672
riskConditions map[string][]metav1.Condition,
@@ -680,7 +680,7 @@ func evaluateConditionalUpdate(
680680
}
681681

682682
var errorMessages []string
683-
for _, risk := range risks {
683+
for _, risk := range conditionalUpdate.Risks {
684684
riskCondition := meta.FindStatusCondition(riskConditions[risk.Name], internal.ConditionalUpdateRiskConditionTypeApplies)
685685
if riskCondition == nil {
686686
// This should never happen
@@ -700,7 +700,7 @@ func evaluateConditionalUpdate(
700700
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionTrue)
701701
recommended.Reason = newRecommendedReason(recommended.Reason, recommendedReasonAllExposedRisksAccepted)
702702
recommended.Message = "The update is recommended, because either risk does not apply to this cluster or it is accepted by cluster admins."
703-
klog.V(2).Infof("Risk with name %q is accepted by the cluster admin and thus not in the evaluation of conditional update", risk.Name)
703+
klog.V(2).Infof("Risk with name %q is accepted by the cluster admin and thus not in the evaluation of conditional update %s", risk.Name, conditionalUpdate.Release.Version)
704704
} else {
705705
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionFalse)
706706
wantReason := recommendedReasonExposed

pkg/cvo/availableupdates_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -517,7 +517,8 @@ func TestEvaluateConditionalUpdate(t *testing.T) {
517517
if tc.riskConditions == nil {
518518
tc.riskConditions = map[string][]metav1.Condition{}
519519
}
520-
actual := evaluateConditionalUpdate(tc.risks, tc.acceptRisks, tc.shouldReconcileAcceptRisks, tc.riskConditions)
520+
conditionalUpdate := configv1.ConditionalUpdate{Risks: tc.risks}
521+
actual := evaluateConditionalUpdate(conditionalUpdate, tc.acceptRisks, tc.shouldReconcileAcceptRisks, tc.riskConditions)
521522
if diff := cmp.Diff(tc.expected, actual); diff != "" {
522523
t.Errorf("actual condition differs from expected:\n%s", diff)
523524
}

0 commit comments

Comments
 (0)