diff --git a/plugins/ipam/host-local/backend/allocator/allocator.go b/plugins/ipam/host-local/backend/allocator/allocator.go index 1d2964b9..d1c2b101 100644 --- a/plugins/ipam/host-local/backend/allocator/allocator.go +++ b/plugins/ipam/host-local/backend/allocator/allocator.go @@ -41,7 +41,7 @@ func NewIPAllocator(s *RangeSet, store backend.Store, id int) *IPAllocator { } // Get alocates an IP -func (a *IPAllocator) Get(id string, requestedIP net.IP) (*current.IPConfig, error) { +func (a *IPAllocator) Get(id string, ifname string, requestedIP net.IP) (*current.IPConfig, error) { a.store.Lock() defer a.store.Unlock() @@ -62,7 +62,7 @@ func (a *IPAllocator) Get(id string, requestedIP net.IP) (*current.IPConfig, err return nil, fmt.Errorf("requested ip %s is subnet's gateway", requestedIP.String()) } - reserved, err := a.store.Reserve(id, requestedIP, a.rangeID) + reserved, err := a.store.Reserve(id, ifname, requestedIP, a.rangeID) if err != nil { return nil, err } @@ -83,7 +83,7 @@ func (a *IPAllocator) Get(id string, requestedIP net.IP) (*current.IPConfig, err break } - reserved, err := a.store.Reserve(id, reservedIP.IP, a.rangeID) + reserved, err := a.store.Reserve(id, ifname, reservedIP.IP, a.rangeID) if err != nil { return nil, err } @@ -110,11 +110,11 @@ func (a *IPAllocator) Get(id string, requestedIP net.IP) (*current.IPConfig, err } // Release clears all IPs allocated for the container with given ID -func (a *IPAllocator) Release(id string) error { +func (a *IPAllocator) Release(id string, ifname string) error { a.store.Lock() defer a.store.Unlock() - return a.store.ReleaseByID(id) + return a.store.ReleaseByID(id, ifname) } type RangeIter struct { diff --git a/plugins/ipam/host-local/backend/allocator/allocator_test.go b/plugins/ipam/host-local/backend/allocator/allocator_test.go index 436aaa52..5888c14b 100644 --- a/plugins/ipam/host-local/backend/allocator/allocator_test.go +++ b/plugins/ipam/host-local/backend/allocator/allocator_test.go @@ -70,7 +70,7 @@ func (t AllocatorTestCase) run(idx int) (*current.IPConfig, error) { rangeID: "rangeid", } - return alloc.Get("ID", nil) + return alloc.Get("ID", "eth0", nil) } var _ = Describe("host-local ip allocator", func() { @@ -88,8 +88,8 @@ var _ = Describe("host-local ip allocator", func() { It("should loop correctly from the end", func() { a := mkalloc() - a.store.Reserve("ID", net.IP{192, 168, 1, 6}, a.rangeID) - a.store.ReleaseByID("ID") + a.store.Reserve("ID", "eth0", net.IP{192, 168, 1, 6}, a.rangeID) + a.store.ReleaseByID("ID", "eth0") r, _ := a.GetIter() Expect(r.nextip()).To(Equal(net.IP{192, 168, 1, 2})) Expect(r.nextip()).To(Equal(net.IP{192, 168, 1, 3})) @@ -100,8 +100,8 @@ var _ = Describe("host-local ip allocator", func() { }) It("should loop correctly from the middle", func() { a := mkalloc() - a.store.Reserve("ID", net.IP{192, 168, 1, 3}, a.rangeID) - a.store.ReleaseByID("ID") + a.store.Reserve("ID", "eth0", net.IP{192, 168, 1, 3}, a.rangeID) + a.store.ReleaseByID("ID", "eth0") r, _ := a.GetIter() Expect(r.nextip()).To(Equal(net.IP{192, 168, 1, 4})) Expect(r.nextip()).To(Equal(net.IP{192, 168, 1, 5})) @@ -221,28 +221,28 @@ 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", nil) + res, err := alloc.Get("ID", "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", nil) + x, err := alloc.Get("ID", "eth0", nil) fmt.Fprintln(GinkgoWriter, "got ip", x) Expect(err).To(HaveOccurred()) }) It("should allocate in a round-robin fashion", func() { alloc := mkalloc() - res, err := alloc.Get("ID", nil) + res, err := alloc.Get("ID", "eth0", nil) Expect(err).ToNot(HaveOccurred()) Expect(res.Address.String()).To(Equal("192.168.1.2/29")) - err = alloc.Release("ID") + err = alloc.Release("ID", "eth0") Expect(err).ToNot(HaveOccurred()) - res, err = alloc.Get("ID", nil) + res, err = alloc.Get("ID", "eth0", nil) Expect(err).ToNot(HaveOccurred()) Expect(res.Address.String()).To(Equal("192.168.1.3/29")) @@ -252,7 +252,7 @@ var _ = Describe("host-local ip allocator", func() { It("must allocate the requested IP", func() { alloc := mkalloc() requestedIP := net.IP{192, 168, 1, 5} - res, err := alloc.Get("ID", requestedIP) + res, err := alloc.Get("ID", "eth0", requestedIP) Expect(err).ToNot(HaveOccurred()) Expect(res.Address.IP.String()).To(Equal(requestedIP.String())) }) @@ -260,11 +260,11 @@ var _ = Describe("host-local ip allocator", func() { It("must fail when the requested IP is allocated", func() { alloc := mkalloc() requestedIP := net.IP{192, 168, 1, 5} - res, err := alloc.Get("ID", requestedIP) + res, err := alloc.Get("ID", "eth0", requestedIP) Expect(err).ToNot(HaveOccurred()) Expect(res.Address.IP.String()).To(Equal(requestedIP.String())) - _, err = alloc.Get("ID", requestedIP) + _, err = alloc.Get("ID", "eth0", requestedIP) Expect(err).To(MatchError(`requested IP address 192.168.1.5 is not available in range set 192.168.1.1-192.168.1.6`)) }) @@ -272,7 +272,7 @@ var _ = Describe("host-local ip allocator", func() { alloc := mkalloc() (*alloc.rangeset)[0].RangeEnd = net.IP{192, 168, 1, 4} requestedIP := net.IP{192, 168, 1, 5} - _, err := alloc.Get("ID", requestedIP) + _, err := alloc.Get("ID", "eth0", requestedIP) Expect(err).To(HaveOccurred()) }) @@ -280,7 +280,7 @@ var _ = Describe("host-local ip allocator", func() { alloc := mkalloc() (*alloc.rangeset)[0].RangeStart = net.IP{192, 168, 1, 3} requestedIP := net.IP{192, 168, 1, 2} - _, err := alloc.Get("ID", requestedIP) + _, err := alloc.Get("ID", "eth0", requestedIP) 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 08bb4eb9..76407ff3 100644 --- a/plugins/ipam/host-local/backend/disk/backend.go +++ b/plugins/ipam/host-local/backend/disk/backend.go @@ -26,6 +26,7 @@ import ( ) const lastIPFilePrefix = "last_reserved_ip." +const LineBreak = "\r\n" var defaultDataDir = "/var/lib/cni/networks" @@ -55,7 +56,7 @@ func New(network, dataDir string) (*Store, error) { return &Store{lk, dir}, nil } -func (s *Store) Reserve(id string, ip net.IP, rangeID string) (bool, error) { +func (s *Store) Reserve(id string, ifname string, ip net.IP, rangeID string) (bool, error) { fname := GetEscapedPath(s.dataDir, ip.String()) f, err := os.OpenFile(fname, os.O_RDWR|os.O_EXCL|os.O_CREATE, 0644) @@ -65,7 +66,7 @@ func (s *Store) Reserve(id string, ip net.IP, rangeID string) (bool, error) { if err != nil { return false, err } - if _, err := f.WriteString(strings.TrimSpace(id)); err != nil { + if _, err := f.WriteString(strings.TrimSpace(id) + LineBreak + ifname); err != nil { f.Close() os.Remove(f.Name()) return false, err @@ -97,9 +98,8 @@ func (s *Store) Release(ip net.IP) error { return os.Remove(GetEscapedPath(s.dataDir, ip.String())) } -// N.B. This function eats errors to be tolerant and -// release as much as possible -func (s *Store) ReleaseByID(id string) error { +func (s *Store) ReleaseByKey(id string, ifname string, match string) (bool, error) { + found := false err := filepath.Walk(s.dataDir, func(path string, info os.FileInfo, err error) error { if err != nil || info.IsDir() { return nil @@ -108,13 +108,30 @@ func (s *Store) ReleaseByID(id string) error { if err != nil { return nil } - if strings.TrimSpace(string(data)) == strings.TrimSpace(id) { + if strings.TrimSpace(string(data)) == match { if err := os.Remove(path); err != nil { return nil } + found = true } return nil }) + return found, err + +} + +// N.B. This function eats errors to be tolerant and +// release as much as possible +func (s *Store) ReleaseByID(id string, ifname string) error { + found := false + match := strings.TrimSpace(id) + LineBreak + ifname + found, err := s.ReleaseByKey(id, ifname, match) + + // For backwards compatibility, look for files written by a previous version + if !found && err == nil { + match := strings.TrimSpace(id) + found, err = s.ReleaseByKey(id, ifname, match) + } return err } diff --git a/plugins/ipam/host-local/backend/store.go b/plugins/ipam/host-local/backend/store.go index 3d695847..4ea845da 100644 --- a/plugins/ipam/host-local/backend/store.go +++ b/plugins/ipam/host-local/backend/store.go @@ -20,8 +20,8 @@ type Store interface { Lock() error Unlock() error Close() error - Reserve(id string, ip net.IP, rangeID string) (bool, error) + Reserve(id string, ifname string, ip net.IP, rangeID string) (bool, error) LastReservedIP(rangeID string) (net.IP, error) Release(ip net.IP) error - ReleaseByID(id string) error + ReleaseByID(id string, ifname string) error } diff --git a/plugins/ipam/host-local/backend/testing/fake_store.go b/plugins/ipam/host-local/backend/testing/fake_store.go index 49a0f554..631fca2e 100644 --- a/plugins/ipam/host-local/backend/testing/fake_store.go +++ b/plugins/ipam/host-local/backend/testing/fake_store.go @@ -45,7 +45,7 @@ func (s *FakeStore) Close() error { return nil } -func (s *FakeStore) Reserve(id string, ip net.IP, rangeID string) (bool, error) { +func (s *FakeStore) Reserve(id string, ifname string, ip net.IP, rangeID string) (bool, error) { key := ip.String() if _, ok := s.ipMap[key]; !ok { s.ipMap[key] = id @@ -68,7 +68,7 @@ func (s *FakeStore) Release(ip net.IP) error { return nil } -func (s *FakeStore) ReleaseByID(id string) error { +func (s *FakeStore) ReleaseByID(id string, ifname string) error { toDelete := []string{} for k, v := range s.ipMap { if v == id { diff --git a/plugins/ipam/host-local/host_local_test.go b/plugins/ipam/host-local/host_local_test.go index 653d6175..46ab0b56 100644 --- a/plugins/ipam/host-local/host_local_test.go +++ b/plugins/ipam/host-local/host_local_test.go @@ -33,6 +33,8 @@ import ( . "github.com/onsi/gomega" ) +const LineBreak = "\r\n" + var _ = Describe("host-local Operations", func() { It("allocates and releases addresses with ADD/DEL", func() { const ifname string = "eth0" @@ -111,12 +113,12 @@ var _ = Describe("host-local Operations", func() { ipFilePath1 := filepath.Join(tmpDir, "mynet", "10.1.2.2") contents, err := ioutil.ReadFile(ipFilePath1) Expect(err).NotTo(HaveOccurred()) - Expect(string(contents)).To(Equal(args.ContainerID)) + Expect(string(contents)).To(Equal(args.ContainerID + LineBreak + ifname)) ipFilePath2 := filepath.Join(tmpDir, disk.GetEscapedPath("mynet", "2001:db8:1::2")) contents, err = ioutil.ReadFile(ipFilePath2) Expect(err).NotTo(HaveOccurred()) - Expect(string(contents)).To(Equal(args.ContainerID)) + Expect(string(contents)).To(Equal(args.ContainerID + LineBreak + ifname)) lastFilePath1 := filepath.Join(tmpDir, "mynet", "last_reserved_ip.0") contents, err = ioutil.ReadFile(lastFilePath1) @@ -139,6 +141,173 @@ var _ = Describe("host-local Operations", func() { Expect(err).To(HaveOccurred()) }) + It("allocates and releases addresses on specific interface with ADD/DEL", func() { + const ifname0 string = "eth0" + const ifname1 string = "eth1" + const nspath string = "/some/where" + + tmpDir, err := getTmpDir() + Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(tmpDir) + + err = ioutil.WriteFile(filepath.Join(tmpDir, "resolv.conf"), []byte("nameserver 192.0.2.3"), 0644) + Expect(err).NotTo(HaveOccurred()) + + conf0 := fmt.Sprintf(`{ + "cniVersion": "0.3.1", + "name": "mynet0", + "type": "ipvlan", + "master": "foo0", + "ipam": { + "type": "host-local", + "dataDir": "%s", + "resolvConf": "%s/resolv.conf", + "ranges": [ + [{ "subnet": "10.1.2.0/24" }] + ] + } + }`, tmpDir, tmpDir) + + conf1 := fmt.Sprintf(`{ + "cniVersion": "0.3.1", + "name": "mynet1", + "type": "ipvlan", + "master": "foo1", + "ipam": { + "type": "host-local", + "dataDir": "%s", + "resolvConf": "%s/resolv.conf", + "ranges": [ + [{ "subnet": "10.2.2.0/24" }] + ] + } + }`, tmpDir, tmpDir) + + args0 := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: nspath, + IfName: ifname0, + StdinData: []byte(conf0), + } + + // Allocate the IP + r0, raw, err := testutils.CmdAddWithArgs(args0, func() error { + return cmdAdd(args0) + }) + Expect(err).NotTo(HaveOccurred()) + Expect(strings.Index(string(raw), "\"version\":")).Should(BeNumerically(">", 0)) + + _, err = current.GetResult(r0) + Expect(err).NotTo(HaveOccurred()) + + args1 := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: nspath, + IfName: ifname1, + StdinData: []byte(conf1), + } + + // Allocate the IP + r1, raw, err := testutils.CmdAddWithArgs(args1, func() error { + return cmdAdd(args1) + }) + Expect(err).NotTo(HaveOccurred()) + Expect(strings.Index(string(raw), "\"version\":")).Should(BeNumerically(">", 0)) + + _, err = current.GetResult(r1) + Expect(err).NotTo(HaveOccurred()) + + ipFilePath0 := filepath.Join(tmpDir, "mynet0", "10.1.2.2") + contents, err := ioutil.ReadFile(ipFilePath0) + Expect(err).NotTo(HaveOccurred()) + Expect(string(contents)).To(Equal(args0.ContainerID + LineBreak + ifname0)) + + ipFilePath1 := filepath.Join(tmpDir, "mynet1", "10.2.2.2") + contents, err = ioutil.ReadFile(ipFilePath1) + Expect(err).NotTo(HaveOccurred()) + Expect(string(contents)).To(Equal(args1.ContainerID + LineBreak + ifname1)) + + // Release the IP on ifname0 + err = testutils.CmdDelWithArgs(args0, func() error { + return cmdDel(args0) + }) + Expect(err).NotTo(HaveOccurred()) + _, err = os.Stat(ipFilePath0) + Expect(err).To(HaveOccurred()) + + // reread ipFilePath1, ensure that ifname1 didn't get deleted + contents, err = ioutil.ReadFile(ipFilePath1) + Expect(err).NotTo(HaveOccurred()) + Expect(string(contents)).To(Equal(args1.ContainerID + LineBreak + ifname1)) + + // Release the IP on ifname1 + err = testutils.CmdDelWithArgs(args1, func() error { + return cmdDel(args1) + }) + Expect(err).NotTo(HaveOccurred()) + + _, err = os.Stat(ipFilePath1) + Expect(err).To(HaveOccurred()) + }) + + It("Verify DEL works on backwards compatible allocate", func() { + const nspath string = "/some/where" + const ifname string = "eth0" + + tmpDir, err := getTmpDir() + Expect(err).NotTo(HaveOccurred()) + defer os.RemoveAll(tmpDir) + + err = ioutil.WriteFile(filepath.Join(tmpDir, "resolv.conf"), []byte("nameserver 192.0.2.3"), 0644) + Expect(err).NotTo(HaveOccurred()) + + conf := fmt.Sprintf(`{ + "cniVersion": "0.3.1", + "name": "mynet", + "type": "ipvlan", + "master": "foo", + "ipam": { + "type": "host-local", + "dataDir": "%s", + "resolvConf": "%s/resolv.conf", + "ranges": [ + [{ "subnet": "10.1.2.0/24" }] + ] + } + }`, tmpDir, tmpDir) + + args := &skel.CmdArgs{ + ContainerID: "dummy", + Netns: nspath, + IfName: ifname, + StdinData: []byte(conf), + } + + // Allocate the IP + r, raw, err := testutils.CmdAddWithArgs(args, func() error { + return cmdAdd(args) + }) + Expect(err).NotTo(HaveOccurred()) + Expect(strings.Index(string(raw), "\"version\":")).Should(BeNumerically(">", 0)) + + _, err = current.GetResult(r) + Expect(err).NotTo(HaveOccurred()) + + ipFilePath := filepath.Join(tmpDir, "mynet", "10.1.2.2") + contents, err := ioutil.ReadFile(ipFilePath) + Expect(err).NotTo(HaveOccurred()) + Expect(string(contents)).To(Equal(args.ContainerID + LineBreak + ifname)) + err = ioutil.WriteFile(ipFilePath, []byte(strings.TrimSpace(args.ContainerID)), 0644) + Expect(err).NotTo(HaveOccurred()) + + err = testutils.CmdDelWithArgs(args, func() error { + return cmdDel(args) + }) + Expect(err).NotTo(HaveOccurred()) + _, err = os.Stat(ipFilePath) + Expect(err).To(HaveOccurred()) + }) + It("doesn't error when passed an unknown ID on DEL", func() { const ifname string = "eth0" const nspath string = "/some/where" @@ -223,7 +392,7 @@ var _ = Describe("host-local Operations", func() { ipFilePath := filepath.Join(tmpDir, "mynet", "10.1.2.2") contents, err := ioutil.ReadFile(ipFilePath) Expect(err).NotTo(HaveOccurred()) - Expect(string(contents)).To(Equal(args.ContainerID)) + Expect(string(contents)).To(Equal(args.ContainerID + LineBreak + ifname)) lastFilePath := filepath.Join(tmpDir, "mynet", "last_reserved_ip.0") contents, err = ioutil.ReadFile(lastFilePath) @@ -281,7 +450,7 @@ var _ = Describe("host-local Operations", func() { ipFilePath := filepath.Join(tmpDir, "mynet", result.IPs[0].Address.IP.String()) contents, err := ioutil.ReadFile(ipFilePath) Expect(err).NotTo(HaveOccurred()) - Expect(string(contents)).To(Equal("dummy")) + Expect(string(contents)).To(Equal("dummy" + LineBreak + ifname)) // Release the IP err = testutils.CmdDelWithArgs(args, func() error { diff --git a/plugins/ipam/host-local/main.go b/plugins/ipam/host-local/main.go index 132391d0..cb1328e4 100644 --- a/plugins/ipam/host-local/main.go +++ b/plugins/ipam/host-local/main.go @@ -85,11 +85,11 @@ func cmdAdd(args *skel.CmdArgs) error { } } - ipConf, err := allocator.Get(args.ContainerID, requestedIP) + ipConf, err := allocator.Get(args.ContainerID, args.IfName, requestedIP) if err != nil { // Deallocate all already allocated IPs for _, alloc := range allocs { - _ = alloc.Release(args.ContainerID) + _ = alloc.Release(args.ContainerID, args.IfName) } return fmt.Errorf("failed to allocate for range %d: %v", idx, err) } @@ -102,7 +102,7 @@ func cmdAdd(args *skel.CmdArgs) error { // If an IP was requested that wasn't fulfilled, fail if len(requestedIPs) != 0 { for _, alloc := range allocs { - _ = alloc.Release(args.ContainerID) + _ = alloc.Release(args.ContainerID, args.IfName) } errstr := "failed to allocate all requested IPs:" for _, ip := range requestedIPs { @@ -133,7 +133,7 @@ func cmdDel(args *skel.CmdArgs) error { for idx, rangeset := range ipamConf.Ranges { ipAllocator := allocator.NewIPAllocator(&rangeset, store, idx) - err := ipAllocator.Release(args.ContainerID) + err := ipAllocator.Release(args.ContainerID, args.IfName) if err != nil { errors = append(errors, err.Error()) }