Skip to content

Commit a46418c

Browse files
committed
OTA-1546: Reconcile cv.spec.desiredUpdate.acceptRisks
1 parent 8b12479 commit a46418c

File tree

11 files changed

+301
-68
lines changed

11 files changed

+301
-68
lines changed

pkg/cvo/availableupdates.go

Lines changed: 73 additions & 27 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,8 @@ type availableUpdates struct {
192205
ConditionRegistry clusterconditions.ConditionRegistry
193206

194207
Condition configv1.ClusterOperatorStatusCondition
208+
209+
RiskConditions map[string][]metav1.Condition
195210
}
196211

197212
func (u *availableUpdates) RecentlyAttempted(interval time.Duration) bool {
@@ -291,14 +306,17 @@ func (optr *Operator) getAvailableUpdates() *availableUpdates {
291306
}
292307

293308
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,
309+
UpdateService: optr.availableUpdates.UpdateService,
310+
Channel: optr.availableUpdates.Channel,
311+
Architecture: optr.availableUpdates.Architecture,
312+
ShouldReconcileAcceptRisks: optr.shouldReconcileAcceptRisks,
313+
AcceptRisks: optr.availableUpdates.AcceptRisks,
314+
RiskConditions: optr.availableUpdates.RiskConditions,
315+
LastAttempt: optr.availableUpdates.LastAttempt,
316+
LastSyncOrConfigChange: optr.availableUpdates.LastSyncOrConfigChange,
317+
Current: *optr.availableUpdates.Current.DeepCopy(),
318+
ConditionRegistry: optr.availableUpdates.ConditionRegistry, // intentionally not a copy, to preserve cache state
319+
Condition: optr.availableUpdates.Condition,
302320
}
303321

304322
if optr.availableUpdates.Updates != nil {
@@ -427,7 +445,7 @@ func (u *availableUpdates) evaluateConditionalUpdates(ctx context.Context) {
427445
return vi.GTE(vj)
428446
})
429447
for i, conditionalUpdate := range u.ConditionalUpdates {
430-
condition := evaluateConditionalUpdate(ctx, conditionalUpdate.Risks, u.ConditionRegistry)
448+
condition := evaluateConditionalUpdate(ctx, conditionalUpdate.Risks, u.ConditionRegistry, u.AcceptRisks, u.ShouldReconcileAcceptRisks, u.RiskConditions)
431449

432450
if condition.Status == metav1.ConditionTrue {
433451
u.addUpdate(conditionalUpdate.Release)
@@ -478,13 +496,16 @@ func newRecommendedStatus(now, want metav1.ConditionStatus) metav1.ConditionStat
478496
}
479497

480498
const (
481-
recommendedReasonRisksNotExposed = "NotExposedToRisks"
482-
recommendedReasonEvaluationFailed = "EvaluationFailed"
483-
recommendedReasonMultiple = "MultipleReasons"
499+
recommendedReasonRisksNotExposed = "NotExposedToRisks"
500+
recommendedReasonExposedOnlyToAcceptedRisks = "ExposedOnlyToAcceptedRisks"
501+
recommendedReasonEvaluationFailed = "EvaluationFailed"
502+
recommendedReasonMultiple = "MultipleReasons"
484503

485504
// recommendedReasonExposed is used instead of the original name if it does
486505
// not match the pattern for a valid k8s condition reason.
487506
recommendedReasonExposed = "ExposedToRisks"
507+
508+
riskConditionReasonEvaluationFailed = "EvaluationFailed"
488509
)
489510

490511
// Reasons follow same pattern as k8s Condition Reasons
@@ -493,38 +514,63 @@ var reasonPattern = regexp.MustCompile(`^[A-Za-z]([A-Za-z0-9_,:]*[A-Za-z0-9_])?$
493514

494515
func newRecommendedReason(now, want string) string {
495516
switch {
496-
case now == recommendedReasonRisksNotExposed:
517+
case now == recommendedReasonRisksNotExposed || now == recommendedReasonExposedOnlyToAcceptedRisks && now != want:
497518
return want
498-
case now == want:
519+
case now == recommendedReasonExposedOnlyToAcceptedRisks && want == recommendedReasonRisksNotExposed || now == want:
499520
return now
500521
default:
501522
return recommendedReasonMultiple
502523
}
503524
}
504525

505-
func evaluateConditionalUpdate(ctx context.Context, risks []configv1.ConditionalUpdateRisk, conditionRegistry clusterconditions.ConditionRegistry) metav1.Condition {
526+
func evaluateConditionalUpdate(
527+
ctx context.Context,
528+
risks []configv1.ConditionalUpdateRisk,
529+
conditionRegistry clusterconditions.ConditionRegistry,
530+
acceptRisks sets.Set[string],
531+
shouldReconcileAcceptRisks func() bool,
532+
riskConditions map[string][]metav1.Condition,
533+
) metav1.Condition {
506534
recommended := metav1.Condition{
507-
Type: ConditionalUpdateConditionTypeRecommended,
535+
Type: internal.ConditionalUpdateConditionTypeRecommended,
508536
Status: metav1.ConditionTrue,
509537
// FIXME: ObservedGeneration? That would capture upstream/channel, but not necessarily the currently-reconciling version.
510538
Reason: recommendedReasonRisksNotExposed,
511-
Message: "The update is recommended, because none of the conditional update risks apply to this cluster.",
539+
Message: "The update is recommended, because either risk does not apply to this cluster or it is accepted by cluster admins.",
512540
}
513541

514542
var errorMessages []string
515543
for _, risk := range risks {
544+
riskCondition := metav1.Condition{
545+
Type: internal.ConditionalUpdateRiskConditionTypeApplies,
546+
Status: metav1.ConditionFalse,
547+
}
516548
if match, err := conditionRegistry.Match(ctx, risk.MatchingRules); err != nil {
549+
msg := unknownExposureMessage(risk, err)
517550
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionUnknown)
518551
recommended.Reason = newRecommendedReason(recommended.Reason, recommendedReasonEvaluationFailed)
519-
errorMessages = append(errorMessages, unknownExposureMessage(risk, err))
552+
errorMessages = append(errorMessages, msg)
553+
riskCondition.Status = metav1.ConditionUnknown
554+
riskCondition.Reason = riskConditionReasonEvaluationFailed
555+
riskCondition.Message = msg
520556
} else if match {
521-
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionFalse)
522-
wantReason := recommendedReasonExposed
523-
if reasonPattern.MatchString(risk.Name) {
524-
wantReason = risk.Name
557+
riskCondition.Status = metav1.ConditionTrue
558+
if shouldReconcileAcceptRisks() && acceptRisks.Has(risk.Name) {
559+
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionTrue)
560+
recommended.Reason = newRecommendedReason(recommended.Reason, recommendedReasonExposedOnlyToAcceptedRisks)
561+
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)
562+
} else {
563+
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionFalse)
564+
wantReason := recommendedReasonExposed
565+
if reasonPattern.MatchString(risk.Name) {
566+
wantReason = risk.Name
567+
}
568+
recommended.Reason = newRecommendedReason(recommended.Reason, wantReason)
569+
errorMessages = append(errorMessages, fmt.Sprintf("%s %s", risk.Message, risk.URL))
525570
}
526-
recommended.Reason = newRecommendedReason(recommended.Reason, wantReason)
527-
errorMessages = append(errorMessages, fmt.Sprintf("%s %s", risk.Message, risk.URL))
571+
}
572+
if _, ok := riskConditions[risk.Name]; !ok {
573+
riskConditions[risk.Name] = []metav1.Condition{riskCondition}
528574
}
529575
}
530576
if len(errorMessages) > 0 {

0 commit comments

Comments
 (0)