daemon, node: Remove old, discarded router IPs from cilium_host

daemon/cmd/daemon.go

@@ -79,6 +79,7 @@ import (
 	cnitypes "github.com/cilium/cilium/plugins/cilium-cni/types"
 
 	"github.com/sirupsen/logrus"
+	"github.com/vishvananda/netlink"
 	"golang.org/x/sync/semaphore"
 )
 
@@ -274,7 +275,50 @@ func restoreCiliumHostIPs(ipv6 bool, fromK8s net.IP) {
 		fromFS = node.GetInternalIPv4Router()
 	}
 
-	node.RestoreHostIPs(ipv6, fromK8s, fromFS, cidr)
+	restoredIP := node.RestoreHostIPs(ipv6, fromK8s, fromFS, cidr)
+	if err := removeOldRouterState(restoredIP); err != nil {
+		log.WithError(err).Warnf(
+			"Failed to remove old router IPs (restored IP: %s) from cilium_host. Manual intervention is required to remove all other old IPs.",
+			restoredIP,
+		)
+	}
+}
+
+// removeOldRouterState will try to ensure that the only IP assigned to the
+// `cilium_host` interface is the given restored IP. If the given IP is nil,
+// then it attempts to clear all IPs from the interface.
+func removeOldRouterState(restoredIP net.IP) error {
+	l, err := netlink.LinkByName(defaults.HostDevice)
+	if err != nil {
+		return err
+	}
+
+	family := netlink.FAMILY_V6
+	if restoredIP.To4() != nil {
+		family = netlink.FAMILY_V4
+	}
+	addrs, err := netlink.AddrList(l, family)
+	if err != nil {
+		return err
+	}
+	if len(addrs) > 1 {
+		log.Info("More than one router IP was found on the cilium_host device after restoration, cleaning up old router IPs.")
+	}
+
+	var errs []error
+	for _, a := range addrs {
+		if restoredIP != nil && restoredIP.Equal(a.IP) {
+			continue
+		}
+		if err := netlink.AddrDel(l, &a); err != nil {
+			errs = append(errs, fmt.Errorf("failed to remove IP %s: %w", a.IP, err))
+		}
+	}
+	if len(errs) > 0 {
+		return fmt.Errorf("failed to remove all old router IPs: %v", errs)
+	}
+
+	return nil
 }
 
 // NewDaemon creates and returns a new Daemon with the parameters set in c.

pkg/node/address.go

@@ -375,9 +375,11 @@ func AutoComplete() error {
 // filesystem to be the most up-to-date source of truth. The chosen router IP
 // is then checked whether it is contained inside node CIDR (pod CIDR) range.
 // If not, then the router IP is discarded and not restored.
-func RestoreHostIPs(ipv6 bool, fromK8s, fromFS net.IP, cidr *cidr.CIDR) {
+//
+// The restored IP is returned.
+func RestoreHostIPs(ipv6 bool, fromK8s, fromFS net.IP, cidr *cidr.CIDR) net.IP {
 	if !option.Config.EnableHostIPRestore {
-		return
+		return nil
 	}
 
 	var (
@@ -398,6 +400,9 @@ func RestoreHostIPs(ipv6 bool, fromK8s, fromFS net.IP, cidr *cidr.CIDR) {
 			"The router IP (%s) considered for restoration does not belong in the Pod CIDR of the node. Discarding old router IP.",
 			ip,
 		)
+		// Indicate that this IP will not be restored by setting to nil after
+		// we've used it to log above.
+		ip = nil
 		setter(nil)
 	case err != nil && errors.Is(err, errMismatch):
 		log.Warnf(
@@ -408,16 +413,31 @@ func RestoreHostIPs(ipv6 bool, fromK8s, fromFS net.IP, cidr *cidr.CIDR) {
 	case err == nil:
 		setter(ip)
 	}
+
+	return ip
 }
 
 func chooseHostIPsToRestore(ipv6 bool, fromK8s, fromFS net.IP, cidr *cidr.CIDR) (ip net.IP, err error) {
 	switch {
+	// If both IPs are available, then check both for validity. We prefer the
+	// local IP from the FS over the K8s IP.
 	case fromK8s != nil && fromFS != nil:
 		if fromK8s.Equal(fromFS) {
 			ip = fromK8s
 		} else {
 			ip = fromFS
 			err = errMismatch
+
+			// Check if we need to fallback to using the fromK8s IP, in the
+			// case that the IP from the FS is not within the CIDR. If we
+			// fallback, then we also need to check the fromK8s IP is also
+			// within the CIDR.
+			if cidr != nil && cidr.Contains(ip) {
+				return
+			} else if cidr != nil && cidr.Contains(fromK8s) {
+				ip = fromK8s
+				return
+			}
 		}
 	case fromK8s == nil && fromFS != nil:
 		ip = fromFS