-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
KUBESAW-12: Convert the health-check goroutine into ToolchainCluster controller #386
Merged
fbm3307
merged 16 commits into
codeready-toolchain:master
from
fbm3307:ks12routinetocontroller
Apr 22, 2024
Merged
Changes from 13 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
4b00df6
KUBESAW-12: Convert the health-check goroutine into ToolchainCluster …
fbm3307 59122ba
Merge branch 'master' into ks12routinetocontroller
fbm3307 84525e0
Fixing go-lint
fbm3307 eb16703
Adding some test cases
fbm3307 aec2edf
adding the test cases as previous
fbm3307 2171041
Review comments 1
fbm3307 70fbeef
Merge branch 'master' into ks12routinetocontroller
fbm3307 391810f
Merge branch 'master' into ks12routinetocontroller
fbm3307 5556309
removing unused clustercacheservice
fbm3307 5e77e0c
Refactoring the tests
fbm3307 762118c
Fixing go lint
fbm3307 3ae2002
Just convert to controller
fbm3307 4ec7dc5
Updated package name
fbm3307 5d6888b
Review comments-2
fbm3307 b8fdb47
better readability
fbm3307 74da909
Merge branch 'master' into ks12routinetocontroller
fbm3307 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
62 changes: 1 addition & 61 deletions
62
...rs/toolchainclustercache/healthchecker.go → ...rollers/toolchaincluster/healthchecker.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,177 @@ | ||
package toolchaincluster | ||
|
||
import ( | ||
"context" | ||
"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 | ||
defer gock.Off() | ||
tcNs := "test-namespace" | ||
gock.New("http://cluster.com"). | ||
Get("healthz"). | ||
Persist(). | ||
Reply(200). | ||
BodyString("ok") | ||
gock.New("http://unstable.com"). | ||
Get("healthz"). | ||
Persist(). | ||
Reply(200). | ||
BodyString("unstable") | ||
gock.New("http://not-found.com"). | ||
Get("healthz"). | ||
Persist(). | ||
Reply(404) | ||
|
||
tests := map[string]struct { | ||
tctype string | ||
apiendpoint string | ||
clusterconditions []toolchainv1alpha1.ToolchainClusterCondition | ||
status toolchainv1alpha1.ToolchainClusterStatus | ||
}{ | ||
//ToolchainCluster.status doesn't contain any conditions | ||
"UnstableNoCondition": { | ||
tctype: "unstable", | ||
apiendpoint: "http://unstable.com", | ||
clusterconditions: []toolchainv1alpha1.ToolchainClusterCondition{unhealthy(), notOffline()}, | ||
status: toolchainv1alpha1.ToolchainClusterStatus{}, | ||
}, | ||
"StableNoCondition": { | ||
tctype: "stable", | ||
apiendpoint: "http://cluster.com", | ||
clusterconditions: []toolchainv1alpha1.ToolchainClusterCondition{healthy()}, | ||
status: toolchainv1alpha1.ToolchainClusterStatus{}, | ||
}, | ||
"NotFoundNoCondition": { | ||
tctype: "not-found", | ||
apiendpoint: "http://not-found.com", | ||
clusterconditions: []toolchainv1alpha1.ToolchainClusterCondition{offline()}, | ||
status: toolchainv1alpha1.ToolchainClusterStatus{}, | ||
}, | ||
//ToolchainCluster.status already contains conditions | ||
"UnstableContainsCondition": { | ||
tctype: "unstable", | ||
apiendpoint: "http://unstable.com", | ||
clusterconditions: []toolchainv1alpha1.ToolchainClusterCondition{unhealthy(), notOffline()}, | ||
status: withStatus(healthy()), | ||
}, | ||
"StableContainsCondition": { | ||
tctype: "stable", | ||
apiendpoint: "http://cluster.com", | ||
clusterconditions: []toolchainv1alpha1.ToolchainClusterCondition{healthy()}, | ||
status: withStatus(healthy()), | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}, | ||
"NotFoundContainsCondition": { | ||
tctype: "not-found", | ||
apiendpoint: "http://not-found.com", | ||
clusterconditions: []toolchainv1alpha1.ToolchainClusterCondition{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.ToolchainClusterCondition{offline()}, | ||
status: toolchainv1alpha1.ToolchainClusterStatus{}, | ||
}, | ||
//if no zones nor region is retrieved, then keep the current | ||
"NoZoneKeepCurrent": { | ||
tctype: "stable", | ||
apiendpoint: "http://cluster.com", | ||
clusterconditions: []toolchainv1alpha1.ToolchainClusterCondition{healthy()}, | ||
status: withStatus(offline()), | ||
}, | ||
} | ||
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) | ||
defer reset() | ||
cachedtc, found := cluster.GetCachedToolchainCluster(tctype.Name) | ||
require.True(t, found) | ||
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...) | ||
}) | ||
} | ||
} | ||
|
||
func withStatus(conditions ...toolchainv1alpha1.ToolchainClusterCondition) toolchainv1alpha1.ToolchainClusterStatus { | ||
return toolchainv1alpha1.ToolchainClusterStatus{ | ||
Conditions: conditions, | ||
} | ||
} | ||
func assertClusterStatus(t *testing.T, cl client.Client, clusterName string, clusterConds ...toolchainv1alpha1.ToolchainClusterCondition) { | ||
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 | ||
} | ||
} | ||
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.ToolchainClusterCondition { | ||
return toolchainv1alpha1.ToolchainClusterCondition{ | ||
Type: toolchainv1alpha1.ToolchainClusterReady, | ||
Status: corev1.ConditionTrue, | ||
Reason: "ClusterReady", | ||
Message: "/healthz responded with ok", | ||
} | ||
} | ||
func unhealthy() toolchainv1alpha1.ToolchainClusterCondition { | ||
return toolchainv1alpha1.ToolchainClusterCondition{Type: toolchainv1alpha1.ToolchainClusterReady, | ||
Status: corev1.ConditionFalse, | ||
Reason: "ClusterNotReady", | ||
Message: "/healthz responded without ok", | ||
} | ||
} | ||
func offline() toolchainv1alpha1.ToolchainClusterCondition { | ||
return toolchainv1alpha1.ToolchainClusterCondition{Type: toolchainv1alpha1.ToolchainClusterOffline, | ||
Status: corev1.ConditionTrue, | ||
Reason: "ClusterNotReachable", | ||
Message: "cluster is not reachable", | ||
} | ||
} | ||
func notOffline() toolchainv1alpha1.ToolchainClusterCondition { | ||
return toolchainv1alpha1.ToolchainClusterCondition{Type: toolchainv1alpha1.ToolchainClusterOffline, | ||
Status: corev1.ConditionFalse, | ||
Reason: "ClusterReachable", | ||
Message: "cluster is reachable", | ||
} | ||
} |
81 changes: 81 additions & 0 deletions
81
controllers/toolchaincluster/toolchaincluster_controller.go
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
package toolchaincluster | ||
|
||
import ( | ||
"context" | ||
"fmt" | ||
"time" | ||
|
||
toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1" | ||
"github.com/codeready-toolchain/toolchain-common/pkg/cluster" | ||
"k8s.io/apimachinery/pkg/api/errors" | ||
"k8s.io/apimachinery/pkg/runtime" | ||
kubeclientset "k8s.io/client-go/kubernetes" | ||
ctrl "sigs.k8s.io/controller-runtime" | ||
"sigs.k8s.io/controller-runtime/pkg/client" | ||
"sigs.k8s.io/controller-runtime/pkg/log" | ||
"sigs.k8s.io/controller-runtime/pkg/reconcile" | ||
) | ||
|
||
// Reconciler reconciles a ToolchainCluster object | ||
type Reconciler struct { | ||
client client.Client | ||
scheme *runtime.Scheme | ||
requeAfter time.Duration | ||
} | ||
|
||
// SetupWithManager sets up the controller with the Manager. | ||
func (r *Reconciler) SetupWithManager(mgr ctrl.Manager) error { | ||
return ctrl.NewControllerManagedBy(mgr). | ||
For(&toolchainv1alpha1.ToolchainCluster{}). | ||
Complete(r) | ||
} | ||
|
||
// Reconcile reads that state of the cluster for a ToolchainCluster object and makes changes based on the state read | ||
// and what is in the ToolchainCluster.Spec. It updates the status of the individual cluster | ||
// Note: | ||
// The Controller will requeue the Request to be processed again if the returned error is non-nil or | ||
// Result.Requeue is true, otherwise upon completion it will remove the work from the queue. | ||
func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.Result, error) { | ||
reqLogger := log.FromContext(ctx).WithName("health") | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
reqLogger.Info("Reconciling ToolchainCluster") | ||
|
||
// Fetch the ToolchainCluster instance | ||
toolchainCluster := &toolchainv1alpha1.ToolchainCluster{} | ||
err := r.client.Get(ctx, request.NamespacedName, toolchainCluster) | ||
if err != nil { | ||
if errors.IsNotFound(err) { | ||
// Stop monitoring the toolchain cluster as it is deleted | ||
return reconcile.Result{}, nil | ||
} | ||
// Error reading the object - requeue the request. | ||
return reconcile.Result{}, err | ||
} | ||
|
||
cachedCluster, ok := cluster.GetCachedToolchainCluster(toolchainCluster.Name) | ||
if !ok { | ||
err := fmt.Errorf("cluster %s not found in cache", toolchainCluster.Name) | ||
toolchainCluster.Status.Conditions = []toolchainv1alpha1.ToolchainClusterCondition{clusterOfflineCondition()} | ||
if err := r.client.Status().Update(ctx, toolchainCluster); err != nil { | ||
reqLogger.Error(err, "failed to update the status of ToolchainCluster") | ||
} | ||
return reconcile.Result{}, err | ||
} | ||
|
||
clientSet, err := kubeclientset.NewForConfig(cachedCluster.RestConfig) | ||
if err != nil { | ||
return reconcile.Result{}, err | ||
fbm3307 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
healthChecker := &HealthChecker{ | ||
localClusterClient: r.client, | ||
remoteClusterClient: cachedCluster.Client, | ||
remoteClusterClientset: clientSet, | ||
logger: reqLogger, | ||
} | ||
//update the status of the individual cluster. | ||
if err := healthChecker.updateIndividualClusterStatus(ctx, toolchainCluster); err != nil { | ||
reqLogger.Error(err, "unable to update cluster status of ToolchainCluster") | ||
return reconcile.Result{}, err | ||
} | ||
|
||
return reconcile.Result{RequeueAfter: r.requeAfter}, nil | ||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this way you improved the unit test 👍 🥇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1