Skip to content

Commit 32fa03f

Browse files
committed
OTA-1546: Reconcile cv.spec.desiredUpdate.acceptRisks
From this pull, CVO starts to reconcile `cv.spec.desiredUpdate.acceptRisks` on a cluser if FeatureGateClusterUpdateAcceptRisks is enabled no a non-hypershift cluster. - CVO does not block a conditional update if all risks that apply to the cluster are accpeted. - CVO fills up the new fields in status: - status.conditionalUpdateRisks, - status.conditionalUpdate.riskNames, and - status.conditionalUpdate.risks.conditions - CVO adds risk names into status.history.acceptedRisks if a conditional update is accepted.
1 parent 8b12479 commit 32fa03f

File tree

11 files changed

+519
-57
lines changed

11 files changed

+519
-57
lines changed

pkg/cvo/availableupdates.go

Lines changed: 76 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,12 +16,14 @@ import (
1616
"k8s.io/apimachinery/pkg/api/equality"
1717
"k8s.io/apimachinery/pkg/api/meta"
1818
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
19+
"k8s.io/apimachinery/pkg/util/sets"
1920
"k8s.io/klog/v2"
2021

2122
configv1 "github.com/openshift/api/config/v1"
2223
"github.com/openshift/cluster-version-operator/lib/resourcemerge"
2324
"github.com/openshift/cluster-version-operator/pkg/cincinnati"
2425
"github.com/openshift/cluster-version-operator/pkg/clusterconditions"
26+
"github.com/openshift/cluster-version-operator/pkg/internal"
2527
)
2628

2729
const noArchitecture string = "NoArchitecture"
@@ -50,6 +52,12 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
5052
channel := config.Spec.Channel
5153
desiredArch := optr.getDesiredArchitecture(config.Spec.DesiredUpdate)
5254
currentArch := optr.getCurrentArchitecture()
55+
acceptRisks := sets.New[string]()
56+
if config.Spec.DesiredUpdate != nil {
57+
for _, risk := range config.Spec.DesiredUpdate.AcceptRisks {
58+
acceptRisks.Insert(risk.Name)
59+
}
60+
}
5361

5462
// updates are only checked at most once per minimumUpdateCheckInterval or if the generation changes
5563
optrAvailableUpdates := optr.getAvailableUpdates()
@@ -129,6 +137,9 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
129137
optrAvailableUpdates.UpdateService = updateService
130138
optrAvailableUpdates.Channel = channel
131139
optrAvailableUpdates.Architecture = desiredArch
140+
optrAvailableUpdates.ShouldReconcileAcceptRisks = optr.shouldReconcileAcceptRisks
141+
optrAvailableUpdates.AcceptRisks = acceptRisks
142+
optrAvailableUpdates.RiskConditions = map[string][]metav1.Condition{}
132143
optrAvailableUpdates.ConditionRegistry = optr.conditionRegistry
133144
optrAvailableUpdates.Condition = condition
134145

@@ -167,9 +178,11 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
167178
}
168179

169180
type availableUpdates struct {
170-
UpdateService string
171-
Channel string
172-
Architecture string
181+
UpdateService string
182+
Channel string
183+
Architecture string
184+
ShouldReconcileAcceptRisks func() bool
185+
AcceptRisks sets.Set[string]
173186

174187
// LastAttempt records the time of the most recent attempt at update
175188
// retrieval, regardless of whether it was successful.
@@ -192,6 +205,11 @@ type availableUpdates struct {
192205
ConditionRegistry clusterconditions.ConditionRegistry
193206

194207
Condition configv1.ClusterOperatorStatusCondition
208+
209+
// RiskConditions stores the condition for every risk (name, url, message, matchingRules).
210+
// The key of the risk is represented by its name which is ensured by validating our graph-data.
211+
// https://github.com/openshift/cincinnati-graph-data/blob/af701850c24b4a53426c2a5400c63895fdf9de60/hack/validate-blocked-edges.py#L25C77-L25C90
212+
RiskConditions map[string][]metav1.Condition
195213
}
196214

197215
func (u *availableUpdates) RecentlyAttempted(interval time.Duration) bool {
@@ -291,14 +309,17 @@ func (optr *Operator) getAvailableUpdates() *availableUpdates {
291309
}
292310

293311
u := &availableUpdates{
294-
UpdateService: optr.availableUpdates.UpdateService,
295-
Channel: optr.availableUpdates.Channel,
296-
Architecture: optr.availableUpdates.Architecture,
297-
LastAttempt: optr.availableUpdates.LastAttempt,
298-
LastSyncOrConfigChange: optr.availableUpdates.LastSyncOrConfigChange,
299-
Current: *optr.availableUpdates.Current.DeepCopy(),
300-
ConditionRegistry: optr.availableUpdates.ConditionRegistry, // intentionally not a copy, to preserve cache state
301-
Condition: optr.availableUpdates.Condition,
312+
UpdateService: optr.availableUpdates.UpdateService,
313+
Channel: optr.availableUpdates.Channel,
314+
Architecture: optr.availableUpdates.Architecture,
315+
ShouldReconcileAcceptRisks: optr.shouldReconcileAcceptRisks,
316+
AcceptRisks: optr.availableUpdates.AcceptRisks,
317+
RiskConditions: optr.availableUpdates.RiskConditions,
318+
LastAttempt: optr.availableUpdates.LastAttempt,
319+
LastSyncOrConfigChange: optr.availableUpdates.LastSyncOrConfigChange,
320+
Current: *optr.availableUpdates.Current.DeepCopy(),
321+
ConditionRegistry: optr.availableUpdates.ConditionRegistry, // intentionally not a copy, to preserve cache state
322+
Condition: optr.availableUpdates.Condition,
302323
}
303324

304325
if optr.availableUpdates.Updates != nil {
@@ -427,7 +448,7 @@ func (u *availableUpdates) evaluateConditionalUpdates(ctx context.Context) {
427448
return vi.GTE(vj)
428449
})
429450
for i, conditionalUpdate := range u.ConditionalUpdates {
430-
condition := evaluateConditionalUpdate(ctx, conditionalUpdate.Risks, u.ConditionRegistry)
451+
condition := evaluateConditionalUpdate(ctx, conditionalUpdate.Risks, u.ConditionRegistry, u.AcceptRisks, u.ShouldReconcileAcceptRisks, u.RiskConditions)
431452

432453
if condition.Status == metav1.ConditionTrue {
433454
u.addUpdate(conditionalUpdate.Release)
@@ -478,13 +499,16 @@ func newRecommendedStatus(now, want metav1.ConditionStatus) metav1.ConditionStat
478499
}
479500

480501
const (
481-
recommendedReasonRisksNotExposed = "NotExposedToRisks"
482-
recommendedReasonEvaluationFailed = "EvaluationFailed"
483-
recommendedReasonMultiple = "MultipleReasons"
502+
recommendedReasonRisksNotExposed = "NotExposedToRisks"
503+
recommendedReasonExposedOnlyToAcceptedRisks = "ExposedOnlyToAcceptedRisks"
504+
recommendedReasonEvaluationFailed = "EvaluationFailed"
505+
recommendedReasonMultiple = "MultipleReasons"
484506

485507
// recommendedReasonExposed is used instead of the original name if it does
486508
// not match the pattern for a valid k8s condition reason.
487509
recommendedReasonExposed = "ExposedToRisks"
510+
511+
riskConditionReasonEvaluationFailed = "EvaluationFailed"
488512
)
489513

490514
// Reasons follow same pattern as k8s Condition Reasons
@@ -493,18 +517,25 @@ var reasonPattern = regexp.MustCompile(`^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
493517

494518
func newRecommendedReason(now, want string) string {
495519
switch {
496-
case now == recommendedReasonRisksNotExposed:
520+
case now == recommendedReasonRisksNotExposed || now == recommendedReasonExposedOnlyToAcceptedRisks && now != want:
497521
return want
498-
case now == want:
522+
case now == recommendedReasonExposedOnlyToAcceptedRisks && want == recommendedReasonRisksNotExposed || now == want:
499523
return now
500524
default:
501525
return recommendedReasonMultiple
502526
}
503527
}
504528

505-
func evaluateConditionalUpdate(ctx context.Context, risks []configv1.ConditionalUpdateRisk, conditionRegistry clusterconditions.ConditionRegistry) metav1.Condition {
529+
func evaluateConditionalUpdate(
530+
ctx context.Context,
531+
risks []configv1.ConditionalUpdateRisk,
532+
conditionRegistry clusterconditions.ConditionRegistry,
533+
acceptRisks sets.Set[string],
534+
shouldReconcileAcceptRisks func() bool,
535+
riskConditions map[string][]metav1.Condition,
536+
) metav1.Condition {
506537
recommended := metav1.Condition{
507-
Type: ConditionalUpdateConditionTypeRecommended,
538+
Type: internal.ConditionalUpdateConditionTypeRecommended,
508539
Status: metav1.ConditionTrue,
509540
// FIXME: ObservedGeneration? That would capture upstream/channel, but not necessarily the currently-reconciling version.
510541
Reason: recommendedReasonRisksNotExposed,
@@ -513,18 +544,37 @@ func evaluateConditionalUpdate(ctx context.Context, risks []configv1.Conditional
513544

514545
var errorMessages []string
515546
for _, risk := range risks {
547+
riskCondition := metav1.Condition{
548+
Type: internal.ConditionalUpdateRiskConditionTypeApplies,
549+
Status: metav1.ConditionFalse,
550+
}
516551
if match, err := conditionRegistry.Match(ctx, risk.MatchingRules); err != nil {
552+
msg := unknownExposureMessage(risk, err)
517553
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionUnknown)
518554
recommended.Reason = newRecommendedReason(recommended.Reason, recommendedReasonEvaluationFailed)
519-
errorMessages = append(errorMessages, unknownExposureMessage(risk, err))
555+
errorMessages = append(errorMessages, msg)
556+
riskCondition.Status = metav1.ConditionUnknown
557+
riskCondition.Reason = riskConditionReasonEvaluationFailed
558+
riskCondition.Message = msg
520559
} else if match {
521-
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionFalse)
522-
wantReason := recommendedReasonExposed
523-
if reasonPattern.MatchString(risk.Name) {
524-
wantReason = risk.Name
560+
riskCondition.Status = metav1.ConditionTrue
561+
if shouldReconcileAcceptRisks() && acceptRisks.Has(risk.Name) {
562+
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionTrue)
563+
recommended.Reason = newRecommendedReason(recommended.Reason, recommendedReasonExposedOnlyToAcceptedRisks)
564+
recommended.Message = "The update is recommended, because either risk does not apply to this cluster or it is accepted by cluster admins."
565+
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)
566+
} else {
567+
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionFalse)
568+
wantReason := recommendedReasonExposed
569+
if reasonPattern.MatchString(risk.Name) {
570+
wantReason = risk.Name
571+
}
572+
recommended.Reason = newRecommendedReason(recommended.Reason, wantReason)
573+
errorMessages = append(errorMessages, fmt.Sprintf("%s %s", risk.Message, risk.URL))
525574
}
526-
recommended.Reason = newRecommendedReason(recommended.Reason, wantReason)
527-
errorMessages = append(errorMessages, fmt.Sprintf("%s %s", risk.Message, risk.URL))
575+
}
576+
if _, ok := riskConditions[risk.Name]; !ok {
577+
riskConditions[risk.Name] = []metav1.Condition{riskCondition}
528578
}
529579
}
530580
if len(errorMessages) > 0 {

0 commit comments

Comments
 (0)