Skip to content

Commit d01b53a

Browse files
claudewking
authored andcommitted
pkg: Update Upgradeable condition to explicitly mention major version updates
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 <noreply@anthropic.com>
1 parent 7cebe65 commit d01b53a

4 files changed

Lines changed: 17 additions & 16 deletions

File tree

pkg/cvo/cvo_test.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2929,7 +2929,7 @@ func TestOperator_upgradeableSync(t *testing.T) {
29292929
Type: configv1.OperatorUpgradeable,
29302930
Status: configv1.ConditionFalse,
29312931
Reason: "RandomReason",
2932-
Message: "Cluster operator default-operator-1 should not be upgraded between minor versions: some random reason why upgrades are not safe.",
2932+
Message: "Cluster operator default-operator-1 should not be upgraded between minor or major versions: some random reason why upgrades are not safe.",
29332933
}},
29342934
},
29352935
},
@@ -2990,7 +2990,7 @@ func TestOperator_upgradeableSync(t *testing.T) {
29902990
Type: configv1.OperatorUpgradeable,
29912991
Status: configv1.ConditionFalse,
29922992
Reason: "RandomReason",
2993-
Message: "Cluster operator default-operator-1 should not be upgraded between minor versions: some random reason why upgrades are not safe.",
2993+
Message: "Cluster operator default-operator-1 should not be upgraded between minor or major versions: some random reason why upgrades are not safe.",
29942994
}},
29952995
},
29962996
},
@@ -3054,7 +3054,7 @@ func TestOperator_upgradeableSync(t *testing.T) {
30543054
Type: configv1.OperatorUpgradeable,
30553055
Status: configv1.ConditionFalse,
30563056
Reason: "RandomReason",
3057-
Message: "Cluster operator default-operator-1 should not be upgraded between minor versions: some random reason why upgrades are not safe.",
3057+
Message: "Cluster operator default-operator-1 should not be upgraded between minor or major versions: some random reason why upgrades are not safe.",
30583058
}},
30593059
},
30603060
},
@@ -3120,7 +3120,7 @@ func TestOperator_upgradeableSync(t *testing.T) {
31203120
Type: configv1.OperatorUpgradeable,
31213121
Status: configv1.ConditionFalse,
31223122
Reason: "ClusterOperatorsNotUpgradeable",
3123-
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.",
3123+
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.",
31243124
}},
31253125
},
31263126
},
@@ -3189,12 +3189,12 @@ func TestOperator_upgradeableSync(t *testing.T) {
31893189
Type: configv1.OperatorUpgradeable,
31903190
Status: configv1.ConditionFalse,
31913191
Reason: "MultipleReasons",
3192-
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.",
3192+
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.",
31933193
}, {
31943194
Type: "UpgradeableClusterOperators",
31953195
Status: configv1.ConditionFalse,
31963196
Reason: "ClusterOperatorsNotUpgradeable",
3197-
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.",
3197+
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.",
31983198
}, {
31993199
Type: "UpgradeableClusterVersionOverrides",
32003200
Status: configv1.ConditionFalse,

pkg/cvo/upgradeable.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ func (optr *Operator) setUpgradeableConditions() {
104104
Type: configv1.OperatorUpgradeable,
105105
Status: configv1.ConditionFalse,
106106
Reason: "MultipleReasons",
107-
Message: fmt.Sprintf("Cluster should not be upgraded between minor versions for multiple reasons: %s\n* %s", strings.Join(reasons, ","), strings.Join(msgs, "\n* ")),
107+
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* ")),
108108
LastTransitionTime: now,
109109
})
110110
} else {
@@ -176,7 +176,7 @@ func (optr *Operator) getUpgradeable() *upgradeable {
176176
}
177177

178178
type upgradeableCheck interface {
179-
// Check returns a not-nil condition that should be addressed before a minor level upgrade when the check fails.
179+
// Check returns a not-nil condition that should be addressed before a minor or major version upgrade when the check fails.
180180
Check() *configv1.ClusterOperatorStatusCondition
181181
}
182182

@@ -217,14 +217,14 @@ func (check *clusterOperatorsUpgradeable) Check() *configv1.ClusterOperatorStatu
217217
reason := ""
218218
if len(notup) == 1 {
219219
reason = notup[0].condition.Reason
220-
msg = fmt.Sprintf("Cluster operator %s should not be upgraded between minor versions: %s", notup[0].name, notup[0].condition.Message)
220+
msg = fmt.Sprintf("Cluster operator %s should not be upgraded between minor or major versions: %s", notup[0].name, notup[0].condition.Message)
221221
} else {
222222
reason = "ClusterOperatorsNotUpgradeable"
223223
var msgs []string
224224
for _, cond := range notup {
225-
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))
225+
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))
226226
}
227-
msg = fmt.Sprintf("Multiple cluster operators should not be upgraded between minor versions:\n* %s", strings.Join(msgs, "\n* "))
227+
msg = fmt.Sprintf("Multiple cluster operators should not be upgraded between minor or major versions:\n* %s", strings.Join(msgs, "\n* "))
228228
}
229229
cond.Reason = reason
230230
cond.Message = msg
@@ -302,7 +302,7 @@ func (check *clusterManifestDeleteInProgressUpgradeable) Check() *configv1.Clust
302302
resources := strings.Join(deletes, ",")
303303
klog.V(2).Infof("Resource deletions in progress; resources=%s", resources)
304304
cond.Reason = "ResourceDeletesInProgress"
305-
cond.Message = fmt.Sprintf("Cluster minor level upgrades are not allowed while resource deletions are in progress; resources=%s", resources)
305+
cond.Message = fmt.Sprintf("Cluster minor or major version upgrades are not allowed while resource deletions are in progress; resources=%s", resources)
306306
return cond
307307
}
308308
return nil

pkg/payload/precondition/clusterversion/upgradeable.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,8 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele
9797
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)
9898
patchOnly := targetVersion.Major == currentVersion.Major && targetVersion.Minor == currentVersion.Minor
9999
if targetVersion.LTE(currentVersion) || patchOnly {
100-
// When Upgradeable==False, a patch level update with the same minor level is allowed unless overrides are set
100+
// When Upgradeable==False, a patch level update with the same minor version is allowed unless overrides are set.
101+
// However, minor or major version updates are blocked when Upgradeable==False (handled below at line 124).
101102
// 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
102103
if condition := ClusterVersionOverridesCondition(cv); condition != nil {
103104
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
111112
// This is to generate an accepted risk for the accepting case 4.y.z -> 4.y+1.z' -> 4.y+1.z''
112113
return &precondition.Error{
113114
Reason: "MinorVersionClusterUpdateInProgress",
114-
Message: fmt.Sprintf("Retarget to %s while a minor level update from %s to %s is in progress", targetVersion, completedVersion, currentVersion),
115+
Message: fmt.Sprintf("Retarget to %s while a minor version update from %s to %s is in progress", targetVersion, completedVersion, currentVersion),
115116
Name: pf.Name(),
116117
NonBlockingWarning: true,
117118
}
@@ -129,7 +130,7 @@ func (pf *Upgradeable) Run(ctx context.Context, releaseContext precondition.Rele
129130
}
130131
}
131132

132-
// minorUpdateFrom returns the version that was installed completed if a minor level upgrade is in progress
133+
// minorUpdateFrom returns the version that was installed completed if a minor version upgrade is in progress
133134
// and the empty string otherwise
134135
func minorUpdateFrom(status configv1.ClusterVersionStatus, currentVersion semver.Version) string {
135136
completedVersion := GetCurrentVersion(status.History)

pkg/payload/precondition/clusterversion/upgradeable_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ func TestUpgradeableRun(t *testing.T) {
136136
expected: &precondition.Error{
137137
NonBlockingWarning: true,
138138
Name: "ClusterVersionUpgradeable",
139-
Message: "Retarget to 4.2.3 while a minor level update from 4.1.1 to 4.2.1 is in progress",
139+
Message: "Retarget to 4.2.3 while a minor version update from 4.1.1 to 4.2.1 is in progress",
140140
Reason: "MinorVersionClusterUpdateInProgress",
141141
},
142142
},

0 commit comments

Comments
 (0)