make sure wellKnownReadyController can't have the same condition twice

This change also preserves the current behavior of conditionally setting
the Progressing condition.

pkg/controllers/common/conditions.go

@@ -44,13 +44,12 @@ func (e *ControllerProgressingError) Unwrap() error {
 	return e.err
 }
 
-func (e *ControllerProgressingError) ToCondition(controllerName string) operatorv1.OperatorCondition {
-	return operatorv1.OperatorCondition{
-		Type:    ControllerProgressingConditionName(controllerName),
-		Status:  operatorv1.ConditionTrue,
-		Reason:  e.reason,
-		Message: e.err.Error(),
-	}
+func (e *ControllerProgressingError) ToCondition(controllerName string) *applyoperatorv1.OperatorConditionApplyConfiguration {
+	return applyoperatorv1.OperatorCondition().
+		WithType(ControllerProgressingConditionName(controllerName)).
+		WithStatus(operatorv1.ConditionTrue).
+		WithReason(e.reason).
+		WithMessage(e.err.Error())
 }
 
 // IsDegraded returns true if the condition matching this error (same type, reason and message)

pkg/controllers/readiness/wellknown_ready_controller.go

@@ -111,8 +111,20 @@ func (c *wellKnownReadyController) sync(ctx context.Context, controllerContext f
 	}
 
 	// the code below this point triggers status updates, unify status update handling in defer
+	var available, progressing *applyoperatorv1.OperatorConditionApplyConfiguration
 	status := applyoperatorv1.OperatorStatus()
 	defer func() {
+		// only add valid conditions to the status. This is to prevent unintended changes in current behavior,
+		// where this function may return without setting a progressing condition.
+		conditions := []*applyoperatorv1.OperatorConditionApplyConfiguration{
+			available,
+			progressing,
+		}
+		for _, condition := range conditions {
+			if condition != nil {
+				status = status.WithConditions(condition)
+			}
+		}
 		if updateErr := c.operatorClient.ApplyOperatorStatus(ctx, c.controllerInstanceName, status); updateErr != nil {
 			// fall through to the generic error handling for degraded and requeue
 			utilruntime.HandleError(updateErr)
@@ -122,48 +134,41 @@ func (c *wellKnownReadyController) sync(ctx context.Context, controllerContext f
 	// the well-known endpoint cannot be ready until we know the oauth-server's hostname
 	_, err = c.routeLister.Routes("openshift-authentication").Get("oauth-openshift")
 	if apierrors.IsNotFound(err) {
-		status = status.WithConditions(applyoperatorv1.OperatorCondition().
+		available = applyoperatorv1.OperatorCondition().
 			WithType("WellKnownAvailable").
 			WithStatus(operatorv1.ConditionFalse).
 			WithReason("PrereqsNotReady").
-			WithMessage(err.Error()))
+			WithMessage(err.Error())
 	}
 	if err != nil {
 		return err
 	}
 
 	if err := c.isWellknownEndpointsReady(ctx, operatorSpec, operatorStatus, authConfig, infraConfig); err != nil {
-		status = status.WithConditions(applyoperatorv1.OperatorCondition().
+		available = applyoperatorv1.OperatorCondition().
 			WithType("WellKnownAvailable").
 			WithStatus(operatorv1.ConditionFalse).
 			WithReason("NotReady").
-			WithMessage(fmt.Sprintf("The well-known endpoint is not yet available: %s", err.Error())))
-
+			WithMessage(fmt.Sprintf("The well-known endpoint is not yet available: %s", err.Error()))
 		if progressingErr, ok := err.(*common.ControllerProgressingError); ok {
 			if progressingErr.IsDegraded(controllerName, operatorStatus) {
 				return progressingErr.Unwrap()
 			}
-			condition := progressingErr.ToCondition(controllerName)
-			status = status.WithConditions(applyoperatorv1.OperatorCondition().
-				WithType(condition.Type).
-				WithStatus(condition.Status).
-				WithReason(condition.Reason).
-				WithMessage(condition.Message))
+			progressing = progressingErr.ToCondition(controllerName)
 			return nil
 		} else {
 			return err
 		}
 	}
 
-	status = status.WithConditions(
-		applyoperatorv1.OperatorCondition().
-			WithType(common.ControllerProgressingConditionName(controllerName)).
-			WithStatus(operatorv1.ConditionFalse),
-		applyoperatorv1.OperatorCondition().
-			WithType("WellKnownAvailable").
-			WithStatus(operatorv1.ConditionTrue).
-			WithReason("AsExpected"),
-	)
+	progressing = applyoperatorv1.OperatorCondition().
+		WithType(common.ControllerProgressingConditionName(controllerName)).
+		WithStatus(operatorv1.ConditionFalse)
+
+	available = applyoperatorv1.OperatorCondition().
+		WithType("WellKnownAvailable").
+		WithStatus(operatorv1.ConditionTrue).
+		WithReason("AsExpected")
 
 	return nil
 }