Skip to content

Commit

Permalink
cvo: set ResourceReconciliationIssues condition
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
petr-muller committed Jan 31, 2024
1 parent 80004b0 commit 9b2c690
Show file tree
Hide file tree
Showing 3 changed files with 98 additions and 2 deletions.
13 changes: 13 additions & 0 deletions pkg/cvo/resource_reconciliation_issues.go
@@ -0,0 +1,13 @@
package cvo

import v1 "github.com/openshift/api/config/v1"

const (
resourceReconciliationIssuesConditionType v1.ClusterStatusConditionType = "ResourceReconciliationIssues"

noResourceReconciliationIssuesReason string = "NoIssues"
noResourceReconciliationIssuesMessage string = "No issues found during resource reconciliation"

resourceReconciliationIssuesFoundReason string = "IssuesFound"
resourceReconciliationIssuesFoundMessage string = "Issues found during resource reconciliation"
)
20 changes: 18 additions & 2 deletions pkg/cvo/status.go
Expand Up @@ -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))
Expand All @@ -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 {
Expand Down Expand Up @@ -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{
Expand Down
67 changes: 67 additions & 0 deletions pkg/cvo/status_test.go
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
})
}
}

0 comments on commit 9b2c690

Please sign in to comment.