From 649a087ef6026b0ecca51d1150a29034dd30ebc6 Mon Sep 17 00:00:00 2001 From: tanle-ux Date: Wed, 10 Jun 2026 06:06:15 -0700 Subject: [PATCH] nodediscovery, k8s/client: make API-server coupling timeouts configurable cilium-agent is hard-coupled to the kube-apiserver on two paths, both of which exit the agent during a control-plane / API server outage and drive a cluster-wide crash-loop: 1. Running agent (nodediscovery): updateCiliumNodeResource retries a fixed 10x / 500ms and then logging.Fatal-exits when it cannot write its CiliumNode resource. 2. Cold start (k8s client): waitForConn retries the initial connection for a hardcoded 1 minute (connTimeout) and then fails the hive start hook, exiting the agent before the datapath comes up. Make both budgets configurable so operators can tune how long the agent tolerates an API server outage before giving up: --cilium-node-update-max-retries (CiliumNode create/update attempts) --cilium-node-update-retry-backoff (backoff between attempts) --k8s-client-connection-retry-timeout (cold-start initial-connection wait) Non-positive values fall back to the built-in defaults. NOTE: for control-plane-outage testing the defaults are temporarily set very large so the agent keeps retrying instead of exiting: - cilium-node-update-max-retries: 1000000 (was 10) - k8s-client-connection-retry-timeout: 168h (was 1m) - HiveStartTimeout: 168h (was 5m, raised so the cold-start wait is not capped by the start-hook timeout) Revert all three to their previous values before merging upstream. Adds TestUpdateCiliumNodeResourceConfigurableRetries. Co-Authored-By: Claude Opus 4.8 Signed-off-by: tanle-ux --- pkg/defaults/defaults.go | 5 +- pkg/k8s/client/cell.go | 12 ++++- pkg/k8s/client/config.go | 17 ++++++- pkg/nodediscovery/cell.go | 19 ++++++++ pkg/nodediscovery/nodediscovery.go | 20 ++++++-- pkg/nodediscovery/nodediscovery_test.go | 64 +++++++++++++++++++++++++ 6 files changed, 130 insertions(+), 7 deletions(-) diff --git a/pkg/defaults/defaults.go b/pkg/defaults/defaults.go index 4ece394a92892..fa0fb98723a1e 100644 --- a/pkg/defaults/defaults.go +++ b/pkg/defaults/defaults.go @@ -10,7 +10,10 @@ import ( // Hive options const ( // HiveStartTimeout is the default value for option.HiveStartTimeout - HiveStartTimeout = 5 * time.Minute + // NOTE: temporarily very large for control-plane-outage testing so the + // agent does not abort start hooks (e.g. the API server cold-start wait) + // too soon. Revert to 5 * time.Minute before merging upstream. + HiveStartTimeout = 168 * time.Hour // HiveStopTimeout is the default value for option.HiveStopTimeout HiveStopTimeout = time.Minute diff --git a/pkg/k8s/client/cell.go b/pkg/k8s/client/cell.go index d1868a96470fe..aca9981c86c2c 100644 --- a/pkg/k8s/client/cell.go +++ b/pkg/k8s/client/cell.go @@ -310,8 +310,18 @@ func (c *compositeClientset) startHeartbeat() { } func (c *compositeClientset) waitForConn(ctx context.Context) error { + // How long to keep retrying the initial connection before giving up is + // configurable so the agent does not terminate too soon during a + // control-plane / API server outage. Fall back to the built-in default + // when unset. The effective wait is still bounded by ctx (the hive + // start-hook timeout). + connRetryTimeout := connTimeout + if c.config.K8sClientConnectionRetryTimeout > 0 { + connRetryTimeout = c.config.K8sClientConnectionRetryTimeout + } + stop := make(chan struct{}) - timeout := time.NewTimer(connTimeout) + timeout := time.NewTimer(connRetryTimeout) defer timeout.Stop() var err error wait.Until(func() { diff --git a/pkg/k8s/client/config.go b/pkg/k8s/client/config.go index ee2c4bb990a28..6231630ea0edc 100644 --- a/pkg/k8s/client/config.go +++ b/pkg/k8s/client/config.go @@ -40,6 +40,14 @@ type SharedConfig struct { // K8sHeartbeatTimeout configures the timeout for apiserver heartbeat K8sHeartbeatTimeout time.Duration + // K8sClientConnectionRetryTimeout configures how long the agent keeps + // retrying to establish its initial connection to the API server at + // startup (cold start) before giving up and exiting. Increase to avoid + // terminating too soon during a control-plane / API server outage. The + // effective wait is min(this, --hive-start-timeout), so raise + // --hive-start-timeout as well for values above its default. + K8sClientConnectionRetryTimeout time.Duration + // EnableAPIDiscovery enables Kubernetes API discovery EnableK8sAPIDiscovery bool } @@ -69,7 +77,13 @@ var defaultSharedConfig = SharedConfig{ K8sClientConnectionTimeout: 30 * time.Second, K8sClientConnectionKeepAlive: 30 * time.Second, K8sHeartbeatTimeout: 30 * time.Second, - EnableK8sAPIDiscovery: defaults.K8sEnableAPIDiscovery, + // NOTE: temporarily very large for control-plane-outage testing so the + // agent does not terminate at cold start when the API server is + // unreachable. Revert to 1 minute (the previous hardcoded connTimeout) + // before merging upstream. The default HiveStartTimeout is raised to + // match so this is not capped at start-hook timeout. + K8sClientConnectionRetryTimeout: 168 * time.Hour, + EnableK8sAPIDiscovery: defaults.K8sEnableAPIDiscovery, } func (def SharedConfig) Flags(flags *pflag.FlagSet) { @@ -79,6 +93,7 @@ func (def SharedConfig) Flags(flags *pflag.FlagSet) { flags.Duration(option.K8sClientConnectionTimeout, def.K8sClientConnectionTimeout, "Configures the timeout of K8s client connections. K8s client is disabled if the value is set to 0") flags.Duration(option.K8sClientConnectionKeepAlive, def.K8sClientConnectionKeepAlive, "Configures the keep alive duration of K8s client connections. K8 client is disabled if the value is set to 0") flags.Duration(option.K8sHeartbeatTimeout, def.K8sHeartbeatTimeout, "Configures the timeout for api-server heartbeat, set to 0 to disable") + flags.Duration("k8s-client-connection-retry-timeout", def.K8sClientConnectionRetryTimeout, "Maximum time the agent keeps retrying its initial API server connection at startup before exiting. Increase to avoid terminating too soon during a control-plane / API server outage. Effective wait is min(this, --hive-start-timeout). A non-positive value falls back to the built-in default") flags.Bool(option.K8sEnableAPIDiscovery, def.EnableK8sAPIDiscovery, "Enable discovery of Kubernetes API groups and resources with the discovery API") } diff --git a/pkg/nodediscovery/cell.go b/pkg/nodediscovery/cell.go index edce21a9d16ab..f071145860b2a 100644 --- a/pkg/nodediscovery/cell.go +++ b/pkg/nodediscovery/cell.go @@ -9,6 +9,7 @@ import ( "github.com/cilium/cilium/pkg/defaults" "github.com/cilium/cilium/pkg/endpoint/regeneration" + "github.com/cilium/cilium/pkg/time" ) // The node discovery cell provides the local node configuration and node discovery @@ -39,6 +40,12 @@ var defaultConfig = config{ IPAMMaxAllocate: 0, IPAMStaticIPTags: map[string]string{}, + // NOTE: temporarily very large for control-plane-outage testing so the + // agent keeps retrying instead of exiting. Revert to maxRetryCount (10) + // before merging upstream. + CiliumNodeUpdateMaxRetries: 1000000, + CiliumNodeUpdateRetryBackoff: backoffDuration, + ENIFirstInterfaceIndex: defaults.ENIFirstInterfaceIndex, ENISubnetIDs: []string{}, ENISubnetTags: map[string]string{}, @@ -63,6 +70,15 @@ type config struct { IPAMMaxAllocate int IPAMStaticIPTags map[string]string + // CiliumNodeUpdateMaxRetries is the number of attempts the agent makes to + // create or update its own CiliumNode resource against the API server + // before giving up and exiting. A higher value lets the agent tolerate a + // longer control-plane / API server outage without crash-looping. + CiliumNodeUpdateMaxRetries int + // CiliumNodeUpdateRetryBackoff is the backoff between CiliumNode create or + // update retries against the API server. + CiliumNodeUpdateRetryBackoff time.Duration + ENIFirstInterfaceIndex int ENISubnetIDs []string ENISubnetTags map[string]string @@ -87,6 +103,9 @@ func (c config) Flags(flags *pflag.FlagSet) { flags.Int("ipam-max-allocate", c.IPAMMaxAllocate, "Maximum number of IPs that can be allocated at the node level") flags.StringToString("ipam-static-ip-tags", c.IPAMStaticIPTags, "List of tags to determine the pool of IPs from which to attribute a static IP to the node at the node level, this currently works with AWS and Azure") + flags.Int("cilium-node-update-max-retries", c.CiliumNodeUpdateMaxRetries, "Number of attempts to create or update the CiliumNode resource against the API server before the agent gives up and exits. Increase to tolerate longer control-plane / API server outages without crash-looping") + flags.Duration("cilium-node-update-retry-backoff", c.CiliumNodeUpdateRetryBackoff, "Backoff duration between CiliumNode create/update retries against the API server") + flags.Int("eni-first-interface-index", c.ENIFirstInterfaceIndex, "Index of the first ENI to use for IP allocation at the node level") flags.StringToString("eni-exclude-interface-tags", c.ENIExcludeInterfaceTags, "List of tags to use when excluding ENIs for Cilium IP allocation at the node level") flags.StringSlice("eni-subnet-ids", c.ENISubnetIDs, "List of subnet ids to use when evaluating what AWS subnets to use for ENI and IP allocation at the node level") diff --git a/pkg/nodediscovery/nodediscovery.go b/pkg/nodediscovery/nodediscovery.go index bf8243836ae2a..127a5af25222a 100644 --- a/pkg/nodediscovery/nodediscovery.go +++ b/pkg/nodediscovery/nodediscovery.go @@ -227,10 +227,22 @@ func (n *NodeDiscovery) updateCiliumNodeResource(ctx context.Context, ln *node.L logfields.Node, nodeTypes.GetName(), ) + // Retry count and backoff are configurable so the agent can tolerate a + // longer control-plane / API server outage before giving up and exiting. + // Fall back to the package defaults if unset (e.g. zero-valued config). + maxRetries := n.config.CiliumNodeUpdateMaxRetries + if maxRetries <= 0 { + maxRetries = maxRetryCount + } + retryBackoff := n.config.CiliumNodeUpdateRetryBackoff + if retryBackoff <= 0 { + retryBackoff = backoffDuration + } + performGet := true var nodeResource *ciliumv2.CiliumNode var lastErr error - for retryCount := range maxRetryCount { + for retryCount := range maxRetries { performUpdate := true if performGet { var err error @@ -271,7 +283,7 @@ func (n *NodeDiscovery) updateCiliumNodeResource(ctx context.Context, ln *node.L lastErr = err n.logger.Info("Unable to update CiliumNode resource, will retry", logfields.Error, err) // Backoff before retrying - time.Sleep(backoffDuration) + time.Sleep(retryBackoff) continue } else { return @@ -281,7 +293,7 @@ func (n *NodeDiscovery) updateCiliumNodeResource(ctx context.Context, ln *node.L lastErr = err n.logger.Info("Unable to create CiliumNode resource, will retry", logfields.Error, err) // Backoff before retrying - time.Sleep(backoffDuration) + time.Sleep(retryBackoff) continue } else { n.logger.Info("Successfully created CiliumNode resource") @@ -289,7 +301,7 @@ func (n *NodeDiscovery) updateCiliumNodeResource(ctx context.Context, ln *node.L } } } - logging.Fatal(n.logger, "Could not create or update CiliumNode resource", logfields.Error, lastErr, logfields.Retries, maxRetryCount) + logging.Fatal(n.logger, "Could not create or update CiliumNode resource", logfields.Error, lastErr, logfields.Retries, maxRetries) } func (n *NodeDiscovery) mutateNodeResource(ctx context.Context, nodeResource *ciliumv2.CiliumNode, ln *node.LocalNode) error { diff --git a/pkg/nodediscovery/nodediscovery_test.go b/pkg/nodediscovery/nodediscovery_test.go index 5176477d82584..a35ff07a951e3 100644 --- a/pkg/nodediscovery/nodediscovery_test.go +++ b/pkg/nodediscovery/nodediscovery_test.go @@ -21,6 +21,7 @@ import ( "github.com/cilium/cilium/pkg/node" nodeTypes "github.com/cilium/cilium/pkg/node/types" "github.com/cilium/cilium/pkg/option" + "github.com/cilium/cilium/pkg/time" cnitypes "github.com/cilium/cilium/plugins/cilium-cni/types" ) @@ -97,3 +98,66 @@ func TestUpdateCiliumNodeResourceTransientErrorCausesFatal(t *testing.T) { require.Equal(t, maxRetryCount, updateCalls, "transient errors should be retried %d times, not fataled immediately", maxRetryCount) } + +// TestUpdateCiliumNodeResourceConfigurableRetries verifies that the number of +// CiliumNode update attempts before the agent gives up and exits is driven by +// the configurable CiliumNodeUpdateMaxRetries, so operators can tune how long +// the agent tolerates an API server outage instead of being fixed at the +// compiled-in default. +func TestUpdateCiliumNodeResourceConfigurableRetries(t *testing.T) { + option.Config.AutoCreateCiliumNodeResource = true + option.Config.IPAM = "" + t.Cleanup(func() { + option.Config.AutoCreateCiliumNodeResource = false + }) + + const nodeName = "test-node" + nodeTypes.SetName(nodeName) + + // Cover values both below and above the default (maxRetryCount == 10) to + // show the retry budget is honored in either direction. + for _, maxRetries := range []int{1, 3, 15} { + t.Run(fmt.Sprintf("maxRetries=%d", maxRetries), func(t *testing.T) { + logging.RegisterExitHandler(func() { panic("fatal called") }) + t.Cleanup(func() { logging.RegisterExitHandler(func() {}) }) + + fakeClient, _ := clienttestutils.NewFakeClientset(hivetest.Logger(t)) + + existingNode := &ciliumv2.CiliumNode{ + ObjectMeta: metav1.ObjectMeta{Name: nodeName}, + } + require.NoError(t, fakeClient.CiliumFakeClientset.Tracker().Add(existingNode)) + + updateCalls := 0 + fakeClient.CiliumFakeClientset.PrependReactor("update", "ciliumnodes", + func(_ k8stesting.Action) (bool, runtime.Object, error) { + updateCalls++ + return true, nil, fmt.Errorf("connection reset by peer") + }, + ) + + nd := &NodeDiscovery{ + logger: hivetest.Logger(t), + clientset: fakeClient, + k8sGetters: &mockK8sGetters{ciliumNode: existingNode}, + cniConfigManager: &mockCNIConfigManager{}, + config: config{ + CiliumNodeUpdateMaxRetries: maxRetries, + CiliumNodeUpdateRetryBackoff: time.Millisecond, + }, + } + + ln := &node.LocalNode{ + Node: nodeTypes.Node{Name: nodeName}, + Local: &node.LocalNodeInfo{}, + } + + require.Panics(t, func() { + nd.updateCiliumNodeResource(context.Background(), ln) + }) + + require.Equal(t, maxRetries, updateCalls, + "update should be retried exactly CiliumNodeUpdateMaxRetries (%d) times before exiting", maxRetries) + }) + } +}