Merge pull request #1 from criteo-forks/make-host-local-add-idempotent

ADD operation now returns the IP if it has already been allocated for the same container id and interface

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

@@ -40,7 +40,7 @@ func NewIPAllocator(s *RangeSet, store backend.Store, id int) *IPAllocator {
 	}
 }
 
-// Get allocates an IP
+// Get allocates an IP or returns the IP if it has already been allocated
 func (a *IPAllocator) Get(id string, ifname string, requestedIP net.IP) (*current.IPConfig, error) {
 	a.store.Lock()
 	defer a.store.Unlock()
@@ -73,14 +73,25 @@ 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
+		// try to get allocated IPs for this given id, if exists, it means it has already been allocated
+		// so just return it
 		// 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)
+				r, err := a.rangeset.RangeFor(allocatedIP)
+				if err != nil {
+					return nil, err
+				}
+				return &current.IPConfig{
+					Version: "4",
+					Address: net.IPNet{
+						IP:   allocatedIP,
+						Mask: r.Subnet.Mask,
+					},
+					Gateway: r.Gateway,
+				}, nil
 			}
 		}
 

plugins/ipam/host-local/host_local_test.go

@@ -299,28 +299,40 @@ var _ = Describe("host-local Operations", func() {
 		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 {
+		r1, raw, err := testutils.CmdAddWithArgs(args, func() error {
 			return cmdAdd(args)
 		})
-		Expect(err).To(HaveOccurred())
+		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.2/24"))
 
 		// Allocate the IP with the another container ID
-		r1, raw, err := testutils.CmdAddWithArgs(args1, func() error {
+		r2, 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)
+		result2, err := current.GetResult(r2)
 		Expect(err).NotTo(HaveOccurred())
-		Expect(len(result1.IPs)).Should(Equal(1))
-		Expect(result1.IPs[0].Address.String()).Should(Equal("10.1.2.3/24"))
+		Expect(len(result2.IPs)).Should(Equal(1))
+		Expect(result2.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 {
+		r3, raw, err := testutils.CmdAddWithArgs(args, func() error {
 			return cmdAdd(args)
 		})
-		Expect(err).To(HaveOccurred())
+		Expect(err).NotTo(HaveOccurred())
+		Expect(strings.Index(string(raw), "\"version\":")).Should(BeNumerically(">", 0))
+
+		result3, err := current.GetResult(r3)
+		Expect(err).NotTo(HaveOccurred())
+		Expect(len(result3.IPs)).Should(Equal(1))
+		Expect(result3.IPs[0].Address.String()).Should(Equal("10.1.2.2/24"))
 
 		ipFilePath := filepath.Join(tmpDir, "mynet0", "10.1.2.2")