Refactoring test cases as per review comments

Signed-off-by: Feny Mehta <[email protected]>

controllers/toolchaincluster/healthchecker.go

@@ -19,8 +19,8 @@ const (
 	clusterReachableMsg    = "cluster is reachable"
 )
 
-// getClusterHealthStatus gets the kubernetes cluster health status by requesting "/healthz"
-func getClusterHealthStatus(ctx context.Context, remoteClusterClientset *kubeclientset.Clientset) []v1alpha1.Condition {
+// GetClusterHealthStatus gets the kubernetes cluster health status by requesting "/healthz"
+func GetClusterHealthStatus(ctx context.Context, remoteClusterClientset *kubeclientset.Clientset) []v1alpha1.Condition {
 	lgr := log.FromContext(ctx)
 	conditions := []v1alpha1.Condition{}
 	body, err := remoteClusterClientset.DiscoveryClient.RESTClient().Get().AbsPath("/healthz").Do(ctx).Raw()

controllers/toolchaincluster/healthchecker_test.go

@@ -97,7 +97,7 @@ func TestClusterHealthChecks(t *testing.T) {
 			require.NoError(t, err)
 
 			//when
-			hcond := getClusterHealthStatus(context.TODO(), cacheclient)
+			hcond := GetClusterHealthStatus(context.TODO(), cacheclient)
 
 			//then
 			assert.Len(t, tc.clusterconditions, len(hcond))

controllers/toolchaincluster/toolchaincluster_controller.go

@@ -57,41 +57,30 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.
 	cachedCluster, ok := cluster.GetCachedToolchainCluster(toolchainCluster.Name)
 	if !ok {
 		err := fmt.Errorf("cluster %s not found in cache", toolchainCluster.Name)
-		toolchainCluster.Status.Conditions = condition.AddOrUpdateStatusConditionsWithLastUpdatedTimestamp(toolchainCluster.Status.Conditions, clusterOfflineCondition())
-		if err := r.Client.Status().Update(ctx, toolchainCluster); err != nil {
-			reqLogger.Error(err, "failed to update the status of ToolchainCluster")
-		}
+		r.updateStatus(ctx, toolchainCluster, clusterOfflineCondition())
 		return reconcile.Result{}, err
 	}
 
 	//Not testing as this as this is being tested in cluster package
 	clientSet, err := kubeclientset.NewForConfig(cachedCluster.RestConfig)
 	if err != nil {
 		reqLogger.Error(err, "cannot create ClientSet for the ToolchainCluster")
-		toolchainCluster.Status.Conditions = condition.AddOrUpdateStatusConditionsWithLastUpdatedTimestamp(toolchainCluster.Status.Conditions, clusterNotReadyCondition())
+		r.updateStatus(ctx, toolchainCluster, clusterNotReadyCondition())
 		return reconcile.Result{}, err
 	}
 
 	// execute healthcheck
-	healthcheckResult := r.runCheckHealthOrDefault(ctx, clientSet)
+	healthcheckResult := r.CheckHealth(ctx, clientSet)
 
 	// update the status of the individual cluster.
-	if err := r.updateStatus(ctx, toolchainCluster, healthcheckResult); err != nil {
+	if err := r.updateStatus(ctx, toolchainCluster, healthcheckResult...); err != nil {
 		reqLogger.Error(err, "unable to update cluster status of ToolchainCluster")
 		return reconcile.Result{}, err
 	}
 	return reconcile.Result{RequeueAfter: r.RequeAfter}, nil
 }
 
-func (r *Reconciler) runCheckHealthOrDefault(ctx context.Context, rcc *kubeclientset.Clientset) []toolchainv1alpha1.Condition {
-	if r.CheckHealth != nil {
-		return r.CheckHealth(ctx, rcc)
-	}
-	hcond := getClusterHealthStatus(ctx, rcc)
-	return hcond
-}
-
-func (r *Reconciler) updateStatus(ctx context.Context, toolchainCluster *toolchainv1alpha1.ToolchainCluster, currentconditions []toolchainv1alpha1.Condition) error {
+func (r *Reconciler) updateStatus(ctx context.Context, toolchainCluster *toolchainv1alpha1.ToolchainCluster, currentconditions ...toolchainv1alpha1.Condition) error {
 
 	toolchainCluster.Status.Conditions = condition.AddOrUpdateStatusConditionsWithLastUpdatedTimestamp(toolchainCluster.Status.Conditions, currentconditions...)
 	if err := r.Client.Status().Update(ctx, toolchainCluster); err != nil {

controllers/toolchaincluster/toolchaincluster_controller_test.go

@@ -103,63 +103,6 @@ func TestClusterControllerChecks(t *testing.T) {
 		assertClusterStatus(t, cl, "stable", healthy())
 	})
 
-	t.Run("Checking the run check health default ", func(t *testing.T) {
-		// given
-		stable, sec := newToolchainCluster("stable", tcNs, "http://cluster.com", withStatus(healthy()))
-
-		cl := test.NewFakeClient(t, stable, sec)
-		reset := setupCachedClusters(t, cl, stable)
-		defer reset()
-		controller, req := prepareCheckHealthDefaultReconcile(stable, cl, requeAfter)
-
-		// when
-		recresult, err := controller.Reconcile(context.TODO(), req)
-
-		// then
-		require.Equal(t, err, nil)
-		require.Equal(t, reconcile.Result{RequeueAfter: requeAfter}, recresult)
-		assertClusterStatus(t, cl, "stable", healthy())
-	})
-
-	t.Run("Updates the empty condition with a new one ", func(t *testing.T) {
-		// given
-		stable, sec := newToolchainCluster("stable", tcNs, "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{})
-
-		cl := test.NewFakeClient(t, stable, sec)
-		reset := setupCachedClusters(t, cl, stable)
-		defer reset()
-		controller, req := prepareReconcile(stable, cl, requeAfter)
-		controller.CheckHealth = func(ctx context.Context, c *kubeclientset.Clientset) []toolchainv1alpha1.Condition {
-			return []toolchainv1alpha1.Condition{healthy()}
-		}
-		// when
-		recresult, err := controller.Reconcile(context.TODO(), req)
-
-		// then
-		require.Equal(t, err, nil)
-		require.Equal(t, reconcile.Result{RequeueAfter: requeAfter}, recresult)
-		assertClusterStatus(t, cl, "stable", healthy())
-	})
-
-	t.Run("adds a new condition when tc already has a condition ", func(t *testing.T) {
-		// given
-		unstable, sec := newToolchainCluster("unstable", tcNs, "http://cluster.com", withStatus(notOffline()))
-
-		cl := test.NewFakeClient(t, unstable, sec)
-		reset := setupCachedClusters(t, cl, unstable)
-		defer reset()
-		controller, req := prepareReconcile(unstable, cl, requeAfter)
-		controller.CheckHealth = func(ctx context.Context, c *kubeclientset.Clientset) []toolchainv1alpha1.Condition {
-			return []toolchainv1alpha1.Condition{healthy()}
-		}
-		// when
-		recresult, err := controller.Reconcile(context.TODO(), req)
-
-		// then
-		require.Equal(t, err, nil)
-		require.Equal(t, reconcile.Result{RequeueAfter: requeAfter}, recresult)
-		assertClusterStatus(t, cl, "unstable", notOffline(), healthy())
-	})
 	t.Run("toolchain cluster cache not found", func(t *testing.T) {
 		// given
 		unstable, _ := newToolchainCluster("unstable", tcNs, "http://cluster.com", toolchainv1alpha1.ToolchainClusterStatus{})
@@ -220,18 +163,6 @@ func prepareReconcile(toolchainCluster *toolchainv1alpha1.ToolchainCluster, cl *
 	return controller, req
 }
 
-func prepareCheckHealthDefaultReconcile(toolchainCluster *toolchainv1alpha1.ToolchainCluster, cl *test.FakeClient, requeAfter time.Duration) (Reconciler, reconcile.Request) {
-	controller := Reconciler{
-		Client:     cl,
-		Scheme:     scheme.Scheme,
-		RequeAfter: requeAfter,
-	}
-	req := reconcile.Request{
-		NamespacedName: test.NamespacedName(toolchainCluster.Namespace, toolchainCluster.Name),
-	}
-	return controller, req
-}
-
 func withStatus(conditions ...toolchainv1alpha1.Condition) toolchainv1alpha1.ToolchainClusterStatus {
 	return toolchainv1alpha1.ToolchainClusterStatus{
 		Conditions: conditions,