(chore): cluster shows not ready when node unschedulable

Signed-off-by: abhijith-darshan <abhijith.ravindra@sap.com>

(chore): set ready false when all nodes are not ready

Signed-off-by: abhijith-darshan <abhijith.ravindra@sap.com>

(chore): e2e test node not ready

Signed-off-by: abhijith-darshan <abhijith.ravindra@sap.com>

Author: abhijith-darshan

cmd/greenhouse/controllers.go

@@ -42,9 +42,9 @@ var knownControllers = map[string]func(controllerName string, mgr ctrl.Manager)
 	"clusterPluginDefinition": (&plugindefinitioncontroller.ClusterPluginDefinitionReconciler{}).SetupWithManager,
 
 	// Cluster controllers
-	"bootStrap":         (&clustercontrollers.BootstrapReconciler{}).SetupWithManager,
-	"clusterReconciler": startClusterReconciler,
-	"kubeconfig":        (&clustercontrollers.KubeconfigReconciler{}).SetupWithManager,
+	"bootstrap":  (&clustercontrollers.BootstrapReconciler{}).SetupWithManager,
+	"cluster":    startClusterReconciler,
+	"kubeconfig": (&clustercontrollers.KubeconfigReconciler{}).SetupWithManager,
 }
 
 // knownControllers lists the name of known controllers.

e2e/cluster/e2e_test.go

@@ -251,4 +251,36 @@ var _ = Describe("Cluster E2E", Ordered, func() {
 			shared.ClusterIsReady(ctx, adminClient, remoteOIDCClusterCName, env.TestNamespace)
 		})
 	})
+
+	Context("Cluster Node Not Ready / Ready 😵 🤖", Ordered, func() {
+		It("should onboard remote cluster", func() {
+			By("onboarding remote cluster")
+			shared.OnboardRemoteCluster(ctx, adminClient, env.RemoteKubeConfigBytes, remoteClusterFName, env.TestNamespace, team.Name)
+		})
+		It("should have a cluster resource created", func() {
+			By("verifying if the cluster resource is created")
+			Eventually(func(g Gomega) bool {
+				err := adminClient.Get(ctx, client.ObjectKey{Name: remoteClusterFName, Namespace: env.TestNamespace}, &greenhousev1alpha1.Cluster{})
+				g.Expect(err).ToNot(HaveOccurred())
+				return true
+			}).Should(BeTrue(), "cluster resource should be created")
+
+			By("verifying the cluster status is ready")
+			shared.ClusterIsReady(ctx, adminClient, remoteClusterFName, env.TestNamespace)
+		})
+		It("should reach not ready state when all nodes are not ready", func() {
+			By("cordoning all nodes in the remote cluster")
+			expect.CordonRemoteNodes(ctx, remoteClient)
+
+			By("verifying the cluster status is not ready")
+			expect.ReconcileReadyNotReady(ctx, adminClient, remoteClusterFName, env.TestNamespace, false)
+		})
+		It("should reach ready state when nodes are ready again", func() {
+			By("un-cordoning all nodes in the remote cluster")
+			expect.UnCordonRemoteNodes(ctx, remoteClient)
+
+			By("verifying the cluster status is ready again")
+			expect.ReconcileReadyNotReady(ctx, adminClient, remoteClusterFName, env.TestNamespace, true)
+		})
+	})
 })

e2e/cluster/expect/expect.go

@@ -6,7 +6,6 @@ package expect
 import (
 	"context"
 	"fmt"
-	"strconv"
 	"time"
 
 	rbacv1 "k8s.io/api/rbac/v1"
@@ -123,12 +122,12 @@ func RevokingRemoteClusterAccess(ctx context.Context, adminClient, remoteClient
 func ReconcileReadyNotReady(ctx context.Context, adminClient client.Client, clusterName, namespace string, readyStatus bool) {
 	err := shared.WaitUntilResourceReadyOrNotReady(ctx, adminClient, &greenhousev1alpha1.Cluster{}, clusterName, namespace, func(resource lifecycle.RuntimeObject) error {
 		By("triggering a reconcile of the cluster resource")
-		resourceLabels := resource.GetLabels()
+		resourceLabels := resource.GetAnnotations()
 		if resourceLabels == nil {
 			resourceLabels = make(map[string]string)
 		}
-		resourceLabels["greenhouse.sap/last-apply"] = strconv.FormatInt(time.Now().Unix(), 10)
-		resource.SetLabels(resourceLabels)
+		resourceLabels[lifecycle.ReconcileAnnotation] = metav1.Now().Format(time.DateTime)
+		resource.SetAnnotations(resourceLabels)
 		return adminClient.Update(ctx, resource)
 	}, readyStatus)
 	Expect(err).NotTo(HaveOccurred(), "cluster should be in desired status")
@@ -139,3 +138,39 @@ func GetRestConfig(restClientGetter *clientutil.RestClientGetter) *rest.Config {
 	Expect(err).NotTo(HaveOccurred(), "there should be no error creating the remote REST config")
 	return restConfig
 }
+
+func CordonRemoteNodes(ctx context.Context, remoteClient client.Client) {
+	nodes := &corev1.NodeList{}
+	err := remoteClient.List(ctx, nodes)
+	Expect(err).NotTo(HaveOccurred(), "there should be no error while listing nodes")
+	for _, node := range nodes.Items {
+		labels := node.GetLabels()
+		_, ok := labels["node-role.kubernetes.io/control-plane"]
+		if ok {
+			By("skipping cordoning control plane node: " + node.Name)
+			continue
+		}
+		if node.Spec.Unschedulable {
+			continue
+		}
+		base := node.DeepCopy()
+		node.Spec.Unschedulable = true
+		err = remoteClient.Patch(ctx, &node, client.MergeFrom(base))
+		Expect(err).NotTo(HaveOccurred(), "there should be no error while patching node")
+	}
+}
+
+func UnCordonRemoteNodes(ctx context.Context, remoteClient client.Client) {
+	nodes := &corev1.NodeList{}
+	err := remoteClient.List(ctx, nodes)
+	Expect(err).NotTo(HaveOccurred(), "there should be no error while listing nodes")
+	for _, node := range nodes.Items {
+		if !node.Spec.Unschedulable {
+			continue
+		}
+		base := node.DeepCopy()
+		node.Spec.Unschedulable = false
+		err = remoteClient.Patch(ctx, &node, client.MergeFrom(base))
+		Expect(err).NotTo(HaveOccurred(), "there should be no error while patching node")
+	}
+}

internal/controller/cluster/cluster_status.go

@@ -51,7 +51,7 @@ func (r *RemoteClusterReconciler) setConditions() lifecycle.Conditioner {
 			kubeConfigValidCondition, k8sVersion = r.reconcileKubeConfigValid(restClientGetter)
 		}
 
-		var allNodesReadyCondition, clusterAccessibleCondition, resourcesDeployedCondition greenhousemetav1alpha1.Condition
+		var allNodesReadyCondition, clusterAccessibleCondition, resourcesDeployedCondition, readyCondition greenhousemetav1alpha1.Condition
 		var nodes *greenhousev1alpha1.Nodes
 		// Can only reconcile detailed status if kubeconfig is valid.
 		if kubeConfigValidCondition.IsFalse() {
@@ -64,7 +64,11 @@ func (r *RemoteClusterReconciler) setConditions() lifecycle.Conditioner {
 			resourcesDeployedCondition = r.reconcileBootstrapResources(ctx, restClientGetter, clusterSecret)
 		}
 
-		readyCondition := r.reconcileReadyStatus(kubeConfigValidCondition, resourcesDeployedCondition)
+		if allNodesReadyCondition.IsFalse() {
+			readyCondition = greenhousemetav1alpha1.FalseCondition(greenhousemetav1alpha1.ReadyCondition, "", allNodesReadyCondition.Message)
+		} else {
+			readyCondition = r.reconcileReadyStatus(kubeConfigValidCondition, resourcesDeployedCondition)
+		}
 
 		ownerLabelCondition := util.ComputeOwnerLabelCondition(ctx, r.Client, cluster)
 		util.UpdateOwnedByLabelMissingMetric(cluster, ownerLabelCondition.IsFalse())
@@ -223,11 +227,14 @@ func (r *RemoteClusterReconciler) reconcileNodeStatus(
 		return greenhousemetav1alpha1.FalseCondition(greenhousev1alpha1.AllNodesReady, "", err.Error()), nil
 	}
 
-	notReadyNodes := []greenhousev1alpha1.NodeStatus{}
+	var notReadyNodes []greenhousev1alpha1.NodeStatus
 	var totalNodes, readyNodes int32
 	allNodesReadyCondition = greenhousemetav1alpha1.TrueCondition(greenhousev1alpha1.AllNodesReady, "", "")
 
 	for _, node := range nodeList.Items {
+		if checkIfControlPlaneNode(node.Labels) {
+			continue
+		}
 		totalNodes++
 
 		nodeStatus := getNodeStatusIfNodeNotReady(node)
@@ -251,6 +258,9 @@ func (r *RemoteClusterReconciler) reconcileNodeStatus(
 
 // getNodeStatusIfNodeNotReady returns a NodeStatus when the NodeReady condition is explicitly False; otherwise nil.
 func getNodeStatusIfNodeNotReady(node corev1.Node) *greenhousev1alpha1.NodeStatus {
+	if node.Spec.Unschedulable {
+		return &greenhousev1alpha1.NodeStatus{Name: node.Name, Message: "Node is unschedulable", LastTransitionTime: metav1.Now()}
+	}
 	var readyCondition *corev1.NodeCondition
 	for i := range node.Status.Conditions {
 		if node.Status.Conditions[i].Type == corev1.NodeReady {
@@ -265,3 +275,14 @@ func getNodeStatusIfNodeNotReady(node corev1.Node) *greenhousev1alpha1.NodeStatu
 
 	return &greenhousev1alpha1.NodeStatus{Name: node.Name, Message: readyCondition.Message, LastTransitionTime: readyCondition.LastTransitionTime}
 }
+
+func checkIfControlPlaneNode(labels map[string]string) (control bool) {
+	control = false
+	for key, _ := range labels {
+		if key == "node-role.kubernetes.io/control-plane" {
+			control = true
+			break
+		}
+	}
+	return
+}