From d01b53a45275ab09a344dcab3ec6ba7c67534457 Mon Sep 17 00:00:00 2001 From: "Claude Sonnet 4.5" Date: Thu, 18 Dec 2025 14:42:50 -0800 Subject: [PATCH 1/3] pkg: Update Upgradeable condition to explicitly mention major version updates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Upgradeable condition already blocks both minor and major version updates when set to False, but all the error messages and comments only mentioned 'minor versions' or 'minor level' updates. This was misleading, especially in the context of enabling updates to version 5.0. Changes: - Update error messages in pkg/cvo/upgradeable.go to say 'minor or major versions' - Update comments in pkg/payload/precondition/clusterversion/upgradeable.go to clarify that major version updates are also blocked - Update all test expectations to match the new messages - Standardize terminology from 'minor level' to 'minor version' for consistency 🤖 Generated with [Claude Code](https://claude.com/claude-code) My prompts were: Change the Upgradeable ClusterVersion condition handling so that it applies to major version updates as well as the current minor version updates. and then: commit these changes, and set yourself as the commit author Co-Authored-By: Claude Sonnet 4.5 --- pkg/cvo/cvo_test.go | 12 ++++++------ pkg/cvo/upgradeable.go | 12 ++++++------ .../precondition/clusterversion/upgradeable.go | 7 ++++--- .../precondition/clusterversion/upgradeable_test.go | 2 +- 4 files changed, 17 insertions(+), 16 deletions(-) diff --git a/pkg/cvo/cvo_test.go b/pkg/cvo/cvo_test.go index 0be327dd05..77fd43b4d7 100644 --- a/pkg/cvo/cvo_test.go +++ b/pkg/cvo/cvo_test.go @@ -2929,7 +2929,7 @@ func TestOperator_upgradeableSync(t *testing.T) { Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, Reason: "RandomReason", - Message: "Cluster operator default-operator-1 should not be upgraded between minor versions: some random reason why upgrades are not safe.", + Message: "Cluster operator default-operator-1 should not be upgraded between minor or major versions: some random reason why upgrades are not safe.", }}, }, }, @@ -2990,7 +2990,7 @@ func TestOperator_upgradeableSync(t *testing.T) { Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, Reason: "RandomReason", - Message: "Cluster operator default-operator-1 should not be upgraded between minor versions: some random reason why upgrades are not safe.", + Message: "Cluster operator default-operator-1 should not be upgraded between minor or major versions: some random reason why upgrades are not safe.", }}, }, }, @@ -3054,7 +3054,7 @@ func TestOperator_upgradeableSync(t *testing.T) { Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, Reason: "RandomReason", - Message: "Cluster operator default-operator-1 should not be upgraded between minor versions: some random reason why upgrades are not safe.", + Message: "Cluster operator default-operator-1 should not be upgraded between minor or major versions: some random reason why upgrades are not safe.", }}, }, }, @@ -3120,7 +3120,7 @@ func TestOperator_upgradeableSync(t *testing.T) { Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, Reason: "ClusterOperatorsNotUpgradeable", - Message: "Multiple cluster operators should not be upgraded between minor versions:\n* Cluster operator default-operator-1 should not be upgraded between minor versions: RandomReason: some random reason why upgrades are not safe.\n* Cluster operator default-operator-2 should not be upgraded between minor versions: RandomReason2: some random reason 2 why upgrades are not safe.", + Message: "Multiple cluster operators should not be upgraded between minor or major versions:\n* Cluster operator default-operator-1 should not be upgraded between minor or major versions: RandomReason: some random reason why upgrades are not safe.\n* Cluster operator default-operator-2 should not be upgraded between minor or major versions: RandomReason2: some random reason 2 why upgrades are not safe.", }}, }, }, @@ -3189,12 +3189,12 @@ func TestOperator_upgradeableSync(t *testing.T) { Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, Reason: "MultipleReasons", - Message: "Cluster should not be upgraded between minor versions for multiple reasons: ClusterVersionOverridesSet,ClusterOperatorsNotUpgradeable\n* Disabling ownership via cluster version overrides prevents upgrades. Please remove overrides before continuing.\n* Multiple cluster operators should not be upgraded between minor versions:\n* Cluster operator default-operator-1 should not be upgraded between minor versions: RandomReason: some random reason why upgrades are not safe.\n* Cluster operator default-operator-2 should not be upgraded between minor versions: RandomReason2: some random reason 2 why upgrades are not safe.", + Message: "Cluster should not be upgraded between minor or major versions for multiple reasons: ClusterVersionOverridesSet,ClusterOperatorsNotUpgradeable\n* Disabling ownership via cluster version overrides prevents upgrades. Please remove overrides before continuing.\n* Multiple cluster operators should not be upgraded between minor or major versions:\n* Cluster operator default-operator-1 should not be upgraded between minor or major versions: RandomReason: some random reason why upgrades are not safe.\n* Cluster operator default-operator-2 should not be upgraded between minor or major versions: RandomReason2: some random reason 2 why upgrades are not safe.", }, { Type: "UpgradeableClusterOperators", Status: configv1.ConditionFalse, Reason: "ClusterOperatorsNotUpgradeable", - Message: "Multiple cluster operators should not be upgraded between minor versions:\n* Cluster operator default-operator-1 should not be upgraded between minor versions: RandomReason: some random reason why upgrades are not safe.\n* Cluster operator default-operator-2 should not be upgraded between minor versions: RandomReason2: some random reason 2 why upgrades are not safe.", + Message: "Multiple cluster operators should not be upgraded between minor or major versions:\n* Cluster operator default-operator-1 should not be upgraded between minor or major versions: RandomReason: some random reason why upgrades are not safe.\n* Cluster operator default-operator-2 should not be upgraded between minor or major versions: RandomReason2: some random reason 2 why upgrades are not safe.", }, { Type: "UpgradeableClusterVersionOverrides", Status: configv1.ConditionFalse, diff --git a/pkg/cvo/upgradeable.go b/pkg/cvo/upgradeable.go index 6d48c36ad6..5533f0bf72 100644 --- a/pkg/cvo/upgradeable.go +++ b/pkg/cvo/upgradeable.go @@ -104,7 +104,7 @@ func (optr *Operator) setUpgradeableConditions() { Type: configv1.OperatorUpgradeable, Status: configv1.ConditionFalse, Reason: "MultipleReasons", - Message: fmt.Sprintf("Cluster should not be upgraded between minor versions for multiple reasons: %s\n* %s", strings.Join(reasons, ","), strings.Join(msgs, "\n* ")), + Message: fmt.Sprintf("Cluster should not be upgraded between minor or major versions for multiple reasons: %s\n* %s", strings.Join(reasons, ","), strings.Join(msgs, "\n* ")), LastTransitionTime: now, }) } else { @@ -176,7 +176,7 @@ func (optr *Operator) getUpgradeable() *upgradeable { } type upgradeableCheck interface { - // Check returns a not-nil condition that should be addressed before a minor level upgrade when the check fails. + // Check returns a not-nil condition that should be addressed before a minor or major version upgrade when the check fails. Check() *configv1.ClusterOperatorStatusCondition } @@ -217,14 +217,14 @@ func (check *clusterOperatorsUpgradeable) Check() *configv1.ClusterOperatorStatu reason := "" if len(notup) == 1 { reason = notup[0].condition.Reason - msg = fmt.Sprintf("Cluster operator %s should not be upgraded between minor versions: %s", notup[0].name, notup[0].condition.Message) + msg = fmt.Sprintf("Cluster operator %s should not be upgraded between minor or major versions: %s", notup[0].name, notup[0].condition.Message) } else { reason = "ClusterOperatorsNotUpgradeable" var msgs []string for _, cond := range notup { - msgs = append(msgs, fmt.Sprintf("Cluster operator %s should not be upgraded between minor versions: %s: %s", cond.name, cond.condition.Reason, cond.condition.Message)) + msgs = append(msgs, fmt.Sprintf("Cluster operator %s should not be upgraded between minor or major versions: %s: %s", cond.name, cond.condition.Reason, cond.condition.Message)) } - msg = fmt.Sprintf("Multiple cluster operators should not be upgraded between minor versions:\n* %s", strings.Join(msgs, "\n* ")) + msg = fmt.Sprintf("Multiple cluster operators should not be upgraded between minor or major versions:\n* %s", strings.Join(msgs, "\n* ")) } cond.Reason = reason cond.Message = msg @@ -302,7 +302,7 @@ func (check *clusterManifestDeleteInProgressUpgradeable) Check() *configv1.Clust resources := strings.Join(deletes, ",") klog.V(2).Infof("Resource deletions in progress; resources=%s", resources) cond.Reason = "ResourceDeletesInProgress" - cond.Message = fmt.Sprintf("Cluster minor level upgrades are not allowed while resource deletions are in progress; resources=%s", resources) + cond.Message = fmt.Sprintf("Cluster minor or major version upgrades are not allowed while resource deletions are in progress; resources=%s", resources) return cond } return nil diff --git a/pkg/payload/precondition/clusterversion/upgradeable.go b/pkg/payload/precondition/clusterversion/upgradeable.go index 83d354ac76..e18ebab855 100644 --- a/pkg/payload/precondition/clusterversion/upgradeable.go +++ b/pkg/payload/precondition/clusterversion/upgradeable.go @@ -97,7 +97,8 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele klog.V(4).Infof("The current version is %s parsed from %s and the target version is %s parsed from %s", currentVersion.String(), cv.Status.Desired.Version, targetVersion.String(), releaseContext.DesiredVersion) patchOnly := targetVersion.Major == currentVersion.Major && targetVersion.Minor == currentVersion.Minor if targetVersion.LTE(currentVersion) || patchOnly { - // When Upgradeable==False, a patch level update with the same minor level is allowed unless overrides are set + // When Upgradeable==False, a patch level update with the same minor version is allowed unless overrides are set. + // However, minor or major version updates are blocked when Upgradeable==False (handled below at line 124). // This Upgradeable precondition is only concerned about moving forward, i.e., do not care about downgrade which is taken care of by the Rollback precondition if condition := ClusterVersionOverridesCondition(cv); condition != nil { klog.V(2).Infof("Retarget from %s to %s is blocked by %s: %s", currentVersion.String(), targetVersion.String(), condition.Reason, condition.Message) @@ -111,7 +112,7 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele // This is to generate an accepted risk for the accepting case 4.y.z -> 4.y+1.z' -> 4.y+1.z'' return &precondition.Error{ Reason: "MinorVersionClusterUpdateInProgress", - Message: fmt.Sprintf("Retarget to %s while a minor level update from %s to %s is in progress", targetVersion, completedVersion, currentVersion), + Message: fmt.Sprintf("Retarget to %s while a minor version update from %s to %s is in progress", targetVersion, completedVersion, currentVersion), Name: pf.Name(), NonBlockingWarning: true, } @@ -129,7 +130,7 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele } } -// minorUpdateFrom returns the version that was installed completed if a minor level upgrade is in progress +// minorUpdateFrom returns the version that was installed completed if a minor version upgrade is in progress // and the empty string otherwise func minorUpdateFrom(status configv1.ClusterVersionStatus, currentVersion semver.Version) string { completedVersion := GetCurrentVersion(status.History) diff --git a/pkg/payload/precondition/clusterversion/upgradeable_test.go b/pkg/payload/precondition/clusterversion/upgradeable_test.go index e888f6befb..885ef5e3f2 100644 --- a/pkg/payload/precondition/clusterversion/upgradeable_test.go +++ b/pkg/payload/precondition/clusterversion/upgradeable_test.go @@ -136,7 +136,7 @@ func TestUpgradeableRun(t *testing.T) { expected: &precondition.Error{ NonBlockingWarning: true, Name: "ClusterVersionUpgradeable", - Message: "Retarget to 4.2.3 while a minor level update from 4.1.1 to 4.2.1 is in progress", + Message: "Retarget to 4.2.3 while a minor version update from 4.1.1 to 4.2.1 is in progress", Reason: "MinorVersionClusterUpdateInProgress", }, }, From f0070658e2cc847726f6a98f4f4d176d882d2782 Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 23 Dec 2025 07:31:14 -0800 Subject: [PATCH 2/3] pkg: Update Upgradeable condition to explicitly mention major version updates (v2) Additional locations caught by David and Claude's collaboration [1,2]. [1]: https://github.com/openshift/cluster-version-operator/pull/1276#discussion_r2643276274 [2]: https://github.com/openshift/cluster-version-operator/pull/1276#discussion_r2643368490 --- ..._90_cluster-version-operator_02_servicemonitor.yaml | 2 +- pkg/internal/constants.go | 10 +++++----- pkg/payload/precondition/clusterversion/upgradeable.go | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/install/0000_90_cluster-version-operator_02_servicemonitor.yaml b/install/0000_90_cluster-version-operator_02_servicemonitor.yaml index 9df75793ef..d4ceb3e9b0 100644 --- a/install/0000_90_cluster-version-operator_02_servicemonitor.yaml +++ b/install/0000_90_cluster-version-operator_02_servicemonitor.yaml @@ -87,7 +87,7 @@ spec: rules: - alert: ClusterNotUpgradeable annotations: - summary: One or more cluster operators have been blocking minor version cluster upgrades for at least an hour. + summary: One or more cluster operators have been blocking minor or major version cluster updates for at least an hour. description: In most cases, you will still be able to apply patch releases. Reason {{ "{{ with $cluster_operator_conditions := \"cluster_operator_conditions\" | query}}{{range $value := .}}{{if and (eq (label \"name\" $value) \"version\") (eq (label \"condition\" $value) \"Upgradeable\") (eq (label \"endpoint\" $value) \"metrics\") (eq (value $value) 0.0) (ne (len (label \"reason\" $value)) 0) }}{{label \"reason\" $value}}.{{end}}{{end}}{{end}}"}} For more information refer to 'oc adm upgrade'{{ "{{ with $console_url := \"console_url\" | query }}{{ if ne (len (label \"url\" (first $console_url ) ) ) 0}} or {{ label \"url\" (first $console_url ) }}/settings/cluster/{{ end }}{{ end }}" }}. expr: | max by (namespace, name, condition, endpoint) (cluster_operator_conditions{name="version", condition="Upgradeable", endpoint="metrics"} == 0) diff --git a/pkg/internal/constants.go b/pkg/internal/constants.go index fa074c9469..ec53376c0e 100644 --- a/pkg/internal/constants.go +++ b/pkg/internal/constants.go @@ -56,18 +56,18 @@ const ( ImplicitlyEnabledCapabilities configv1.ClusterStatusConditionType = "ImplicitlyEnabledCapabilities" // UpgradeableAdminAckRequired is False if there is API removed from the Kubernetes API server which requires admin - // consideration, and thus update to the next minor version is blocked. + // consideration, and thus update to the next minor or major version is blocked. UpgradeableAdminAckRequired configv1.ClusterStatusConditionType = "UpgradeableAdminAckRequired" - // UpgradeableDeletesInProgress is False if deleting resources is in progress, and thus update to the next minor + // UpgradeableDeletesInProgress is False if deleting resources is in progress, and thus update to the next minor or major // version is blocked. UpgradeableDeletesInProgress configv1.ClusterStatusConditionType = "UpgradeableDeletesInProgress" - // UpgradeableClusterOperators is False if something is wrong with Cluster Operators, and thus update to the next minor + // UpgradeableClusterOperators is False if something is wrong with Cluster Operators, and thus update to the next minor or major // version is blocked. UpgradeableClusterOperators configv1.ClusterStatusConditionType = "UpgradeableClusterOperators" - // UpgradeableClusterVersionOverrides is False if there are overrides in the Cluster Version, and thus update to the next minor + // UpgradeableClusterVersionOverrides is False if there are overrides in the Cluster Version, and thus update to the next minor or major // version is blocked. UpgradeableClusterVersionOverrides configv1.ClusterStatusConditionType = "UpgradeableClusterVersionOverrides" - // UpgradeableUpgradeInProgress is True if an update is in progress + // UpgradeableUpgradeInProgress is True if an update is in progress. UpgradeableUpgradeInProgress configv1.ClusterStatusConditionType = "UpgradeableUpgradeInProgress" ) diff --git a/pkg/payload/precondition/clusterversion/upgradeable.go b/pkg/payload/precondition/clusterversion/upgradeable.go index e18ebab855..cddd74d244 100644 --- a/pkg/payload/precondition/clusterversion/upgradeable.go +++ b/pkg/payload/precondition/clusterversion/upgradeable.go @@ -98,7 +98,7 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele patchOnly := targetVersion.Major == currentVersion.Major && targetVersion.Minor == currentVersion.Minor if targetVersion.LTE(currentVersion) || patchOnly { // When Upgradeable==False, a patch level update with the same minor version is allowed unless overrides are set. - // However, minor or major version updates are blocked when Upgradeable==False (handled below at line 124). + // However, minor or major version updates are blocked when Upgradeable==False. // This Upgradeable precondition is only concerned about moving forward, i.e., do not care about downgrade which is taken care of by the Rollback precondition if condition := ClusterVersionOverridesCondition(cv); condition != nil { klog.V(2).Infof("Retarget from %s to %s is blocked by %s: %s", currentVersion.String(), targetVersion.String(), condition.Reason, condition.Message) From 07c1bb84b78bfbe3d97e07cf75eb58a582687dfd Mon Sep 17 00:00:00 2001 From: "W. Trevor King" Date: Tue, 23 Dec 2025 07:59:13 -0800 Subject: [PATCH 3/3] pkg/payload/precondition/clusterversion: MajorVersionClusterUpdateInProgress David pointed out [1] that the existing MinorVersionClusterUpdateInProgress needed to be extended to have a major-version update form too. [1]: https://github.com/openshift/cluster-version-operator/pull/1276#discussion_r2643320292 --- .../clusterversion/upgradeable.go | 32 +++++++++++-------- .../clusterversion/upgradeable_test.go | 13 ++++++++ 2 files changed, 32 insertions(+), 13 deletions(-) diff --git a/pkg/payload/precondition/clusterversion/upgradeable.go b/pkg/payload/precondition/clusterversion/upgradeable.go index cddd74d244..094dcdef84 100644 --- a/pkg/payload/precondition/clusterversion/upgradeable.go +++ b/pkg/payload/precondition/clusterversion/upgradeable.go @@ -3,6 +3,7 @@ package clusterversion import ( "context" "fmt" + "strings" "time" "github.com/blang/semver/v4" @@ -108,11 +109,11 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele Name: pf.Name(), } } else { - if completedVersion := minorUpdateFrom(cv.Status, currentVersion); completedVersion != "" && patchOnly { + if completedVersion, majorOrMinor := majorOrMinorUpdateFrom(cv.Status, currentVersion); completedVersion != "" && patchOnly { // This is to generate an accepted risk for the accepting case 4.y.z -> 4.y+1.z' -> 4.y+1.z'' return &precondition.Error{ - Reason: "MinorVersionClusterUpdateInProgress", - Message: fmt.Sprintf("Retarget to %s while a minor version update from %s to %s is in progress", targetVersion, completedVersion, currentVersion), + Reason: fmt.Sprintf("%sVersionClusterUpdateInProgress", majorOrMinor), + Message: fmt.Sprintf("Retarget to %s while a %s version update from %s to %s is in progress", targetVersion, strings.ToLower(majorOrMinor), completedVersion, currentVersion), Name: pf.Name(), NonBlockingWarning: true, } @@ -130,24 +131,29 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele } } -// minorUpdateFrom returns the version that was installed completed if a minor version upgrade is in progress -// and the empty string otherwise -func minorUpdateFrom(status configv1.ClusterVersionStatus, currentVersion semver.Version) string { +// majorOrMinorUpdateFrom returns the version that was installed +// completed if a minor or major version upgrade is in progress and the +// empty string otherwise. It also returns "Major", "Minor", or "" to name +// the transition. +func majorOrMinorUpdateFrom(status configv1.ClusterVersionStatus, currentVersion semver.Version) (string, string) { completedVersion := GetCurrentVersion(status.History) if completedVersion == "" { - return "" + return "", "" } v, err := semver.Parse(completedVersion) if err != nil { - return "" + return "", "" } if cond := resourcemerge.FindOperatorStatusCondition(status.Conditions, configv1.OperatorProgressing); cond != nil && - cond.Status == configv1.ConditionTrue && - v.Major == currentVersion.Major && - v.Minor < currentVersion.Minor { - return completedVersion + cond.Status == configv1.ConditionTrue { + if v.Major < currentVersion.Major { + return completedVersion, "Major" + } + if v.Major == currentVersion.Major && v.Minor < currentVersion.Minor { + return completedVersion, "Minor" + } } - return "" + return "", "" } // Name returns the name of the precondition. diff --git a/pkg/payload/precondition/clusterversion/upgradeable_test.go b/pkg/payload/precondition/clusterversion/upgradeable_test.go index 885ef5e3f2..aae0709cb4 100644 --- a/pkg/payload/precondition/clusterversion/upgradeable_test.go +++ b/pkg/payload/precondition/clusterversion/upgradeable_test.go @@ -140,6 +140,19 @@ func TestUpgradeableRun(t *testing.T) { Reason: "MinorVersionClusterUpdateInProgress", }, }, + { + name: "move-x-then-z, with false condition", + upgradeable: ptr(configv1.ConditionFalse), + completedVersion: "4.1.1", + currVersion: "5.0.1", + desiredVersion: "5.0.3", + expected: &precondition.Error{ + NonBlockingWarning: true, + Name: "ClusterVersionUpgradeable", + Message: "Retarget to 5.0.3 while a major version update from 4.1.1 to 5.0.1 is in progress", + Reason: "MajorVersionClusterUpdateInProgress", + }, + }, { name: "move-z-then-z, with false condition", upgradeable: ptr(configv1.ConditionFalse),