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"