diff --git a/pkg/controllers/routing/network_routes_controller.go b/pkg/controllers/routing/network_routes_controller.go index 906e1908d..157dbb670 100644 --- a/pkg/controllers/routing/network_routes_controller.go +++ b/pkg/controllers/routing/network_routes_controller.go @@ -177,13 +177,10 @@ func (nrc *NetworkRoutingController) Run( stopCh <-chan struct{}, wg *sync.WaitGroup, ) { - var err error - if nrc.enableCNI { - nrc.updateCNIConfig() - } + mtu, cniNetConf := nrc.initCNIConfig() klog.V(1).Info("Populating ipsets.") - err = nrc.syncNodeIPSets() + err := nrc.syncNodeIPSets() if err != nil { klog.Errorf("Failed initial ipset setup: %s", err) } @@ -289,48 +286,33 @@ func (nrc *NetworkRoutingController) Run( } } - if nrc.autoMTU { - mtu, err := nrc.krNode.GetNodeMTU() + if mtu > 0 { + klog.Infof("Setting MTU of kube-bridge interface to: %d", mtu) + err = netlink.LinkSetMTU(kubeBridgeIf, mtu) if err != nil { klog.Errorf( - "Failed to find MTU for node IP: %s for intelligently setting the kube-bridge MTU "+ - "due to %s.", - nrc.krNode.GetPrimaryNodeIP(), + "Failed to set MTU for kube-bridge interface due to: %s (kubeBridgeIf: %#v, mtu: %v)", err.Error(), + kubeBridgeIf, + mtu, ) - } - if mtu > 0 { - klog.Infof("Setting MTU of kube-bridge interface to: %d", mtu) - err = netlink.LinkSetMTU(kubeBridgeIf, mtu) - if err != nil { - klog.Errorf( - "Failed to set MTU for kube-bridge interface due to: %s (kubeBridgeIf: %#v, mtu: %v)", - err.Error(), - kubeBridgeIf, - mtu, + // Update the CNI config to include the effective MTU, if any. + if currentMTU := kubeBridgeIf.Attrs().MTU; currentMTU > 0 && cniNetConf != nil && currentMTU != mtu { + klog.Warningf( + "Updating CNI config with current MTU for kube-bridge: %d", + currentMTU, ) - // need to correct kuberouter.conf because autoConfigureMTU() may have set an invalid value! - currentMTU := kubeBridgeIf.Attrs().MTU - if currentMTU > 0 && currentMTU != mtu { - klog.Warningf( - "Updating config file with current MTU for kube-bridge: %d", - currentMTU, - ) - cniNetConf, err := utils.NewCNINetworkConfig(nrc.cniConfFile) - if err == nil { - cniNetConf.SetMTU(currentMTU) - if err = cniNetConf.WriteCNIConfig(); err != nil { - klog.Errorf("Failed to update CNI config file due to: %v", err) - } - } else { - klog.Errorf("Failed to load CNI config file to reset MTU due to: %v", err) - } - } + cniNetConf.SetMTU(currentMTU) } - } else { - klog.Infof("Not setting MTU of kube-bridge interface") } } + + if cniNetConf != nil { + if err := cniNetConf.WriteCNIConfig(); err != nil { + klog.Fatalf("failed to write CNI file: %v", err) + } + } + // enable netfilter for the bridge func() { ctx, cancel := context.WithTimeout(context.Background(), maxModprobeTimeout) @@ -460,11 +442,23 @@ func (nrc *NetworkRoutingController) Run( } } -func (nrc *NetworkRoutingController) updateCNIConfig() { +func (nrc *NetworkRoutingController) initCNIConfig() (mtu int, _ *utils.CNINetworkConfig) { + if nrc.autoMTU { + if nodeMTU, err := nrc.krNode.GetNodeMTU(); err != nil { + klog.Errorf("Not setting MTU of kube-bridge interface: cannot determine the node's MTU: %v", err) + } else { + mtu = nodeMTU + } + } + + if !nrc.enableCNI { + return mtu, nil + } + // Parse the existing IPAM CIDRs from the CNI conf file cniNetConf, err := utils.NewCNINetworkConfig(nrc.cniConfFile) if err != nil { - klog.Errorf("failed to parse CNI Config: %v", err) + klog.Fatalf("failed to parse CNI Config: %v", err) } // Insert any IPv4 CIDRs that are missing from the IPAM configuration in the CNI @@ -483,20 +477,11 @@ func (nrc *NetworkRoutingController) updateCNIConfig() { } } - if nrc.autoMTU { - // Get the MTU by looking at the node's interface that is associated with the primary IP of the cluster - mtu, err := nrc.krNode.GetNodeMTU() - if err != nil { - klog.Fatalf("failed to generate MTU: %v", err) - } - + if mtu > 0 { cniNetConf.SetMTU(mtu) } - err = cniNetConf.WriteCNIConfig() - if err != nil { - klog.Fatalf("failed to write CNI file: %v", err) - } + return mtu, cniNetConf } func (nrc *NetworkRoutingController) watchBgpUpdates() { diff --git a/pkg/utils/cni.go b/pkg/utils/cni.go index 46f4f624e..a93f9e724 100644 --- a/pkg/utils/cni.go +++ b/pkg/utils/cni.go @@ -9,15 +9,15 @@ import ( "strings" ) -type cniNetworkConfig struct { - FilePath string - Conf *Conf - ConfList *ConfList +type CNINetworkConfig struct { + filePath string + conf *Conf + confList *ConfList } -func NewCNINetworkConfig(cniConfFilePath string) (*cniNetworkConfig, error) { - cniNetConf := cniNetworkConfig{ - FilePath: cniConfFilePath, +func NewCNINetworkConfig(cniConfFilePath string) (*CNINetworkConfig, error) { + cniNetConf := CNINetworkConfig{ + filePath: cniConfFilePath, } cniFileBytes, err := os.ReadFile(cniConfFilePath) @@ -35,7 +35,7 @@ func NewCNINetworkConfig(cniConfFilePath string) (*cniNetworkConfig, error) { if len(confList.Plugins) == 0 { return nil, fmt.Errorf("CNI config list %s has no plugins", cniConfFilePath) } - cniNetConf.ConfList = confList + cniNetConf.confList = confList } else { // If we're working with a conf setup conf := new(Conf) @@ -46,7 +46,7 @@ func NewCNINetworkConfig(cniConfFilePath string) (*cniNetworkConfig, error) { if conf.Type == "" { return nil, fmt.Errorf("error load CNI config, file appears to have no type: %s", cniConfFilePath) } - cniNetConf.Conf = conf + cniNetConf.conf = conf } if err = cniNetConf.consolidateSubnets(); err != nil { @@ -59,7 +59,7 @@ func NewCNINetworkConfig(cniConfFilePath string) (*cniNetworkConfig, error) { // consolidateSubnets Many people still define the legacy single subnet variation of the IPAM plugin instead of the // newer ranges variation. To account for this and make parsing simpler, we do the same thing that the official IPAM // config loader does and collapse them into ranges. -func (c *cniNetworkConfig) consolidateSubnets() error { +func (c *CNINetworkConfig) consolidateSubnets() error { brPlug := c.getBridgePlugin() if brPlug.IPAM.Subnet != "" { err := c.InsertPodCIDRIntoIPAM(brPlug.IPAM.Subnet) @@ -75,12 +75,12 @@ func (c *cniNetworkConfig) consolidateSubnets() error { // IsConfList checks to see if this CNI configuration is a *.conflist file or if it is a *.conf file. Returns true for // *.conflist, returns false for anything else. -func (c *cniNetworkConfig) IsConfList() bool { - return strings.HasSuffix(strings.ToLower(c.FilePath), ".conflist") +func (c *CNINetworkConfig) IsConfList() bool { + return strings.HasSuffix(strings.ToLower(c.filePath), ".conflist") } // getPodCIDRsMapFromCNISpec gets pod CIDR allocated to the node as a map from CNI spec file and returns it -func (c *cniNetworkConfig) getPodCIDRsMapFromCNISpec() (map[string]*net.IPNet, error) { +func (c *CNINetworkConfig) getPodCIDRsMapFromCNISpec() (map[string]*net.IPNet, error) { podCIDRs := make(map[string]*net.IPNet) ipamConfig := c.getBridgePlugin().IPAM @@ -93,7 +93,7 @@ func (c *cniNetworkConfig) getPodCIDRsMapFromCNISpec() (map[string]*net.IPNet, e _, netCIDR, err := net.ParseCIDR(item.Subnet) if err != nil { return nil, fmt.Errorf("unable to parse CIDR '%s' contained in CNI: %s", - item.Subnet, c.FilePath) + item.Subnet, c.filePath) } podCIDRs[netCIDR.String()] = netCIDR } @@ -105,7 +105,7 @@ func (c *cniNetworkConfig) getPodCIDRsMapFromCNISpec() (map[string]*net.IPNet, e } // GetPodCIDRsFromCNISpec gets pod CIDR allocated to the node from CNI spec file and returns it -func (c *cniNetworkConfig) GetPodCIDRsFromCNISpec() ([]*net.IPNet, error) { +func (c *CNINetworkConfig) GetPodCIDRsFromCNISpec() ([]*net.IPNet, error) { podCIDRMap, err := c.getPodCIDRsMapFromCNISpec() if err != nil { return nil, err @@ -118,21 +118,21 @@ func (c *cniNetworkConfig) GetPodCIDRsFromCNISpec() ([]*net.IPNet, error) { } // getBridgePlugin get the bridge plugin configuration out of the cniNetworkConfig in a consistent manner -func (c *cniNetworkConfig) getBridgePlugin() *Conf { - if c.ConfList != nil { - for _, conf := range c.ConfList.Plugins { +func (c *CNINetworkConfig) getBridgePlugin() *Conf { + if c.confList != nil { + for _, conf := range c.confList.Plugins { if conf.Type == "bridge" { return conf } } } - return c.Conf + return c.conf } // InsertPodCIDRIntoIPAM insert a new cidr into the CNI file. If the CIDR already exists in the CNI ranges, then // operation is a noop. Throws an error if either the passed cidr cannot be parsed or if there is a problem with the // CIDRs already in the CNI config. -func (c *cniNetworkConfig) InsertPodCIDRIntoIPAM(cidr string) error { +func (c *CNINetworkConfig) InsertPodCIDRIntoIPAM(cidr string) error { ipamConfig := c.getBridgePlugin().IPAM // This should have already been sanitized by the GetPodCIDR* functions before it comes to us, but you can never be @@ -158,27 +158,27 @@ func (c *cniNetworkConfig) InsertPodCIDRIntoIPAM(cidr string) error { return nil } -func (c *cniNetworkConfig) SetMTU(mtu int) { +func (c *CNINetworkConfig) SetMTU(mtu int) { brPlugin := c.getBridgePlugin() brPlugin.MTU = float64(mtu) } -func (c *cniNetworkConfig) WriteCNIConfig() error { +func (c *CNINetworkConfig) WriteCNIConfig() error { var cniBytes []byte var err error if c.IsConfList() { - cniBytes, err = json.Marshal(c.ConfList) + cniBytes, err = json.Marshal(c.confList) if err != nil { return fmt.Errorf("unable to marshal CNI ConfList: %w", err) } } else { - cniBytes, err = json.Marshal(c.Conf) + cniBytes, err = json.Marshal(c.conf) if err != nil { return fmt.Errorf("unable to marshal CNI Conf: %w", err) } } - err = os.WriteFile(c.FilePath, cniBytes, 0644) + err = os.WriteFile(c.filePath, cniBytes, 0644) if err != nil { return fmt.Errorf("failed to write into CNI conf file: %w", err) } diff --git a/pkg/utils/node.go b/pkg/utils/node.go index e3ec7c3d8..d14cb4f93 100644 --- a/pkg/utils/node.go +++ b/pkg/utils/node.go @@ -199,7 +199,8 @@ func (n *LocalKRNode) GetNodeMTU() (int, error) { } } } - return 0, errors.New("failed to find interface with specified node IP") + + return 0, fmt.Errorf("primary node IP %s is not assigned to any interface", n.PrimaryIP) } // GetNodeIPAddrs returns all of the node's IP addresses (whether internal or external) as defined by the Kubernetes