Merge pull request #1401 from jcaamano/master

Concern only about flannel ip addresses

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