Update reconciler so that GitopsClusters are only Ready if ClusterConnected is Ready too

Author: sarataha

api/v1alpha1/condition_types.go

@@ -11,6 +11,10 @@ const (
 	ControlPlaneReadyStatusReason string = "ControlPlaneReadyStatus"
 	// WaitingForCAPIClusterReason signals that a given CAPI cluster has not been found.
 	WaitingForCAPIClusterReason string = "WaitingForCAPICluster"
+	// ClusterConnectedReason signals that a given cluster is connected.
+	ClusterConnectedReason string = "ClusterConnected"
+	// ClusterNotConnectedReason signals that a given cluster is not connected.
+	ClusterNotConnectedReason string = "ClusterNotConnected"
 
 	// WaitingForCAPIClusterDeletionReason signals that this cluster has been
 	// deleted, but the referenced CAPI Cluster still exists.

controllers/gitopscluster_controller.go

@@ -25,7 +25,6 @@ import (
 	"github.com/fluxcd/pkg/apis/meta"
 	"github.com/fluxcd/pkg/runtime/conditions"
 	corev1 "k8s.io/api/core/v1"
-	v1 "k8s.io/api/core/v1"
 	apierrors "k8s.io/apimachinery/pkg/api/errors"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/runtime"
@@ -230,6 +229,21 @@ func (r *GitopsClusterReconciler) Reconcile(ctx context.Context, req ctrl.Reques
 		return ctrl.Result{}, err
 	}
 
+	// Cluster is ready only if it has a secret and is connected
+	connectedCondition := conditions.Get(cluster, gitopsv1alpha1.ClusterConnectivity)
+	if connectedCondition == nil || !conditions.IsTrue(cluster, gitopsv1alpha1.ClusterConnectivity) {
+		readyConditionReason := conditions.GetReason(cluster, meta.ReadyCondition)
+		if readyConditionReason != gitopsv1alpha1.WaitingForControlPlaneReadyStatusReason {
+			conditions.MarkFalse(cluster, meta.ReadyCondition, gitopsv1alpha1.ClusterNotConnectedReason, "No connectivity")
+			if err := r.Status().Update(ctx, cluster); err != nil {
+				log.Error(err, "failed to update Cluster status")
+				return ctrl.Result{}, err
+			}
+		}
+	} else {
+		conditions.MarkTrue(cluster, meta.ReadyCondition, gitopsv1alpha1.ClusterConnectedReason, "")
+	}
+
 	return ctrl.Result{RequeueAfter: r.Options.DefaultRequeueTime}, nil
 }
 
@@ -443,7 +457,7 @@ func (r *GitopsClusterReconciler) restConfigFromSecret(ctx context.Context, clus
 		Namespace: cluster.Namespace,
 	}
 
-	var secret v1.Secret
+	var secret corev1.Secret
 	if err := r.Get(ctx, key, &secret); err != nil {
 		log.Error(err, "unable to fetch secret for GitOps Cluster", "cluster", cluster.Name)
 

controllers/gitopscluster_controller_test.go

@@ -73,7 +73,7 @@ func TestReconcile(t *testing.T) {
 				makeTestSecret(types.NamespacedName{
 					Name:      "dev",
 					Namespace: testNamespace,
-				}, map[string][]byte{"value": []byte("testing")}),
+				}, map[string][]byte{"value": kubeConfig}),
 			},
 			obj: types.NamespacedName{Namespace: testNamespace, Name: testName},
 			opts: controllers.Options{
@@ -282,6 +282,29 @@ func TestReconcile(t *testing.T) {
 			wantStatus:        "False",
 			wantStatusMessage: `failed creating rest config from secret: invalid configuration: no server found for cluster "envtest"`,
 		},
+		{
+			name: "No connectivity causes ready condition to be false",
+			state: []runtime.Object{
+				makeTestCluster(func(c *gitopsv1alpha1.GitopsCluster) {
+					c.Spec.SecretRef = &meta.LocalObjectReference{
+						Name: "dev",
+					}
+				}),
+				makeTestSecret(types.NamespacedName{
+					Name:      "dev",
+					Namespace: testNamespace,
+				}, map[string][]byte{"value": kubeconfigWithError(t)}),
+			},
+			obj: types.NamespacedName{Namespace: testNamespace, Name: testName},
+			opts: controllers.Options{
+				CAPIEnabled:        true,
+				DefaultRequeueTime: defaultRequeueTime,
+			},
+			requeueAfter:      defaultRequeueTime,
+			wantCondition:     meta.ReadyCondition,
+			wantStatus:        "False",
+			wantStatusMessage: "No connectivity",
+		},
 	}
 
 	for _, tt := range tests {