Removes cluster node status reporting from agent

.github/scripts/label-downstream-cluster.sh

@@ -4,6 +4,6 @@ set -euxo pipefail
 
 ns=${FLEET_E2E_NS_DOWNSTREAM-fleet-default}
 
-{ grep -q -m 1 -e "k3d-downstream"; kill $!; } < <(kubectl get clusters.fleet.cattle.io -n "$ns" -w)
+{ grep -q -m 1 -e "1/1"; kill $!; } < <(kubectl get clusters.fleet.cattle.io -n "$ns" -w)
 name=$(kubectl get clusters.fleet.cattle.io -o=jsonpath='{.items[0].metadata.name}' -n "$ns")
 kubectl patch clusters.fleet.cattle.io -n "$ns" "$name" --type=json -p '[{"op": "add", "path": "/metadata/labels/env", "value": "test" }]'

.github/workflows/e2e-multicluster-ci.yml

@@ -121,7 +121,7 @@ jobs:
                 --set token="$token"
 
           echo "waiting for downstream cluster to be registered..."
-          { grep -q -m 1 "k3d-downstream"; kill $!; } < <(kubectl get cluster -n fleet-default -w)
+          { grep -q -m 1 "1/1"; kill $!; } < <(kubectl get cluster -n fleet-default -w)
 
           echo "waiting for cluster to report being ready..."
           while [ $(kubectl -n fleet-default get cluster -o jsonpath='{.items[0].status.summary.ready}') -ne 1 ]; do

charts/fleet-crd/templates/crds.yaml

@@ -3626,12 +3626,6 @@ spec:
         - jsonPath: .status.display.readyBundles
           name: Bundles-Ready
           type: string
-        - jsonPath: .status.display.readyNodes
-          name: Nodes-Ready
-          type: string
-        - jsonPath: .status.display.sampleNode
-          name: Sample-Node
-          type: string
         - jsonPath: .status.agent.lastSeen
           name: Last-Seen
           type: string
@@ -5109,29 +5103,6 @@ spec:
                         e.g. "cattle-fleet-system".
                       nullable: true
                       type: string
-                    nonReadyNodeNames:
-                      description: 'NonReadyNode contains the names of non-ready nodes.
-                        The list is
-
-                        limited to at most 3 names.'
-                      items:
-                        type: string
-                      type: array
-                    nonReadyNodes:
-                      description: NonReadyNodes is the number of nodes that are not
-                        ready.
-                      type: integer
-                    readyNodeNames:
-                      description: 'ReadyNodes contains the names of ready nodes.
-                        The list is limited to
-
-                        at most 3 names.'
-                      items:
-                        type: string
-                      type: array
-                    readyNodes:
-                      description: ReadyNodes is the number of nodes that are ready.
-                      type: integer
                   type: object
                 agentAffinityHash:
                   description: 'AgentAffinityHash is a hash of the agent''s affinity
@@ -5260,19 +5231,6 @@ spec:
 
                         to be ready.'
                       type: string
-                    readyNodes:
-                      description: 'ReadyNodes is a string in the form "%d/%d", that
-                        describes the
-
-                        number of nodes that are ready vs. the number of expected
-                        nodes.'
-                      type: string
-                    sampleNode:
-                      description: 'SampleNode is the name of one of the nodes that
-                        are ready. If no
-
-                        node is ready, it''s the name of a node that is not ready.'
-                      type: string
                     state:
                       description: State of the cluster, either one of the bundle
                         states, or "WaitCheckIn".

internal/cmd/agent/clusterstatus/suite_test.go

@@ -0,0 +1,22 @@
+package clusterstatus_test
+
+import (
+	"testing"
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+)
+
+const (
+	timeout = 30 * time.Second
+)
+
+func TestFleet(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "ClusterStatus Suite")
+}
+
+var _ = BeforeSuite(func() {
+	SetDefaultEventuallyTimeout(timeout)
+})

internal/cmd/agent/clusterstatus/ticker.go

@@ -1,10 +1,9 @@
-// Package clusterstatus updates the cluster.fleet.cattle.io status in the upstream cluster with the current node status.
+// Package clusterstatus updates the cluster.fleet.cattle.io status in the upstream cluster with the current cluster status.
 package clusterstatus
 
 import (
 	"context"
 	"encoding/json"
-	"sort"
 	"time"
 
 	"github.com/sirupsen/logrus"
@@ -16,7 +15,6 @@ import (
 	corecontrollers "github.com/rancher/wrangler/v2/pkg/generated/controllers/core/v1"
 	"github.com/rancher/wrangler/v2/pkg/ticker"
 
-	corev1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/equality"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
@@ -52,50 +50,31 @@ func Ticker(ctx context.Context,
 
 	go func() {
 		time.Sleep(durations.ClusterRegisterDelay)
-		logger.V(1).Info("Reporting cluster node status once")
+		logger.V(1).Info("Reporting cluster status once")
 		if err := h.Update(); err != nil {
-			logrus.Errorf("failed to report cluster node status: %v", err)
+			logrus.Errorf("failed to report cluster status: %v", err)
 		}
 	}()
 	go func() {
 		if checkinInterval == 0 {
 			checkinInterval = durations.DefaultClusterCheckInterval
 		}
 		for range ticker.Context(ctx, checkinInterval) {
-			logger.V(1).Info("Reporting cluster node status")
+			logger.V(1).Info("Reporting cluster status")
 			if err := h.Update(); err != nil {
-				logrus.Errorf("failed to report cluster node status: %v", err)
+				logrus.Errorf("failed to report cluster status: %v", err)
 			}
 		}
 	}()
 }
 
-// Update the cluster.fleet.cattle.io status in the upstream cluster with the current node status
+// Update the cluster.fleet.cattle.io status in the upstream cluster with the current cluster status
 func (h *handler) Update() error {
-	nodes, err := h.nodes.List(metav1.ListOptions{})
-	if err != nil {
-		return err
-	}
-
-	ready, nonReady := sortReadyUnready(nodes.Items)
-
 	agentStatus := fleet.AgentStatus{
-		LastSeen:      metav1.Now(),
-		Namespace:     h.agentNamespace,
-		NonReadyNodes: len(nonReady),
-		ReadyNodes:    len(ready),
+		LastSeen:  metav1.Now(),
+		Namespace: h.agentNamespace,
 	}
 
-	if len(ready) > 3 {
-		ready = ready[:3]
-	}
-	if len(nonReady) > 3 {
-		nonReady = nonReady[:3]
-	}
-
-	agentStatus.ReadyNodeNames = ready
-	agentStatus.NonReadyNodeNames = nonReady
-
 	if equality.Semantic.DeepEqual(h.reported, agentStatus) {
 		return nil
 	}
@@ -117,43 +96,3 @@ func (h *handler) Update() error {
 	h.reported = agentStatus
 	return nil
 }
-
-func sortReadyUnready(nodes []corev1.Node) (ready []string, nonReady []string) {
-	var (
-		masterNodeNames         []string
-		nonReadyMasterNodeNames []string
-		readyNodes              []string
-		nonReadyNodes           []string
-	)
-
-	for _, node := range nodes {
-		ready := false
-		for _, cond := range node.Status.Conditions {
-			if cond.Type == corev1.NodeReady && cond.Status == corev1.ConditionTrue {
-				ready = true
-				break
-			}
-		}
-
-		if node.Annotations["node-role.kubernetes.io/master"] == "true" {
-			if ready {
-				masterNodeNames = append(masterNodeNames, node.Name)
-			} else {
-				nonReadyMasterNodeNames = append(nonReadyMasterNodeNames, node.Name)
-			}
-		} else {
-			if ready {
-				readyNodes = append(readyNodes, node.Name)
-			} else {
-				nonReadyNodes = append(nonReadyNodes, node.Name)
-			}
-		}
-	}
-
-	sort.Strings(masterNodeNames)
-	sort.Strings(nonReadyMasterNodeNames)
-	sort.Strings(readyNodes)
-	sort.Strings(nonReadyNodes)
-
-	return append(masterNodeNames, readyNodes...), append(nonReadyMasterNodeNames, nonReadyNodes...)
-}

internal/cmd/agent/clusterstatus/ticker_test.go

@@ -0,0 +1,77 @@
+package clusterstatus
+
+import (
+	"context"
+	"encoding/json"
+	"time"
+
+	"github.com/golang/mock/gomock"
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+	fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1"
+	"github.com/rancher/wrangler/v2/pkg/generic/fake"
+	v1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
+)
+
+var _ = Describe("ClusterStatus Ticker", func() {
+	var (
+		clusterClient        *fake.MockClientInterface[*fleet.Cluster, *fleet.ClusterList]
+		nodeClient           *fake.MockNonNamespacedClientInterface[*v1.Node, *v1.NodeList]
+		ctx                  context.Context
+		cancel               context.CancelFunc
+		agentNamespace       string
+		clusterName          string
+		clusterNamespace     string
+		baseTime             metav1.Time
+		lastTime             metav1.Time
+		agentStatusNamespace string
+	)
+
+	BeforeEach(func() {
+		ctrl := gomock.NewController(GinkgoT())
+		clusterClient = fake.NewMockClientInterface[*fleet.Cluster, *fleet.ClusterList](ctrl)
+		nodeClient = fake.NewMockNonNamespacedClientInterface[*v1.Node, *v1.NodeList](ctrl)
+		agentNamespace = "cattle-fleet-system"
+		clusterName = "cluster-name"
+		clusterNamespace = "cluster-namespace"
+		baseTime = metav1.Now()
+		ctx, cancel = context.WithCancel(context.TODO())
+		clusterClient.EXPECT().Patch(clusterNamespace, clusterName, types.MergePatchType, gomock.Any(), "status").
+			DoAndReturn(func(namespace, name string, pt types.PatchType, data []byte, subresources ...string) (fleet.Cluster, error) {
+				cluster := &fleet.Cluster{}
+				err := json.Unmarshal(data, cluster)
+				if err != nil {
+					return fleet.Cluster{}, err
+				}
+				// only storing the lastseen time value here.
+				// We're not checking for values here because calling Expect,
+				// for example, makes the mock call panic when it doesn't succeed
+				lastTime = cluster.Status.Agent.LastSeen
+				agentStatusNamespace = cluster.Status.Agent.Namespace
+				return *cluster, nil
+			}).AnyTimes()
+		Ticker(ctx, agentNamespace, clusterNamespace, clusterName, time.Second*1, nodeClient, clusterClient)
+	})
+
+	It("Increases the timestamp used to call Patch", func() {
+		By("Comparing every 2 seconds for a 6 seconds period")
+		Consistently(func() bool {
+			// return true when we're calling before Patch was even called
+			if lastTime.IsZero() {
+				return true
+			}
+			// check that the timestamp increases and the namespace is the expected one
+			result := baseTime.Before(&lastTime) && agentStatusNamespace == agentNamespace
+			baseTime = lastTime
+			return result
+		}, 6*time.Second, 2*time.Second).Should(BeTrue())
+		// ensure that lastTime was set (which means Patch was successfully called)
+		Expect(lastTime).ShouldNot(BeZero())
+	})
+
+	AfterEach(func() {
+		cancel()
+	})
+})

internal/cmd/controller/reconciler/cluster_controller.go

@@ -137,10 +137,6 @@ func (r *ClusterReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
 	cluster.Status.Display.ReadyBundles = fmt.Sprintf("%d/%d",
 		cluster.Status.Summary.Ready,
 		cluster.Status.Summary.DesiredReady)
-	cluster.Status.Display.ReadyNodes = fmt.Sprintf("%d/%d",
-		cluster.Status.Agent.ReadyNodes,
-		cluster.Status.Agent.NonReadyNodes+cluster.Status.Agent.ReadyNodes)
-	cluster.Status.Display.SampleNode = sampleNode(cluster.Status)
 
 	var state fleet.BundleState
 	for _, nonReady := range cluster.Status.Summary.NonReadyResources {
@@ -190,13 +186,3 @@ func (r *ClusterReconciler) SetupWithManager(mgr ctrl.Manager) error {
 		// cluster.Status.Namespace to create the namespace.
 		Complete(r)
 }
-
-func sampleNode(status fleet.ClusterStatus) string {
-	if len(status.Agent.ReadyNodeNames) > 0 {
-		return status.Agent.ReadyNodeNames[0]
-	}
-	if len(status.Agent.NonReadyNodeNames) > 0 {
-		return status.Agent.NonReadyNodeNames[0]
-	}
-	return ""
-}

pkg/apis/fleet.cattle.io/v1alpha1/cluster_types.go

@@ -46,8 +46,6 @@ var (
 // +kubebuilder:object:root=true
 // +kubebuilder:subresource:status
 // +kubebuilder:printcolumn:name="Bundles-Ready",type=string,JSONPath=`.status.display.readyBundles`
-// +kubebuilder:printcolumn:name="Nodes-Ready",type=string,JSONPath=`.status.display.readyNodes`
-// +kubebuilder:printcolumn:name="Sample-Node",type=string,JSONPath=`.status.display.sampleNode`
 // +kubebuilder:printcolumn:name="Last-Seen",type=string,JSONPath=`.status.agent.lastSeen`
 // +kubebuilder:printcolumn:name="Status",type=string,JSONPath=`.status.conditions[?(@.type=="Ready")].message`
 
@@ -200,12 +198,6 @@ type ClusterDisplay struct {
 	// number of bundles that are ready vs. the number of bundles desired
 	// to be ready.
 	ReadyBundles string `json:"readyBundles,omitempty"`
-	// ReadyNodes is a string in the form "%d/%d", that describes the
-	// number of nodes that are ready vs. the number of expected nodes.
-	ReadyNodes string `json:"readyNodes,omitempty"`
-	// SampleNode is the name of one of the nodes that are ready. If no
-	// node is ready, it's the name of a node that is not ready.
-	SampleNode string `json:"sampleNode,omitempty"`
 	// State of the cluster, either one of the bundle states, or "WaitCheckIn".
 	State string `json:"state,omitempty"`
 }
@@ -220,18 +212,4 @@ type AgentStatus struct {
 	// +nullable
 	// +optional
 	Namespace string `json:"namespace"`
-	// NonReadyNodes is the number of nodes that are not ready.
-	// +optional
-	NonReadyNodes int `json:"nonReadyNodes"`
-	// ReadyNodes is the number of nodes that are ready.
-	// +optional
-	ReadyNodes int `json:"readyNodes"`
-	// NonReadyNode contains the names of non-ready nodes. The list is
-	// limited to at most 3 names.
-	// +optional
-	NonReadyNodeNames []string `json:"nonReadyNodeNames"`
-	// ReadyNodes contains the names of ready nodes. The list is limited to
-	// at most 3 names.
-	// +optional
-	ReadyNodeNames []string `json:"readyNodeNames"`
 }