From bc9874a7dd77476bdb21a7463d2cebefb0e870fc Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Tue, 4 Jul 2023 19:17:44 +0300 Subject: [PATCH 1/4] Simplify updateDRClusterManifestWorkStatus retry loop Return update result instead of keeping it in temporary error and remove unneeded error definition. Signed-off-by: Nir Soffer --- controllers/drcluster_controller_test.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/controllers/drcluster_controller_test.go b/controllers/drcluster_controller_test.go index 7d8ff64ab..0c9bb7e2a 100644 --- a/controllers/drcluster_controller_test.go +++ b/controllers/drcluster_controller_test.go @@ -244,18 +244,14 @@ func updateDRClusterManifestWorkStatus(clusterNamespace string) { } retryErr := retry.RetryOnConflict(retry.DefaultBackoff, func() error { - var err error - - err = apiReader.Get(context.TODO(), manifestLookupKey, mw) + err := apiReader.Get(context.TODO(), manifestLookupKey, mw) if err != nil { return err } mw.Status = DRClusterStatusConditions - err = k8sClient.Status().Update(context.TODO(), mw) - - return err + return k8sClient.Status().Update(context.TODO(), mw) }) Expect(retryErr).NotTo(HaveOccurred()) From b59fb5e3791d40d30779c2fd05bc5712e55859e6 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Tue, 4 Jul 2023 20:00:38 +0300 Subject: [PATCH 2/4] Disable nlreturn golangci-lint rule The current nlreturn golangci-lint rule is bad, leading to worse code style like: if err != nil { log.Info("Something failed"); return err; } Disable the rule so we can format code properly. Signed-off-by: Nir Soffer --- .golangci.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/.golangci.yaml b/.golangci.yaml index d253391a5..74ba6584b 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -43,6 +43,7 @@ linters: - godot - godox - paralleltest + - nlreturn - goerr113 # TODO: Need to introduce error definition and bring this back - goheader # TODO: Introduce back post fixing linter errors - gci From 5d65afe2cd759bd5ca3dcb9d5515760cd9e8e727 Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Tue, 4 Jul 2023 20:33:49 +0300 Subject: [PATCH 3/4] Disable wsl golangci-lint linter The rules of this linter forces bad style and I could not find how to disable the rule for adding blank line before return. An example of bad style required by wsl: if condition { log.Info("Long message so we move extra arguments to next line...", "name", name, "namespace", namespace) return true } Signed-off-by: Nir Soffer --- .golangci.yaml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/.golangci.yaml b/.golangci.yaml index 74ba6584b..a40b4a6fa 100644 --- a/.golangci.yaml +++ b/.golangci.yaml @@ -30,9 +30,6 @@ linters-settings: SPDX-License-Identifier: Apache-2.0 misspell: locale: US - wsl: - allow-trailing-comment: true - enforce-err-cuddling: true linters: enable-all: true @@ -44,6 +41,7 @@ linters: - godox - paralleltest - nlreturn + - wsl - goerr113 # TODO: Need to introduce error definition and bring this back - goheader # TODO: Introduce back post fixing linter errors - gci From 2290ee3f4375cd36f5a08f5f9219f32a737fb8cf Mon Sep 17 00:00:00 2001 From: Nir Soffer Date: Tue, 4 Jul 2023 20:05:15 +0300 Subject: [PATCH 4/4] Remove unneeded blank lines before return In drcluster module and tests. This was not possible before because of the golangci-lint nlreturn rule and wsl linter. Same change is needed in rest of the code, starting with these modules as example for possible changes. Signed-off-by: Nir Soffer --- controllers/drcluster_controller.go | 13 ------------- controllers/drcluster_controller_test.go | 1 - 2 files changed, 14 deletions(-) diff --git a/controllers/drcluster_controller.go b/controllers/drcluster_controller.go index 9931c7a62..961795e17 100644 --- a/controllers/drcluster_controller.go +++ b/controllers/drcluster_controller.go @@ -84,7 +84,6 @@ func (r *DRClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { } ctrl.Log.Info(fmt.Sprintf("DRCluster: Filtering DRPC (%s/%s)", drpc.Name, drpc.Namespace)) - return filterDRPC(drpc) })) @@ -97,7 +96,6 @@ func (r *DRClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { } ctrl.Log.Info(fmt.Sprintf("DRCluster: Filtering ManifestWork (%s/%s)", mw.Name, mw.Namespace)) - return filterDRClusterMW(mw) })) @@ -110,7 +108,6 @@ func (r *DRClusterReconciler) SetupWithManager(mgr ctrl.Manager) error { } ctrl.Log.Info(fmt.Sprintf("DRCluster: Filtering MCV (%s/%s)", mcv.Name, mcv.Namespace)) - return filterDRClusterMCV(mcv) })) @@ -180,7 +177,6 @@ func DRPCUpdateOfInterest(oldDRPC, newDRPC *ramen.DRPlacementControl) bool { log.Info("Processing DRPC failover cluster change event", "name", newDRPC.GetName(), "namespace", newDRPC.GetNamespace()) - return true } @@ -193,14 +189,12 @@ func DRPCUpdateOfInterest(oldDRPC, newDRPC *ramen.DRPlacementControl) bool { log.Info("Processing DRPC failed over event", "name", newDRPC.GetName(), "namespace", newDRPC.GetNamespace()) - return true } log.Info("Ignoring DRPC failed over event", "name", newDRPC.GetName(), "namespace", newDRPC.GetNamespace()) - return false } @@ -509,17 +503,14 @@ func (u *drclusterInstance) statusUpdate() error { if err := u.client.Status().Update(u.ctx, u.object); err != nil { u.log.Info(fmt.Sprintf("Failed to update drCluster status (%s/%s/%v)", u.object.Name, u.object.Namespace, err)) - return fmt.Errorf("failed to update drCluster status (%s/%s)", u.object.Name, u.object.Namespace) } u.log.Info(fmt.Sprintf("Updated drCluster Status %+v", u.object.Status)) - return nil } u.log.Info(fmt.Sprintf("Nothing to update %+v", u.object.Status)) - return nil } @@ -647,7 +638,6 @@ func (u *drclusterInstance) clusterUnfence() (bool, error) { if requeue { u.log.Info("requing as cluster unfence operation is not complete") - return requeue, nil } @@ -710,7 +700,6 @@ func (u *drclusterInstance) fenceClusterOnCluster(peerCluster *ramen.DRCluster) "fencing operation not successful") u.log.Info("Fencing operation not successful", "cluster", u.object.Name) - return true, fmt.Errorf("fencing operation result not successful") } @@ -776,7 +765,6 @@ func (u *drclusterInstance) unfenceClusterOnCluster(peerCluster *ramen.DRCluster "unfencing operation not successful") u.log.Info("Unfencing operation not successful", "cluster", u.object.Name) - return true, fmt.Errorf("un operation result not successful") } @@ -1241,7 +1229,6 @@ func (u *drclusterInstance) createNFManifestWork(targetCluster *ramen.DRCluster, u.object.Name, u.object.Namespace, peerCluster.Name, nf, annotations); err != nil { log.Error(err, "failed to create or update NetworkFence manifest") - return fmt.Errorf("failed to create or update NetworkFence manifest in cluster %s to fence off cluster %s (%w)", peerCluster.Name, targetCluster.Name, err) } diff --git a/controllers/drcluster_controller_test.go b/controllers/drcluster_controller_test.go index 0c9bb7e2a..898c5a279 100644 --- a/controllers/drcluster_controller_test.go +++ b/controllers/drcluster_controller_test.go @@ -217,7 +217,6 @@ func updateDRClusterManifestWorkStatus(clusterNamespace string) { Eventually(func() bool { err := apiReader.Get(context.TODO(), manifestLookupKey, mw) - return err == nil }, timeout, interval).Should(BeTrue(), fmt.Sprintf("failed to get manifest for DRCluster %s", clusterNamespace))