From 87d31e2befd7d6fbdb2e7271c6a568a203253bb3 Mon Sep 17 00:00:00 2001 From: Jason van Brackel Date: Fri, 16 Jul 2021 07:39:55 -0400 Subject: [PATCH] feat: add functionality to update the state of a group object upon error --- controllers/gitlab/group_controller.go | 28 +++++++- controllers/gitlab/group_controller_test.go | 75 +++++++++++++++------ 2 files changed, 81 insertions(+), 22 deletions(-) diff --git a/controllers/gitlab/group_controller.go b/controllers/gitlab/group_controller.go index 0e37aa7..e2cb55d 100644 --- a/controllers/gitlab/group_controller.go +++ b/controllers/gitlab/group_controller.go @@ -49,6 +49,7 @@ const ( errorGettingGroupVariablesFromGitlab = "Error getting group variables from Gitlab" errorCreatingGroupVariableInGitlab = "Error creating group variable in Gitlab" errorUpdatingGroupVariableInGitLab = "Error updating group variable in Gitlab" + errorWhileUpdatingGroupState = "Error while updating group state in status" ) // Group Status @@ -113,6 +114,7 @@ func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl } if r.gitlabClient, err = r.gitlabClientConfiguration.SetupClient(r.Client, group.Spec.GitlabCredentialsName); err != nil { r.Log.Error(err, errorUnableToSetupGitlabClient) + _ = r.updateState(ctx, group, errorUnableToSetupGitlabClient) return ctrl.Result{Requeue: true}, err } } @@ -121,34 +123,40 @@ func (r *GroupReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ctrl var gitlabGroup *gitlab.Group found, gitlabGroup, err := r.groupExists(group) if err != nil { - r.Log.Error(err, errorWhileSearchingGroups, "status") + r.Log.Error(err, errorWhileSearchingGroups) + _ = r.updateState(ctx, group, errorWhileSearchingGroups) return ctrl.Result{Requeue: true}, err } if !found { if gitlabGroup, _, err = r.createGroup(group); err != nil { r.Log.Error(err, errorWhileCreatingGitlabGroup, "group", group) + _ = r.updateState(ctx, group, errorWhileCreatingGitlabGroup) return ctrl.Result{Requeue: true}, err } } else { if gitlabGroup, _, err = r.updateGroup(group); err != nil { r.Log.Error(err, errorWhileUpdatingGitlabGroup, "group", group) + _ = r.updateState(ctx, group, errorWhileUpdatingGitlabGroup) return ctrl.Result{Requeue: true}, err } } if err = r.updateCiVariables(gitlabGroup.ID, group); err != nil { r.Log.Error(err, errorWhileUpdatingGitlabGroupCiVariables, "group", group) + _ = r.updateState(ctx, group, errorWhileUpdatingGitlabGroupCiVariables) return ctrl.Result{Requeue: true}, err } if err = r.updateStatus(ctx, gitlabGroup.ID, group); err != nil { r.Log.Error(err, errorUnableToUpdateStatus, "group", group) + _ = r.updateState(ctx, group, errorUnableToUpdateStatus) return ctrl.Result{Requeue: true}, err } if err = r.processProjects(ctx, group, group.Spec.ProjectSpecs); err != nil { r.Log.Error(err, errorUnableToProcessProjects, "group", group) + _ = r.updateState(ctx, group, errorUnableToProcessProjects) return ctrl.Result{Requeue: true}, err } @@ -291,12 +299,17 @@ func (r *GroupReconciler) updateProject(ctx context.Context, groupID int, spec g func (r *GroupReconciler) updateStatus(ctx context.Context, id int, group *gitlabv1alpha1.Group) error { if id != 0 { + group.Status.State = "OK" group.Status.LastUpdatedTime = metav1.Time{ Time: time.Now(), } } - return r.Status().Update(ctx, group) + if err := r.Status().Update(ctx, group); err != nil { + r.Log.Error(err, errorUnableToUpdateStatus) + return err + } + return nil } // SetupWithManager sets up the controller with the Manager. @@ -308,7 +321,7 @@ func (r *GroupReconciler) SetupWithManager(mgr ctrl.Manager) error { } func (r *GroupReconciler) updateCiVariables(ID int, group *gitlabv1alpha1.Group) error { - var variablesToUpdate []string = []string{KustomizeStagingPathVariableName, KustomizeProductionPathVariableName, ManifestRepoURLVariableName} + var variablesToUpdate = []string{KustomizeStagingPathVariableName, KustomizeProductionPathVariableName, ManifestRepoURLVariableName} var err error var groupVariable *gitlab.GroupVariable @@ -379,3 +392,12 @@ func (r *GroupReconciler) GetGroupVariableValueByName(variableToCreate string, g } return "" } + +func (r *GroupReconciler) updateState(ctx context.Context, group *gitlabv1alpha1.Group, state string) error { + group.Status.State = state + if err := r.Client.Status().Update(ctx, group) ; err != nil { + r.Log.Error(err, errorWhileUpdatingGroupState, "group", group) + return err + } + return nil +} diff --git a/controllers/gitlab/group_controller_test.go b/controllers/gitlab/group_controller_test.go index 7937de9..0865a03 100755 --- a/controllers/gitlab/group_controller_test.go +++ b/controllers/gitlab/group_controller_test.go @@ -192,6 +192,9 @@ var _ = Describe("Reconcile", func() { })) Expect(err).ToNot(BeNil()) }) + It("should update the state of the object", func() { + Expect(sut.Status().(*MockStatusWriter).updatedObject.(*gitlabv1alpha1.Group).Status.State).To(Equal(errorUnableToSetupGitlabClient)) + }) }) Context("groupExists fails", func() { sut, logger, _, _, _ := getGroupControllerWithMocksInGreenTestState() @@ -217,6 +220,9 @@ var _ = Describe("Reconcile", func() { It("should return an error", func() { Expect(err).NotTo(BeNil()) }) + It("should update the state of the object", func() { + Expect(sut.Status().(*MockStatusWriter).updatedObject.(*gitlabv1alpha1.Group).Status.State).To(Equal(errorWhileSearchingGroups)) + }) }) Context("createGroup fails", func() { sut, logger, _, _, _ := getGroupControllerWithMocksInGreenTestState() @@ -246,6 +252,9 @@ var _ = Describe("Reconcile", func() { It("should return an error", func() { Expect(err).NotTo(BeNil()) }) + It("should update the state of the object", func() { + Expect(sut.Status().(*MockStatusWriter).updatedObject.(*gitlabv1alpha1.Group).Status.State).To(Equal(errorWhileCreatingGitlabGroup)) + }) }) Context("updateGroup fails", func() { sut, logger, _, _, gitlabClient := getGroupControllerWithMocksInGreenTestState() @@ -267,6 +276,9 @@ var _ = Describe("Reconcile", func() { It("should return an error", func() { Expect(err).NotTo(BeNil()) }) + It("should update the state of the object", func() { + Expect(sut.Status().(*MockStatusWriter).updatedObject.(*gitlabv1alpha1.Group).Status.State).To(Equal(errorWhileUpdatingGitlabGroup)) + }) }) Context("getGroupVariable fails", func() { sut, log, _, _, gitlabClient := getGroupControllerWithMocksInGreenTestState() @@ -284,6 +296,9 @@ var _ = Describe("Reconcile", func() { It("should return an error", func() { Expect(err).ToNot(BeNil()) }) + It("should update the state of the object", func() { + Expect(sut.Status().(*MockStatusWriter).updatedObject.(*gitlabv1alpha1.Group).Status.State).To(Equal(errorWhileUpdatingGitlabGroupCiVariables)) + }) }) Context("createCiVariable fails", func() { sut, log, _, _, gitlabClient := getGroupControllerWithMocksInGreenTestState() @@ -301,6 +316,9 @@ var _ = Describe("Reconcile", func() { It("should return an error", func() { Expect(err).ToNot(BeNil()) }) + It("should update the state of the object", func() { + Expect(sut.Status().(*MockStatusWriter).updatedObject.(*gitlabv1alpha1.Group).Status.State).To(Equal(errorWhileUpdatingGitlabGroupCiVariables)) + }) }) Context("updateGroupVariable fails", func() { sut, log, _, _, gitlabClient := getGroupControllerWithMocksInGreenTestState() @@ -318,25 +336,8 @@ var _ = Describe("Reconcile", func() { It("should return an error", func() { Expect(err).ToNot(BeNil()) }) - }) - Context("updateStatus fails", func() { - sut, logger, _, kubernetesClient, _ := getGroupControllerWithMocksInGreenTestState() - - kubernetesClient.statusWriter = &MockStatusWriter{ - updateFunction: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { - return &MockError{"updateStatus fails"} - }, - } - result, err := sut.Reconcile(context.TODO(), getGreenGroupControllerRequest()) - It("should requeue the object", func() { - Expect(result).To(Equal(ctrl.Result{Requeue: true})) - }) - It("should log the error", func() { - Expect(logger.logLevelCalled).To(Equal("Error")) - Expect(logger.loggedMessage).To(Equal(errorUnableToUpdateStatus)) - }) - It("should return an error", func() { - Expect(err).NotTo(BeNil()) + It("should update the state of the object", func() { + Expect(sut.Status().(*MockStatusWriter).updatedObject.(*gitlabv1alpha1.Group).Status.State).To(Equal(errorWhileUpdatingGitlabGroupCiVariables)) }) }) Context("processProjects fails", func() { @@ -362,6 +363,42 @@ var _ = Describe("Reconcile", func() { }) }) +var _ = Describe("updateState fails", func() { + sut, logger, _, _, _ := getGroupControllerWithMocksInGreenTestState() + sut.Status().(*MockStatusWriter).updateFunction = func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return &MockError{message: "mock updateState error"} + } + result := sut.updateState(context.TODO(), getGreenGroup(), "") + + It("should log the error", func() { + Expect(logger.logLevelCalled).To(Equal("Error")) + Expect(logger.loggedMessage).To(Equal(errorWhileUpdatingGroupState)) + }) + It("should return an error", func() { + Expect(result).ToNot(BeNil()) + }) +}) + +var _ = Describe("updateStatus fails", func() { + sut, logger, _, kubernetesClient, _ := getGroupControllerWithMocksInGreenTestState() + + kubernetesClient.statusWriter = &MockStatusWriter{ + updateFunction: func(ctx context.Context, obj client.Object, opts ...client.UpdateOption) error { + return &MockError{"updateStatus fails"} + }, + } + ctx := context.TODO() + group := getGreenGroup() + err := sut.updateStatus(ctx, 1, group) + It("should log the error", func() { + Expect(logger.logLevelCalled).To(Equal("Error")) + Expect(logger.loggedMessage).To(Equal(errorUnableToUpdateStatus)) + }) + It("should return an error", func() { + Expect(err).NotTo(BeNil()) + }) +}) + var _ = Describe("groupExists", func() { When("the gitlab client fails", func() { sut, _, _, _, gitlabClient := getGroupControllerWithMocksInGreenTestState() -- GitLab