Skip to content

Commit

Permalink
remove redundant startRange in RangeIter due to overlap check on mult…
Browse files Browse the repository at this point in the history
…i ranges

Signed-off-by: Bruce Ma <[email protected]>
  • Loading branch information
mars1024 committed Mar 4, 2021
1 parent d385120 commit b811967
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 6 deletions.
10 changes: 4 additions & 6 deletions plugins/ipam/host-local/backend/allocator/allocator.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"strconv"

current "github.com/containernetworking/cni/pkg/types/100"

"github.com/containernetworking/plugins/pkg/ip"
"github.com/containernetworking/plugins/plugins/ipam/host-local/backend"
)
Expand Down Expand Up @@ -132,9 +133,8 @@ type RangeIter struct {
// Our current position
cur net.IP

// The IP and range index where we started iterating; if we hit this again, we're done.
startIP net.IP
startRange int
// The IP where we started iterating; if we hit this again, we're done.
startIP net.IP
}

// GetIter encapsulates the strategy for this allocator.
Expand Down Expand Up @@ -164,7 +164,6 @@ func (a *IPAllocator) GetIter() (*RangeIter, error) {
for i, r := range *a.rangeset {
if r.Contains(lastReservedIP) {
iter.rangeIdx = i
iter.startRange = i

// We advance the cursor on every Next(), so the first call
// to next() will return lastReservedIP + 1
Expand All @@ -174,7 +173,6 @@ func (a *IPAllocator) GetIter() (*RangeIter, error) {
}
} else {
iter.rangeIdx = 0
iter.startRange = 0
iter.startIP = (*a.rangeset)[0].RangeStart
}
return &iter, nil
Expand Down Expand Up @@ -210,7 +208,7 @@ func (i *RangeIter) Next() (*net.IPNet, net.IP) {

if i.startIP == nil {
i.startIP = i.cur
} else if i.rangeIdx == i.startRange && i.cur.Equal(i.startIP) {
} else if i.cur.Equal(i.startIP) {
// IF we've looped back to where we started, give up
return nil, nil
}
Expand Down
50 changes: 50 additions & 0 deletions plugins/ipam/host-local/backend/allocator/allocator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,23 @@ func mkalloc() IPAllocator {
return alloc
}

func newAllocatorWithMultiRanges() IPAllocator {
p := RangeSet{
Range{RangeStart: net.IP{192, 168, 1, 0}, RangeEnd: net.IP{192, 168, 1, 3}, Subnet: mustSubnet("192.168.1.1/30")},
Range{RangeStart: net.IP{192, 168, 2, 0}, RangeEnd: net.IP{192, 168, 2, 3}, Subnet: mustSubnet("192.168.2.1/30")},
}
_ = p.Canonicalize()
store := fakestore.NewFakeStore(map[string]string{}, map[string]net.IP{})

alloc := IPAllocator{
rangeset: &p,
store: store,
rangeID: "rangeid",
}

return alloc
}

func (t AllocatorTestCase) run(idx int) (*current.IPConfig, error) {
fmt.Fprintln(GinkgoWriter, "Index:", idx)
p := RangeSet{}
Expand Down Expand Up @@ -320,6 +337,39 @@ var _ = Describe("host-local ip allocator", func() {
}
})
})

Context("when lastReservedIP is at the end of one of multi ranges", func() {
It("should use the first IP of next range as startIP after Next", func() {
a := newAllocatorWithMultiRanges()

// reserve the last IP of the first range
reserved, err := a.store.Reserve("ID", "eth0", net.IP{192, 168, 1, 3}, a.rangeID)
Expect(reserved).To(BeTrue())
Expect(err).NotTo(HaveOccurred())

// get range iterator and do the first Next
r, err := a.GetIter()
Expect(err).NotTo(HaveOccurred())
ip := r.nextip()
Expect(ip).NotTo(BeNil())

Expect(r.startIP).To(Equal(net.IP{192, 168, 2, 0}))
})
})

Context("when no lastReservedIP", func() {
It("should use the first IP of the first range as startIP after Next", func() {
a := newAllocatorWithMultiRanges()

// get range iterator and do the first Next
r, err := a.GetIter()
Expect(err).NotTo(HaveOccurred())
ip := r.nextip()
Expect(ip).NotTo(BeNil())

Expect(r.startIP).To(Equal(net.IP{192, 168, 1, 0}))
})
})
})

// nextip is a convenience function used for testing
Expand Down

0 comments on commit b811967

Please sign in to comment.