diff --git a/plugins/ipam/host-local/backend/allocator/allocator.go b/plugins/ipam/host-local/backend/allocator/allocator.go index d1c2b101..4cec1a74 100644 --- a/plugins/ipam/host-local/backend/allocator/allocator.go +++ b/plugins/ipam/host-local/backend/allocator/allocator.go @@ -40,7 +40,7 @@ func NewIPAllocator(s *RangeSet, store backend.Store, id int) *IPAllocator { } } -// Get alocates an IP +// Get allocates an IP func (a *IPAllocator) Get(id string, ifname string, requestedIP net.IP) (*current.IPConfig, error) { a.store.Lock() defer a.store.Unlock() @@ -73,6 +73,17 @@ 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 + // 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) + } + } + iter, err := a.GetIter() if err != nil { return nil, err diff --git a/plugins/ipam/host-local/backend/allocator/allocator_test.go b/plugins/ipam/host-local/backend/allocator/allocator_test.go index 5888c14b..a7027c15 100644 --- a/plugins/ipam/host-local/backend/allocator/allocator_test.go +++ b/plugins/ipam/host-local/backend/allocator/allocator_test.go @@ -221,14 +221,14 @@ var _ = Describe("host-local ip allocator", func() { It("should not allocate the broadcast address", func() { alloc := mkalloc() for i := 2; i < 7; i++ { - res, err := alloc.Get("ID", "eth0", nil) + res, err := alloc.Get(fmt.Sprintf("ID%d", i), "eth0", nil) Expect(err).ToNot(HaveOccurred()) s := fmt.Sprintf("192.168.1.%d/29", i) Expect(s).To(Equal(res.Address.String())) fmt.Fprintln(GinkgoWriter, "got ip", res.Address.String()) } - x, err := alloc.Get("ID", "eth0", nil) + x, err := alloc.Get("ID8", "eth0", nil) fmt.Fprintln(GinkgoWriter, "got ip", x) Expect(err).To(HaveOccurred()) }) diff --git a/plugins/ipam/host-local/backend/disk/backend.go b/plugins/ipam/host-local/backend/disk/backend.go index c3e6b496..56f1638a 100644 --- a/plugins/ipam/host-local/backend/disk/backend.go +++ b/plugins/ipam/host-local/backend/disk/backend.go @@ -19,10 +19,10 @@ import ( "net" "os" "path/filepath" + "runtime" "strings" "github.com/containernetworking/plugins/plugins/ipam/host-local/backend" - "runtime" ) const lastIPFilePrefix = "last_reserved_ip." @@ -172,6 +172,35 @@ func (s *Store) ReleaseByID(id string, ifname string) error { return err } +// GetByID returns the IPs which have been allocated to the specific ID +func (s *Store) GetByID(id string, ifname string) []net.IP { + var ips []net.IP + + match := strings.TrimSpace(id) + LineBreak + ifname + // matchOld for backwards compatibility + matchOld := strings.TrimSpace(id) + + // walk through all ips in this network to get the ones which belong to a specific ID + _ = filepath.Walk(s.dataDir, func(path string, info os.FileInfo, err error) error { + if err != nil || info.IsDir() { + return nil + } + data, err := ioutil.ReadFile(path) + if err != nil { + return nil + } + if strings.TrimSpace(string(data)) == match || strings.TrimSpace(string(data)) == matchOld { + _, ipString := filepath.Split(path) + if ip := net.ParseIP(ipString); ip != nil { + ips = append(ips, ip) + } + } + return nil + }) + + return ips +} + func GetEscapedPath(dataDir string, fname string) string { if runtime.GOOS == "windows" { fname = strings.Replace(fname, ":", "_", -1) diff --git a/plugins/ipam/host-local/backend/store.go b/plugins/ipam/host-local/backend/store.go index 4ea845da..7211ddf6 100644 --- a/plugins/ipam/host-local/backend/store.go +++ b/plugins/ipam/host-local/backend/store.go @@ -24,4 +24,5 @@ type Store interface { LastReservedIP(rangeID string) (net.IP, error) Release(ip net.IP) error ReleaseByID(id string, ifname string) error + GetByID(id string, ifname string) []net.IP } diff --git a/plugins/ipam/host-local/backend/testing/fake_store.go b/plugins/ipam/host-local/backend/testing/fake_store.go index 631fca2e..4f3a934e 100644 --- a/plugins/ipam/host-local/backend/testing/fake_store.go +++ b/plugins/ipam/host-local/backend/testing/fake_store.go @@ -81,6 +81,16 @@ func (s *FakeStore) ReleaseByID(id string, ifname string) error { return nil } +func (s *FakeStore) GetByID(id string, ifname string) []net.IP { + var ips []net.IP + for k, v := range s.ipMap { + if v == id { + ips = append(ips, net.ParseIP(k)) + } + } + return ips +} + func (s *FakeStore) SetIPMap(m map[string]string) { s.ipMap = m } diff --git a/plugins/ipam/host-local/host_local_test.go b/plugins/ipam/host-local/host_local_test.go index 46ab0b56..6f32c0a3 100644 --- a/plugins/ipam/host-local/host_local_test.go +++ b/plugins/ipam/host-local/host_local_test.go @@ -250,6 +250,94 @@ var _ = Describe("host-local Operations", func() { Expect(err).To(HaveOccurred()) }) + It("repeat allocating addresses on specific interface for same container ID with ADD", func() { + const ifname string = "eth0" + const nspath string = "/some/where" + + tmpDir, err := getTmpDir() + Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(tmpDir) + + conf := fmt.Sprintf(`{ + "cniVersion": "0.3.1", + "name": "mynet0", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "dataDir": "%s", + "ranges": [ + [{ "subnet": "10.1.2.0/24" }] + ] + } + }`, tmpDir) + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: nspath, + IfName: ifname, + StdinData: []byte(conf), + } + + args1 := &skel.CmdArgs{ + ContainerID: "dummy1", + Netns: nspath, + IfName: ifname, + StdinData: []byte(conf), + } + + // Allocate the IP + r0, raw, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).NotTo(HaveOccurred()) + Expect(strings.Index(string(raw), "\"version\":")).Should(BeNumerically(">", 0)) + + result0, err := current.GetResult(r0) + Expect(err).NotTo(HaveOccurred()) + Expect(len(result0.IPs)).Should(Equal(1)) + 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 { + 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.3/24")) + + // Allocate the IP with the same container ID again + _, _, err = testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).To(HaveOccurred()) + + ipFilePath := filepath.Join(tmpDir, "mynet0", "10.1.2.2") + + // Release the IPs + err = testutils.CmdDelWithArgs(args, func() error { + return cmdDel(args) + }) + Expect(err).NotTo(HaveOccurred()) + _, err = os.Stat(ipFilePath) + Expect(err).To(HaveOccurred()) + + err = testutils.CmdDelWithArgs(args1, func() error { + return cmdDel(args1) + }) + Expect(err).NotTo(HaveOccurred()) + }) + It("Verify DEL works on backwards compatible allocate", func() { const nspath string = "/some/where" const ifname string = "eth0"