Merge pull request #328 from mars1024/feature/hostlocal_idempotent

host-local support idempotent allocation

plugins/ipam/host-local/backend/allocator/allocator.go

@@ -40,7 +40,7 @@ func NewIPAllocator(s *RangeSet, store backend.Store, id int) *IPAllocator {
 	}
 }
 
-// Get alocates an IP
+// Get allocates an IP
 func (a *IPAllocator) Get(id string, ifname string, requestedIP net.IP) (*current.IPConfig, error) {
 	a.store.Lock()
 	defer a.store.Unlock()
@@ -73,6 +73,17 @@ func (a *IPAllocator) Get(id string, ifname string, requestedIP net.IP) (*curren
 		gw = r.Gateway
 
 	} else {
+		// try to get allocated IPs for this given id, if exists, just return error
+		// because duplicate allocation is not allowed in SPEC
+		// https://github.com/containernetworking/cni/blob/master/SPEC.md
+		allocatedIPs := a.store.GetByID(id, ifname)
+		for _, allocatedIP := range allocatedIPs {
+			// check whether the existing IP belong to this range set
+			if _, err := a.rangeset.RangeFor(allocatedIP); err == nil {
+				return nil, fmt.Errorf("%s has been allocated to %s, duplicate allocation is not allowed", allocatedIP.String(), id)
+			}
+		}
+
 		iter, err := a.GetIter()
 		if err != nil {
 			return nil, err

plugins/ipam/host-local/backend/allocator/allocator_test.go

@@ -221,14 +221,14 @@ var _ = Describe("host-local ip allocator", func() {
 		It("should not allocate the broadcast address", func() {
 			alloc := mkalloc()
 			for i := 2; i < 7; i++ {
-				res, err := alloc.Get("ID", "eth0", nil)
+				res, err := alloc.Get(fmt.Sprintf("ID%d", i), "eth0", nil)
 				Expect(err).ToNot(HaveOccurred())
 				s := fmt.Sprintf("192.168.1.%d/29", i)
 				Expect(s).To(Equal(res.Address.String()))
 				fmt.Fprintln(GinkgoWriter, "got ip", res.Address.String())
 			}
 
-			x, err := alloc.Get("ID", "eth0", nil)
+			x, err := alloc.Get("ID8", "eth0", nil)
 			fmt.Fprintln(GinkgoWriter, "got ip", x)
 			Expect(err).To(HaveOccurred())
 		})

plugins/ipam/host-local/backend/disk/backend.go

@@ -19,10 +19,10 @@ import (
 	"net"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 
 	"github.com/containernetworking/plugins/plugins/ipam/host-local/backend"
-	"runtime"
 )
 
 const lastIPFilePrefix = "last_reserved_ip."
@@ -172,6 +172,35 @@ func (s *Store) ReleaseByID(id string, ifname string) error {
 	return err
 }
 
+// GetByID returns the IPs which have been allocated to the specific ID
+func (s *Store) GetByID(id string, ifname string) []net.IP {
+	var ips []net.IP
+
+	match := strings.TrimSpace(id) + LineBreak + ifname
+	// matchOld for backwards compatibility
+	matchOld := strings.TrimSpace(id)
+
+	// walk through all ips in this network to get the ones which belong to a specific ID
+	_ = filepath.Walk(s.dataDir, func(path string, info os.FileInfo, err error) error {
+		if err != nil || info.IsDir() {
+			return nil
+		}
+		data, err := ioutil.ReadFile(path)
+		if err != nil {
+			return nil
+		}
+		if strings.TrimSpace(string(data)) == match || strings.TrimSpace(string(data)) == matchOld {
+			_, ipString := filepath.Split(path)
+			if ip := net.ParseIP(ipString); ip != nil {
+				ips = append(ips, ip)
+			}
+		}
+		return nil
+	})
+
+	return ips
+}
+
 func GetEscapedPath(dataDir string, fname string) string {
 	if runtime.GOOS == "windows" {
 		fname = strings.Replace(fname, ":", "_", -1)

plugins/ipam/host-local/backend/store.go

@@ -24,4 +24,5 @@ type Store interface {
 	LastReservedIP(rangeID string) (net.IP, error)
 	Release(ip net.IP) error
 	ReleaseByID(id string, ifname string) error
+	GetByID(id string, ifname string) []net.IP
 }

plugins/ipam/host-local/backend/testing/fake_store.go

@@ -81,6 +81,16 @@ func (s *FakeStore) ReleaseByID(id string, ifname string) error {
 	return nil
 }
 
+func (s *FakeStore) GetByID(id string, ifname string) []net.IP {
+	var ips []net.IP
+	for k, v := range s.ipMap {
+		if v == id {
+			ips = append(ips, net.ParseIP(k))
+		}
+	}
+	return ips
+}
+
 func (s *FakeStore) SetIPMap(m map[string]string) {
 	s.ipMap = m
 }

plugins/ipam/host-local/host_local_test.go

@@ -250,6 +250,94 @@ var _ = Describe("host-local Operations", func() {
 		Expect(err).To(HaveOccurred())
 	})
 
+	It("repeat allocating addresses on specific interface for same container ID with ADD", func() {
+		const ifname string = "eth0"
+		const nspath string = "/some/where"
+
+		tmpDir, err := getTmpDir()
+		Expect(err).NotTo(HaveOccurred())
+		defer os.RemoveAll(tmpDir)
+
+		conf := fmt.Sprintf(`{
+		"cniVersion": "0.3.1",
+		"name": "mynet0",
+		"type": "ipvlan",
+		"master": "foo0",
+			"ipam": {
+				"type": "host-local",
+				"dataDir": "%s",
+				"ranges": [
+					[{ "subnet": "10.1.2.0/24" }]
+				]
+			}
+		}`, tmpDir)
+
+		args := &skel.CmdArgs{
+			ContainerID: "dummy",
+			Netns:       nspath,
+			IfName:      ifname,
+			StdinData:   []byte(conf),
+		}
+
+		args1 := &skel.CmdArgs{
+			ContainerID: "dummy1",
+			Netns:       nspath,
+			IfName:      ifname,
+			StdinData:   []byte(conf),
+		}
+
+		// Allocate the IP
+		r0, raw, err := testutils.CmdAddWithArgs(args, func() error {
+			return cmdAdd(args)
+		})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(strings.Index(string(raw), "\"version\":")).Should(BeNumerically(">", 0))
+
+		result0, err := current.GetResult(r0)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(len(result0.IPs)).Should(Equal(1))
+		Expect(result0.IPs[0].Address.String()).Should(Equal("10.1.2.2/24"))
+
+		// Allocate the IP with the same container ID
+		_, _, err = testutils.CmdAddWithArgs(args, func() error {
+			return cmdAdd(args)
+		})
+		Expect(err).To(HaveOccurred())
+
+		// Allocate the IP with the another container ID
+		r1, raw, err := testutils.CmdAddWithArgs(args1, func() error {
+			return cmdAdd(args1)
+		})
+		Expect(err).NotTo(HaveOccurred())
+		Expect(strings.Index(string(raw), "\"version\":")).Should(BeNumerically(">", 0))
+
+		result1, err := current.GetResult(r1)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(len(result1.IPs)).Should(Equal(1))
+		Expect(result1.IPs[0].Address.String()).Should(Equal("10.1.2.3/24"))
+
+		// Allocate the IP with the same container ID again
+		_, _, err = testutils.CmdAddWithArgs(args, func() error {
+			return cmdAdd(args)
+		})
+		Expect(err).To(HaveOccurred())
+
+		ipFilePath := filepath.Join(tmpDir, "mynet0", "10.1.2.2")
+
+		// Release the IPs
+		err = testutils.CmdDelWithArgs(args, func() error {
+			return cmdDel(args)
+		})
+		Expect(err).NotTo(HaveOccurred())
+		_, err = os.Stat(ipFilePath)
+		Expect(err).To(HaveOccurred())
+
+		err = testutils.CmdDelWithArgs(args1, func() error {
+			return cmdDel(args1)
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
 	It("Verify DEL works on backwards compatible allocate", func() {
 		const nspath string = "/some/where"
 		const ifname string = "eth0"