Skip to content

Commit eb30e14

Browse files
hongkailiuhongkailiu
committed
OTA-1546: Reconcile cv.spec.desiredUpdate.acceptRisks
1 parent 83c1a18 commit eb30e14

6 files changed

Lines changed: 169 additions & 32 deletions

File tree

‎pkg/cvo/availableupdates.go‎

Lines changed: 66 additions & 22 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,10 @@ 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+
for _, risk := range config.Spec.DesiredUpdate.AcceptRisks {
57+
acceptRisks.Insert(risk.Name)
58+
}
5359

5460
// updates are only checked at most once per minimumUpdateCheckInterval or if the generation changes
5561
optrAvailableUpdates := optr.getAvailableUpdates()
@@ -129,6 +135,8 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
129135
optrAvailableUpdates.UpdateService = updateService
130136
optrAvailableUpdates.Channel = channel
131137
optrAvailableUpdates.Architecture = desiredArch
138+
optrAvailableUpdates.ShouldReconcileAcceptRisks = optr.shouldReconcileAcceptRisks
139+
optrAvailableUpdates.AcceptRisks = acceptRisks
132140
optrAvailableUpdates.ConditionRegistry = optr.conditionRegistry
133141
optrAvailableUpdates.Condition = condition
134142

@@ -167,9 +175,11 @@ func (optr *Operator) syncAvailableUpdates(ctx context.Context, config *configv1
167175
}
168176

169177
type availableUpdates struct {
170-
UpdateService string
171-
Channel string
172-
Architecture string
178+
UpdateService string
179+
Channel string
180+
Architecture string
181+
ShouldReconcileAcceptRisks func() bool
182+
AcceptRisks sets.Set[string]
173183

174184
// LastAttempt records the time of the most recent attempt at update
175185
// retrieval, regardless of whether it was successful.
@@ -192,6 +202,8 @@ type availableUpdates struct {
192202
ConditionRegistry clusterconditions.ConditionRegistry
193203

194204
Condition configv1.ClusterOperatorStatusCondition
205+
206+
RiskConditions map[string][]metav1.Condition
195207
}
196208

197209
func (u *availableUpdates) RecentlyAttempted(interval time.Duration) bool {
@@ -291,14 +303,16 @@ func (optr *Operator) getAvailableUpdates() *availableUpdates {
291303
}
292304

293305
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,
306+
UpdateService: optr.availableUpdates.UpdateService,
307+
Channel: optr.availableUpdates.Channel,
308+
Architecture: optr.availableUpdates.Architecture,
309+
ShouldReconcileAcceptRisks: optr.shouldReconcileAcceptRisks,
310+
AcceptRisks: optr.availableUpdates.AcceptRisks,
311+
LastAttempt: optr.availableUpdates.LastAttempt,
312+
LastSyncOrConfigChange: optr.availableUpdates.LastSyncOrConfigChange,
313+
Current: *optr.availableUpdates.Current.DeepCopy(),
314+
ConditionRegistry: optr.availableUpdates.ConditionRegistry, // intentionally not a copy, to preserve cache state
315+
Condition: optr.availableUpdates.Condition,
302316
}
303317

304318
if optr.availableUpdates.Updates != nil {
@@ -427,7 +441,7 @@ func (u *availableUpdates) evaluateConditionalUpdates(ctx context.Context) {
427441
return vi.GTE(vj)
428442
})
429443
for i, conditionalUpdate := range u.ConditionalUpdates {
430-
condition := evaluateConditionalUpdate(ctx, conditionalUpdate.Risks, u.ConditionRegistry)
444+
condition := evaluateConditionalUpdate(ctx, conditionalUpdate.Risks, u.ConditionRegistry, u.AcceptRisks, u.ShouldReconcileAcceptRisks, u.RiskConditions)
431445

432446
if condition.Status == metav1.ConditionTrue {
433447
u.addUpdate(conditionalUpdate.Release)
@@ -485,6 +499,8 @@ const (
485499
// recommendedReasonExposed is used instead of the original name if it does
486500
// not match the pattern for a valid k8s condition reason.
487501
recommendedReasonExposed = "ExposedToRisks"
502+
503+
riskConditionReasonEvaluationFailed = "EvaluationFailed"
488504
)
489505

490506
// Reasons follow same pattern as k8s Condition Reasons
@@ -502,29 +518,57 @@ func newRecommendedReason(now, want string) string {
502518
}
503519
}
504520

505-
func evaluateConditionalUpdate(ctx context.Context, risks []configv1.ConditionalUpdateRisk, conditionRegistry clusterconditions.ConditionRegistry) metav1.Condition {
521+
func evaluateConditionalUpdate(
522+
ctx context.Context,
523+
risks []configv1.ConditionalUpdateRisk,
524+
conditionRegistry clusterconditions.ConditionRegistry,
525+
acceptRisks sets.Set[string],
526+
shouldReconcileAcceptRisks func() bool,
527+
riskConditions map[string][]metav1.Condition,
528+
) metav1.Condition {
506529
recommended := metav1.Condition{
507-
Type: ConditionalUpdateConditionTypeRecommended,
530+
Type: internal.ConditionalUpdateConditionTypeRecommended,
508531
Status: metav1.ConditionTrue,
509532
// FIXME: ObservedGeneration? That would capture upstream/channel, but not necessarily the currently-reconciling version.
510533
Reason: recommendedReasonRisksNotExposed,
511534
Message: "The update is recommended, because none of the conditional update risks apply to this cluster.",
512535
}
513536

514537
var errorMessages []string
515-
for _, risk := range risks {
538+
for i, risk := range risks {
539+
riskCondition := metav1.Condition{
540+
Type: internal.ConditionalUpdateRiskConditionTypeApplies,
541+
Status: metav1.ConditionFalse,
542+
LastTransitionTime: metav1.Now(),
543+
}
516544
if match, err := conditionRegistry.Match(ctx, risk.MatchingRules); err != nil {
545+
msg := unknownExposureMessage(risk, err)
517546
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionUnknown)
518547
recommended.Reason = newRecommendedReason(recommended.Reason, recommendedReasonEvaluationFailed)
519-
errorMessages = append(errorMessages, unknownExposureMessage(risk, err))
548+
errorMessages = append(errorMessages, msg)
549+
riskCondition.Status = metav1.ConditionUnknown
550+
riskCondition.Reason = riskConditionReasonEvaluationFailed
551+
riskCondition.Message = msg
520552
} else if match {
521-
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionFalse)
522-
wantReason := recommendedReasonExposed
523-
if reasonPattern.MatchString(risk.Name) {
524-
wantReason = risk.Name
553+
riskCondition.Status = metav1.ConditionTrue
554+
riskCondition.Message = fmt.Sprintf("The matchingRules[%d] matches", i)
555+
if shouldReconcileAcceptRisks() && acceptRisks.Has(risk.Name) {
556+
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)
557+
} else {
558+
recommended.Status = newRecommendedStatus(recommended.Status, metav1.ConditionFalse)
559+
wantReason := recommendedReasonExposed
560+
if reasonPattern.MatchString(risk.Name) {
561+
wantReason = risk.Name
562+
}
563+
recommended.Reason = newRecommendedReason(recommended.Reason, wantReason)
564+
errorMessages = append(errorMessages, fmt.Sprintf("%s %s", risk.Message, risk.URL))
525565
}
526-
recommended.Reason = newRecommendedReason(recommended.Reason, wantReason)
527-
errorMessages = append(errorMessages, fmt.Sprintf("%s %s", risk.Message, risk.URL))
566+
}
567+
if riskConditions == nil {
568+
riskConditions = map[string][]metav1.Condition{}
569+
}
570+
if _, ok := riskConditions[risk.Name]; !ok {
571+
riskConditions[risk.Name] = []metav1.Condition{riskCondition}
528572
}
529573
}
530574
if len(errorMessages) > 0 {

‎pkg/cvo/cvo.go‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1091,3 +1091,12 @@ func (optr *Operator) shouldReconcileCVOConfiguration() bool {
10911091
// The relevant CRD and CR are not applied in HyperShift, which configures the CVO via a configuration file
10921092
return optr.enabledFeatureGates.CVOConfiguration() && !optr.hypershift
10931093
}
1094+
1095+
// shouldReconcileAcceptRisks returns whether the CVO should reconcile spec.desiredUpdate.acceptRisks and populate the
1096+
// relevant fields in status.
1097+
//
1098+
// enabledFeatureGates must be initialized before the function is called.
1099+
func (optr *Operator) shouldReconcileAcceptRisks() bool {
1100+
// HyperShift will be supported later if needed
1101+
return optr.enabledFeatureGates.AcceptRisks() && !optr.hypershift
1102+
}

‎pkg/cvo/status.go‎

Lines changed: 71 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import (
1717
apierrors "k8s.io/apimachinery/pkg/api/errors"
1818
"k8s.io/apimachinery/pkg/api/meta"
1919
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
20+
"k8s.io/apimachinery/pkg/util/sets"
2021
"k8s.io/apimachinery/pkg/util/validation/field"
2122

2223
configv1 "github.com/openshift/api/config/v1"
@@ -29,18 +30,14 @@ import (
2930
)
3031

3132
const (
32-
// ConditionalUpdateConditionTypeRecommended is a type of the condition present on a conditional update
33-
// that indicates whether the conditional update is recommended or not
34-
ConditionalUpdateConditionTypeRecommended = "Recommended"
35-
3633
// MaxHistory is the maximum size of ClusterVersion history. Once exceeded
3734
// ClusterVersion history will be pruned. It is declared here and passed
3835
// into the pruner function to allow easier testing.
3936
MaxHistory = 100
4037
)
4138

4239
func findRecommendedCondition(conditions []metav1.Condition) *metav1.Condition {
43-
return meta.FindStatusCondition(conditions, ConditionalUpdateConditionTypeRecommended)
40+
return meta.FindStatusCondition(conditions, internal.ConditionalUpdateConditionTypeRecommended)
4441
}
4542

4643
func mergeEqualVersions(current *configv1.UpdateHistory, desired configv1.Release) bool {
@@ -180,7 +177,7 @@ func (optr *Operator) syncStatus(ctx context.Context, original, config *configv1
180177
original = config.DeepCopy()
181178
}
182179

183-
updateClusterVersionStatus(&config.Status, status, optr.release, optr.getAvailableUpdates, optr.enabledFeatureGates, validationErrs)
180+
updateClusterVersionStatus(&config.Status, status, optr.release, optr.getAvailableUpdates, optr.enabledFeatureGates, validationErrs, optr.shouldReconcileAcceptRisks)
184181

185182
if klog.V(6).Enabled() {
186183
klog.Infof("Apply config: %s", cmp.Diff(original, config))
@@ -191,9 +188,15 @@ func (optr *Operator) syncStatus(ctx context.Context, original, config *configv1
191188
}
192189

193190
// updateClusterVersionStatus updates the passed cvStatus with the latest status information
194-
func updateClusterVersionStatus(cvStatus *configv1.ClusterVersionStatus, status *SyncWorkerStatus,
195-
release configv1.Release, getAvailableUpdates func() *availableUpdates, enabledGates featuregates.CvoGateChecker,
196-
validationErrs field.ErrorList) {
191+
func updateClusterVersionStatus(
192+
cvStatus *configv1.ClusterVersionStatus,
193+
status *SyncWorkerStatus,
194+
release configv1.Release,
195+
getAvailableUpdates func() *availableUpdates,
196+
enabledGates featuregates.CvoGateChecker,
197+
validationErrs field.ErrorList,
198+
shouldReconcileAcceptRisks func() bool,
199+
) {
197200

198201
cvStatus.ObservedGeneration = status.Generation
199202
if len(status.VersionHash) > 0 {
@@ -222,9 +225,22 @@ func updateClusterVersionStatus(cvStatus *configv1.ClusterVersionStatus, status
222225
desired.Architecture = configv1.ClusterVersionArchitecture("")
223226
}
224227

228+
var riskNamesForDesiredImage []string
229+
if shouldReconcileAcceptRisks() {
230+
cvStatus.ConditionalUpdates, riskNamesForDesiredImage = conditionalUpdateWithRiskNamesAndRiskConditions(cvStatus.ConditionalUpdates, getAvailableUpdates, desired.Image)
231+
cvStatus.ConditionalUpdateRisks = conditionalUpdateRisks(cvStatus.ConditionalUpdates)
232+
}
233+
225234
risksMsg := ""
226235
if desired.Image == status.loadPayloadStatus.Update.Image {
227236
risksMsg = status.loadPayloadStatus.AcceptedRisks
237+
if shouldReconcileAcceptRisks() && len(riskNamesForDesiredImage) > 0 {
238+
if risksMsg == "" {
239+
risksMsg = fmt.Sprintf("The target release %s is exposed to the risks [%s] and accepted by CVO because either the risk is considered acceptable by the cluster admin or it does not apply to the cluster.", desired.Image, strings.Join(riskNamesForDesiredImage, ","))
240+
} else {
241+
risksMsg = fmt.Sprintf("%s; It is exposed to the risks [%s] and accepted by CVO because either the risk is considered acceptable by the cluster admin or it does not apply to the cluster.", risksMsg, strings.Join(riskNamesForDesiredImage, ","))
242+
}
243+
}
228244
}
229245

230246
mergeOperatorHistory(cvStatus, desired, status.Verified, now, status.Completed > 0, risksMsg, status.loadPayloadStatus.Local)
@@ -402,6 +418,52 @@ func updateClusterVersionStatus(cvStatus *configv1.ClusterVersionStatus, status
402418
}
403419
}
404420

421+
func conditionalUpdateWithRiskNamesAndRiskConditions(conditionalUpdates []configv1.ConditionalUpdate, getAvailableUpdates func() *availableUpdates, desiredImage string) ([]configv1.ConditionalUpdate, []string) {
422+
var result []configv1.ConditionalUpdate
423+
var riskNamesForDesiredImage []string
424+
riskConditions := getAvailableUpdates().RiskConditions
425+
for _, conditionalUpdate := range conditionalUpdates {
426+
if desiredImage == conditionalUpdate.Release.Image {
427+
riskNamesForDesiredImage = conditionalUpdate.RiskNames
428+
}
429+
riskNames := sets.New[string]()
430+
for _, risk := range conditionalUpdate.Risks {
431+
riskNames.Insert(risk.Name)
432+
riskTypesToRemove := sets.New[string]()
433+
conditions := riskConditions[risk.Name]
434+
for _, condition := range risk.Conditions {
435+
if found := meta.FindStatusCondition(conditions, condition.Type); found == nil {
436+
riskTypesToRemove.Insert(condition.Type)
437+
}
438+
}
439+
for riskTypeToRemove := range riskTypesToRemove {
440+
meta.RemoveStatusCondition(&risk.Conditions, riskTypeToRemove)
441+
}
442+
for _, condition := range conditions {
443+
meta.SetStatusCondition(&risk.Conditions, condition)
444+
}
445+
}
446+
conditionalUpdate.RiskNames = sets.List[string](riskNames)
447+
result = append(result, conditionalUpdate)
448+
}
449+
return result, riskNamesForDesiredImage
450+
}
451+
452+
func conditionalUpdateRisks(conditionalUpdates []configv1.ConditionalUpdate) []configv1.ConditionalUpdateRisk {
453+
var result []configv1.ConditionalUpdateRisk
454+
riskNames := sets.New[string]()
455+
for _, conditionalUpdate := range conditionalUpdates {
456+
for _, risk := range conditionalUpdate.Risks {
457+
if riskNames.Has(risk.Name) {
458+
continue
459+
}
460+
riskNames.Insert(risk.Name)
461+
result = append(result, risk)
462+
}
463+
}
464+
return result
465+
}
466+
405467
// getReasonMessageFromError returns the reason and the message from an error.
406468
// If the reason or message is not available, an empty string is returned.
407469
func getReasonMessageFromError(err error) (reason, message string) {

‎pkg/featuregates/featuregates.go‎

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ type CvoGateChecker interface {
3535
// CVOConfiguration controls whether the CVO reconciles the ClusterVersionOperator resource that corresponds
3636
// to its configuration.
3737
CVOConfiguration() bool
38+
39+
// AcceptRisks controls whether the CVO reconciles spec.desiredUpdate.acceptRisks.
40+
AcceptRisks() bool
3841
}
3942

4043
// CvoGates contains flags that control CVO functionality gated by product feature gates. The
@@ -49,6 +52,7 @@ type CvoGates struct {
4952
unknownVersion bool
5053
statusReleaseArchitecture bool
5154
cvoConfiguration bool
55+
acceptRisks bool
5256
}
5357

5458
func (c CvoGates) StatusReleaseArchitecture() bool {
@@ -63,13 +67,18 @@ func (c CvoGates) CVOConfiguration() bool {
6367
return c.cvoConfiguration
6468
}
6569

70+
func (c CvoGates) AcceptRisks() bool {
71+
return c.acceptRisks
72+
}
73+
6674
// DefaultCvoGates apply when actual features for given version are unknown
6775
func DefaultCvoGates(version string) CvoGates {
6876
return CvoGates{
6977
desiredVersion: version,
7078
unknownVersion: true,
7179
statusReleaseArchitecture: false,
7280
cvoConfiguration: false,
81+
acceptRisks: false,
7382
}
7483
}
7584

@@ -91,6 +100,8 @@ func CvoGatesFromFeatureGate(gate *configv1.FeatureGate, version string) CvoGate
91100
enabledGates.statusReleaseArchitecture = true
92101
case features.FeatureGateCVOConfiguration:
93102
enabledGates.cvoConfiguration = true
103+
case features.FeatureGateClusterUpdateAcceptRisks:
104+
enabledGates.acceptRisks = true
94105
}
95106
}
96107
for _, disabled := range g.Disabled {
@@ -99,6 +110,8 @@ func CvoGatesFromFeatureGate(gate *configv1.FeatureGate, version string) CvoGate
99110
enabledGates.statusReleaseArchitecture = false
100111
case features.FeatureGateCVOConfiguration:
101112
enabledGates.cvoConfiguration = false
113+
case features.FeatureGateClusterUpdateAcceptRisks:
114+
enabledGates.acceptRisks = false
102115
}
103116
}
104117
}

‎pkg/internal/constants.go‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,4 +70,12 @@ const (
7070

7171
// UpgradeableUpgradeInProgress is True if an update is in progress
7272
UpgradeableUpgradeInProgress configv1.ClusterStatusConditionType = "UpgradeableUpgradeInProgress"
73+
74+
// ConditionalUpdateConditionTypeRecommended is a type of the condition present on a conditional update
75+
// that indicates whether the conditional update is recommended or not
76+
ConditionalUpdateConditionTypeRecommended = "Recommended"
77+
78+
// ConditionalUpdateRiskConditionTypeApplies is a type of the condition present on a conditional update risk
79+
// that indicates whether the conditional update risk applies to the cluster
80+
ConditionalUpdateRiskConditionTypeApplies = "Applies"
7381
)

‎pkg/payload/precondition/clusterversion/recommendedupdate.go‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1212

1313
"github.com/openshift/cluster-version-operator/lib/resourcemerge"
14+
"github.com/openshift/cluster-version-operator/pkg/internal"
1415
precondition "github.com/openshift/cluster-version-operator/pkg/payload/precondition"
1516
)
1617

@@ -51,7 +52,7 @@ func (ru *RecommendedUpdate) Run(ctx context.Context, releaseContext preconditio
5152
for _, conditionalUpdate := range clusterVersion.Status.ConditionalUpdates {
5253
if conditionalUpdate.Release.Version == releaseContext.DesiredVersion {
5354
for _, condition := range conditionalUpdate.Conditions {
54-
if condition.Type == "Recommended" {
55+
if condition.Type == internal.ConditionalUpdateConditionTypeRecommended {
5556
switch condition.Status {
5657
case metav1.ConditionTrue:
5758
return nil

0 commit comments

Comments
 (0)