Skip to content

Commit

Permalink
KUBESAW-100: Refactoring toolchaincluster controller (#401)
Browse files Browse the repository at this point in the history 
* Refactoring toolchaincluster controller

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

update

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

refac

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

* Review Comments

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

* Review Comments-2

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

* Toolchaincluster controller test cases

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

* Some more test for healthchecker

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

* Some more

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

* test cases

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

* cleaning the test cases

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

* go lint

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

* review comments

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

* Refactoring test cases as per review comments

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

* Linter

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

* Rc-1

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

* Reviewcomments

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

* Go lint

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

* code cov patch

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

* Review coments-2

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

* lint speliing

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

* review cosmetic

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

* use latupdatetime

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

* review

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

* review

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

* camel case

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

* clean up

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

* silly misses and face palm moments

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

* the remaining misses

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

* 2

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

* i

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

---------

Signed-off-by: Feny Mehta <[email protected]>
Co-authored-by: Francisc Munteanu <[email protected]>
  • Loading branch information
@fbm3307 @mfrancisc
fbm3307 and mfrancisc authored Jul 16, 2024
1 parent 3789520 commit 8604fe4
Show file tree
Hide file tree
Showing 4 changed files with 223 additions and 255 deletions.
106 changes: 10 additions & 96 deletions controllers/toolchaincluster/healthchecker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,110 +4,24 @@ 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
}
}
}
// getClusterHealth gets the kubernetes cluster health status by requesting "/healthz"
func getClusterHealthStatus(ctx context.Context, remoteClusterClientset *kubeclientset.Clientset) (bool, error) {

toolchainCluster.Status = *currentClusterStatus
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()
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,
lgr.Error(err, "Failed to do cluster health check for a ToolchainCluster")
return false, err
}
}
return strings.EqualFold(string(body), "ok"), nil

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,
}
}
161 changes: 34 additions & 127 deletions controllers/toolchaincluster/healthchecker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",

})
}
}
Loading

0 comments on commit 8604fe4

Please sign in to comment.