Skip to content

Commit

Permalink
Avoid early return (#28)
Browse files Browse the repository at this point in the history
  • Loading branch information
luizbafilho authored Oct 25, 2022
1 parent f8e7260 commit b3ca3ac
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 18 deletions.
6 changes: 5 additions & 1 deletion controllers/gitopscluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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}
Expand Down
38 changes: 21 additions & 17 deletions controllers/gitopscluster_controller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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,
Expand All @@ -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",
},
Expand Down Expand Up @@ -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,
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
},
Expand Down Expand Up @@ -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",
Expand All @@ -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\\\"\" }",
Expand All @@ -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",
Expand Down Expand Up @@ -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)...)

Expand Down Expand Up @@ -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)...)

Expand Down

0 comments on commit b3ca3ac

Please sign in to comment.