From b3ca3ac783cd65a73b4003434d70c243fd40b43d Mon Sep 17 00:00:00 2001 From: Luiz Filho Date: Tue, 25 Oct 2022 13:12:02 -0300 Subject: [PATCH] Avoid early return (#28) --- controllers/gitopscluster_controller.go | 6 +++- controllers/gitopscluster_controller_test.go | 38 +++++++++++--------- 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/controllers/gitopscluster_controller.go b/controllers/gitopscluster_controller.go index a2544ae..702f897 100644 --- a/controllers/gitopscluster_controller.go +++ b/controllers/gitopscluster_controller.go @@ -225,7 +225,6 @@ func (r *GitopsClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques log.Error(err, "failed to update Cluster status") return ctrl.Result{}, err } - return ctrl.Result{}, nil } if err := r.verifyConnectivity(ctx, cluster); err != nil { @@ -378,6 +377,11 @@ func (r *GitopsClusterReconciler) requestsForCAPIClusterChange(o client.Object) func (r *GitopsClusterReconciler) verifyConnectivity(ctx context.Context, cluster *gitopsv1alpha1.GitopsCluster) error { log := log.FromContext(ctx) + // avoid checking the cluster if it's under deletion. + if !cluster.ObjectMeta.DeletionTimestamp.IsZero() { + return nil + } + log.Info("checking connectivity", "cluster", cluster.Name) nsName := types.NamespacedName{Namespace: cluster.Namespace, Name: cluster.Name} diff --git a/controllers/gitopscluster_controller_test.go b/controllers/gitopscluster_controller_test.go index 6d749ff..614d7ae 100644 --- a/controllers/gitopscluster_controller_test.go +++ b/controllers/gitopscluster_controller_test.go @@ -27,8 +27,9 @@ import ( ) const ( - testName = "test-cluster" - testNamespace = "testing" + testName = "test-cluster" + testNamespace = "testing" + defaultRequeueTime = 1 * time.Minute ) func TestReconcile(t *testing.T) { @@ -55,7 +56,7 @@ func TestReconcile(t *testing.T) { obj: types.NamespacedName{Namespace: testNamespace, Name: testName}, opts: controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, }, requeueAfter: controllers.MissingSecretRequeueTime, wantCondition: meta.ReadyCondition, @@ -78,9 +79,9 @@ func TestReconcile(t *testing.T) { obj: types.NamespacedName{Namespace: testNamespace, Name: testName}, opts: controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, }, - requeueAfter: 1 * time.Minute, + requeueAfter: defaultRequeueTime, wantCondition: meta.ReadyCondition, wantStatus: "True", }, @@ -116,7 +117,7 @@ func TestReconcile(t *testing.T) { obj: types.NamespacedName{Namespace: testNamespace, Name: testName}, opts: controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, }, errString: "failed to get CAPI cluster.*missing.*not found", wantCondition: meta.ReadyCondition, @@ -139,8 +140,9 @@ func TestReconcile(t *testing.T) { obj: types.NamespacedName{Namespace: testNamespace, Name: testName}, opts: controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, }, + requeueAfter: defaultRequeueTime, wantCondition: meta.ReadyCondition, wantStatus: "False", wantStatusMessage: "Waiting for ControlPlaneReady status", @@ -163,8 +165,9 @@ func TestReconcile(t *testing.T) { obj: types.NamespacedName{Namespace: testNamespace, Name: testName}, opts: controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, }, + requeueAfter: defaultRequeueTime, wantCondition: gitopsv1alpha1.ClusterProvisionedCondition, wantStatus: "True", wantStatusMessage: "CAPI Cluster has been provisioned", @@ -188,8 +191,9 @@ func TestReconcile(t *testing.T) { obj: types.NamespacedName{Namespace: testNamespace, Name: testName}, opts: controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, }, + requeueAfter: defaultRequeueTime, wantCondition: meta.ReadyCondition, wantStatus: "True", }, @@ -226,9 +230,9 @@ func TestReconcile(t *testing.T) { obj: types.NamespacedName{Namespace: testNamespace, Name: testName}, opts: controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, }, - requeueAfter: 1 * time.Minute, + requeueAfter: defaultRequeueTime, wantCondition: gitopsv1alpha1.ClusterConnectivity, wantStatus: "True", wantStatusMessage: "cluster connectivity is ok", @@ -249,9 +253,9 @@ func TestReconcile(t *testing.T) { obj: types.NamespacedName{Namespace: testNamespace, Name: testName}, opts: controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, }, - requeueAfter: 1 * time.Minute, + requeueAfter: defaultRequeueTime, wantCondition: gitopsv1alpha1.ClusterConnectivity, wantStatus: "False", wantStatusMessage: "failed creating rest config from secret: couldn't get version/kind; json parse error: json: cannot unmarshal string into Go value of type struct { APIVersion string \"json:\\\"apiVersion,omitempty\\\"\"; Kind string \"json:\\\"kind,omitempty\\\"\" }", @@ -272,9 +276,9 @@ func TestReconcile(t *testing.T) { obj: types.NamespacedName{Namespace: testNamespace, Name: testName}, opts: controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, }, - requeueAfter: 1 * time.Minute, + requeueAfter: defaultRequeueTime, wantCondition: gitopsv1alpha1.ClusterConnectivity, wantStatus: "False", wantStatusMessage: "failed connecting to the cluster: the server rejected our request for an unknown reason", @@ -374,7 +378,7 @@ func TestFinalizedDeletion(t *testing.T) { controllerutil.AddFinalizer(tt.gitopsCluster, controllers.GitOpsClusterFinalizer) opts := controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, } r := makeTestReconciler(t, opts, append(tt.additionalObjs, tt.gitopsCluster)...) @@ -473,7 +477,7 @@ func TestFinalizers(t *testing.T) { t.Run(tt.name, func(t *testing.T) { opts := controllers.Options{ CAPIEnabled: true, - DefaultRequeueTime: 1 * time.Minute, + DefaultRequeueTime: defaultRequeueTime, } r := makeTestReconciler(t, opts, append(tt.additionalObjs, tt.gitopsCluster)...)