Skip to content

NO-JIRA: Move constants of ClusterStatusConditionType into one place #1252

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
+309 −277
Open

NO-JIRA: Move constants of ClusterStatusConditionType into one place #1252

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0f873c1
Move constants of ClusterStatusConditionType into one place
hongkailiu Oct 29, 2025
ce96105
Fix unit tests
hongkailiu Oct 29, 2025
bfc7f69
Remove redundant type casting
hongkailiu Oct 29, 2025
76f0dd4
Rename DesiredReleaseAccepted to ReleaseAccepted
hongkailiu Nov 19, 2025
cea8161
Rename setDesiredReleaseAcceptedCondition to setReleaseAcceptedCondition
hongkailiu Nov 19, 2025
d5212c0
Move UpgradeableUpgradeInProgress into the internal pkg
hongkailiu Nov 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
149 changes: 75 additions & 74 deletions pkg/cvo/cvo_scenarios_test.go
View file
Open in desktop

Large diffs are not rendered by default.

225 changes: 113 additions & 112 deletions pkg/cvo/cvo_test.go
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion pkg/cvo/metrics.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ func (m *operatorMetrics) Collect(ch chan<- prometheus.Metric) {
}

// answers "if we are failing, are we updating or reconciling"
failing := resourcemerge.FindOperatorStatusCondition(cv.Status.Conditions, ClusterStatusFailing)
failing := resourcemerge.FindOperatorStatusCondition(cv.Status.Conditions, internal.ClusterStatusFailing)
if failing != nil && failing.Status == configv1.ConditionTrue {
if update := cv.Spec.DesiredUpdate; update != nil && update.Image != current.Image {
g = m.version.WithLabelValues("failure", update.Version, update.Image, completed.Version)
Expand Down
20 changes: 11 additions & 9 deletions pkg/cvo/metrics_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import (
"k8s.io/client-go/tools/record"

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

"github.com/openshift/cluster-version-operator/pkg/internal"
)

func Test_operatorMetrics_Collect(t *testing.T) {
Expand Down Expand Up @@ -329,8 +331,8 @@ func Test_operatorMetrics_Collect(t *testing.T) {
Status: configv1.ClusterOperatorStatus{
Conditions: []configv1.ClusterOperatorStatusCondition{
{Type: configv1.OperatorAvailable, Status: configv1.ConditionTrue},
{Type: configv1.ClusterStatusConditionType("Custom"), Status: configv1.ConditionFalse, Reason: "CustomReason"},
{Type: configv1.ClusterStatusConditionType("Unknown"), Status: configv1.ConditionUnknown},
{Type: "Custom", Status: configv1.ConditionFalse, Reason: "CustomReason"},
{Type: "Unknown", Status: configv1.ConditionUnknown},
},
},
},
Expand Down Expand Up @@ -479,7 +481,7 @@ func Test_operatorMetrics_Collect(t *testing.T) {
{State: configv1.CompletedUpdate, Version: "0.0.2", Image: "test/image:1", CompletionTime: &([]metav1.Time{{Time: time.Unix(2, 0)}}[0])},
},
Conditions: []configv1.ClusterOperatorStatusCondition{
{Type: ClusterStatusFailing, Status: configv1.ConditionTrue, LastTransitionTime: metav1.Time{Time: time.Unix(4, 0)}, Reason: "Because stuff"},
{Type: internal.ClusterStatusFailing, Status: configv1.ConditionTrue, LastTransitionTime: metav1.Time{Time: time.Unix(4, 0)}, Reason: "Because stuff"},
},
},
},
Expand Down Expand Up @@ -517,7 +519,7 @@ func Test_operatorMetrics_Collect(t *testing.T) {
},
Status: configv1.ClusterVersionStatus{
Conditions: []configv1.ClusterOperatorStatusCondition{
{Type: ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "Because stuff"},
{Type: internal.ClusterStatusFailing, Status: configv1.ConditionTrue, Reason: "Because stuff"},
},
},
},
Expand Down Expand Up @@ -717,7 +719,7 @@ func Test_operatorMetrics_CollectTransitions(t *testing.T) {
Status: configv1.ClusterOperatorStatus{
Conditions: []configv1.ClusterOperatorStatusCondition{
{Type: configv1.OperatorAvailable, Status: configv1.ConditionTrue},
{Type: configv1.ClusterStatusConditionType("Custom"), Status: configv1.ConditionFalse},
{Type: "Custom", Status: configv1.ConditionFalse},
},
},
},
Expand All @@ -726,17 +728,17 @@ func Test_operatorMetrics_CollectTransitions(t *testing.T) {
Status: configv1.ClusterOperatorStatus{
Conditions: []configv1.ClusterOperatorStatusCondition{
{Type: configv1.OperatorAvailable, Status: configv1.ConditionFalse},
{Type: configv1.ClusterStatusConditionType("Custom"), Status: configv1.ConditionFalse},
{Type: configv1.ClusterStatusConditionType("Unknown"), Status: configv1.ConditionUnknown},
{Type: "Custom", Status: configv1.ConditionFalse},
{Type: "Unknown", Status: configv1.ConditionUnknown},
},
},
},
&configv1.ClusterOperator{
ObjectMeta: metav1.ObjectMeta{Name: "test"},
Status: configv1.ClusterOperatorStatus{
Conditions: []configv1.ClusterOperatorStatusCondition{
{Type: configv1.ClusterStatusConditionType("Custom"), Status: configv1.ConditionTrue},
{Type: configv1.ClusterStatusConditionType("Unknown"), Status: configv1.ConditionTrue},
{Type: "Custom", Status: configv1.ConditionTrue},
{Type: "Unknown", Status: configv1.ConditionTrue},
},
},
},
Expand Down
45 changes: 13 additions & 32 deletions pkg/cvo/status.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,15 +24,11 @@ import (

"github.com/openshift/cluster-version-operator/lib/resourcemerge"
"github.com/openshift/cluster-version-operator/pkg/featuregates"
"github.com/openshift/cluster-version-operator/pkg/internal"
"github.com/openshift/cluster-version-operator/pkg/payload"
)

const (
// ClusterStatusFailing is set on the ClusterVersion status when a cluster
// cannot reach the desired state. It is considered more serious than Degraded
// and indicates the cluster is not healthy.
ClusterStatusFailing = configv1.ClusterStatusConditionType("Failing")

// ConditionalUpdateConditionTypeRecommended is a type of the condition present on a conditional update
// that indicates whether the conditional update is recommended or not
ConditionalUpdateConditionTypeRecommended = "Recommended"
Expand Down Expand Up @@ -159,21 +155,6 @@ func mergeOperatorHistory(cvStatus *configv1.ClusterVersionStatus, desired confi
cvStatus.Desired = desired
}

// ClusterVersionInvalid indicates that the cluster version has an error that prevents the server from
// taking action. The cluster version operator will only reconcile the current state as long as this
// condition is set.
const ClusterVersionInvalid configv1.ClusterStatusConditionType = "Invalid"

// DesiredReleaseAccepted indicates whether the requested (desired) release payload was successfully loaded
// and no failures occurred during image verification and precondition checking.
const DesiredReleaseAccepted configv1.ClusterStatusConditionType = "ReleaseAccepted"

// ImplicitlyEnabledCapabilities is True if there are enabled capabilities which the user is not currently
// requesting via spec.capabilities, because the cluster version operator does not support uninstalling
// capabilities if any associated resources were previously managed by the CVO or disabling previously
// enabled capabilities.
const ImplicitlyEnabledCapabilities configv1.ClusterStatusConditionType = "ImplicitlyEnabledCapabilities"

// syncStatus calculates the new status of the ClusterVersion based on the current sync state and any
// validation errors found. We allow the caller to pass the original object to avoid DeepCopying twice.
func (optr *Operator) syncStatus(ctx context.Context, original, config *configv1.ClusterVersion, status *SyncWorkerStatus, validationErrs field.ErrorList) error {
Expand Down Expand Up @@ -265,21 +246,21 @@ func updateClusterVersionStatus(cvStatus *configv1.ClusterVersionStatus, status
reason = "InvalidClusterVersion"

resourcemerge.SetOperatorStatusCondition(&cvStatus.Conditions, configv1.ClusterOperatorStatusCondition{
Type: ClusterVersionInvalid,
Type: internal.ClusterVersionInvalid,
Status: configv1.ConditionTrue,
Reason: reason,
Message: buf.String(),
LastTransitionTime: now,
})
} else {
resourcemerge.RemoveOperatorStatusCondition(&cvStatus.Conditions, ClusterVersionInvalid)
resourcemerge.RemoveOperatorStatusCondition(&cvStatus.Conditions, internal.ClusterVersionInvalid)
}

// set the implicitly enabled capabilities condition
setImplicitlyEnabledCapabilitiesCondition(cvStatus, status.CapabilitiesStatus.ImplicitlyEnabledCaps, now)

// set the desired release accepted condition
setDesiredReleaseAcceptedCondition(cvStatus, status.loadPayloadStatus, now)
// set the release accepted condition
setReleaseAcceptedCondition(cvStatus, status.loadPayloadStatus, now)

// set the available condition
if status.Completed > 0 {
Expand Down Expand Up @@ -318,7 +299,7 @@ func updateClusterVersionStatus(cvStatus *configv1.ClusterVersionStatus, status

// set the failing condition
failingCondition := configv1.ClusterOperatorStatusCondition{
Type: ClusterStatusFailing,
Type: internal.ClusterStatusFailing,
Status: configv1.ConditionFalse,
LastTransitionTime: now,
}
Expand Down Expand Up @@ -482,15 +463,15 @@ func setImplicitlyEnabledCapabilitiesCondition(cvStatus *configv1.ClusterVersion
message = message + strings.Join([]string(caps), ", ")

resourcemerge.SetOperatorStatusCondition(&cvStatus.Conditions, configv1.ClusterOperatorStatusCondition{
Type: ImplicitlyEnabledCapabilities,
Type: internal.ImplicitlyEnabledCapabilities,
Status: configv1.ConditionTrue,
Reason: "CapabilitiesImplicitlyEnabled",
Message: message,
LastTransitionTime: now,
})
} else {
resourcemerge.SetOperatorStatusCondition(&cvStatus.Conditions, configv1.ClusterOperatorStatusCondition{
Type: ImplicitlyEnabledCapabilities,
Type: internal.ImplicitlyEnabledCapabilities,
Status: configv1.ConditionFalse,
Reason: "AsExpected",
Message: "Capabilities match configured spec",
Expand All @@ -499,10 +480,10 @@ func setImplicitlyEnabledCapabilitiesCondition(cvStatus *configv1.ClusterVersion
}
}

func setDesiredReleaseAcceptedCondition(cvStatus *configv1.ClusterVersionStatus, status LoadPayloadStatus, now metav1.Time) {
func setReleaseAcceptedCondition(cvStatus *configv1.ClusterVersionStatus, status LoadPayloadStatus, now metav1.Time) {
if status.Step == "PayloadLoaded" {
resourcemerge.SetOperatorStatusCondition(&cvStatus.Conditions, configv1.ClusterOperatorStatusCondition{
Type: DesiredReleaseAccepted,
Type: internal.ReleaseAccepted,
Status: configv1.ConditionTrue,
Reason: status.Step,
Message: status.Message,
Expand All @@ -511,15 +492,15 @@ func setDesiredReleaseAcceptedCondition(cvStatus *configv1.ClusterVersionStatus,
} else if status.Step != "" {
if status.Failure != nil {
resourcemerge.SetOperatorStatusCondition(&cvStatus.Conditions, configv1.ClusterOperatorStatusCondition{
Type: DesiredReleaseAccepted,
Type: internal.ReleaseAccepted,
Status: configv1.ConditionFalse,
Reason: status.Step,
Message: status.Message,
LastTransitionTime: now,
})
} else {
resourcemerge.SetOperatorStatusCondition(&cvStatus.Conditions, configv1.ClusterOperatorStatusCondition{
Type: DesiredReleaseAccepted,
Type: internal.ReleaseAccepted,
Status: configv1.ConditionUnknown,
Reason: status.Step,
Message: status.Message,
Expand Down Expand Up @@ -644,7 +625,7 @@ func (optr *Operator) syncFailingStatus(ctx context.Context, original *configv1.

// reset the failing message
resourcemerge.SetOperatorStatusCondition(&config.Status.Conditions, configv1.ClusterOperatorStatusCondition{
Type: ClusterStatusFailing,
Type: internal.ClusterStatusFailing,
Status: configv1.ConditionTrue,
Message: ierr.Error(),
LastTransitionTime: now,
Expand Down
Loading