diff --git a/pkg/cvo/internal/operatorstatus.go b/pkg/cvo/internal/operatorstatus.go index 6a7ff206d..774bbf691 100644 --- a/pkg/cvo/internal/operatorstatus.go +++ b/pkg/cvo/internal/operatorstatus.go @@ -232,20 +232,15 @@ func checkOperatorHealth(ctx context.Context, client ClusterOperatorsGetter, exp } } + var degradedError error if degraded { if degradedCondition != nil && len(degradedCondition.Message) > 0 { nestedMessage = fmt.Errorf("cluster operator %s is %s=%s: %s, %s", actual.Name, degradedCondition.Type, degradedCondition.Status, degradedCondition.Reason, degradedCondition.Message) } - var updateEffect payload.UpdateEffectType - if mode == resourcebuilder.InitializingMode { - updateEffect = payload.UpdateEffectReport - } else { - updateEffect = payload.UpdateEffectFailAfterInterval - } - return &payload.UpdateError{ + degradedError = &payload.UpdateError{ Nested: nestedMessage, - UpdateEffect: updateEffect, + UpdateEffect: payload.UpdateEffectReport, Reason: "ClusterOperatorDegraded", PluralReason: "ClusterOperatorsDegraded", Message: fmt.Sprintf("Cluster operator %s is degraded", actual.Name), @@ -256,10 +251,19 @@ func checkOperatorHealth(ctx context.Context, client ClusterOperatorsGetter, exp // during initialization we allow undone versions if len(undone) > 0 && mode != resourcebuilder.InitializingMode { - nestedMessage = fmt.Errorf("cluster operator %s is available and not degraded but has not finished updating to target version", actual.Name) + var updateEffect payload.UpdateEffectType + + if degradedError == nil { + nestedMessage = fmt.Errorf("cluster operator %s is available and not degraded but has not finished updating to target version", actual.Name) + updateEffect = payload.UpdateEffectNone // block on versions, but nothing to go Failing=True over + } else { + nestedMessage = fmt.Errorf("cluster operator %s is available but %s=%s and has not finished updating to target version", actual.Name, degradedCondition.Type, degradedCondition.Status) + updateEffect = payload.UpdateEffectFail // block on versions, go Failing=True on Degraded=True + } + return &payload.UpdateError{ Nested: nestedMessage, - UpdateEffect: payload.UpdateEffectNone, + UpdateEffect: updateEffect, Reason: "ClusterOperatorUpdating", PluralReason: "ClusterOperatorsUpdating", Message: fmt.Sprintf("Cluster operator %s is updating versions", actual.Name), @@ -268,5 +272,5 @@ func checkOperatorHealth(ctx context.Context, client ClusterOperatorsGetter, exp } } - return nil + return degradedError } diff --git a/pkg/cvo/internal/operatorstatus_test.go b/pkg/cvo/internal/operatorstatus_test.go index 875540ded..289238264 100644 --- a/pkg/cvo/internal/operatorstatus_test.go +++ b/pkg/cvo/internal/operatorstatus_test.go @@ -432,7 +432,7 @@ func Test_checkOperatorHealth(t *testing.T) { }, expErr: &payload.UpdateError{ Nested: fmt.Errorf("cluster operator test-co is Degraded=True: RandomReason, random error"), - UpdateEffect: payload.UpdateEffectFailAfterInterval, + UpdateEffect: payload.UpdateEffectReport, Reason: "ClusterOperatorDegraded", PluralReason: "ClusterOperatorsDegraded", Message: "Cluster operator test-co is degraded", @@ -504,7 +504,7 @@ func Test_checkOperatorHealth(t *testing.T) { }, expErr: &payload.UpdateError{ Nested: fmt.Errorf("cluster operator test-co is Degraded=True: RandomReason, random error"), - UpdateEffect: payload.UpdateEffectFailAfterInterval, + UpdateEffect: payload.UpdateEffectReport, Reason: "ClusterOperatorDegraded", PluralReason: "ClusterOperatorsDegraded", Message: "Cluster operator test-co is degraded", @@ -573,7 +573,7 @@ func Test_checkOperatorHealth(t *testing.T) { }, expErr: &payload.UpdateError{ Nested: fmt.Errorf("cluster operator test-co: available=true, progressing=true, degraded=true, undone="), - UpdateEffect: payload.UpdateEffectFailAfterInterval, + UpdateEffect: payload.UpdateEffectReport, Reason: "ClusterOperatorDegraded", PluralReason: "ClusterOperatorsDegraded", Message: "Cluster operator test-co is degraded", diff --git a/pkg/cvo/status.go b/pkg/cvo/status.go index 9736dfd24..900a5209b 100644 --- a/pkg/cvo/status.go +++ b/pkg/cvo/status.go @@ -552,23 +552,6 @@ func convertErrorToProgressing(now time.Time, statusFailure error) (reason strin return uErr.Reason, fmt.Sprintf("waiting on %s", uErr.Name), true case payload.UpdateEffectFail: return "", "", false - case payload.UpdateEffectFailAfterInterval: - var exceeded []string - threshold := now.Add(-(40 * time.Minute)) - names := uErr.Names - if len(names) == 0 { - names = []string{uErr.Name} - } - for _, name := range names { - if payload.COUpdateStartTimesGet(name).Before(threshold) { - exceeded = append(exceeded, name) - } - } - if len(exceeded) > 0 { - return uErr.Reason, fmt.Sprintf("wait has exceeded 40 minutes for these operators: %s", strings.Join(exceeded, ", ")), false - } else { - return uErr.Reason, fmt.Sprintf("waiting up to 40 minutes on %s", uErr.Name), true - } } return "", "", false } diff --git a/pkg/cvo/status_test.go b/pkg/cvo/status_test.go index 1403c71f9..b6d15e7cb 100644 --- a/pkg/cvo/status_test.go +++ b/pkg/cvo/status_test.go @@ -439,7 +439,7 @@ func TestUpdateClusterVersionStatus_FilteringMultipleErrorsForFailingCondition(t }, }, { - name: "MultipleErrors of UpdateEffectFail and UpdateEffectFailAfterInterval", + name: "MultipleErrors of UpdateEffectFail and UpdateEffectReport", args: args{ syncWorkerStatus: &SyncWorkerStatus{ Failure: &payload.UpdateError{ @@ -450,14 +450,14 @@ func TestUpdateClusterVersionStatus_FilteringMultipleErrorsForFailingCondition(t Message: "Cluster operator A is not available", }, &payload.UpdateError{ - UpdateEffect: payload.UpdateEffectFailAfterInterval, + UpdateEffect: payload.UpdateEffectReport, Reason: "ClusterOperatorDegraded", Message: "Cluster operator B is degraded", }, }), UpdateEffect: payload.UpdateEffectFail, - Reason: "MultipleErrors", - Message: "Multiple errors are preventing progress:\n* Cluster operator A is not available\n* Cluster operator B is degraded", + Reason: "ClusterOperatorNotAvailable", + Message: "Cluster operator A is not available", }, }, }, @@ -541,7 +541,7 @@ func TestUpdateClusterVersionStatus_FilteringMultipleErrorsForFailingCondition(t }, }, { - name: "MultipleErrors of UpdateEffectFail, UpdateEffectFailAfterInterval, and UpdateEffectNone", + name: "MultipleErrors of UpdateEffectFail, UpdateEffectReport, and UpdateEffectNone", args: args{ syncWorkerStatus: &SyncWorkerStatus{ Failure: &payload.UpdateError{ @@ -557,14 +557,14 @@ func TestUpdateClusterVersionStatus_FilteringMultipleErrorsForFailingCondition(t Message: "Cluster operator B is updating versions", }, &payload.UpdateError{ - UpdateEffect: payload.UpdateEffectFailAfterInterval, + UpdateEffect: payload.UpdateEffectReport, Reason: "ClusterOperatorDegraded", Message: "Cluster operator C is degraded", }, }), UpdateEffect: payload.UpdateEffectFail, Reason: "MultipleErrors", - 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", + Message: "Multiple errors are preventing progress:\n* Cluster operator A is not available\n* Cluster operator B is updating versions", }, }, }, @@ -664,10 +664,6 @@ func Test_filterOutUpdateErrors(t *testing.T) { Name: "Report", UpdateEffect: payload.UpdateEffectReport, }, - &payload.UpdateError{ - Name: "Fail After Interval", - UpdateEffect: payload.UpdateEffectFailAfterInterval, - }, }, updateEffectType: payload.UpdateEffectNone, }, @@ -680,10 +676,6 @@ func Test_filterOutUpdateErrors(t *testing.T) { Name: "Report", UpdateEffect: payload.UpdateEffectReport, }, - &payload.UpdateError{ - Name: "Fail After Interval", - UpdateEffect: payload.UpdateEffectFailAfterInterval, - }, }, }, { @@ -691,8 +683,8 @@ func Test_filterOutUpdateErrors(t *testing.T) { args: args{ errs: []error{ &payload.UpdateError{ - Name: "Fail After Interval", - UpdateEffect: payload.UpdateEffectFailAfterInterval, + Name: "Fail", + UpdateEffect: payload.UpdateEffectFail, }, &payload.UpdateError{ Name: "None #1", @@ -711,8 +703,8 @@ func Test_filterOutUpdateErrors(t *testing.T) { }, want: []error{ &payload.UpdateError{ - Name: "Fail After Interval", - UpdateEffect: payload.UpdateEffectFailAfterInterval, + Name: "Fail", + UpdateEffect: payload.UpdateEffectFail, }, &payload.UpdateError{ Name: "Report", @@ -725,8 +717,8 @@ func Test_filterOutUpdateErrors(t *testing.T) { args: args{ errs: []error{ &payload.UpdateError{ - Name: "Fail After Interval", - UpdateEffect: payload.UpdateEffectFailAfterInterval, + Name: "Fail", + UpdateEffect: payload.UpdateEffectFail, }, &payload.UpdateError{ Name: "None #1", @@ -745,8 +737,8 @@ func Test_filterOutUpdateErrors(t *testing.T) { }, want: []error{ &payload.UpdateError{ - Name: "Fail After Interval", - UpdateEffect: payload.UpdateEffectFailAfterInterval, + Name: "Fail", + UpdateEffect: payload.UpdateEffectFail, }, &payload.UpdateError{ Name: "None #1", diff --git a/pkg/cvo/sync_worker.go b/pkg/cvo/sync_worker.go index b581e4d48..bf32c2153 100644 --- a/pkg/cvo/sync_worker.go +++ b/pkg/cvo/sync_worker.go @@ -1264,10 +1264,6 @@ func condenseClusterOperators(errs []error) []error { if updateEffect == payload.UpdateEffectReport { updateEffect = payload.UpdateEffectNone } - case payload.UpdateEffectFailAfterInterval: - if updateEffect != payload.UpdateEffectFail { - updateEffect = payload.UpdateEffectFailAfterInterval - } case payload.UpdateEffectFail: updateEffect = payload.UpdateEffectFail } diff --git a/pkg/cvo/sync_worker_test.go b/pkg/cvo/sync_worker_test.go index 440e127b2..ac1870957 100644 --- a/pkg/cvo/sync_worker_test.go +++ b/pkg/cvo/sync_worker_test.go @@ -256,8 +256,8 @@ func Test_condenseClusterOperators(t *testing.T) { Name: "test-co-B", Task: task("test-co-B", configv1.GroupVersion.WithKind("ClusterOperator")), } - coCDegradedFailAfterInterval := &payload.UpdateError{ - UpdateEffect: payload.UpdateEffectFailAfterInterval, + coCDegraded := &payload.UpdateError{ + UpdateEffect: payload.UpdateEffectReport, Reason: "ClusterOperatorDegraded", PluralReason: "ClusterOperatorsDegraded", Message: "Cluster operator test-co-C is degraded", @@ -318,10 +318,9 @@ func Test_condenseClusterOperators(t *testing.T) { input: []error{ coBUpdating, coAUpdating, - coCDegradedFailAfterInterval, + coCDegraded, }, expected: []error{ - coCDegradedFailAfterInterval, // Degraded sorts before Updating &payload.UpdateError{ Nested: errors.NewAggregate([]error{coBUpdating, coAUpdating}), UpdateEffect: payload.UpdateEffectNone, @@ -332,17 +331,18 @@ func Test_condenseClusterOperators(t *testing.T) { Name: "test-co-A, test-co-B", Names: []string{"test-co-A", "test-co-B"}, }, + coCDegraded, }, }, { name: "there ClusterOperator with the same reason but different effects", input: []error{ coBDegradedFail, coADegradedNone, - coCDegradedFailAfterInterval, + coCDegraded, }, expected: []error{ &payload.UpdateError{ - Nested: errors.NewAggregate([]error{coBDegradedFail, coADegradedNone, coCDegradedFailAfterInterval}), + Nested: errors.NewAggregate([]error{coBDegradedFail, coADegradedNone, coCDegraded}), UpdateEffect: payload.UpdateEffectFail, Reason: "ClusterOperatorsDegraded", PluralReason: "ClusterOperatorsDegraded", @@ -353,15 +353,15 @@ func Test_condenseClusterOperators(t *testing.T) { }, }, }, { - name: "to ClusterOperator with the same reason but None and FailAfterInterval effects", + name: "to ClusterOperator with the same reason but None and Report effects", input: []error{ coADegradedNone, - coCDegradedFailAfterInterval, + coCDegraded, }, expected: []error{ &payload.UpdateError{ - Nested: errors.NewAggregate([]error{coADegradedNone, coCDegradedFailAfterInterval}), - UpdateEffect: payload.UpdateEffectFailAfterInterval, + Nested: errors.NewAggregate([]error{coADegradedNone, coCDegraded}), + UpdateEffect: payload.UpdateEffectFail, Reason: "ClusterOperatorsDegraded", PluralReason: "ClusterOperatorsDegraded", Message: "Cluster operators test-co-A, test-co-C are degraded", diff --git a/pkg/payload/task.go b/pkg/payload/task.go index 1ac592a56..aec0c56cf 100644 --- a/pkg/payload/task.go +++ b/pkg/payload/task.go @@ -161,10 +161,6 @@ const ( // UpdateEffectFail defines an error as indicating the update is failing. UpdateEffectFail UpdateEffectType = "Fail" - - // UpdateEffectFailAfterInterval defines an error as one which indicates the update - // is failing if the error continues for a defined interval. - UpdateEffectFailAfterInterval UpdateEffectType = "FailAfterInterval" ) // UpdateError is a wrapper for errors that occur during a payload sync.