cvo: set ResourceReconciliationIssues condition

When an appropriate feature flag is set, maintain a `ResourceReconciliationIssues`
condition on the CV status. This condition is False when no issues were
encountered (signalled by the `Failure` field on the `SyncWorkerStatus`
 parameter) and True otherwise.

Author: petr-muller

pkg/cvo/resource_reconciliation_issues.go

@@ -0,0 +1,12 @@
+package cvo
+
+import v1 "github.com/openshift/api/config/v1"
+
+const (
+	resourceReconciliationIssuesConditionType v1.ClusterStatusConditionType = "ResourceReconciliationIssues"
+	noResourceReconciliationIssuesReason                                    = "NoIssues"
+	noResourceReconciliationIssuesMessage                                   = "No issues found during resource reconciliation"
+
+	resourceReconciliationIssuesFoundReason  = "IssuesFound"
+	resourceReconciliationissuesFoundMessage = "Issues found during resource reconciliation"
+)

pkg/cvo/status.go

@@ -198,7 +198,7 @@ func (optr *Operator) syncStatus(ctx context.Context, original, config *configv1
 		original = config.DeepCopy()
 	}
 
-	updateClusterVersionStatus(&config.Status, status, optr.release, optr.getAvailableUpdates, validationErrs)
+	updateClusterVersionStatus(&config.Status, status, optr.release, optr.getAvailableUpdates, optr.enabledGates, validationErrs)
 
 	if klog.V(6).Enabled() {
 		klog.Infof("Apply config: %s", diff.ObjectReflectDiff(original, config))
@@ -210,7 +210,8 @@ func (optr *Operator) syncStatus(ctx context.Context, original, config *configv1
 
 // updateClusterVersionStatus updates the passed cvStatus with the latest status information
 func updateClusterVersionStatus(cvStatus *configv1.ClusterVersionStatus, status *SyncWorkerStatus,
-	release configv1.Release, getAvailableUpdates func() *availableUpdates, validationErrs field.ErrorList) {
+	release configv1.Release, getAvailableUpdates func() *availableUpdates, enabledGates FeatureGates,
+	validationErrs field.ErrorList) {
 
 	cvStatus.ObservedGeneration = status.Generation
 	if len(status.VersionHash) > 0 {
@@ -379,6 +380,21 @@ func updateClusterVersionStatus(cvStatus *configv1.ClusterVersionStatus, status
 		}
 	}
 
+	if enabledGates.ResourceReconciliationIssuesCondition {
+		rriCondition := configv1.ClusterOperatorStatusCondition{
+			Type:    resourceReconciliationIssuesConditionType,
+			Status:  configv1.ConditionFalse,
+			Reason:  noResourceReconciliationIssuesReason,
+			Message: noResourceReconciliationIssuesMessage,
+		}
+		if status.Failure != nil {
+			rriCondition.Status = configv1.ConditionTrue
+			rriCondition.Reason = resourceReconciliationIssuesFoundReason
+			rriCondition.Message = fmt.Sprintf("%s: %s", resourceReconciliationissuesFoundMessage, status.Failure.Error())
+		}
+		resourcemerge.SetOperatorStatusCondition(&cvStatus.Conditions, rriCondition)
+	}
+
 	// default retrieved updates if it is not set
 	if resourcemerge.FindOperatorStatusCondition(cvStatus.Conditions, configv1.RetrievedUpdates) == nil {
 		resourcemerge.SetOperatorStatusCondition(&cvStatus.Conditions, configv1.ClusterOperatorStatusCondition{

pkg/cvo/status_test.go

@@ -6,8 +6,12 @@ import (
 	"reflect"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
+	"github.com/google/go-cmp/cmp/cmpopts"
+	"github.com/openshift/cluster-version-operator/lib/resourcemerge"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/diff"
+	"k8s.io/apimachinery/pkg/util/validation/field"
 	"k8s.io/client-go/tools/record"
 
 	configv1 "github.com/openshift/api/config/v1"
@@ -190,3 +194,66 @@ func TestOperator_syncFailingStatus(t *testing.T) {
 		})
 	}
 }
+
+func TestUpdateClusterVersionStatus_ResourceReconciliationIssues(t *testing.T) {
+	ignoreLastTransitionTime := cmpopts.IgnoreFields(configv1.ClusterOperatorStatusCondition{}, "LastTransitionTime")
+
+	testCases := []struct {
+		name             string
+		originalCvStatus configv1.ClusterVersionStatus
+		syncWorkerStatus SyncWorkerStatus
+
+		enabled bool
+
+		expectedCondition *configv1.ClusterOperatorStatusCondition
+	}{
+		{
+			name:             "ResourceReconciliationIssues present and happy when gate is enabled and no failures happened",
+			originalCvStatus: configv1.ClusterVersionStatus{},
+			syncWorkerStatus: SyncWorkerStatus{},
+			enabled:          true,
+			expectedCondition: &configv1.ClusterOperatorStatusCondition{
+				Type:    resourceReconciliationIssuesConditionType,
+				Status:  configv1.ConditionFalse,
+				Reason:  noResourceReconciliationIssuesReason,
+				Message: noResourceReconciliationIssuesMessage,
+			},
+		},
+		{
+			name:             "ResourceReconciliationIssues present and unhappy when gate is enabled and failures happened",
+			originalCvStatus: configv1.ClusterVersionStatus{},
+			syncWorkerStatus: SyncWorkerStatus{
+				Failure: fmt.Errorf("Something happened"),
+			},
+			enabled: true,
+			expectedCondition: &configv1.ClusterOperatorStatusCondition{
+				Type:    resourceReconciliationIssuesConditionType,
+				Status:  configv1.ConditionTrue,
+				Reason:  resourceReconciliationIssuesFoundReason,
+				Message: fmt.Sprintf("Issues found during resource reconciliation: Something happened"),
+			},
+		},
+		{
+			name:              "ResourceReconciliationIssues not present when gate is enabled and failures happened",
+			originalCvStatus:  configv1.ClusterVersionStatus{},
+			syncWorkerStatus:  SyncWorkerStatus{},
+			enabled:           false,
+			expectedCondition: nil,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		t.Run(tc.name, func(t *testing.T) {
+			gates := FeatureGates{ResourceReconciliationIssuesCondition: tc.enabled}
+			release := configv1.Release{}
+			getAvailableUpdates := func() *availableUpdates { return nil }
+			var noErrors field.ErrorList
+			updateClusterVersionStatus(&tc.originalCvStatus, &tc.syncWorkerStatus, release, getAvailableUpdates, gates, noErrors)
+			condition := resourcemerge.FindOperatorStatusCondition(tc.originalCvStatus.Conditions, resourceReconciliationIssuesConditionType)
+			if diff := cmp.Diff(tc.expectedCondition, condition, ignoreLastTransitionTime); diff != "" {
+				t.Errorf("unexpected condition\n:%s", diff)
+			}
+		})
+	}
+}