MGMT-19979: Propagate ClusterVersion upgrade status message

Adds the upgrade message from the ClusterVersion in the
workload cluster to the OACP's upgrade status so it's
transparent to the user. Also adds a failure reason
in case the upgrade fails.

Author: CrystalChun

controlplane/api/v1alpha2/condition_consts.go

@@ -36,6 +36,9 @@ const (
 	// UpgradeInProgressReason (Severity=Info) documents that an upgrade is in progress.
 	UpgradeInProgressReason = "UpgradeInProgress"
 
+	// UpgradeFailedReason (Severity=Error) documents that an upgrade has failed.
+	UpgradeFailedReason = "UpgradeFailed"
+
 	// UpgradeImageUnavailableReason (Severity=Error) documents whether an upgrade image is available
 	UpgradeImageUnavailableReason = "UpgradeImageUnavailable"
 

controlplane/internal/controller/openshiftassistedcontrolplane_controller.go

@@ -286,16 +286,11 @@ func (r *OpenshiftAssistedControlPlaneReconciler) upgradeWorkloadCluster(ctx con
 	log := ctrl.LoggerFrom(ctx)
 
 	var isUpdateInProgress bool
+	var upgradeConditionMessage string
 	defer func() {
 		if isUpdateInProgress || !isWorkloadClusterRunningDesiredVersion(oacp) {
-			conditions.MarkFalse(
-				oacp,
-				controlplanev1alpha2.UpgradeCompletedCondition,
-				controlplanev1alpha2.UpgradeInProgressReason,
-				clusterv1.ConditionSeverityInfo,
-				"upgrade to version %s in progress",
-				oacp.Spec.DistributionVersion,
-			)
+			// Either upgrade is in progress or it failed
+			setUpgradeStatus(oacp, isUpdateInProgress, upgradeConditionMessage)
 			return
 		}
 		if conditions.IsFalse(oacp, controlplanev1alpha2.UpgradeCompletedCondition) {
@@ -316,6 +311,10 @@ func (r *OpenshiftAssistedControlPlaneReconciler) upgradeWorkloadCluster(ctx con
 	if err != nil {
 		return ctrl.Result{}, err
 	}
+	upgradeConditionMessage, err = upgrader.GetUpgradeStatus(ctx)
+	if err != nil {
+		return ctrl.Result{}, err
+	}
 
 	oacp.Status.DistributionVersion, err = upgrader.GetCurrentVersion(ctx)
 	if err != nil {
@@ -354,6 +353,26 @@ func (r *OpenshiftAssistedControlPlaneReconciler) upgradeWorkloadCluster(ctx con
 		)
 }
 
+func setUpgradeStatus(oacp *controlplanev1alpha2.OpenshiftAssistedControlPlane, upgradeInProgress bool, conditionMessage string) {
+	reason := controlplanev1alpha2.UpgradeInProgressReason
+	severity := clusterv1.ConditionSeverityInfo
+	msg := "upgrade to version %s in progress\n%s"
+	if !upgradeInProgress {
+		reason = controlplanev1alpha2.UpgradeFailedReason
+		severity = clusterv1.ConditionSeverityError
+		msg = "upgrade to version %s has failed\n%s"
+	}
+	conditions.MarkFalse(
+		oacp,
+		controlplanev1alpha2.UpgradeCompletedCondition,
+		reason,
+		severity,
+		msg,
+		oacp.Spec.DistributionVersion,
+		conditionMessage,
+	)
+}
+
 func getUpgradeOptions(oacp *controlplanev1alpha2.OpenshiftAssistedControlPlane, pullSecret []byte) []upgrade.ClusterUpgradeOption {
 	upgradeOptions := []upgrade.ClusterUpgradeOption{
 		{

controlplane/internal/controller/openshiftassistedcontrolplane_controller_test.go

@@ -81,6 +81,7 @@ var _ = Describe("OpenshiftAssistedControlPlane Controller", func() {
 			mockUpgrader.EXPECT().IsUpgradeInProgress(gomock.Any()).Return(false, nil).AnyTimes()
 			mockUpgrader.EXPECT().GetCurrentVersion(gomock.Any()).Return("4.18.0", nil).AnyTimes()
 			mockUpgrader.EXPECT().IsDesiredVersionUpdated(gomock.Any(), gomock.Any()).Return(true, nil).AnyTimes()
+			mockUpgrader.EXPECT().GetUpgradeStatus(gomock.Any()).Return("", nil).AnyTimes()
 
 			mockUpgradeFactory = upgrade.NewMockClusterUpgradeFactory(ctrl)
 			mockUpgradeFactory.EXPECT().NewUpgrader(gomock.Any()).Return(mockUpgrader, nil).AnyTimes()
@@ -384,6 +385,7 @@ var _ = Describe("Upgrade scenarios", func() {
 		mockUpgradeFactory.EXPECT().NewUpgrader(gomock.Any()).Return(mockUpgrader, nil)
 		mockUpgrader.EXPECT().IsUpgradeInProgress(gomock.Any()).Return(false, nil)
 		mockUpgrader.EXPECT().GetCurrentVersion(gomock.Any()).Return(currentVersion, nil)
+		mockUpgrader.EXPECT().GetUpgradeStatus(gomock.Any()).Return("", nil)
 		mockUpgrader.EXPECT().IsDesiredVersionUpdated(gomock.Any(), desiredVersion).Return(false, nil)
 		mockUpgrader.EXPECT().UpdateClusterVersionDesiredUpdate(gomock.Any(), desiredVersion, gomock.Any(), gomock.Any()).Return(nil)
 
@@ -403,6 +405,7 @@ var _ = Describe("Upgrade scenarios", func() {
 		mockUpgradeFactory.EXPECT().NewUpgrader(gomock.Any()).Return(mockUpgrader, nil)
 		mockUpgrader.EXPECT().IsUpgradeInProgress(gomock.Any()).Return(true, nil)
 		mockUpgrader.EXPECT().GetCurrentVersion(gomock.Any()).Return(currentVersion, nil)
+		mockUpgrader.EXPECT().GetUpgradeStatus(gomock.Any()).Return("upgrading cluster", nil)
 		mockUpgrader.EXPECT().IsDesiredVersionUpdated(gomock.Any(), desiredVersion).Return(true, nil)
 		conditions.MarkFalse(openshiftAssistedControlPlane, controlplanev1alpha2.UpgradeCompletedCondition, controlplanev1alpha2.UpgradeInProgressReason, clusterv1.ConditionSeverityInfo, "upgrade in progress")
 
@@ -417,12 +420,14 @@ var _ = Describe("Upgrade scenarios", func() {
 		Expect(condition).NotTo(BeNil())
 		Expect(condition.Status).To(Equal(corev1.ConditionFalse))
 		Expect(condition.Reason).To(Equal(controlplanev1alpha2.UpgradeInProgressReason))
+		Expect(condition.Message).To(Equal("upgrade to version 4.15.0 in progress\nupgrading cluster"))
 	})
 
 	It("should handle completed upgrade", func() {
 		mockUpgradeFactory.EXPECT().NewUpgrader(gomock.Any()).Return(mockUpgrader, nil)
 		mockUpgrader.EXPECT().IsUpgradeInProgress(gomock.Any()).Return(false, nil)
 		mockUpgrader.EXPECT().GetCurrentVersion(gomock.Any()).Return(desiredVersion, nil)
+		mockUpgrader.EXPECT().GetUpgradeStatus(gomock.Any()).Return("", nil)
 		mockUpgrader.EXPECT().IsDesiredVersionUpdated(gomock.Any(), desiredVersion).Return(true, nil)
 
 		// Make sure upgrade is in progress: once we reconcile it should be marked as complete
@@ -453,6 +458,7 @@ var _ = Describe("Upgrade scenarios", func() {
 		mockUpgradeFactory.EXPECT().NewUpgrader(gomock.Any()).Return(mockUpgrader, nil)
 		mockUpgrader.EXPECT().IsUpgradeInProgress(gomock.Any()).Return(false, nil)
 		mockUpgrader.EXPECT().GetCurrentVersion(gomock.Any()).Return(currentVersion, nil)
+		mockUpgrader.EXPECT().GetUpgradeStatus(gomock.Any()).Return("failed to upgrade", nil)
 		mockUpgrader.EXPECT().IsDesiredVersionUpdated(gomock.Any(), desiredVersion).Return(false, nil)
 		mockUpgrader.EXPECT().UpdateClusterVersionDesiredUpdate(gomock.Any(), desiredVersion, gomock.Any(), gomock.Any()).Return(expectedError)
 
@@ -469,17 +475,22 @@ var _ = Describe("Upgrade scenarios", func() {
 		Expect(err).To(HaveOccurred())
 		Expect(err).To(Equal(expectedError))
 
-		// Upgrade is in progres: spec.distributionVersion is 4.15 and status.distributionVersion is 4.14
+		err = k8sClient.Get(ctx, typeNamespacedName, openshiftAssistedControlPlane)
+		Expect(err).NotTo(HaveOccurred())
+		// Upgrade is in progress but failed: spec.distributionVersion is 4.15 and status.distributionVersion is 4.14
 		condition := conditions.Get(openshiftAssistedControlPlane, controlplanev1alpha2.UpgradeCompletedCondition)
 		Expect(condition).NotTo(BeNil())
 		Expect(condition.Status).To(Equal(corev1.ConditionFalse))
+		Expect(condition.Reason).To(Equal(controlplanev1alpha2.UpgradeFailedReason))
+		Expect(condition.Message).To(Equal("upgrade to version 4.15.0 has failed\nfailed to upgrade"))
 	})
 
 	It("should handle errors getting current version", func() {
 		expectedError := fmt.Errorf("failed to get version")
 		mockUpgradeFactory.EXPECT().NewUpgrader(gomock.Any()).Return(mockUpgrader, nil)
 		mockUpgrader.EXPECT().IsUpgradeInProgress(gomock.Any()).Return(false, nil)
 		mockUpgrader.EXPECT().GetCurrentVersion(gomock.Any()).Return("", expectedError)
+		mockUpgrader.EXPECT().GetUpgradeStatus(gomock.Any()).Return("failed to upgrade", nil)
 		mockUpgrader.EXPECT().IsDesiredVersionUpdated(gomock.Any(), desiredVersion).Return(true, nil)
 		mockUpgrader.EXPECT().UpdateClusterVersionDesiredUpdate(ctx, desiredVersion, gomock.Any(), gomock.Any()).Return(nil)
 
@@ -494,8 +505,8 @@ var _ = Describe("Upgrade scenarios", func() {
 		condition := conditions.Get(openshiftAssistedControlPlane, controlplanev1alpha2.UpgradeCompletedCondition)
 		Expect(condition).NotTo(BeNil())
 		Expect(condition.Status).To(Equal(corev1.ConditionFalse))
-		Expect(condition.Reason).To(Equal(controlplanev1alpha2.UpgradeInProgressReason))
-		Expect(condition.Message).To(Equal("upgrade to version 4.15.0 in progress"))
+		Expect(condition.Reason).To(Equal(controlplanev1alpha2.UpgradeFailedReason))
+		Expect(condition.Message).To(Equal("upgrade to version 4.15.0 has failed\nfailed to upgrade"))
 	})
 
 	It("should handle upgrade with repository override", func() {
@@ -506,8 +517,9 @@ var _ = Describe("Upgrade scenarios", func() {
 		Expect(k8sClient.Update(ctx, openshiftAssistedControlPlane)).To(Succeed())
 
 		mockUpgradeFactory.EXPECT().NewUpgrader(gomock.Any()).Return(mockUpgrader, nil)
-		mockUpgrader.EXPECT().IsUpgradeInProgress(gomock.Any()).Return(false, nil)
+		mockUpgrader.EXPECT().IsUpgradeInProgress(gomock.Any()).Return(true, nil) // should this be successfully starting upgrade?
 		mockUpgrader.EXPECT().GetCurrentVersion(gomock.Any()).Return(currentVersion, nil)
+		mockUpgrader.EXPECT().GetUpgradeStatus(gomock.Any()).Return("", nil)
 		mockUpgrader.EXPECT().IsDesiredVersionUpdated(gomock.Any(), desiredVersion).Return(false, nil)
 		expectedParams := []upgrade.ClusterUpgradeOption{
 			{Name: upgrade.ReleaseImagePullSecretOption, Value: "{\"auths\":{\"fake-pull-secret\":{\"auth\":\"cGxhY2Vob2xkZXI6c2VjcmV0Cg==\"}}}"},
@@ -528,6 +540,7 @@ var _ = Describe("Upgrade scenarios", func() {
 		condition := conditions.Get(openshiftAssistedControlPlane, controlplanev1alpha2.UpgradeCompletedCondition)
 		Expect(condition).NotTo(BeNil())
 		Expect(condition.Status).To(Equal(corev1.ConditionFalse))
+		Expect(condition.Message).To(Equal("upgrade to version 4.15.0 in progress\n"))
 	})
 })
 

controlplane/internal/upgrade/mock_upgrade.go

@@ -35,6 +35,7 @@ type ClusterUpgrade interface {
 	IsUpgradeInProgress(ctx context.Context) (bool, error)
 	GetCurrentVersion(ctx context.Context) (string, error)
 	IsDesiredVersionUpdated(ctx context.Context, desiredVersion string) (bool, error)
+	GetUpgradeStatus(ctx context.Context) (string, error)
 	UpdateClusterVersionDesiredUpdate(ctx context.Context, desiredVersion string, architecture string, options ...ClusterUpgradeOption) error
 }
 
@@ -113,6 +114,24 @@ func (u *OpenshiftUpgrader) GetCurrentVersion(ctx context.Context) (string, erro
 	return "", fmt.Errorf("no completed update found in ClusterVersion history")
 }
 
+// Returns the cluster version's current upgrade condition message for the client's cluster.
+// If any error occurs while performing this operation, it will be returned.
+func (u *OpenshiftUpgrader) GetUpgradeStatus(ctx context.Context) (string, error) {
+	clusterVersion, err := u.getClusterVersion(ctx)
+	if err != nil {
+		return "", err
+	}
+	for _, condition := range clusterVersion.Status.Conditions {
+		// Condition is only set to one of Available, Progressing, and Degraded
+		// and they are mutually exclusive, so only one of these is true
+		// at any given point.
+		if condition.Status == configv1.ConditionTrue {
+			return condition.Message, nil
+		}
+	}
+	return "", nil
+}
+
 // Returns true if the desired version matches the cluster's desired version, false otherwise. If any error occurs while
 // performing this operation, it will be returned
 func (u *OpenshiftUpgrader) IsDesiredVersionUpdated(ctx context.Context, desiredVersion string) (bool, error) {

controlplane/internal/upgrade/upgrade.go

@@ -176,6 +176,13 @@ var _ = Describe("OpenShift Upgrader", func() {
 				Expect(updatedCV.Spec.DesiredUpdate.Force).To(BeTrue())
 			})
 		})
+		Context("GetUpgradeStatus", func() {
+			It("should return an upgrade is in progress message", func() {
+				msg, err := upgrader.GetUpgradeStatus(ctx)
+				Expect(err).To(BeNil())
+				Expect(msg).To(Equal("upgrade is in progress"))
+			})
+		})
 	})
 })
 
@@ -189,6 +196,16 @@ func getClusterVersion(history []configv1.UpdateHistory) configv1.ClusterVersion
 			Desired: configv1.Release{
 				Version: "4.10.0",
 			},
+			Conditions: []configv1.ClusterOperatorStatusCondition{
+				{
+					Message: "upgrade is in progress",
+					Status:  configv1.ConditionTrue,
+				},
+				{
+					Message: "upgrade is not in progress",
+					Status:  configv1.ConditionFalse,
+				},
+			},
 		},
 	}
 }