Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Concern only about flannel ip addresses #1401

Merged
merged 1 commit into from
Mar 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions backend/ipip/ipip.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
}

Expand Down
4 changes: 2 additions & 2 deletions backend/vxlan/device.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
2 changes: 1 addition & 1 deletion backend/vxlan/vxlan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down
32 changes: 17 additions & 15 deletions pkg/ip/iface.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import (
"syscall"

"github.com/vishvananda/netlink"
log "k8s.io/klog"
)

func getIfaceAddrs(iface *net.Interface) ([]netlink.Addr, error) {
Expand Down Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we log this as Info nevertheless? Useful since we are doing kind of a 'destructive' operation.

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

Expand Down
18 changes: 13 additions & 5 deletions pkg/ip/iface_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
}
}