Skip to content

Commit dc830db

Browse files
committed
pkg/cvo/status_test.go: Test setting Failing condition in updateClusterVersionStatus
This commit will add additional testing regarding setting the Failing condition using the `updateClusterVersionStatus` function. This is to ensure no functionality is lost upon new changes.
1 parent b9e63d8 commit dc830db

File tree

1 file changed

+256
-0
lines changed

1 file changed

+256
-0
lines changed

pkg/cvo/status_test.go

Lines changed: 256 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,10 @@ import (
88

99
"github.com/google/go-cmp/cmp"
1010
"github.com/google/go-cmp/cmp/cmpopts"
11+
"github.com/openshift/cluster-version-operator/pkg/payload"
1112
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
1213
"k8s.io/apimachinery/pkg/util/diff"
14+
apierrors "k8s.io/apimachinery/pkg/util/errors"
1315
"k8s.io/apimachinery/pkg/util/validation/field"
1416
"k8s.io/client-go/tools/record"
1517

@@ -345,3 +347,257 @@ func TestUpdateClusterVersionStatus_ReconciliationIssues(t *testing.T) {
345347
})
346348
}
347349
}
350+
351+
func TestUpdateClusterVersionStatus_FilteringMultipleErrorsForFailingCondition(t *testing.T) {
352+
ignoreLastTransitionTime := cmpopts.IgnoreFields(configv1.ClusterOperatorStatusCondition{}, "LastTransitionTime")
353+
type args struct {
354+
syncWorkerStatus *SyncWorkerStatus
355+
}
356+
tests := []struct {
357+
name string
358+
args args
359+
shouldModifyWhenNotReconcilingAndHistoryNotEmpty bool
360+
expectedConditionNotModified *configv1.ClusterOperatorStatusCondition
361+
expectedConditionModified *configv1.ClusterOperatorStatusCondition
362+
}{
363+
{
364+
name: "no errors are present",
365+
args: args{
366+
syncWorkerStatus: &SyncWorkerStatus{
367+
Failure: nil,
368+
},
369+
},
370+
expectedConditionNotModified: &configv1.ClusterOperatorStatusCondition{
371+
Type: ClusterStatusFailing,
372+
Status: configv1.ConditionFalse,
373+
},
374+
},
375+
{
376+
name: "single generic error",
377+
args: args{
378+
syncWorkerStatus: &SyncWorkerStatus{
379+
Failure: fmt.Errorf("error has happened"),
380+
},
381+
},
382+
expectedConditionNotModified: &configv1.ClusterOperatorStatusCondition{
383+
Type: ClusterStatusFailing,
384+
Status: configv1.ConditionTrue,
385+
Message: "error has happened",
386+
},
387+
},
388+
{
389+
name: "single UpdateEffectNone error",
390+
args: args{
391+
syncWorkerStatus: &SyncWorkerStatus{
392+
Failure: &payload.UpdateError{
393+
UpdateEffect: payload.UpdateEffectNone,
394+
Reason: "ClusterOperatorUpdating",
395+
Message: "Cluster operator A is updating",
396+
},
397+
},
398+
},
399+
expectedConditionNotModified: &configv1.ClusterOperatorStatusCondition{
400+
Type: ClusterStatusFailing,
401+
Status: configv1.ConditionTrue,
402+
Reason: "ClusterOperatorUpdating",
403+
Message: "Cluster operator A is updating",
404+
},
405+
shouldModifyWhenNotReconcilingAndHistoryNotEmpty: true,
406+
expectedConditionModified: &configv1.ClusterOperatorStatusCondition{
407+
Type: ClusterStatusFailing,
408+
Status: configv1.ConditionFalse,
409+
},
410+
},
411+
{
412+
name: "single condensed UpdateEffectFail UpdateError",
413+
args: args{
414+
syncWorkerStatus: &SyncWorkerStatus{
415+
Failure: &payload.UpdateError{
416+
Nested: apierrors.NewAggregate([]error{
417+
&payload.UpdateError{
418+
UpdateEffect: payload.UpdateEffectFail,
419+
Reason: "ClusterOperatorNotAvailable",
420+
Message: "Cluster operator A is not available",
421+
},
422+
&payload.UpdateError{
423+
UpdateEffect: payload.UpdateEffectFail,
424+
Reason: "ClusterOperatorNotAvailable",
425+
Message: "Cluster operator B is not available",
426+
},
427+
}),
428+
UpdateEffect: payload.UpdateEffectFail,
429+
Reason: "ClusterOperatorNotAvailable",
430+
Message: "Cluster operators A, B are not available",
431+
},
432+
},
433+
},
434+
expectedConditionNotModified: &configv1.ClusterOperatorStatusCondition{
435+
Type: ClusterStatusFailing,
436+
Status: configv1.ConditionTrue,
437+
Reason: "ClusterOperatorNotAvailable",
438+
Message: "Cluster operators A, B are not available",
439+
},
440+
},
441+
{
442+
name: "MultipleErrors of UpdateEffectFail and UpdateEffectFailAfterInterval",
443+
args: args{
444+
syncWorkerStatus: &SyncWorkerStatus{
445+
Failure: &payload.UpdateError{
446+
Nested: apierrors.NewAggregate([]error{
447+
&payload.UpdateError{
448+
UpdateEffect: payload.UpdateEffectFail,
449+
Reason: "ClusterOperatorNotAvailable",
450+
Message: "Cluster operator A is not available",
451+
},
452+
&payload.UpdateError{
453+
UpdateEffect: payload.UpdateEffectFailAfterInterval,
454+
Reason: "ClusterOperatorDegraded",
455+
Message: "Cluster operator B is degraded",
456+
},
457+
}),
458+
UpdateEffect: payload.UpdateEffectFail,
459+
Reason: "MultipleErrors",
460+
Message: "Multiple errors are preventing progress:\n* Cluster operator A is not available\n* Cluster operator B is degraded",
461+
},
462+
},
463+
},
464+
expectedConditionNotModified: &configv1.ClusterOperatorStatusCondition{
465+
Type: ClusterStatusFailing,
466+
Status: configv1.ConditionTrue,
467+
Reason: "MultipleErrors",
468+
Message: "Multiple errors are preventing progress:\n* Cluster operator A is not available\n* Cluster operator B is degraded",
469+
},
470+
},
471+
{
472+
name: "MultipleErrors of UpdateEffectFail and UpdateEffectNone",
473+
args: args{
474+
syncWorkerStatus: &SyncWorkerStatus{
475+
Failure: &payload.UpdateError{
476+
Nested: apierrors.NewAggregate([]error{
477+
&payload.UpdateError{
478+
UpdateEffect: payload.UpdateEffectFail,
479+
Reason: "ClusterOperatorNotAvailable",
480+
Message: "Cluster operator A is not available",
481+
},
482+
&payload.UpdateError{
483+
UpdateEffect: payload.UpdateEffectNone,
484+
Reason: "ClusterOperatorUpdating",
485+
Message: "Cluster operator B is updating versions",
486+
},
487+
}),
488+
UpdateEffect: payload.UpdateEffectFail,
489+
Reason: "MultipleErrors",
490+
Message: "Multiple errors are preventing progress:\n* Cluster operator A is not available\n* Cluster operator B is updating versions",
491+
},
492+
},
493+
},
494+
expectedConditionNotModified: &configv1.ClusterOperatorStatusCondition{
495+
Type: ClusterStatusFailing,
496+
Status: configv1.ConditionTrue,
497+
Reason: "MultipleErrors",
498+
Message: "Multiple errors are preventing progress:\n* Cluster operator A is not available\n* Cluster operator B is updating versions",
499+
},
500+
},
501+
{
502+
name: "MultipleErrors of UpdateEffectNone and UpdateEffectNone",
503+
args: args{
504+
syncWorkerStatus: &SyncWorkerStatus{
505+
Failure: &payload.UpdateError{
506+
Nested: apierrors.NewAggregate([]error{
507+
&payload.UpdateError{
508+
UpdateEffect: payload.UpdateEffectNone,
509+
Reason: "ClusterOperatorUpdating",
510+
Message: "Cluster operator A is updating versions",
511+
},
512+
&payload.UpdateError{
513+
UpdateEffect: payload.UpdateEffectNone,
514+
Reason: "NewUpdateEffectNoneReason",
515+
Message: "Cluster operator B is getting conscious",
516+
},
517+
}),
518+
UpdateEffect: payload.UpdateEffectFail,
519+
Reason: "MultipleErrors",
520+
Message: "Multiple errors are preventing progress:\n* Cluster operator A is updating versions\n* Cluster operator B is getting conscious",
521+
},
522+
},
523+
},
524+
expectedConditionNotModified: &configv1.ClusterOperatorStatusCondition{
525+
Type: ClusterStatusFailing,
526+
Status: configv1.ConditionTrue,
527+
Reason: "MultipleErrors",
528+
Message: "Multiple errors are preventing progress:\n* Cluster operator A is updating versions\n* Cluster operator B is getting conscious",
529+
},
530+
},
531+
{
532+
name: "MultipleErrors of UpdateEffectFail, UpdateEffectFailAfterInterval, and UpdateEffectNone",
533+
args: args{
534+
syncWorkerStatus: &SyncWorkerStatus{
535+
Failure: &payload.UpdateError{
536+
Nested: apierrors.NewAggregate([]error{
537+
&payload.UpdateError{
538+
UpdateEffect: payload.UpdateEffectFail,
539+
Reason: "ClusterOperatorNotAvailable",
540+
Message: "Cluster operator A is not available",
541+
},
542+
&payload.UpdateError{
543+
UpdateEffect: payload.UpdateEffectNone,
544+
Reason: "ClusterOperatorUpdating",
545+
Message: "Cluster operator B is updating versions",
546+
},
547+
&payload.UpdateError{
548+
UpdateEffect: payload.UpdateEffectFailAfterInterval,
549+
Reason: "ClusterOperatorDegraded",
550+
Message: "Cluster operator C is degraded",
551+
},
552+
}),
553+
UpdateEffect: payload.UpdateEffectFail,
554+
Reason: "MultipleErrors",
555+
Message: "Multiple errors are preventing progress:\n* Cluster operator A is not available\n* Cluster operator B is updating versions\n* Cluster operator C is degraded",
556+
},
557+
},
558+
},
559+
expectedConditionNotModified: &configv1.ClusterOperatorStatusCondition{
560+
Type: ClusterStatusFailing,
561+
Status: configv1.ConditionTrue,
562+
Reason: "MultipleErrors",
563+
Message: "Multiple errors are preventing progress:\n* Cluster operator A is not available\n* Cluster operator B is updating versions\n* Cluster operator C is degraded",
564+
},
565+
},
566+
}
567+
for _, tc := range tests {
568+
tc := tc
569+
gates := fakeRiFlags{}
570+
release := configv1.Release{}
571+
getAvailableUpdates := func() *availableUpdates { return nil }
572+
var noErrors field.ErrorList
573+
574+
t.Run(tc.name, func(t *testing.T) {
575+
type helper struct {
576+
isReconciling bool
577+
isHistoryEmpty bool
578+
}
579+
combinations := []helper{
580+
{false, false},
581+
{false, true},
582+
{true, false},
583+
{true, true},
584+
}
585+
for _, c := range combinations {
586+
tc.args.syncWorkerStatus.Reconciling = c.isReconciling
587+
cvStatus := &configv1.ClusterVersionStatus{}
588+
if !c.isHistoryEmpty {
589+
cvStatus.History = []configv1.UpdateHistory{{State: configv1.PartialUpdate}}
590+
}
591+
expectedCondition := tc.expectedConditionNotModified
592+
if tc.shouldModifyWhenNotReconcilingAndHistoryNotEmpty && !c.isReconciling && !c.isHistoryEmpty {
593+
expectedCondition = tc.expectedConditionModified
594+
}
595+
updateClusterVersionStatus(cvStatus, tc.args.syncWorkerStatus, release, getAvailableUpdates, gates, noErrors)
596+
condition := resourcemerge.FindOperatorStatusCondition(cvStatus.Conditions, ClusterStatusFailing)
597+
if diff := cmp.Diff(expectedCondition, condition, ignoreLastTransitionTime); diff != "" {
598+
t.Errorf("unexpected condition when Reconciling == %t && isHistoryEmpty == %t\n:%s", c.isReconciling, c.isHistoryEmpty, diff)
599+
}
600+
}
601+
})
602+
}
603+
}

0 commit comments

Comments
 (0)