Set default messages & reconcile clusteroperator status conditions (#584) (#588)

Author: tremes

pkg/controller/status/conditions.go

@@ -53,28 +53,21 @@ func newConditions(cos *configv1.ClusterOperatorStatus, time metav1.Time) *condi
 	}
 }
 
-func (c *conditions) setCondition(condition configv1.ClusterStatusConditionType,
+func (c *conditions) setCondition(conditionType configv1.ClusterStatusConditionType,
 	status configv1.ConditionStatus, reason, message string, lastTime metav1.Time) {
-	entries := make(conditionsMap)
-	for k, v := range c.entryMap {
-		entries[k] = v
+	originalCondition, ok := c.entryMap[conditionType]
+	// if condition is defined and there is not new status then don't update transition time
+	if ok && originalCondition.Status == status {
+		lastTime = originalCondition.LastTransitionTime
 	}
 
-	existing, ok := c.entryMap[condition]
-	if !ok || existing.Status != status || existing.Reason != reason {
-		if lastTime.IsZero() {
-			lastTime = metav1.Now()
-		}
-		entries[condition] = configv1.ClusterOperatorStatusCondition{
-			Type:               condition,
-			Status:             status,
-			Reason:             reason,
-			Message:            message,
-			LastTransitionTime: lastTime,
-		}
+	c.entryMap[conditionType] = configv1.ClusterOperatorStatusCondition{
+		Type:               conditionType,
+		Reason:             reason,
+		Status:             status,
+		Message:            message,
+		LastTransitionTime: lastTime,
 	}
-
-	c.entryMap = entries
 }
 
 func (c *conditions) removeCondition(condition configv1.ClusterStatusConditionType) {

pkg/controller/status/controller.go

@@ -30,6 +30,8 @@ const (
 	// OCMAPIFailureCountThreshold defines how many unsuccessful responses from the OCM API in a row is tolerated
 	// before the operator is marked as Degraded
 	OCMAPIFailureCountThreshold = 5
+
+	insightsAvailableMessage = "Insights works as expected"
 )
 
 type Reported struct {
@@ -146,10 +148,6 @@ func (c *Controller) merge(clusterOperator *configv1.ClusterOperator) *configv1.
 	// cluster operator conditions
 	cs := newConditions(&clusterOperator.Status, metav1.Time{Time: now})
 	updateControllerConditions(cs, c.ctrlStatus, isInitializing, lastTransition)
-
-	// once the operator is running it is always considered available
-	cs.setCondition(configv1.OperatorAvailable, configv1.ConditionTrue, "AsExpected", "", metav1.Now())
-
 	updateControllerConditionsByStatus(cs, c.ctrlStatus, isInitializing, lastTransition)
 
 	// all status conditions from conditions to cluster operator
@@ -265,7 +263,7 @@ func (c *Controller) Start(ctx context.Context) error {
 				}
 			}
 			if err := c.updateStatus(ctx, false); err != nil {
-				klog.Errorf("Unable to write cluster operator statusMessage: %v", err)
+				klog.Errorf("Unable to write cluster operator status: %v", err)
 			}
 		}
 	}, time.Second, ctx.Done())
@@ -285,32 +283,31 @@ func (c *Controller) updateStatus(ctx context.Context, initial bool) error {
 			var reported Reported
 			if len(existing.Status.Extension.Raw) > 0 {
 				if err := json.Unmarshal(existing.Status.Extension.Raw, &reported); err != nil { //nolint: govet
-					klog.Errorf("The initial operator extension statusMessage is invalid: %v", err)
+					klog.Errorf("The initial operator extension status is invalid: %v", err)
 				}
 			}
 			c.SetLastReportedTime(reported.LastReportTime.Time.UTC())
 			cs := newConditions(&existing.Status, metav1.Now())
 			if con := cs.findCondition(configv1.OperatorDegraded); con == nil ||
 				con != nil && con.Status == configv1.ConditionFalse {
-				klog.Info("The initial operator extension statusMessage is healthy")
+				klog.Info("The initial operator extension status is healthy")
 			}
 		}
 	}
 
-	updated := c.merge(existing)
+	updatedClusterOperator := c.merge(existing)
 	if existing == nil {
-		created, err := c.client.ClusterOperators().Create(ctx, updated, metav1.CreateOptions{}) //nolint: govet
+		created, err := c.client.ClusterOperators().Create(ctx, updatedClusterOperator, metav1.CreateOptions{}) //nolint: govet
 		if err != nil {
 			return err
 		}
-		updated.ObjectMeta = created.ObjectMeta
-		updated.Spec = created.Spec
-	} else if reflect.DeepEqual(updated.Status, existing.Status) {
-		klog.V(4).Infof("No statusMessage update necessary, objects are identical")
+		updatedClusterOperator.ObjectMeta = created.ObjectMeta
+		updatedClusterOperator.Spec = created.Spec
+	} else if reflect.DeepEqual(updatedClusterOperator.Status, existing.Status) {
+		klog.V(4).Infof("No status update necessary, objects are identical")
 		return nil
 	}
-
-	_, err = c.client.ClusterOperators().UpdateStatus(ctx, updated, metav1.UpdateOptions{})
+	_, err = c.client.ClusterOperators().UpdateStatus(ctx, updatedClusterOperator, metav1.UpdateOptions{})
 	return err
 }
 
@@ -339,7 +336,7 @@ func updateControllerConditions(cs *conditions, ctrlStatus *controllerStatus,
 	if es := ctrlStatus.getStatus(ErrorStatus); es != nil {
 		cs.setCondition(configv1.OperatorDegraded, configv1.ConditionTrue, es.reason, es.message, metav1.Time{Time: lastTransition})
 	} else {
-		cs.setCondition(configv1.OperatorDegraded, configv1.ConditionFalse, "AsExpected", "", metav1.Now())
+		cs.setCondition(configv1.OperatorDegraded, configv1.ConditionFalse, "AsExpected", insightsAvailableMessage, metav1.Now())
 	}
 
 	// handle when upload fails
@@ -379,6 +376,8 @@ func updateControllerConditionsByStatus(cs *conditions, ctrlStatus *controllerSt
 	if es := ctrlStatus.getStatus(ErrorStatus); es != nil {
 		klog.V(4).Infof("The operator has some internal errors: %s", es.message)
 		cs.setCondition(configv1.OperatorProgressing, configv1.ConditionFalse, "Degraded", "An error has occurred", metav1.Now())
+		cs.setCondition(configv1.OperatorAvailable, configv1.ConditionFalse, es.reason, es.message, metav1.Now())
+		cs.setCondition(configv1.OperatorUpgradeable, configv1.ConditionFalse, "InsightsNotUpgradeable", es.message, metav1.Now())
 	}
 
 	if ds := ctrlStatus.getStatus(DisabledStatus); ds != nil {
@@ -389,6 +388,9 @@ func updateControllerConditionsByStatus(cs *conditions, ctrlStatus *controllerSt
 	if ctrlStatus.isHealthy() {
 		klog.V(4).Infof("The operator is healthy")
 		cs.setCondition(configv1.OperatorProgressing, configv1.ConditionFalse, "AsExpected", "Monitoring the cluster", metav1.Now())
+		cs.setCondition(configv1.OperatorAvailable, configv1.ConditionTrue, "AsExpected", insightsAvailableMessage, metav1.Now())
+		cs.setCondition(configv1.OperatorUpgradeable, configv1.ConditionTrue, "InsightsUpgradeable",
+			"Insights operator can be upgraded", metav1.Now())
 	}
 }
 
@@ -403,6 +405,7 @@ func handleControllerStatusError(errs []string, errorReason string) (reason, mes
 			reason = "UnknownError"
 		}
 		message = errs[0]
+		reason = errorReason
 	}
 	return reason, message
 }

pkg/controllerstatus/controllerstatus.go

@@ -50,12 +50,12 @@ func (s *Simple) UpdateStatus(summary Summary) { //nolint: gocritic
 	s.lock.Lock()
 	defer s.lock.Unlock()
 
+	if summary.LastTransitionTime.IsZero() {
+		s.summary.LastTransitionTime = time.Now()
+	}
+
 	if s.summary.Healthy != summary.Healthy {
 		klog.V(2).Infof("name=%s healthy=%t reason=%s message=%s", s.Name, summary.Healthy, summary.Reason, summary.Message)
-		if summary.LastTransitionTime.IsZero() {
-			summary.LastTransitionTime = time.Now()
-		}
-
 		s.summary = summary
 		s.summary.Count = 1
 		return