Merge remote-tracking branch 'upstream/master' into create-tc-based-o…

…n-creds-secret

# Conflicts:
#	controllers/toolchaincluster/healthchecker.go
#	controllers/toolchaincluster/toolchaincluster_controller.go

.github/workflows/linters.yml

@@ -14,13 +14,13 @@ jobs:
     name: GolangCI Lint
     runs-on: ubuntu-20.04
     steps:
+    - name: Checkout code
+      uses: actions/checkout@v4
+
     - name: Install Go
       uses: actions/setup-go@v5
       with:
-        go-version: 1.20.x
-
-    - name: Checkout code
-      uses: actions/checkout@v4
+        go-version-file: go.mod
 
     - name: Lint
       uses: golangci/golangci-lint-action@v6

.github/workflows/test-with-coverage.yml

@@ -15,17 +15,17 @@ jobs:
     name: Test with Coverage
     runs-on: ubuntu-20.04
     steps:
-    - name: Install Go
-      uses: actions/setup-go@v5
-      with:
-        go-version: 1.20.x
-
     - name: Checkout code
       uses: actions/checkout@v4
       with:
         ref: ${{github.event.pull_request.head.ref}}
         repository: ${{github.event.pull_request.head.repo.full_name}}
         fetch-depth: 0
+
+    - name: Install Go
+      uses: actions/setup-go@v5
+      with:
+        go-version-file: go.mod
 
     - name: Test
       run: |

controllers/toolchaincluster/healthchecker.go

@@ -4,109 +4,22 @@ import (
 	"context"
 	"strings"
 
-	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
-	"github.com/go-logr/logr"
-	"github.com/pkg/errors"
-	corev1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	kubeclientset "k8s.io/client-go/kubernetes"
-	"sigs.k8s.io/controller-runtime/pkg/client"
+	"sigs.k8s.io/controller-runtime/pkg/log"
 )
 
 const (
-	healthzOk              = "/healthz responded with ok"
-	healthzNotOk           = "/healthz responded without ok"
-	clusterNotReachableMsg = "cluster is not reachable"
-	clusterReachableMsg    = "cluster is reachable"
+	healthzOk    = "/healthz responded with ok"
+	healthzNotOk = "/healthz responded without ok"
 )
 
-type HealthChecker struct {
-	localClusterClient     client.Client
-	remoteClusterClient    client.Client
-	remoteClusterClientset *kubeclientset.Clientset
-	logger                 logr.Logger
-}
-
-func (hc *HealthChecker) updateIndividualClusterStatus(ctx context.Context, toolchainCluster *toolchainv1alpha1.ToolchainCluster) error {
-	currentClusterStatus := hc.getClusterHealthStatus(ctx)
-
-	for index, currentCond := range currentClusterStatus.Conditions {
-		for _, previousCond := range toolchainCluster.Status.Conditions {
-			if currentCond.Type == previousCond.Type && currentCond.Status == previousCond.Status {
-				currentClusterStatus.Conditions[index].LastTransitionTime = previousCond.LastTransitionTime
-			}
-		}
-	}
-
-	toolchainCluster.Status.Conditions = currentClusterStatus.Conditions
-	if err := hc.localClusterClient.Status().Update(ctx, toolchainCluster); err != nil {
-		return errors.Wrapf(err, "Failed to update the status of cluster %s", toolchainCluster.Name)
-	}
-	return nil
-}
-
-// getClusterHealthStatus gets the kubernetes cluster health status by requesting "/healthz"
-func (hc *HealthChecker) getClusterHealthStatus(ctx context.Context) *toolchainv1alpha1.ToolchainClusterStatus {
-	clusterStatus := toolchainv1alpha1.ToolchainClusterStatus{}
-	body, err := hc.remoteClusterClientset.DiscoveryClient.RESTClient().Get().AbsPath("/healthz").Do(ctx).Raw()
+// getClusterHealth gets the kubernetes cluster health status by requesting "/healthz"
+func getClusterHealthStatus(ctx context.Context, remoteClusterClientset *kubeclientset.Clientset) (bool, error) {
+	lgr := log.FromContext(ctx)
+	body, err := remoteClusterClientset.DiscoveryClient.RESTClient().Get().AbsPath("/healthz").Do(ctx).Raw()
 	if err != nil {
-		hc.logger.Error(err, "Failed to do cluster health check for a ToolchainCluster")
-		clusterStatus.Conditions = append(clusterStatus.Conditions, clusterOfflineCondition())
-	} else {
-		if !strings.EqualFold(string(body), "ok") {
-			clusterStatus.Conditions = append(clusterStatus.Conditions, clusterNotReadyCondition(), clusterNotOfflineCondition())
-		} else {
-			clusterStatus.Conditions = append(clusterStatus.Conditions, clusterReadyCondition())
-		}
-	}
-
-	return &clusterStatus
-}
-
-func clusterReadyCondition() toolchainv1alpha1.Condition {
-	currentTime := metav1.Now()
-	return toolchainv1alpha1.Condition{
-		Type:               toolchainv1alpha1.ConditionReady,
-		Status:             corev1.ConditionTrue,
-		Reason:             toolchainv1alpha1.ToolchainClusterClusterReadyReason,
-		Message:            healthzOk,
-		LastUpdatedTime:    &currentTime,
-		LastTransitionTime: currentTime,
-	}
-}
-
-func clusterNotReadyCondition() toolchainv1alpha1.Condition {
-	currentTime := metav1.Now()
-	return toolchainv1alpha1.Condition{
-		Type:               toolchainv1alpha1.ConditionReady,
-		Status:             corev1.ConditionFalse,
-		Reason:             toolchainv1alpha1.ToolchainClusterClusterNotReadyReason,
-		Message:            healthzNotOk,
-		LastUpdatedTime:    &currentTime,
-		LastTransitionTime: currentTime,
-	}
-}
-
-func clusterOfflineCondition() toolchainv1alpha1.Condition {
-	currentTime := metav1.Now()
-	return toolchainv1alpha1.Condition{
-		Type:               toolchainv1alpha1.ToolchainClusterOffline,
-		Status:             corev1.ConditionTrue,
-		Reason:             toolchainv1alpha1.ToolchainClusterClusterNotReachableReason,
-		Message:            clusterNotReachableMsg,
-		LastUpdatedTime:    &currentTime,
-		LastTransitionTime: currentTime,
-	}
-}
-
-func clusterNotOfflineCondition() toolchainv1alpha1.Condition {
-	currentTime := metav1.Now()
-	return toolchainv1alpha1.Condition{
-		Type:               toolchainv1alpha1.ToolchainClusterOffline,
-		Status:             corev1.ConditionFalse,
-		Reason:             toolchainv1alpha1.ToolchainClusterClusterReachableReason,
-		Message:            clusterReachableMsg,
-		LastUpdatedTime:    &currentTime,
-		LastTransitionTime: currentTime,
+		lgr.Error(err, "Failed to do cluster health check for a ToolchainCluster")
+		return false, err
 	}
+	return strings.EqualFold(string(body), "ok"), nil
 }

controllers/toolchaincluster/healthchecker_test.go

@@ -2,22 +2,17 @@ package toolchaincluster
 
 import (
 	"context"
+	"fmt"
 	"testing"
 
 	toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
 	"github.com/codeready-toolchain/toolchain-common/pkg/cluster"
 	"github.com/codeready-toolchain/toolchain-common/pkg/test"
-	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"gopkg.in/h2non/gock.v1"
-	corev1 "k8s.io/api/core/v1"
 	kubeclientset "k8s.io/client-go/kubernetes"
-	"sigs.k8s.io/controller-runtime/pkg/client"
-	logf "sigs.k8s.io/controller-runtime/pkg/log"
 )
 
-var logger = logf.Log.WithName("toolchaincluster_healthcheck")
-
 func TestClusterHealthChecks(t *testing.T) {
 
 	// given
@@ -39,139 +34,51 @@ func TestClusterHealthChecks(t *testing.T) {
 		Reply(404)
 
 	tests := map[string]struct {
-		tctype            string
-		apiendpoint       string
-		clusterconditions []toolchainv1alpha1.Condition
-		status            toolchainv1alpha1.ToolchainClusterStatus
+		tcType      string
+		apiEndPoint string
+		healthCheck bool
+		err         error
 	}{
-		//ToolchainCluster.status doesn't contain any conditions
-		"UnstableNoCondition": {
-			tctype:            "unstable",
-			apiendpoint:       "http://unstable.com",
-			clusterconditions: []toolchainv1alpha1.Condition{unhealthy(), notOffline()},
-			status:            toolchainv1alpha1.ToolchainClusterStatus{},
-		},
-		"StableNoCondition": {
-			tctype:            "stable",
-			apiendpoint:       "http://cluster.com",
-			clusterconditions: []toolchainv1alpha1.Condition{healthy()},
-			status:            toolchainv1alpha1.ToolchainClusterStatus{},
-		},
-		"NotFoundNoCondition": {
-			tctype:            "not-found",
-			apiendpoint:       "http://not-found.com",
-			clusterconditions: []toolchainv1alpha1.Condition{offline()},
-			status:            toolchainv1alpha1.ToolchainClusterStatus{},
-		},
-		//ToolchainCluster.status already contains conditions
-		"UnstableContainsCondition": {
-			tctype:            "unstable",
-			apiendpoint:       "http://unstable.com",
-			clusterconditions: []toolchainv1alpha1.Condition{unhealthy(), notOffline()},
-			status:            withStatus(healthy()),
+		"HealthOkay": {
+			tcType:      "stable",
+			apiEndPoint: "http://cluster.com",
+			healthCheck: true,
 		},
-		"StableContainsCondition": {
-			tctype:            "stable",
-			apiendpoint:       "http://cluster.com",
-			clusterconditions: []toolchainv1alpha1.Condition{healthy()},
-			status:            withStatus(offline()),
+		"HealthNotOkayButNoError": {
+			tcType:      "unstable",
+			apiEndPoint: "http://unstable.com",
+			healthCheck: false,
 		},
-		"NotFoundContainsCondition": {
-			tctype:            "not-found",
-			apiendpoint:       "http://not-found.com",
-			clusterconditions: []toolchainv1alpha1.Condition{offline()},
-			status:            withStatus(healthy()),
-		},
-		//if the connection cannot be established at beginning, then it should be offline
-		"OfflineConnectionNotEstablished": {
-			tctype:            "failing",
-			apiendpoint:       "http://failing.com",
-			clusterconditions: []toolchainv1alpha1.Condition{offline()},
-			status:            toolchainv1alpha1.ToolchainClusterStatus{},
-		},
-		//if no zones nor region is retrieved, then keep the current
-		"NoZoneKeepCurrent": {
-			tctype:            "stable",
-			apiendpoint:       "http://cluster.com",
-			clusterconditions: []toolchainv1alpha1.Condition{healthy()},
-			status:            withStatus(offline()),
+		"ErrorWhileDoingHealth": {
+			tcType:      "Notfound",
+			apiEndPoint: "http://not-found.com",
+			healthCheck: false,
+			err:         fmt.Errorf("the server could not find the requested resource"),
 		},
 	}
 	for k, tc := range tests {
 		t.Run(k, func(t *testing.T) {
-			tctype, sec := newToolchainCluster(tc.tctype, tcNs, tc.apiendpoint, tc.status)
-			cl := test.NewFakeClient(t, tctype, sec)
-			reset := setupCachedClusters(t, cl, tctype)
+			//given
+			tcType, sec := newToolchainCluster(tc.tcType, tcNs, tc.apiEndPoint, toolchainv1alpha1.ToolchainClusterStatus{})
+			cl := test.NewFakeClient(t, tcType, sec)
+			reset := setupCachedClusters(t, cl, tcType)
 			defer reset()
-			cachedtc, found := cluster.GetCachedToolchainCluster(tctype.Name)
+			cachedTC, found := cluster.GetCachedToolchainCluster(tcType.Name)
 			require.True(t, found)
-			cacheclient, err := kubeclientset.NewForConfig(cachedtc.RestConfig)
+			cacheClient, err := kubeclientset.NewForConfig(cachedTC.RestConfig)
 			require.NoError(t, err)
-			healthChecker := &HealthChecker{
-				localClusterClient:     cl,
-				remoteClusterClient:    cachedtc.Client,
-				remoteClusterClientset: cacheclient,
-				logger:                 logger,
-			}
-			// when
-			err = healthChecker.updateIndividualClusterStatus(context.TODO(), tctype)
 
-			//then
-			require.NoError(t, err)
-			assertClusterStatus(t, cl, tc.tctype, tc.clusterconditions...)
-		})
-	}
-}
+			//when
+			healthCheck, err := getClusterHealthStatus(context.TODO(), cacheClient)
 
-func withStatus(conditions ...toolchainv1alpha1.Condition) toolchainv1alpha1.ToolchainClusterStatus {
-	return toolchainv1alpha1.ToolchainClusterStatus{
-		Conditions: conditions,
-	}
-}
-func assertClusterStatus(t *testing.T, cl client.Client, clusterName string, clusterConds ...toolchainv1alpha1.Condition) {
-	tc := &toolchainv1alpha1.ToolchainCluster{}
-	err := cl.Get(context.TODO(), test.NamespacedName("test-namespace", clusterName), tc)
-	require.NoError(t, err)
-	assert.Len(t, tc.Status.Conditions, len(clusterConds))
-ExpConditions:
-	for _, expCond := range clusterConds {
-		for _, cond := range tc.Status.Conditions {
-			if expCond.Type == cond.Type {
-				assert.Equal(t, expCond.Status, cond.Status)
-				assert.Equal(t, expCond.Reason, cond.Reason)
-				assert.Equal(t, expCond.Message, cond.Message)
-				continue ExpConditions
+			//then
+			require.Equal(t, tc.healthCheck, healthCheck)
+			if tc.err != nil {
+				require.EqualError(t, err, tc.err.Error())
+			} else {
+				require.NoError(t, err)
 			}
-		}
-		assert.Failf(t, "condition not found", "the list of conditions %v doesn't contain the expected condition %v", tc.Status.Conditions, expCond)
-	}
-}
-func healthy() toolchainv1alpha1.Condition {
-	return toolchainv1alpha1.Condition{
-		Type:    toolchainv1alpha1.ConditionReady,
-		Status:  corev1.ConditionTrue,
-		Reason:  "ClusterReady",
-		Message: "/healthz responded with ok",
-	}
-}
-func unhealthy() toolchainv1alpha1.Condition {
-	return toolchainv1alpha1.Condition{Type: toolchainv1alpha1.ConditionReady,
-		Status:  corev1.ConditionFalse,
-		Reason:  "ClusterNotReady",
-		Message: "/healthz responded without ok",
-	}
-}
-func offline() toolchainv1alpha1.Condition {
-	return toolchainv1alpha1.Condition{Type: toolchainv1alpha1.ToolchainClusterOffline,
-		Status:  corev1.ConditionTrue,
-		Reason:  "ClusterNotReachable",
-		Message: "cluster is not reachable",
-	}
-}
-func notOffline() toolchainv1alpha1.Condition {
-	return toolchainv1alpha1.Condition{Type: toolchainv1alpha1.ToolchainClusterOffline,
-		Status:  corev1.ConditionFalse,
-		Reason:  "ClusterReachable",
-		Message: "cluster is reachable",
+
+		})
 	}
 }