pkg/cvo/internal: Do not block on Degraded=True ClusterOperator

We have blocking on this condition since 545c342 (api: make status
substruct on operatorstatus, 2018-10-15, #31) when it was Failing.
We'd softened our install-time handling to act this way back in
b0b4902 (clusteroperator: Don't block on failing during
initialization, 2019-03-11, #136), motivated by install speed [1].
And a degraded operator may slow dependent components in their own
transitions.  But as long as the operator/operand are available at
all, it should not block depndent components from transitioning, so
this commit removes the Degraded=True block from the remaining modes.

We still have the critical ClusterOperatorDegraded waking admins up
when an operator goes Degraded=True for a while, we will just no
longer block updates at that point.  We won't block ReconcilingMode
manifest application either, but since that's already flattened and
permuted, and ClusterOperator tend to be towards the end of their
TaskNode, the impact on ReconcilingMode is minimal (except that we
will no longer go Failing=True in ClusterVersion when the only issue
is some Degraded=True ClusterOperator).

[1]: #136 (comment)

Author: wking

docs/user/reconciliation.md

@@ -135,7 +135,6 @@ The ClusterOperator builder only monitors the in-cluster object and blocks until
     would block until the in-cluster ClusterOperator reported `operator` at version 4.1.0.
 
     The progressing check is deprecated and will be removed once all operators are reporting versions.
-* Not degraded (except during initialization, where we ignore the degraded status)
 
 ### CustomResourceDefinition
 

pkg/cvo/internal/operatorstatus.go

@@ -169,63 +169,23 @@ func waitForOperatorStatusToBeDone(ctx context.Context, interval time.Duration,
 		}
 
 		available := false
+		degraded := true
 		progressing := true
-		failing := true
-		var failingCondition *configv1.ClusterOperatorStatusCondition
-		degradedValue := true
-		var degradedCondition *configv1.ClusterOperatorStatusCondition
 		for i := range actual.Status.Conditions {
 			condition := &actual.Status.Conditions[i]
 			switch {
 			case condition.Type == configv1.OperatorAvailable && condition.Status == configv1.ConditionTrue:
 				available = true
+			case condition.Type == configv1.OperatorDegraded && condition.Status == configv1.ConditionFalse:
+				degraded = false
 			case condition.Type == configv1.OperatorProgressing && condition.Status == configv1.ConditionFalse:
 				progressing = false
-			case condition.Type == configv1.OperatorDegraded:
-				if condition.Status == configv1.ConditionFalse {
-					degradedValue = false
-				}
-				degradedCondition = condition
-			}
-		}
-
-		// If degraded was an explicitly set condition, use that. If not, use the deprecated failing.
-		degraded := failing
-		if degradedCondition != nil {
-			degraded = degradedValue
-		}
-
-		switch mode {
-		case resourcebuilder.InitializingMode:
-			// during initialization we allow degraded as long as the component goes available
-			if available && (!progressing || len(expected.Status.Versions) > 0) {
-				return true, nil
-			}
-		default:
-			// if we're at the correct version, and available, and not degraded, we are done
-			// if we're available, not degraded, and not progressing, we're also done
-			// TODO: remove progressing once all cluster operators report expected versions
-			if available && (!progressing || len(expected.Status.Versions) > 0) && !degraded {
-				return true, nil
 			}
 		}
 
-		condition := failingCondition
-		if degradedCondition != nil {
-			condition = degradedCondition
-		}
-		if condition != nil && condition.Status == configv1.ConditionTrue {
-			message := fmt.Sprintf("Cluster operator %s is reporting a failure", actual.Name)
-			if len(condition.Message) > 0 {
-				message = fmt.Sprintf("Cluster operator %s is reporting a failure: %s", actual.Name, condition.Message)
-			}
-			lastErr = &payload.UpdateError{
-				Nested:  errors.New(lowerFirst(message)),
-				Reason:  "ClusterOperatorDegraded",
-				Message: message,
-				Name:    actual.Name,
-			}
-			return false, nil
+		// TODO: remove progressing once all cluster operators report expected versions
+		if available && (!progressing || len(expected.Status.Versions) > 0) {
+			return true, nil
 		}
 
 		lastErr = &payload.UpdateError{

pkg/cvo/internal/operatorstatus_test.go

@@ -315,9 +315,9 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
 			},
 		},
 		expErr: &payload.UpdateError{
-			Nested:  fmt.Errorf("cluster operator test-co is reporting a failure: random error"),
-			Reason:  "ClusterOperatorDegraded",
-			Message: "Cluster operator test-co is reporting a failure: random error",
+			Nested:  fmt.Errorf("cluster operator test-co is not done; it is available=false, progressing=true, degraded=true"),
+			Reason:  "ClusterOperatorNotAvailable",
+			Message: "Cluster operator test-co has not yet reported success",
 			Name:    "test-co",
 		},
 	}, {
@@ -343,12 +343,6 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
 				}},
 			},
 		},
-		expErr: &payload.UpdateError{
-			Nested:  fmt.Errorf("cluster operator test-co is not done; it is available=true, progressing=true, degraded=true"),
-			Reason:  "ClusterOperatorNotAvailable",
-			Message: "Cluster operator test-co has not yet reported success",
-			Name:    "test-co",
-		},
 	}, {
 		name: "cluster operator reporting available=true degraded=true",
 		actual: &configv1.ClusterOperator{
@@ -372,12 +366,6 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
 				}},
 			},
 		},
-		expErr: &payload.UpdateError{
-			Nested:  fmt.Errorf("cluster operator test-co is reporting a failure: random error"),
-			Reason:  "ClusterOperatorDegraded",
-			Message: "Cluster operator test-co is reporting a failure: random error",
-			Name:    "test-co",
-		},
 	}, {
 		name: "cluster operator reporting available=true progressing=true degraded=true",
 		actual: &configv1.ClusterOperator{
@@ -401,12 +389,6 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
 				}},
 			},
 		},
-		expErr: &payload.UpdateError{
-			Nested:  fmt.Errorf("cluster operator test-co is reporting a failure: random error"),
-			Reason:  "ClusterOperatorDegraded",
-			Message: "Cluster operator test-co is reporting a failure: random error",
-			Name:    "test-co",
-		},
 	}, {
 		name: "cluster operator reporting available=true no progressing or degraded",
 		actual: &configv1.ClusterOperator{
@@ -430,12 +412,6 @@ func Test_waitForOperatorStatusToBeDone(t *testing.T) {
 				}},
 			},
 		},
-		expErr: &payload.UpdateError{
-			Nested:  fmt.Errorf("cluster operator test-co is not done; it is available=true, progressing=true, degraded=true"),
-			Reason:  "ClusterOperatorNotAvailable",
-			Message: "Cluster operator test-co has not yet reported success",
-			Name:    "test-co",
-		},
 	}, {
 		name: "cluster operator reporting available=true progressing=false degraded=false",
 		actual: &configv1.ClusterOperator{

pkg/payload/task.go

@@ -195,11 +195,6 @@ func SummaryForReason(reason, name string) string {
 	case "UpgradePreconditionCheckFailed":
 		return "it may not be safe to apply this update"
 
-	case "ClusterOperatorDegraded":
-		if len(name) > 0 {
-			return fmt.Sprintf("the cluster operator %s is degraded", name)
-		}
-		return "a cluster operator is degraded"
 	case "ClusterOperatorNotAvailable":
 		if len(name) > 0 {
 			return fmt.Sprintf("the cluster operator %s has not yet successfully rolled out", name)