Skip to content

Commit

Permalink
iproute2: address: make Exact idempotent
Browse files Browse the repository at this point in the history
Idempotence against address not available for Exact is not a bad idea if
we look at the name of that func

This should accomodate the race condition where keepalived will remove
vips should the primary address used for vrrp comms is removed, causing
later addr del to fail with "cannot assign requested address"
  • Loading branch information
yousong committed Jul 14, 2020
1 parent 1881789 commit 4b6d385
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 16 deletions.
16 changes: 15 additions & 1 deletion pkg/util/iproute2/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ package iproute2

import (
"net"
"syscall"

"github.com/vishvananda/netlink"
)
Expand All @@ -25,12 +26,17 @@ type Address struct {

addrBad bool
addrs []*netlink.Addr

testcb func()
}

func nop() {}

func NewAddress(ifname string, addresses ...string) *Address {
l := NewLink(ifname)
r := &Address{
Link: l,
Link: l,
testcb: nop,
}

addrs := make([]*netlink.Addr, len(addresses))
Expand Down Expand Up @@ -76,6 +82,7 @@ func (address *Address) Exact() *Address {
if err != nil {
address.addErr(err, "Exact: AddrList")
}
address.testcb()
for _, oldAddr := range oldAddrs {
del := true
for _, addr := range address.addrs {
Expand All @@ -87,6 +94,13 @@ func (address *Address) Exact() *Address {
if del {
err := netlink.AddrDel(link, &oldAddr)
if err != nil {
if errno, ok := err.(syscall.Errno); ok {
switch errno {
case syscall.EADDRNOTAVAIL:
// "cannot assign requested address"
continue
}
}
address.addErr(err, "Exact: AddrDel %s", oldAddr)
}
}
Expand Down
59 changes: 44 additions & 15 deletions pkg/util/iproute2/address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,24 +19,24 @@ import (
)

func TestAddress(t *testing.T) {
ifname := genDummyName(t)
dum := addDummy(t, ifname)
defer delDummy(t, dum)
t.Run("exact some", func(t *testing.T) {
ifname := genDummyName(t)
dum := addDummy(t, ifname)
defer delDummy(t, dum)

emptyT := func(t *testing.T) {
l := NewAddress(ifname)
l.Exact()
if err := l.Err(); err != nil {
t.Fatalf("got err: %v", err)
}
if ipnets, err := l.List4(); err != nil {
t.Fatalf("list4 err: %v", err)
} else if len(ipnets) > 0 {
t.Fatalf("want empty, got %#v", ipnets)
emptyT := func(t *testing.T) {
l := NewAddress(ifname)
l.Exact()
if err := l.Err(); err != nil {
t.Fatalf("got err: %v", err)
}
if ipnets, err := l.List4(); err != nil {
t.Fatalf("list4 err: %v", err)
} else if len(ipnets) > 0 {
t.Fatalf("want empty, got %#v", ipnets)
}
}
}

t.Run("exact some", func(t *testing.T) {
want := "10.168.222.236/24"
l := NewAddress(ifname, want, "fe80::222:d5ff:fe9e:28d1/64")
l.Exact()
Expand All @@ -54,6 +54,35 @@ func TestAddress(t *testing.T) {
t.Run("empty empty", emptyT)
})

t.Run("exact indempotent", func(t *testing.T) {
ifname := genDummyName(t)
dum := addDummy(t, ifname)
defer delDummy(t, dum)

surprise := "10.127.190.240/32"
{
l := NewAddress(ifname, "10.127.190.217/24", surprise)
l.Exact()
if err := l.Err(); err != nil {
t.Fatalf("got err: %v", err)
}
}
{
l := NewAddress(ifname)
l.testcb = func() {
if err := NewAddress(ifname, surprise).Del().Err(); err != nil {
t.Fatalf("sneak failed: %v", err)
}
}
if addrs, _ := l.List4(); len(addrs) == 0 {
t.Fatalf("expect at least 1 v4 addr")
}
l.Exact()
if err := l.Err(); err != nil {
t.Fatalf("got err: %v", err)
}
}
})
}

func TestAddress_nopriv(t *testing.T) {
Expand Down

0 comments on commit 4b6d385

Please sign in to comment.