diff --git a/pkg/upgraders/controlplanestep.go b/pkg/upgraders/controlplanestep.go index 01b99089..28bec093 100644 --- a/pkg/upgraders/controlplanestep.go +++ b/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 } } diff --git a/pkg/upgraders/controlplanestep_test.go b/pkg/upgraders/controlplanestep_test.go index 2b15fecc..f1ddc7ef 100644 --- a/pkg/upgraders/controlplanestep_test.go +++ b/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()) }) }) - })