host-local: return error if duplicate allocation is requested for a given ID

Signed-off-by: Bruce Ma <brucema19901024@gmail.com>

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

@@ -73,39 +73,35 @@ func (a *IPAllocator) Get(id string, ifname string, requestedIP net.IP) (*curren
 		gw = r.Gateway
 
 	} else {
-		// try to get existing IPs which have been allocated to this id
+		// try to get allocated IPs for this given id, if exists, just return error
-		existIPs := a.store.GetByID(id, ifname)
+		// because duplicate allocation is not allowed in SPEC
-		for _, existIP := range existIPs {
+		// 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 r, err := a.rangeset.RangeFor(existIP); err == nil {
+			if _, err := a.rangeset.RangeFor(allocatedIP); err == nil {
-				reservedIP = &net.IPNet{IP: existIP, Mask: r.Subnet.Mask}
+				return nil, fmt.Errorf("%s has been allocated to %s, duplicate allocation is not allowed", allocatedIP.String(), id)
-				gw = r.Gateway
-				break
 			}
 		}
 
-		// if no existing IP was found, try to reserve a new one
+		iter, err := a.GetIter()
-		if reservedIP == nil {
+		if err != nil {
-			iter, err := a.GetIter()
+			return nil, err
+		}
+		for {
+			reservedIP, gw = iter.Next()
+			if reservedIP == nil {
+				break
+			}
+
+			reserved, err := a.store.Reserve(id, ifname, reservedIP.IP, a.rangeID)
 			if err != nil {
 				return nil, err
 			}
-			for {
-				reservedIP, gw = iter.Next()
-				if reservedIP == nil {
-					break
-				}
 
-				reserved, err := a.store.Reserve(id, ifname, reservedIP.IP, a.rangeID)
+			if reserved {
-				if err != nil {
+				break
-					return nil, err
-				}
-
-				if reserved {
-					break
-				}
 			}
-
 		}
 	}
 

plugins/ipam/host-local/host_local_test.go

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