Concern only about flannel ip addresses

Currently flannel interface ip addresses are checked on startup when
using vxlan and ipip backends. If multiple addresses are found, startup
fails fatally. If only one address is found and is not the currently
leased one, it will be assumed that it comes from a previous lease and
be removed.

This criteria seems arbitrary both in how it is done and in its timing.
It may cause failures in situations where it might not be strictly
necessary like for example if the node is running a dhcp client that is
assigning link local addresses to all interfaces. It also might fail at
flannel unexpected restarts which are completly unrelated to
the external event that caused the unexpected modification in the
flannel interface.

This patch proposes to concern and check only ip address within the
flannel network and takes the simple approach to ignore any other ip
addresses assuming these would pose no problem on flannel operation.

A discarded but more agressive alternative would be to remove all
addresses that are not the currently leased one.

Fixes #1060

Signed-off-by: Jaime Caamaño Ruiz <[email protected]>

backend/ipip/ipip.go

@@ -88,7 +88,7 @@ func (be *IPIPBackend) RegisterNetwork(ctx context.Context, wg *sync.WaitGroup,
 		return nil, fmt.Errorf("failed to acquire lease: %v", err)
 	}
 
-	link, err := be.configureIPIPDevice(n.SubnetLease)
+	link, err := be.configureIPIPDevice(n.SubnetLease, config.Network)
 
 	if err != nil {
 		return nil, err
@@ -123,7 +123,7 @@ func (be *IPIPBackend) RegisterNetwork(ctx context.Context, wg *sync.WaitGroup,
 	return n, nil
 }
 
-func (be *IPIPBackend) configureIPIPDevice(lease *subnet.Lease) (*netlink.Iptun, error) {
+func (be *IPIPBackend) configureIPIPDevice(lease *subnet.Lease, flannelnet ip.IP4Net) (*netlink.Iptun, error) {
 	// When modprobe ipip module, a tunl0 ipip device is created automatically per network namespace by ipip kernel module.
 	// It is the namespace default IPIP device with attributes local=any and remote=any.
 	// When receiving IPIP protocol packets, kernel will forward them to tunl0 as a fallback device
@@ -195,7 +195,7 @@ func (be *IPIPBackend) configureIPIPDevice(lease *subnet.Lease) (*netlink.Iptun,
 	// Ensure that the device has a /32 address so that no broadcast routes are created.
 	// This IP is just used as a source address for host to workload traffic (so
 	// the return path for the traffic has an address on the flannel network to use as the destination)
-	if err := ip.EnsureV4AddressOnLink(ip.IP4Net{IP: lease.Subnet.IP, PrefixLen: 32}, link); err != nil {
+	if err := ip.EnsureV4AddressOnLink(ip.IP4Net{IP: lease.Subnet.IP, PrefixLen: 32}, flannelnet, link); err != nil {
 		return nil, fmt.Errorf("failed to ensure address of interface %s: %s", link.Attrs().Name, err)
 	}
 

backend/vxlan/device.go

@@ -112,8 +112,8 @@ func ensureLink(vxlan *netlink.Vxlan) (*netlink.Vxlan, error) {
 	return vxlan, nil
 }
 
-func (dev *vxlanDevice) Configure(ipn ip.IP4Net) error {
-	if err := ip.EnsureV4AddressOnLink(ipn, dev.link); err != nil {
+func (dev *vxlanDevice) Configure(ipa ip.IP4Net, flannelnet ip.IP4Net) error {
+	if err := ip.EnsureV4AddressOnLink(ipa, flannelnet, dev.link); err != nil {
 		return fmt.Errorf("failed to ensure address of interface %s: %s", dev.link.Attrs().Name, err)
 	}
 

backend/vxlan/vxlan.go

@@ -155,7 +155,7 @@ func (be *VXLANBackend) RegisterNetwork(ctx context.Context, wg *sync.WaitGroup,
 	// Ensure that the device has a /32 address so that no broadcast routes are created.
 	// This IP is just used as a source address for host to workload traffic (so
 	// the return path for the traffic has an address on the flannel network to use as the destination)
-	if err := dev.Configure(ip.IP4Net{IP: lease.Subnet.IP, PrefixLen: 32}); err != nil {
+	if err := dev.Configure(ip.IP4Net{IP: lease.Subnet.IP, PrefixLen: 32}, config.Network); err != nil {
 		return nil, fmt.Errorf("failed to configure interface %s: %s", dev.link.Attrs().Name, err)
 	}
 

pkg/ip/iface.go

@@ -23,6 +23,7 @@ import (
 	"syscall"
 
 	"github.com/vishvananda/netlink"
+	log "k8s.io/klog"
 )
 
 func getIfaceAddrs(iface *net.Interface) ([]netlink.Addr, error) {
@@ -131,32 +132,33 @@ func DirectRouting(ip net.IP) (bool, error) {
 	return false, nil
 }
 
-// EnsureV4AddressOnLink ensures that there is only one v4 Addr on `link` and it equals `ipn`.
-// If there exist multiple addresses on link, it returns an error message to tell callers to remove additional address.
-func EnsureV4AddressOnLink(ipn IP4Net, link netlink.Link) error {
-	addr := netlink.Addr{IPNet: ipn.ToIPNet()}
+// EnsureV4AddressOnLink ensures that there is only one v4 Addr on `link` within the `ipn` address space and it equals `ipa`.
+func EnsureV4AddressOnLink(ipa IP4Net, ipn IP4Net, link netlink.Link) error {
+	addr := netlink.Addr{IPNet: ipa.ToIPNet()}
 	existingAddrs, err := netlink.AddrList(link, netlink.FAMILY_V4)
 	if err != nil {
 		return err
 	}
 
-	// flannel will never make this happen. This situation can only be caused by a user, so get them to sort it out.
-	if len(existingAddrs) > 1 {
-		return fmt.Errorf("link has incompatible addresses. Remove additional addresses and try again. %#v", link)
-	}
+	var hasAddr bool
+	for _, existingAddr := range existingAddrs {
+		if existingAddr.Equal(addr) {
+			hasAddr = true
+			continue
+		}
 
-	// If the device has an incompatible address then delete it. This can happen if the lease changes for example.
-	if len(existingAddrs) == 1 && !existingAddrs[0].Equal(addr) {
-		if err := netlink.AddrDel(link, &existingAddrs[0]); err != nil {
-			return fmt.Errorf("failed to remove IP address %s from %s: %s", ipn.String(), link.Attrs().Name, err)
+		if ipn.Contains(FromIP(existingAddr.IP)) {
+			if err := netlink.AddrDel(link, &existingAddr); err != nil {
+				return fmt.Errorf("failed to remove IP address %s from %s: %s", existingAddr.String(), link.Attrs().Name, err)
+			}
+			log.Infof("removed IP address %s from %s", existingAddr.String(), link.Attrs().Name)
 		}
-		existingAddrs = []netlink.Addr{}
 	}
 
 	// Actually add the desired address to the interface if needed.
-	if len(existingAddrs) == 0 {
+	if !hasAddr {
 		if err := netlink.AddrAdd(link, &addr); err != nil {
-			return fmt.Errorf("failed to add IP address %s to %s: %s", ipn.String(), link.Attrs().Name, err)
+			return fmt.Errorf("failed to add IP address %s to %s: %s", addr.String(), link.Attrs().Name, err)
 		}
 	}
 

pkg/ip/iface_test.go

@@ -35,7 +35,8 @@ func TestEnsureV4AddressOnLink(t *testing.T) {
 		t.Fatal(err)
 	}
 	// check changing address
-	if err := EnsureV4AddressOnLink(IP4Net{IP: FromIP(net.ParseIP("127.0.0.2")), PrefixLen: 24}, lo); err != nil {
+	ipn := IP4Net{IP: FromIP(net.ParseIP("127.0.0.2")), PrefixLen: 24}
+	if err := EnsureV4AddressOnLink(ipn, ipn, lo); err != nil {
 		t.Fatal(err)
 	}
 	addrs, err := netlink.AddrList(lo, netlink.FAMILY_V4)
@@ -46,11 +47,18 @@ func TestEnsureV4AddressOnLink(t *testing.T) {
 		t.Fatalf("addrs %v is not expected", addrs)
 	}
 
-	// check changing address if there exist multiple addresses
-	if err := netlink.AddrAdd(lo, &netlink.Addr{IPNet: &net.IPNet{IP: net.ParseIP("127.0.0.3"), Mask: net.CIDRMask(24, 32)}}); err != nil {
+	// check changing address if there exist unknown addresses
+	if err := netlink.AddrAdd(lo, &netlink.Addr{IPNet: &net.IPNet{IP: net.ParseIP("127.0.1.1"), Mask: net.CIDRMask(24, 32)}}); err != nil {
 		t.Fatal(err)
 	}
-	if err := EnsureV4AddressOnLink(IP4Net{IP: FromIP(net.ParseIP("127.0.0.2")), PrefixLen: 24}, lo); err == nil {
-		t.Fatal("EnsureV4AddressOnLink should return error if there exist multiple address on link")
+	if err := EnsureV4AddressOnLink(ipn, ipn, lo); err != nil {
+		t.Fatal(err)
+	}
+	addrs, err = netlink.AddrList(lo, netlink.FAMILY_V4)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(addrs) != 2 {
+		t.Fatalf("two addresses expected, addrs: %v", addrs)
 	}
 }