From 48000786cc83cf7eedbce4e5d744f70e32dcf8f6 Mon Sep 17 00:00:00 2001
From: Yousong Zhou <zhouyousong@yunionyun.com>
Date: Tue, 14 Jul 2020 16:31:48 +0800
Subject: [PATCH] iproute2: address: make Exact idempotent

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"
---
 pkg/util/iproute2/address.go      | 16 +++++++-
 pkg/util/iproute2/address_test.go | 63 ++++++++++++++++++++++---------
 2 files changed, 61 insertions(+), 18 deletions(-)

diff --git a/pkg/util/iproute2/address.go b/pkg/util/iproute2/address.go
index 2c81c80d36..bac3ce1e4c 100644
--- a/pkg/util/iproute2/address.go
+++ b/pkg/util/iproute2/address.go
@@ -16,6 +16,7 @@ package iproute2
 
 import (
 	"net"
+	"syscall"
 
 	"github.com/vishvananda/netlink"
 )
@@ -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))
@@ -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 {
@@ -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)
 			}
 		}
diff --git a/pkg/util/iproute2/address_test.go b/pkg/util/iproute2/address_test.go
index 7c36c94f4a..be883021ef 100644
--- a/pkg/util/iproute2/address_test.go
+++ b/pkg/util/iproute2/address_test.go
@@ -19,24 +19,24 @@ import (
 )
 
 func TestAddress(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)
-		}
-	}
-
 	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)
+			}
+		}
+
 		want := "10.168.222.236/24"
 		l := NewAddress(ifname, want, "fe80::222:d5ff:fe9e:28d1/64")
 		l.Exact()
@@ -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) {