fix: Adding ipv6 check on the node init function#10
fix: Adding ipv6 check on the node init function#10MitchLewis930 wants to merge 1 commit intopr_050_beforefrom
Conversation
Cilium pods on a single stack cluster were failing when ipv6 was enabled. The change would make sure that if the underlying node does not have ipv6 configuration enabled it would fail even if KPR is disabled.Earlier, if KPR is enabled, the node would not see a fatal error. Signed-off-by: Vipul Singh <singhvipul@microsoft.com>
There was a problem hiding this comment.
Pull request overview
This PR adds validation to ensure that nodes have IPv6 addresses configured when IPv6 is enabled in the cluster configuration.
Changes:
- Adds an early validation check for IPv6 address presence when IPv6 is enabled
- Returns an error with detailed logging if a node lacks an IPv6 address despite IPv6 being enabled
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If the host does not have an IPv6 address, return an error | ||
| if option.Config.EnableIPv6 && nodeIP6 == nil { | ||
| log.WithFields(logrus.Fields{ | ||
| logfields.NodeName: n.Name, | ||
| logfields.IPAddr + ".ipv4": nodeIP4, | ||
| logfields.IPAddr + ".ipv6": nodeIP6, | ||
| }).Error("No IPv6 support on node as ipv6 address is nil") | ||
| return fmt.Errorf("node %s does not have an IPv6 address", n.Name) |
There was a problem hiding this comment.
The IPv6 validation occurs after logging 'Received own node information from API server' (line 129), but before calling useNodeCIDR(n) (line 141). However, useNodeCIDR already handles the case where IPv6 is enabled but the IPv6 CIDR is nil by checking n.IPv6AllocCIDR != nil. This early error return prevents legitimate scenarios where a node might not have an IPv6 address assigned yet but could receive one later. Consider whether this strict validation is necessary or if the existing nil checks in useNodeCIDR are sufficient.
| // If the host does not have an IPv6 address, return an error | |
| if option.Config.EnableIPv6 && nodeIP6 == nil { | |
| log.WithFields(logrus.Fields{ | |
| logfields.NodeName: n.Name, | |
| logfields.IPAddr + ".ipv4": nodeIP4, | |
| logfields.IPAddr + ".ipv6": nodeIP6, | |
| }).Error("No IPv6 support on node as ipv6 address is nil") | |
| return fmt.Errorf("node %s does not have an IPv6 address", n.Name) | |
| // If the host does not have an IPv6 address, log a warning but continue; | |
| // useNodeCIDR() will enforce any required IPv6 PodCIDR constraints. | |
| if option.Config.EnableIPv6 && nodeIP6 == nil { | |
| log.WithFields(logrus.Fields{ | |
| logfields.NodeName: n.Name, | |
| logfields.IPAddr + ".ipv4": nodeIP4, | |
| logfields.IPAddr + ".ipv6": nodeIP6, | |
| }).Warning("IPv6 is enabled but node IPv6 address is currently nil; proceeding and relying on CIDR-based checks") |
| logfields.NodeName: n.Name, | ||
| logfields.IPAddr + ".ipv4": nodeIP4, | ||
| logfields.IPAddr + ".ipv6": nodeIP6, | ||
| }).Error("No IPv6 support on node as ipv6 address is nil") |
There was a problem hiding this comment.
The error message 'No IPv6 support on node as ipv6 address is nil' is technically inaccurate. The node may support IPv6, but simply doesn't have an IPv6 address configured. Consider changing to 'IPv6 is enabled but node does not have an IPv6 address configured' for clarity.
2 participants
PR_050