OSD-26790 continue upgrade despite notif failure

pkg/upgraders/controlplanestep.go

@@ -14,7 +14,6 @@ import (
 
 // CommenceUpgrade will update the clusterversion object to apply the desired version to trigger real OCP upgrade
 func (c *clusterUpgrader) CommenceUpgrade(ctx context.Context, logger logr.Logger) (bool, error) {
-
 	// We can reset the window breached metric if we're commencing
 	c.metrics.UpdateMetricUpgradeWindowNotBreached(c.upgradeConfig.Name)
 
@@ -29,7 +28,7 @@ func (c *clusterUpgrader) CommenceUpgrade(ctx context.Context, logger logr.Logge
 
 	err = c.notifier.Notify(notifier.MuoStateControlPlaneUpgradeStartedSL)
 	if err != nil {
-		return false, err
+		logger.Error(err, fmt.Sprintf("Failed to notify upon upgrade start %v", err))
 	}
 
 	isComplete, err := c.cvClient.EnsureDesiredConfig(c.upgradeConfig)
@@ -52,7 +51,7 @@ func (c *clusterUpgrader) ControlPlaneUpgraded(ctx context.Context, logger logr.
 	if isCompleted {
 		err = c.notifier.Notify(notifier.MuoStateControlPlaneUpgradeFinishedSL)
 		if err != nil {
-			return false, err
+			logger.Error(err, fmt.Sprintf("Failed to notify upon upgrade completion %v", err))
 		}
 		c.metrics.ResetMetricUpgradeControlPlaneTimeout(c.upgradeConfig.Name, c.upgradeConfig.Spec.Desired.Version)
 		return true, nil
@@ -65,7 +64,7 @@ func (c *clusterUpgrader) ControlPlaneUpgraded(ctx context.Context, logger logr.
 	} else {
 		upgradeStartTime, err = time.Parse(time.RFC3339, c.upgradeConfig.Spec.UpgradeAt)
 		if err != nil {
-			return false, err //error parsing time string
+			return false, err // error parsing time string
 		}
 	}
 

pkg/upgraders/controlplanestep_test.go

@@ -103,10 +103,11 @@ var _ = Describe("ControlPlaneStep", func() {
 					mockCVClient.EXPECT().GetClusterVersion().Return(nil, nil),
 					mockCVClient.EXPECT().HasUpgradeCompleted(gomock.Any(), gomock.Any()).Return(true),
 					mockEMClient.EXPECT().Notify(gomock.Any()).Return(fakeError),
+					mockMetricsClient.EXPECT().ResetMetricUpgradeControlPlaneTimeout(upgradeConfig.Name, upgradeConfig.Spec.Desired.Version),
 				)
 				result, err := upgrader.ControlPlaneUpgraded(context.TODO(), logger)
-				Expect(err).To(HaveOccurred())
-				Expect(result).To(BeFalse())
+				Expect(err).NotTo(HaveOccurred())
+				Expect(result).To(BeTrue())
 			})
 		})
 
@@ -238,16 +239,17 @@ var _ = Describe("ControlPlaneStep", func() {
 		})
 
 		Context("When sending notification for control plane upgrade start fails", func() {
-			It("Should report error", func() {
+			It("Should continue with upgrade", func() {
 				fakeError := fmt.Errorf("fake notification error")
 				gomock.InOrder(
 					mockMetricsClient.EXPECT().UpdateMetricUpgradeWindowNotBreached(gomock.Any()),
 					mockCVClient.EXPECT().HasUpgradeCommenced(gomock.Any()).Return(false, nil),
 					mockEMClient.EXPECT().Notify(gomock.Any()).Return(fakeError),
+					mockCVClient.EXPECT().EnsureDesiredConfig(gomock.Any()).Return(true, nil),
 				)
 				result, err := upgrader.CommenceUpgrade(context.TODO(), logger)
-				Expect(err).To(HaveOccurred())
-				Expect(result).To(BeFalse())
+				Expect(err).ToNot(HaveOccurred())
+				Expect(result).To(BeTrue())
 			})
 		})
 
@@ -263,7 +265,6 @@ var _ = Describe("ControlPlaneStep", func() {
 				Expect(err).ToNot(HaveOccurred())
 				Expect(result).To(BeTrue())
 			})
-
 		})
 	})
 
@@ -280,7 +281,7 @@ var _ = Describe("ControlPlaneStep", func() {
 	})
 
 	Context("When the upgrader can't tell if the cluster's upgrade has commenced", func() {
-		var fakeError = fmt.Errorf("fake upgradeCommenced error")
+		fakeError := fmt.Errorf("fake upgradeCommenced error")
 		It("will abort the commencing of an upgrade", func() {
 			gomock.InOrder(
 				mockMetricsClient.EXPECT().UpdateMetricUpgradeWindowNotBreached(gomock.Any()),
@@ -292,5 +293,4 @@ var _ = Describe("ControlPlaneStep", func() {
 			Expect(result).To(BeFalse())
 		})
 	})
-
 })