From d651b780dc732b227bfae9c266911c886ffbaf88 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20N=C3=A9au?= Date: Mon, 14 Oct 2024 10:47:40 +0200 Subject: [PATCH 1/4] Prevent agent check-in interval from being 0 This ensures that creating a new agent bundle fails with an agent check-in interval set to 0. --- .../controllers/manageagent/manageagent.go | 5 ++++ .../manageagent/manageagent_test.go | 29 +++++++++++++++++-- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent.go b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent.go index 3f65f61925..486a83b8d4 100644 --- a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent.go +++ b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent.go @@ -11,6 +11,7 @@ import ( "crypto/sha256" "encoding/hex" "encoding/json" + "errors" "fmt" "sort" @@ -335,6 +336,10 @@ func (h *handler) newAgentBundle(ns string, cluster *fleet.Cluster) ([]runtime.O priorityClassName = scheduling.FleetAgentPriorityClassName } + if cfg.AgentCheckinInterval.Seconds() == 0 { + return nil, errors.New("agent check-in interval cannot be 0") + } + if cluster.Spec.AgentTolerations != nil { sortTolerations(cluster.Spec.AgentTolerations) } diff --git a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go index 9a6eb7a05d..7481f73449 100644 --- a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go +++ b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go @@ -5,6 +5,7 @@ import ( "reflect" "strings" "testing" + "time" "github.com/rancher/wrangler/v3/pkg/generic/fake" "github.com/rancher/wrangler/v3/pkg/schemes" @@ -14,14 +15,34 @@ import ( metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" + "github.com/rancher/fleet/internal/config" fleet "github.com/rancher/fleet/pkg/apis/fleet.cattle.io/v1alpha1" appsv1 "k8s.io/api/apps/v1" networkv1 "k8s.io/api/networking/v1" "sigs.k8s.io/yaml" - - "github.com/rancher/fleet/internal/config" ) +func TestNewAgentBundle(t *testing.T) { + config.Set(&config.Config{AgentCheckinInterval: metav1.Duration{Duration: 0 * time.Second}}) + + // ensure leader election env is set so NewLeaderElectionOptionsWithPrefix doesn't error + t.Setenv("FLEET_AGENT_ELECTION_LEASE_DURATION", "15s") + t.Setenv("FLEET_AGENT_ELECTION_RENEW_DEADLINE", "10s") + t.Setenv("FLEET_AGENT_ELECTION_RETRY_PERIOD", "2s") + + h := handler{systemNamespace: "blah"} + obj, err := h.newAgentBundle("foo", &fleet.Cluster{Spec: fleet.ClusterSpec{AgentNamespace: "bar"}}) + + if obj != nil { + t.Fatalf("expected obj returned by newAgentBundle to be nil") + } + + expectedStr := "interval cannot be 0" + if !strings.Contains(err.Error(), expectedStr) { + t.Fatalf("expected error %q returned by newAgentBundle to contain %q", err, expectedStr) + } +} + func TestOnClusterChangeAffinity(t *testing.T) { ctrl := gomock.NewController(t) namespaces := fake.NewMockNonNamespacedControllerInterface[*corev1.Namespace, *corev1.NamespaceList](ctrl) @@ -119,7 +140,9 @@ func TestOnClusterChangeAffinity(t *testing.T) { func TestNewAgentBundle_SortsAgentTolerations(t *testing.T) { // make sure config is set for newAgentBundle - config.Set(config.DefaultConfig()) + cfg := config.DefaultConfig() + cfg.AgentCheckinInterval = metav1.Duration{Duration: 1 * time.Second} // non-zero to prevent errors covered elsewhere. + config.Set(cfg) checkRegisterAddToScheme(t, appsv1.AddToScheme) checkRegisterAddToScheme(t, networkv1.AddToScheme) From f8606419dec3462297214a567c0f325e43dc4319 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20N=C3=A9au?= Date: Mon, 14 Oct 2024 11:08:12 +0200 Subject: [PATCH 2/4] Prevent check-in interval from being 0 when importing cluster This adds a check on the agent check-in interval to cluster import, for consistency with agent bundle updates. --- .../controller/agentmanagement/controllers/cluster/import.go | 4 ++++ .../controllers/manageagent/manageagent_test.go | 2 ++ 2 files changed, 6 insertions(+) diff --git a/internal/cmd/controller/agentmanagement/controllers/cluster/import.go b/internal/cmd/controller/agentmanagement/controllers/cluster/import.go index 829fecdd82..cbe79e455f 100644 --- a/internal/cmd/controller/agentmanagement/controllers/cluster/import.go +++ b/internal/cmd/controller/agentmanagement/controllers/cluster/import.go @@ -357,6 +357,10 @@ func (i *importHandler) importCluster(cluster *fleet.Cluster, status fleet.Clust apiServerCA = secret.Data[config.APIServerCAKey] ) + if cfg.AgentCheckinInterval.Seconds() == 0 { + return status, errors.New("agent check-in interval cannot be 0") + } + if apiServerURL == "" { if len(cfg.APIServerURL) == 0 { // Current config cannot be deployed, so remove the "config changed" mark diff --git a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go index 7481f73449..72162401d4 100644 --- a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go +++ b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go @@ -292,6 +292,8 @@ func TestNewAgentBundle_PropagatesAgentImagePullSecrets(t *testing.T) { if tc.configPullSecrets != nil { cfg.ImagePullSecrets = *tc.configPullSecrets } + + cfg.AgentCheckinInterval = metav1.Duration{Duration: 1 * time.Second} // non-zero to prevent errors covered elsewhere. config.Set(cfg) checkRegisterAddToScheme(t, appsv1.AddToScheme) From d7367f71d799ba2f05e0abf4a98854fa0be19945 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20N=C3=A9au?= Date: Tue, 16 Jun 2026 17:45:25 +0200 Subject: [PATCH 3/4] Fix e2e test failure Patching the `fleet-controller` config map must include a non-zero agent check-in interval, otherwise cluster registration fails. --- e2e/multi-cluster/installation/agent_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/multi-cluster/installation/agent_test.go b/e2e/multi-cluster/installation/agent_test.go index c2ae99c6d4..9ceca2093e 100644 --- a/e2e/multi-cluster/installation/agent_test.go +++ b/e2e/multi-cluster/installation/agent_test.go @@ -37,7 +37,7 @@ var _ = Describe("Fleet installation with TLS agent modes", func() { "--type=merge", "-p", fmt.Sprintf( - `{"data":{"config":"{\"apiServerURL\": \"https://google.com\", \"apiServerCA\": \"\", \"agentTLSMode\": \"%s\"}"}}`, + `{"data":{"config":"{\"apiServerURL\": \"https://google.com\", \"apiServerCA\": \"\", \"agentTLSMode\": \"%s\", \"agentCheckinInterval\": \"15m\"}"}}`, // agent check-in interval must be non-zero agentMode, ), ) From 53d3d658f0e87a064cfb64e251852beb8783025b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Corentin=20N=C3=A9au?= Date: Wed, 17 Jun 2026 08:45:19 +0200 Subject: [PATCH 4/4] Harden zero checks for agent check-in interval Negative durations are now also taken into account. --- .../controllers/cluster/import.go | 4 +- .../controllers/manageagent/manageagent.go | 4 +- .../manageagent/manageagent_test.go | 44 +++++++++++++------ 3 files changed, 35 insertions(+), 17 deletions(-) diff --git a/internal/cmd/controller/agentmanagement/controllers/cluster/import.go b/internal/cmd/controller/agentmanagement/controllers/cluster/import.go index cbe79e455f..2ab872b57a 100644 --- a/internal/cmd/controller/agentmanagement/controllers/cluster/import.go +++ b/internal/cmd/controller/agentmanagement/controllers/cluster/import.go @@ -357,8 +357,8 @@ func (i *importHandler) importCluster(cluster *fleet.Cluster, status fleet.Clust apiServerCA = secret.Data[config.APIServerCAKey] ) - if cfg.AgentCheckinInterval.Seconds() == 0 { - return status, errors.New("agent check-in interval cannot be 0") + if cfg.AgentCheckinInterval.Seconds() <= 0 { + return status, errors.New("agent check-in interval cannot be 0 or less") } if apiServerURL == "" { diff --git a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent.go b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent.go index 486a83b8d4..18f11dd535 100644 --- a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent.go +++ b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent.go @@ -336,8 +336,8 @@ func (h *handler) newAgentBundle(ns string, cluster *fleet.Cluster) ([]runtime.O priorityClassName = scheduling.FleetAgentPriorityClassName } - if cfg.AgentCheckinInterval.Seconds() == 0 { - return nil, errors.New("agent check-in interval cannot be 0") + if cfg.AgentCheckinInterval.Seconds() <= 0 { + return nil, errors.New("agent check-in interval cannot be 0 or less") } if cluster.Spec.AgentTolerations != nil { diff --git a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go index 72162401d4..236d2f6fe2 100644 --- a/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go +++ b/internal/cmd/controller/agentmanagement/controllers/manageagent/manageagent_test.go @@ -1,6 +1,7 @@ package manageagent import ( + "fmt" "maps" "reflect" "strings" @@ -23,23 +24,40 @@ import ( ) func TestNewAgentBundle(t *testing.T) { - config.Set(&config.Config{AgentCheckinInterval: metav1.Duration{Duration: 0 * time.Second}}) + testCases := []struct { + agentCheckInInterval time.Duration + }{ + { + 0 * time.Second, + }, + { + -5 * time.Second, + }, + } - // ensure leader election env is set so NewLeaderElectionOptionsWithPrefix doesn't error - t.Setenv("FLEET_AGENT_ELECTION_LEASE_DURATION", "15s") - t.Setenv("FLEET_AGENT_ELECTION_RENEW_DEADLINE", "10s") - t.Setenv("FLEET_AGENT_ELECTION_RETRY_PERIOD", "2s") + for _, tc := range testCases { + t.Run(fmt.Sprintf("agent check-in interval of %s", tc.agentCheckInInterval), func(t *testing.T) { + config.Set(&config.Config{ + AgentCheckinInterval: metav1.Duration{Duration: tc.agentCheckInInterval}, + }) + + // ensure leader election env is set so NewLeaderElectionOptionsWithPrefix doesn't error + t.Setenv("FLEET_AGENT_ELECTION_LEASE_DURATION", "15s") + t.Setenv("FLEET_AGENT_ELECTION_RENEW_DEADLINE", "10s") + t.Setenv("FLEET_AGENT_ELECTION_RETRY_PERIOD", "2s") - h := handler{systemNamespace: "blah"} - obj, err := h.newAgentBundle("foo", &fleet.Cluster{Spec: fleet.ClusterSpec{AgentNamespace: "bar"}}) + h := handler{systemNamespace: "blah"} + obj, err := h.newAgentBundle("foo", &fleet.Cluster{Spec: fleet.ClusterSpec{AgentNamespace: "bar"}}) - if obj != nil { - t.Fatalf("expected obj returned by newAgentBundle to be nil") - } + if obj != nil { + t.Fatalf("expected obj returned by newAgentBundle to be nil") + } - expectedStr := "interval cannot be 0" - if !strings.Contains(err.Error(), expectedStr) { - t.Fatalf("expected error %q returned by newAgentBundle to contain %q", err, expectedStr) + expectedStr := "interval cannot be 0 or less" + if err == nil || !strings.Contains(err.Error(), expectedStr) { + t.Fatalf("expected error %v returned by newAgentBundle to contain %q", err, expectedStr) + } + }) } }