From e2984e784011ad0f04fd9f1459d2d01254ee0bd3 Mon Sep 17 00:00:00 2001 From: Bruce Ma Date: Sat, 6 Jul 2019 10:05:18 +0800 Subject: [PATCH] host-local: return error if duplicate allocation is requested for a given ID Signed-off-by: Bruce Ma --- .../host-local/backend/allocator/allocator.go | 44 +++++++++---------- plugins/ipam/host-local/host_local_test.go | 32 +++++--------- 2 files changed, 30 insertions(+), 46 deletions(-) diff --git a/plugins/ipam/host-local/backend/allocator/allocator.go b/plugins/ipam/host-local/backend/allocator/allocator.go index 2d9f80ec..4cec1a74 100644 --- a/plugins/ipam/host-local/backend/allocator/allocator.go +++ b/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 - existIPs := a.store.GetByID(id, ifname) - for _, existIP := range existIPs { + // 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 r, err := a.rangeset.RangeFor(existIP); err == nil { - reservedIP = &net.IPNet{IP: existIP, Mask: r.Subnet.Mask} - gw = r.Gateway - break + 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) } } - // if no existing IP was found, try to reserve a new one - if reservedIP == nil { - iter, err := a.GetIter() + iter, err := a.GetIter() + 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 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 err != nil { - return nil, err - } - - if reserved { - break - } + if reserved { + break } - } } diff --git a/plugins/ipam/host-local/host_local_test.go b/plugins/ipam/host-local/host_local_test.go index d3b487ba..6f32c0a3 100644 --- a/plugins/ipam/host-local/host_local_test.go +++ b/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")) - - // 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")) + Expect(result1.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(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")) + Expect(err).To(HaveOccurred()) ipFilePath := filepath.Join(tmpDir, "mynet0", "10.1.2.2")