From dfc4f7cd2af04016fc7deedc2cc23925533bbf0a Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 2 Sep 2016 15:55:53 -0500 Subject: [PATCH] host-local: don't allocate past RangeEnd When RangeEnd is given, a.end = RangeEnd+1. If when getSearchRange() is called and lastReservedIP equals RangeEnd, a.nextIP() only compares lastReservedIP (which in this example is RangeEnd) against a.end (which in this example is RangeEnd+1) and they clearly don't match, so a.nextIP() returns start=RangeEnd+1 and end=RangeEnd. Get() happily allocates RangeEnd+1 because it only compares 'cur' to the end returned by getSearchRange(), not to a.end, and thus allocates past RangeEnd. Since a.end is inclusive (eg, host-local will allocate a.end) the fix is to simply set a.end equal to RangeEnd. --- plugins/ipam/host-local/allocator.go | 4 +- plugins/ipam/host-local/allocator_test.go | 74 +++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/plugins/ipam/host-local/allocator.go b/plugins/ipam/host-local/allocator.go index 3610646b..e0915ed8 100644 --- a/plugins/ipam/host-local/allocator.go +++ b/plugins/ipam/host-local/allocator.go @@ -25,7 +25,9 @@ import ( ) type IPAllocator struct { + // start is inclusive and may be allocated start net.IP + // end is inclusive and may be allocated end net.IP conf *IPAMConfig store backend.Store @@ -56,7 +58,7 @@ func NewIPAllocator(conf *IPAMConfig, store backend.Store) (*IPAllocator, error) return nil, err } // RangeEnd is inclusive - end = ip.NextIP(conf.RangeEnd) + end = conf.RangeEnd } return &IPAllocator{start, end, conf, store}, nil } diff --git a/plugins/ipam/host-local/allocator_test.go b/plugins/ipam/host-local/allocator_test.go index 200618a9..bb2b4f6f 100644 --- a/plugins/ipam/host-local/allocator_test.go +++ b/plugins/ipam/host-local/allocator_test.go @@ -15,6 +15,7 @@ package main import ( + "fmt" "github.com/containernetworking/cni/pkg/types" fakestore "github.com/containernetworking/cni/plugins/ipam/host-local/backend/testing" . "github.com/onsi/ginkgo" @@ -115,6 +116,79 @@ var _ = Describe("host-local ip allocator", func() { } }) + It("should allocate the last address if RangeEnd not given", func() { + subnet, err := types.ParseCIDR("192.168.1.0/24") + Expect(err).ToNot(HaveOccurred()) + + conf := IPAMConfig{ + Name: "test", + Type: "host-local", + Subnet: types.IPNet{IP: subnet.IP, Mask: subnet.Mask}, + } + store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP("")) + alloc, err := NewIPAllocator(&conf, store) + Expect(err).ToNot(HaveOccurred()) + + for i := 1; i < 255; i++ { + res, err := alloc.Get("ID") + Expect(err).ToNot(HaveOccurred()) + // i+1 because the gateway address is skipped + s := fmt.Sprintf("192.168.1.%d/24", i+1) + Expect(s).To(Equal(res.IP.String())) + } + + _, err = alloc.Get("ID") + Expect(err).To(HaveOccurred()) + }) + + It("should allocate RangeStart first", func() { + subnet, err := types.ParseCIDR("192.168.1.0/24") + Expect(err).ToNot(HaveOccurred()) + + conf := IPAMConfig{ + Name: "test", + Type: "host-local", + Subnet: types.IPNet{IP: subnet.IP, Mask: subnet.Mask}, + RangeStart: net.ParseIP("192.168.1.10"), + } + store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP("")) + alloc, err := NewIPAllocator(&conf, store) + Expect(err).ToNot(HaveOccurred()) + + res, err := alloc.Get("ID") + Expect(err).ToNot(HaveOccurred()) + Expect(res.IP.String()).To(Equal("192.168.1.10/24")) + + res, err = alloc.Get("ID") + Expect(err).ToNot(HaveOccurred()) + Expect(res.IP.String()).To(Equal("192.168.1.11/24")) + }) + + It("should allocate RangeEnd but not past RangeEnd", func() { + subnet, err := types.ParseCIDR("192.168.1.0/24") + Expect(err).ToNot(HaveOccurred()) + + conf := IPAMConfig{ + Name: "test", + Type: "host-local", + Subnet: types.IPNet{IP: subnet.IP, Mask: subnet.Mask}, + RangeEnd: net.ParseIP("192.168.1.5"), + } + store := fakestore.NewFakeStore(map[string]string{}, net.ParseIP("")) + alloc, err := NewIPAllocator(&conf, store) + Expect(err).ToNot(HaveOccurred()) + + for i := 1; i < 5; i++ { + res, err := alloc.Get("ID") + Expect(err).ToNot(HaveOccurred()) + // i+1 because the gateway address is skipped + Expect(res.IP.String()).To(Equal(fmt.Sprintf("192.168.1.%d/24", i+1))) + } + + _, err = alloc.Get("ID") + Expect(err).To(HaveOccurred()) + }) + Context("when requesting a specific IP", func() { It("must allocate the requested IP", func() { subnet, err := types.ParseCIDR("10.0.0.0/29")