Skip to content

Commit

Permalink
fix: forward context in toolchaincluster controller (#339)
Browse files Browse the repository at this point in the history 
Signed-off-by: Francesco Ilario <[email protected]>
  • Loading branch information
@filariow
filariow authored Nov 9, 2023
1 parent 4700ff0 commit 0d0e1b2
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 21 deletions.
20 changes: 10 additions & 10 deletions controllers/toolchaincluster/healthchecker.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const (
func StartHealthChecks(ctx context.Context, mgr manager.Manager, namespace string, period time.Duration) {
logger.Info("starting health checks", "period", period)
go wait.Until(func() {
updateClusterStatuses(namespace, mgr.GetClient())
updateClusterStatuses(ctx, namespace, mgr.GetClient())
}, period, ctx.Done())
}

Expand All @@ -43,9 +43,9 @@ type HealthChecker struct {
}

// updateClusterStatuses checks cluster health and updates status of all ToolchainClusters
func updateClusterStatuses(namespace string, cl client.Client) {
func updateClusterStatuses(ctx context.Context, namespace string, cl client.Client) {
clusters := &toolchainv1alpha1.ToolchainClusterList{}
err := cl.List(context.TODO(), clusters, client.InNamespace(namespace))
err := cl.List(ctx, clusters, client.InNamespace(namespace))
if err != nil {
logger.Error(err, "unable to list existing ToolchainClusters")
return
Expand All @@ -62,7 +62,7 @@ func updateClusterStatuses(namespace string, cl client.Client) {
if !ok {
clusterLogger.Error(fmt.Errorf("cluster %s not found in cache", clusterObj.Name), "failed to retrieve stored data for cluster")
clusterObj.Status.Conditions = []toolchainv1alpha1.ToolchainClusterCondition{clusterOfflineCondition()}
if err := cl.Status().Update(context.TODO(), clusterObj); err != nil {
if err := cl.Status().Update(ctx, clusterObj); err != nil {
clusterLogger.Error(err, "failed to update the status of ToolchainCluster")
}
continue
Expand All @@ -81,15 +81,15 @@ func updateClusterStatuses(namespace string, cl client.Client) {
logger: clusterLogger,
}
// clusterLogger.Info("getting the current state of ToolchainCluster")
if err := healthChecker.updateIndividualClusterStatus(clusterObj); err != nil {
if err := healthChecker.updateIndividualClusterStatus(ctx, clusterObj); err != nil {
clusterLogger.Error(err, "unable to update cluster status of ToolchainCluster")
}
}
}

func (hc *HealthChecker) updateIndividualClusterStatus(toolchainCluster *toolchainv1alpha1.ToolchainCluster) error {
func (hc *HealthChecker) updateIndividualClusterStatus(ctx context.Context, toolchainCluster *toolchainv1alpha1.ToolchainCluster) error {

currentClusterStatus := hc.getClusterHealthStatus()
currentClusterStatus := hc.getClusterHealthStatus(ctx)

for index, currentCond := range currentClusterStatus.Conditions {
for _, previousCond := range toolchainCluster.Status.Conditions {
Expand All @@ -100,16 +100,16 @@ func (hc *HealthChecker) updateIndividualClusterStatus(toolchainCluster *toolcha
}

toolchainCluster.Status = *currentClusterStatus
if err := hc.localClusterClient.Status().Update(context.TODO(), toolchainCluster); err != nil {
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() *toolchainv1alpha1.ToolchainClusterStatus {
func (hc *HealthChecker) getClusterHealthStatus(ctx context.Context) *toolchainv1alpha1.ToolchainClusterStatus {
clusterStatus := toolchainv1alpha1.ToolchainClusterStatus{}
body, err := hc.remoteClusterClientset.DiscoveryClient.RESTClient().Get().AbsPath("/healthz").Do(context.TODO()).Raw()
body, err := hc.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())
Expand Down
8 changes: 4 additions & 4 deletions controllers/toolchaincluster/healthchecker_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ func TestClusterHealthChecks(t *testing.T) {
defer reset()

// when
updateClusterStatuses("test-namespace", cl)
updateClusterStatuses(context.TODO(), "test-namespace", cl)

// then
assertClusterStatus(t, cl, "unstable", notOffline(), unhealthy())
Expand All @@ -62,7 +62,7 @@ func TestClusterHealthChecks(t *testing.T) {
defer resetCache()

// when
updateClusterStatuses("test-namespace", cl)
updateClusterStatuses(context.TODO(), "test-namespace", cl)

// then
assertClusterStatus(t, cl, "unstable", notOffline(), unhealthy())
Expand All @@ -78,7 +78,7 @@ func TestClusterHealthChecks(t *testing.T) {
defer resetCache()

// when
updateClusterStatuses("test-namespace", cl)
updateClusterStatuses(context.TODO(), "test-namespace", cl)

// then
assertClusterStatus(t, cl, "stable", healthy())
Expand All @@ -90,7 +90,7 @@ func TestClusterHealthChecks(t *testing.T) {
cl := test.NewFakeClient(t, stable, sec)

// when
updateClusterStatuses("test-namespace", cl)
updateClusterStatuses(context.TODO(), "test-namespace", cl)

// then
assertClusterStatus(t, cl, "failing", offline())
Expand Down
15 changes: 8 additions & 7 deletions controllers/toolchaincluster/toolchaincluster_controller.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

toolchainv1alpha1 "github.com/codeready-toolchain/api/api/v1alpha1"
"github.com/codeready-toolchain/toolchain-common/pkg/cluster"
"github.com/go-logr/logr"
"k8s.io/apimachinery/pkg/api/errors"
"k8s.io/apimachinery/pkg/runtime"
ctrl "sigs.k8s.io/controller-runtime"
Expand Down Expand Up @@ -64,27 +63,29 @@ func (r *Reconciler) Reconcile(ctx context.Context, request ctrl.Request) (ctrl.

// add toolchaincluster role label if not present
reqLogger.Info("adding cluster role label based on type")
if err := r.addToolchainClusterRoleLabelFromType(reqLogger, toolchainCluster); err != nil {
if err := r.addToolchainClusterRoleLabelFromType(ctx, toolchainCluster); err != nil {
return reconcile.Result{}, err
}

return reconcile.Result{}, r.clusterCacheService.AddOrUpdateToolchainCluster(toolchainCluster)
}

func (r *Reconciler) addToolchainClusterRoleLabelFromType(log logr.Logger, toolchainCluster *toolchainv1alpha1.ToolchainCluster) error {
func (r *Reconciler) addToolchainClusterRoleLabelFromType(ctx context.Context, toolchainCluster *toolchainv1alpha1.ToolchainCluster) error {
logger := log.FromContext(ctx)

if clusterType, found := toolchainCluster.Labels[cluster.LabelType]; !found {
log.Info("cluster `type` label not found, unable to add toolchain cluster role label from type")
logger.Info("cluster `type` label not found, unable to add toolchain cluster role label from type")
return nil
} else if clusterType != string(cluster.Member) {
log.Info("cluster `type` is not member, skipping cluster role label setting")
logger.Info("cluster `type` is not member, skipping cluster role label setting")
return nil
}
clusterRoleLabel := cluster.RoleLabel(cluster.Tenant)
if _, exists := toolchainCluster.Labels[clusterRoleLabel]; !exists {
log.Info("setting cluster role label for toolchaincluster", clusterRoleLabel, toolchainCluster.Name)
logger.Info("setting cluster role label for toolchaincluster", clusterRoleLabel, toolchainCluster.Name)
// We use only the label key, the value can remain empty.
toolchainCluster.Labels[clusterRoleLabel] = ""
if err := r.client.Update(context.TODO(), toolchainCluster); err != nil {
if err := r.client.Update(ctx, toolchainCluster); err != nil {
return err
}
}
Expand Down

0 comments on commit 0d0e1b2

Please sign in to comment.