Skip to content

Commit

Permalink
Merge pull request #2190 from 0xavi0/1973-remove-node-clusterstatus-a…
Browse files Browse the repository at this point in the history
…gent

Removes cluster node status reporting from agent
  • Loading branch information
0xavi0 authored Mar 4, 2024
2 parents dea8b6c + aa041a4 commit 05316c4
Show file tree
Hide file tree
Showing 9 changed files with 109 additions and 159 deletions.
2 changes: 1 addition & 1 deletion .github/scripts/label-downstream-cluster.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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" }]'
2 changes: 1 addition & 1 deletion .github/workflows/e2e-multicluster-ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
42 changes: 0 additions & 42 deletions charts/fleet-crd/templates/crds.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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".
Expand Down
22 changes: 22 additions & 0 deletions internal/cmd/agent/clusterstatus/suite_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
77 changes: 8 additions & 69 deletions internal/cmd/agent/clusterstatus/ticker.go
Original file line number Diff line number Diff line change
@@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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
}
Expand All @@ -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...)
}
77 changes: 77 additions & 0 deletions internal/cmd/agent/clusterstatus/ticker_test.go
Original file line number Diff line number Diff line change
@@ -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()
})
})
14 changes: 0 additions & 14 deletions internal/cmd/controller/reconciler/cluster_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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 ""
}
22 changes: 0 additions & 22 deletions pkg/apis/fleet.cattle.io/v1alpha1/cluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down Expand Up @@ -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"`
}
Expand All @@ -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"`
}
10 changes: 0 additions & 10 deletions pkg/apis/fleet.cattle.io/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 05316c4

Please sign in to comment.