(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>

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

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

(chore): set PayloadSchedulable condition

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

Automatic generation of CRD API Docs

(chore): fix eventually

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

Author: abhijith-darshan

api/v1alpha1/cluster_types.go

@@ -40,6 +40,9 @@ const (
 	// AllNodesReady reflects the readiness status of all nodes of a cluster.
 	AllNodesReady greenhousemetav1alpha1.ConditionType = "AllNodesReady"
 
+	// PayloadSchedulable reflects whether workloads can be scheduled on the cluster.
+	PayloadSchedulable greenhousemetav1alpha1.ConditionType = "PayloadSchedulable"
+
 	// KubeConfigValid reflects the validity of the kubeconfig of a cluster.
 	KubeConfigValid greenhousemetav1alpha1.ConditionType = "KubeConfigValid"
 
@@ -74,9 +77,9 @@ type ClusterConditionType string
 // Nodes contains node count metrics and tracks non-ready nodes in a cluster.
 type Nodes struct {
 	// Total represent the number of all the nodes in the cluster.
-	Total int32 `json:"total,omitempty"`
+	Total int `json:"total,omitempty"`
 	// ReadyNodes represent the number of ready nodes in the cluster.
-	Ready int32 `json:"ready,omitempty"`
+	Ready int `json:"ready,omitempty"`
 	// NotReady is slice of non-ready nodes status details.
 	// +listType="map"
 	// +listMapKey=name

charts/manager/crds/greenhouse.sap_clusters.yaml

@@ -119,12 +119,10 @@ spec:
                   ready:
                     description: ReadyNodes represent the number of ready nodes in
                       the cluster.
-                    format: int32
                     type: integer
                   total:
                     description: Total represent the number of all the nodes in the
                       cluster.
-                    format: int32
                     type: integer
                 type: object
               statusConditions:

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.

docs/reference/api/index.html

@@ -1879,7 +1879,7 @@ <h3 id="greenhouse.sap/v1alpha1.Nodes">Nodes
 <td>
 <code>total</code><br>
 <em>
-int32
+int
 </em>
 </td>
 <td>
@@ -1890,7 +1890,7 @@ <h3 id="greenhouse.sap/v1alpha1.Nodes">Nodes
 <td>
 <code>ready</code><br>
 <em>
-int32
+int
 </em>
 </td>
 <td>

docs/reference/api/openapi.yaml

@@ -592,11 +592,9 @@ components:
                   x-kubernetes-list-type: map
                 ready:
                   description: ReadyNodes represent the number of ready nodes in the cluster.
-                  format: int32
                   type: integer
                 total:
                   description: Total represent the number of all the nodes in the cluster.
-                  format: int32
                   type: integer
               type: object
             statusConditions:

e2e/cluster/e2e_test.go

@@ -33,6 +33,7 @@ import (
 const (
 	remoteClusterHName               = "remote-int-h-cluster"
 	remoteClusterFName               = "remote-int-f-cluster"
+	remoteClusterNodeName            = "remote-int-n-cluster"
 	remoteOIDCClusterHName           = "remote-int-oidc-h-cluster"
 	remoteOIDCClusterCName           = "remote-int-oidc-c-cluster"
 	remoteOIDCClusterFName           = "remote-int-oidc-f-cluster"
@@ -69,13 +70,14 @@ var _ = BeforeSuite(func() {
 	env = env.WithOrganization(ctx, adminClient, "./testdata/organization.yaml")
 	team = test.NewTeam(ctx, "test-cluster-e2e-team", env.TestNamespace, test.WithTeamLabel(greenhouseapis.LabelKeySupportGroup, "true"), test.WithMappedIDPGroup("SOME_IDP_GROUP_NAME"))
 	err = adminClient.Create(ctx, team)
-	Expect(err).ToNot(HaveOccurred(), "there should be no error creating a Team")
+	Expect(client.IgnoreAlreadyExists(err)).ToNot(HaveOccurred(), "there should be no error creating a Team")
 	testStartTime = time.Now().UTC()
 })
 
 var _ = AfterSuite(func() {
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteClusterHName, env.TestNamespace)
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteClusterFName, env.TestNamespace)
+	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteClusterNodeName, env.TestNamespace)
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteOIDCClusterHName, env.TestNamespace)
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteOIDCClusterFName, env.TestNamespace)
 	shared.OffBoardRemoteCluster(ctx, adminClient, remoteClient, testStartTime, remoteOIDCClusterCName, env.TestNamespace)
@@ -251,4 +253,52 @@ 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, remoteClusterNodeName, 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: remoteClusterNodeName, 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, remoteClusterNodeName, 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 payload is not schedulable")
+			Eventually(func(g Gomega) {
+				cluster := &greenhousev1alpha1.Cluster{}
+				err := adminClient.Get(ctx, client.ObjectKey{Name: remoteClusterNodeName, Namespace: env.TestNamespace}, cluster)
+				g.Expect(err).ToNot(HaveOccurred(), "there should be no error getting the cluster resource")
+				conditions := cluster.GetConditions()
+				payloadCondition := conditions.GetConditionByType(greenhousev1alpha1.PayloadSchedulable)
+				g.Expect(payloadCondition).ToNot(BeNil(), "cluster should have PayloadSchedulable condition")
+				g.Expect(payloadCondition.IsTrue()).To(BeFalse(), "cluster should not be able to schedule payloads")
+			}).Should(Succeed(), "cluster should not be able to schedule payloads when all nodes are cordoned")
+		})
+		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")
+			Eventually(func(g Gomega) {
+				cluster := &greenhousev1alpha1.Cluster{}
+				err := adminClient.Get(ctx, client.ObjectKey{Name: remoteClusterNodeName, Namespace: env.TestNamespace}, cluster)
+				g.Expect(err).ToNot(HaveOccurred(), "there should be no error getting the cluster resource")
+				conditions := cluster.GetConditions()
+				payloadCondition := conditions.GetConditionByType(greenhousev1alpha1.PayloadSchedulable)
+				g.Expect(payloadCondition).ToNot(BeNil(), "cluster should have PayloadSchedulable condition")
+				g.Expect(payloadCondition.IsTrue()).To(BeTrue(), "cluster be able to schedule payloads")
+			}).Should(Succeed(), "cluster should be able to schedule payloads when nodes are uncordoned")
+		})
+	})
 })

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")
+	}
+}

e2e/shared/cluster.go

@@ -82,7 +82,9 @@ func OffBoardRemoteCluster(ctx context.Context, adminClient, remoteClient client
 	Eventually(func(g Gomega) {
 		crb := &rbacv1.ClusterRoleBinding{}
 		err := remoteClient.Get(ctx, client.ObjectKey{Name: ManagedResourceName}, crb)
-		GinkgoWriter.Printf("crb err: %v\n", err)
+		if err != nil {
+			GinkgoWriter.Printf("crb err: %v\n", err)
+		}
 		g.Expect(apierrors.IsNotFound(err)).To(BeTrue(), "cluster role binding should be deleted")
 		managedSA := &corev1.ServiceAccount{}
 		err = remoteClient.Get(ctx, client.ObjectKey{Name: ManagedResourceName, Namespace: namespace}, managedSA)

internal/controller/cluster/cluster_status.go

@@ -30,7 +30,10 @@ import (
 	"github.com/cloudoperators/greenhouse/pkg/lifecycle"
 )
 
-const clusterK8sVersionUnknown = "unknown"
+const (
+	clusterK8sVersionUnknown = "unknown"
+	controlPlaneNodeLabel    = "node-role.kubernetes.io/control-plane"
+)
 
 func (r *RemoteClusterReconciler) setConditions() lifecycle.Conditioner {
 	return func(ctx context.Context, resource lifecycle.RuntimeObject) {
@@ -51,17 +54,22 @@ func (r *RemoteClusterReconciler) setConditions() lifecycle.Conditioner {
 			kubeConfigValidCondition, k8sVersion = r.reconcileKubeConfigValid(restClientGetter)
 		}
 
-		var allNodesReadyCondition, clusterAccessibleCondition, resourcesDeployedCondition greenhousemetav1alpha1.Condition
+		var allNodesReadyCondition, clusterAccessibleCondition, resourcesDeployedCondition, payloadSchedulable greenhousemetav1alpha1.Condition
+		payloadSchedulable = greenhousemetav1alpha1.TrueCondition(greenhousev1alpha1.PayloadSchedulable, "", "")
 		var nodes *greenhousev1alpha1.Nodes
 		// Can only reconcile detailed status if kubeconfig is valid.
 		if kubeConfigValidCondition.IsFalse() {
+			payloadSchedulable = greenhousemetav1alpha1.FalseCondition(greenhousev1alpha1.PayloadSchedulable, "", "kubeconfig not valid - payloads cannot be scheduled")
 			allNodesReadyCondition = greenhousemetav1alpha1.UnknownCondition(greenhousev1alpha1.AllNodesReady, "", "kubeconfig not valid - cannot know node status")
 			clusterAccessibleCondition = greenhousemetav1alpha1.UnknownCondition(greenhousev1alpha1.PermissionsVerified, "", "kubeconfig not valid - cannot validate cluster access")
 			resourcesDeployedCondition = greenhousemetav1alpha1.UnknownCondition(greenhousev1alpha1.ManagedResourcesDeployed, "", "kubeconfig not valid - cannot validate managed resources")
 		} else {
 			allNodesReadyCondition, nodes = r.reconcileNodeStatus(ctx, restClientGetter)
 			clusterAccessibleCondition = r.reconcilePermissions(ctx, restClientGetter)
 			resourcesDeployedCondition = r.reconcileBootstrapResources(ctx, restClientGetter, clusterSecret)
+			if nodes != nil && len(nodes.NotReady) == nodes.Total {
+				payloadSchedulable = greenhousemetav1alpha1.FalseCondition(greenhousev1alpha1.PayloadSchedulable, "", "all nodes are not ready")
+			}
 		}
 
 		readyCondition := r.reconcileReadyStatus(kubeConfigValidCondition, resourcesDeployedCondition)
@@ -76,6 +84,7 @@ func (r *RemoteClusterReconciler) setConditions() lifecycle.Conditioner {
 			ownerLabelCondition,
 			clusterAccessibleCondition,
 			resourcesDeployedCondition,
+			payloadSchedulable,
 		}
 		deletionCondition := r.checkDeletionSchedule(logger, cluster)
 		if !deletionCondition.IsUnknown() {
@@ -223,11 +232,14 @@ func (r *RemoteClusterReconciler) reconcileNodeStatus(
 		return greenhousemetav1alpha1.FalseCondition(greenhousev1alpha1.AllNodesReady, "", err.Error()), nil
 	}
 
-	notReadyNodes := []greenhousev1alpha1.NodeStatus{}
-	var totalNodes, readyNodes int32
+	var notReadyNodes []greenhousev1alpha1.NodeStatus
+	var totalNodes, readyNodes int
 	allNodesReadyCondition = greenhousemetav1alpha1.TrueCondition(greenhousev1alpha1.AllNodesReady, "", "")
 
 	for _, node := range nodeList.Items {
+		if checkIfControlPlaneNode(node.Labels) {
+			continue
+		}
 		totalNodes++
 
 		nodeStatus := getNodeStatusIfNodeNotReady(node)
@@ -251,6 +263,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 +280,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 == controlPlaneNodeLabel {
+			control = true
+			break
+		}
+	}
+	return
+}

internal/controller/cluster/status_test.go

@@ -155,9 +155,9 @@ var _ = Describe("Cluster status", Ordered, func() {
 			g.Expect(validCluster.Status.GetConditionByType(greenhousev1alpha1.AllNodesReady).Status).To(Equal(metav1.ConditionFalse))
 			g.Expect(validCluster.Status.GetConditionByType(greenhousev1alpha1.AllNodesReady).Message).To(ContainSubstring("test-node not ready, test-node-3 not ready"))
 			// Validate the counts of total and ready nodes.
-			g.Expect(validCluster.Status.Nodes).ToNot((BeNil()))
-			g.Expect(validCluster.Status.Nodes.Total).To(Equal(int32(3)))
-			g.Expect(validCluster.Status.Nodes.Ready).To(Equal(int32(1)))
+			g.Expect(validCluster.Status.Nodes).ToNot(BeNil())
+			g.Expect(validCluster.Status.Nodes.Total).To(Equal(3))
+			g.Expect(validCluster.Status.Nodes.Ready).To(Equal(1))
 			g.Expect(validCluster.Status.Nodes.NotReady).To(HaveLen(2))
 		}).Should(Succeed())
 
@@ -220,9 +220,9 @@ var _ = Describe("Cluster status", Ordered, func() {
 			g.Expect(validCluster.Status.GetConditionByType(greenhousev1alpha1.AllNodesReady).Status).To(Equal(metav1.ConditionTrue), "The AllNodesReady condition should be true")
 			g.Expect(validCluster.Status.GetConditionByType(greenhousev1alpha1.AllNodesReady).Message).To(BeEmpty())
 			// Validate the counts of total and ready nodes.
-			g.Expect(validCluster.Status.Nodes).ToNot((BeNil()))
-			g.Expect(validCluster.Status.Nodes.Total).To(Equal(int32(3)))
-			g.Expect(validCluster.Status.Nodes.Ready).To(Equal(int32(3)))
+			g.Expect(validCluster.Status.Nodes).ToNot(BeNil())
+			g.Expect(validCluster.Status.Nodes.Total).To(Equal(3))
+			g.Expect(validCluster.Status.Nodes.Ready).To(Equal(3))
 			g.Expect(validCluster.Status.Nodes.NotReady).To(BeEmpty())
 		}).Should(Succeed())