nodediscovery: make CiliumNode update retry budget configurable

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

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

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

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

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,15 +293,15 @@ 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")
 				return
 			}
 		}
 	}
-	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 {

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