From 076760eb3b370e49e41ba95589e5de3c9f4d36c6 Mon Sep 17 00:00:00 2001 From: Alex Vulaj Date: Fri, 28 Mar 2025 16:08:57 -0400 Subject: [PATCH] Lift upgradeable condition from CVO to cluster deployment label. This commit lifts the "Upgradeable" status from the ClusterVersion into a new hive.openshift.io/minor-version-upgrade-unavailable label on the ClusterDeployment. Higher level tools can consume this message to warn users about minor version upgrades that CVO would reject with a reason. --- pkg/constants/constants.go | 4 + .../clusterversion_controller.go | 17 +++ .../clusterversion_controller_test.go | 114 ++++++++++++++++-- 3 files changed, 125 insertions(+), 10 deletions(-) diff --git a/pkg/constants/constants.go b/pkg/constants/constants.go index 00d5f113dff..bdf581d3a9a 100644 --- a/pkg/constants/constants.go +++ b/pkg/constants/constants.go @@ -357,6 +357,10 @@ const ( // in the form "[MAJOR].[MINOR].[PATCH]". VersionMajorMinorPatchLabel = "hive.openshift.io/version-major-minor-patch" + // MinorVersionUpgradeUnavailable is a label applied to ClusterDeployments to show concerns about upgrades to the + // next minor version. i.e 4.y -> 4.(y+1) + MinorVersionUpgradeUnavailable = "hive.openshift.io/minor-version-upgrade-unavailable" + // OvirtCredentialsDir is the directory containing Ovirt credentials files. OvirtCredentialsDir = "/.ovirt" diff --git a/pkg/controller/clusterversion/clusterversion_controller.go b/pkg/controller/clusterversion/clusterversion_controller.go index 6c5b5e51b56..7f43099a1ed 100644 --- a/pkg/controller/clusterversion/clusterversion_controller.go +++ b/pkg/controller/clusterversion/clusterversion_controller.go @@ -1,6 +1,7 @@ package clusterversion import ( + "cmp" "context" "fmt" "math/rand" @@ -211,6 +212,22 @@ func (r *ReconcileClusterVersion) updateClusterVersionLabels(cd *hivev1.ClusterD changed = changed || origLen != len(cd.Labels) } + upgradeableCondition := "" + for _, condition := range clusterVersion.Status.Conditions { + if condition.Type == openshiftapiv1.OperatorUpgradeable { + if condition.Status != openshiftapiv1.ConditionTrue { + upgradeableCondition = cmp.Or(condition.Message, fmt.Sprintf("%s: %s", condition.Type, condition.Status)) + } + break + } + } + changed = changed || upgradeableCondition != cd.Labels[constants.MinorVersionUpgradeUnavailable] + if upgradeableCondition == "" { + delete(cd.Labels, constants.MinorVersionUpgradeUnavailable) + } else { + cd.Labels[constants.MinorVersionUpgradeUnavailable] = upgradeableCondition + } + if !changed { cdLog.Debug("labels have not changed, nothing to update") return nil diff --git a/pkg/controller/clusterversion/clusterversion_controller_test.go b/pkg/controller/clusterversion/clusterversion_controller_test.go index 7e4b60742b7..5d28a864e3d 100644 --- a/pkg/controller/clusterversion/clusterversion_controller_test.go +++ b/pkg/controller/clusterversion/clusterversion_controller_test.go @@ -45,11 +45,12 @@ func init() { func TestClusterVersionReconcile(t *testing.T) { tests := []struct { - name string - existing []runtime.Object - noRemoteCall bool - expectError bool - validate func(*testing.T, *hivev1.ClusterDeployment) + name string + existing []runtime.Object + clusterVersionStatus configv1.ClusterVersionStatus + noRemoteCall bool + expectError bool + validate func(*testing.T, *hivev1.ClusterDeployment) }{ { // no cluster deployment, no error expected @@ -69,6 +70,7 @@ func TestClusterVersionReconcile(t *testing.T) { testClusterDeployment(), testKubeconfigSecret(), }, + clusterVersionStatus: testRemoteClusterVersionStatus(), validate: func(t *testing.T, cd *hivev1.ClusterDeployment) { assert.Equal(t, "2.3.4+somebuild", cd.Labels[constants.VersionLabel], "unexpected version label") assert.Equal(t, "2", cd.Labels[constants.VersionMajorLabel], "unexpected version major label") @@ -76,6 +78,97 @@ func TestClusterVersionReconcile(t *testing.T) { assert.Equal(t, "2.3.4", cd.Labels[constants.VersionMajorMinorPatchLabel], "unexpected version major-minor-patch label") }, }, + { + name: "upgradeable condition true", + existing: []runtime.Object{ + testClusterDeployment(), + testKubeconfigSecret(), + }, + clusterVersionStatus: testRemoteClusterVersionStatus(configv1.ClusterOperatorStatusCondition{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionTrue, + }), + validate: func(t *testing.T, cd *hivev1.ClusterDeployment) { + assert.Equal(t, "", cd.Labels[constants.MinorVersionUpgradeUnavailable], "unexpected version major-minor-patch label") + }, + }, + { + name: "upgradeable condition false", + existing: []runtime.Object{ + testClusterDeployment(), + testKubeconfigSecret(), + }, + clusterVersionStatus: testRemoteClusterVersionStatus(configv1.ClusterOperatorStatusCondition{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + }), + validate: func(t *testing.T, cd *hivev1.ClusterDeployment) { + assert.Equal(t, "Upgradeable: False", cd.Labels[constants.MinorVersionUpgradeUnavailable], "unexpected version major-minor-patch label") + }, + }, + { + name: "upgradeable condition false with message", + existing: []runtime.Object{ + testClusterDeployment(), + testKubeconfigSecret(), + }, + clusterVersionStatus: testRemoteClusterVersionStatus(configv1.ClusterOperatorStatusCondition{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Message: "Can't do the upgrade", + }), + validate: func(t *testing.T, cd *hivev1.ClusterDeployment) { + assert.Equal(t, "Can't do the upgrade", cd.Labels[constants.MinorVersionUpgradeUnavailable], "unexpected version major-minor-patch label") + }, + }, + { + name: "upgradeable condition unknown", + existing: []runtime.Object{ + testClusterDeployment(), + testKubeconfigSecret(), + }, + clusterVersionStatus: testRemoteClusterVersionStatus(configv1.ClusterOperatorStatusCondition{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionUnknown, + }), + validate: func(t *testing.T, cd *hivev1.ClusterDeployment) { + assert.Equal(t, "Upgradeable: Unknown", cd.Labels[constants.MinorVersionUpgradeUnavailable], "unexpected version major-minor-patch label") + }, + }, + { + name: "upgradeable condition unknown with message", + existing: []runtime.Object{ + testClusterDeployment(), + testKubeconfigSecret(), + }, + clusterVersionStatus: testRemoteClusterVersionStatus(configv1.ClusterOperatorStatusCondition{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionUnknown, + Message: "Can't read status", + }), + validate: func(t *testing.T, cd *hivev1.ClusterDeployment) { + assert.Equal(t, "Can't read status", cd.Labels[constants.MinorVersionUpgradeUnavailable], "unexpected version major-minor-patch label") + }, + }, + { + name: "upgradeable condition when multiple conditions", + existing: []runtime.Object{ + testClusterDeployment(), + testKubeconfigSecret(), + }, + clusterVersionStatus: testRemoteClusterVersionStatus( + configv1.ClusterOperatorStatusCondition{ + Type: configv1.OperatorProgressing, + }, + configv1.ClusterOperatorStatusCondition{ + Type: configv1.OperatorUpgradeable, + Status: configv1.ConditionFalse, + Message: "It can't upgrade", + }), + validate: func(t *testing.T, cd *hivev1.ClusterDeployment) { + assert.Equal(t, "It can't upgrade", cd.Labels[constants.MinorVersionUpgradeUnavailable], "unexpected version major-minor-patch label") + }, + }, } for _, test := range tests { @@ -85,7 +178,7 @@ func TestClusterVersionReconcile(t *testing.T) { mockCtrl := gomock.NewController(t) mockRemoteClientBuilder := remoteclientmock.NewMockBuilder(mockCtrl) if !test.noRemoteCall { - mockRemoteClientBuilder.EXPECT().Build().Return(testRemoteClusterAPIClient(), nil) + mockRemoteClientBuilder.EXPECT().Build().Return(testRemoteClusterAPIClient(test.clusterVersionStatus), nil) } rcd := &ReconcileClusterVersion{ Client: fakeClient, @@ -183,19 +276,19 @@ func testSecret(name, key, value string) *corev1.Secret { return s } -func testRemoteClusterAPIClient() client.Client { +func testRemoteClusterAPIClient(status configv1.ClusterVersionStatus) client.Client { remoteClusterVersion := &configv1.ClusterVersion{ ObjectMeta: metav1.ObjectMeta{ Name: remoteClusterVersionObjectName, }, } - remoteClusterVersion.Status = *testRemoteClusterVersionStatus() + remoteClusterVersion.Status = status return testfake.NewFakeClientBuilder().WithRuntimeObjects(remoteClusterVersion).Build() } -func testRemoteClusterVersionStatus() *configv1.ClusterVersionStatus { +func testRemoteClusterVersionStatus(conditions ...configv1.ClusterOperatorStatusCondition) configv1.ClusterVersionStatus { zeroTime := metav1.NewTime(time.Unix(0, 0)) - return &configv1.ClusterVersionStatus{ + return configv1.ClusterVersionStatus{ Desired: configv1.Release{ Version: "2.3.4+somebuild", }, @@ -209,5 +302,6 @@ func testRemoteClusterVersionStatus() *configv1.ClusterVersionStatus { }, ObservedGeneration: 123456789, VersionHash: "TESTVERSIONHASH", + Conditions: conditions, } }