pkg/ip: Return correct error if container name provided exists, and test cases

If interface name for a container provided by a user is already present,
Veth creation fails with incorrect error.
If os.IsExist error is returned by makeVethPair:
* Check for peer name, if exists generate another random peer name,
* else, IsExist error is due to container interface present, return error.

Fixes #155

diff --git a/ip/link.go b/ip/link.go
index c9662d9..1cab50c 100644 (file)
--- a/ip/link.go
+++ b/ip/link.go
@@ -41,6 +41,13 @@ func makeVethPair(name, peer string, mtu int) (netlink.Link, error) {
        return veth, nil
 }
 
+func peerExists(name string) bool {
+       if _, err := netlink.LinkByName(name); err != nil {
+               return false
+       }
+       return true
+}
+
 func makeVeth(name string, mtu int) (peerName string, veth netlink.Link, err error) {
        for i := 0; i < 10; i++ {
                peerName, err = RandomVethName()
@@ -54,7 +61,11 @@ func makeVeth(name string, mtu int) (peerName string, veth netlink.Link, err err
                        return
 
                case os.IsExist(err):
-                       continue
+                       if peerExists(peerName) {
+                               continue
+                       }
+                       err = fmt.Errorf("container veth name provided (%v) already exists", name)
+                       return
 
                default:
                        err = fmt.Errorf("failed to make veth pair: %v", err)

diff --git a/ip/link_test.go b/ip/link_test.go
index 5c139d9..d4eff9b 100644 (file)
--- a/ip/link_test.go
+++ b/ip/link_test.go
@@ -15,6 +15,8 @@
 package ip_test
 
 import (
+       "bytes"
+       "crypto/rand"
        "fmt"
        "net"
 
@@ -47,8 +49,9 @@ var _ = Describe("Link", func() {
                hostVethName      string
                containerVethName string
 
-               ip4one = net.ParseIP("1.1.1.1")
-               ip4two = net.ParseIP("1.1.1.2")
+               ip4one             = net.ParseIP("1.1.1.1")
+               ip4two             = net.ParseIP("1.1.1.2")
+               originalRandReader = rand.Reader
        )
 
        BeforeEach(func() {
@@ -60,6 +63,10 @@ var _ = Describe("Link", func() {
                containerNetNS, err = ns.NewNS()
                Expect(err).NotTo(HaveOccurred())
 
+               fakeBytes := make([]byte, 20)
+               //to be reset in AfterEach block
+               rand.Reader = bytes.NewReader(fakeBytes)
+
                _ = containerNetNS.Do(func(ns.NetNS) error {
                        defer GinkgoRecover()
 
@@ -80,6 +87,7 @@ var _ = Describe("Link", func() {
                Expect(containerNetNS.Close()).To(Succeed())
                Expect(hostNetNS.Close()).To(Succeed())
                ifaceCounter++
+               rand.Reader = originalRandReader
        })
 
        It("SetupVeth must put the veth endpoints into the separate namespaces", func() {
@@ -102,6 +110,74 @@ var _ = Describe("Link", func() {
                })
        })
 
+       Context("when container already has an interface with the same name", func() {
+               It("returns useful error", func() {
+                       _ = containerNetNS.Do(func(ns.NetNS) error {
+                               defer GinkgoRecover()
+
+                               _, _, err := ip.SetupVeth(containerVethName, mtu, hostNetNS)
+                               Expect(err.Error()).To(Equal(fmt.Sprintf("container veth name provided (%s) already exists", containerVethName)))
+
+                               return nil
+                       })
+               })
+       })
+
+       Context("when there is no name available for the host-side", func() {
+               BeforeEach(func() {
+                       //adding different interface to container ns
+                       containerVethName += "0"
+               })
+               It("returns useful error", func() {
+                       _ = containerNetNS.Do(func(ns.NetNS) error {
+                               defer GinkgoRecover()
+
+                               _, _, err := ip.SetupVeth(containerVethName, mtu, hostNetNS)
+                               Expect(err.Error()).To(Equal("failed to move veth to host netns: file exists"))
+
+                               return nil
+                       })
+               })
+       })
+
+       Context("when there is no name conflict for the host or container interfaces", func() {
+               BeforeEach(func() {
+                       //adding different interface to container and host ns
+                       containerVethName += "0"
+                       rand.Reader = originalRandReader
+               })
+               It("successfully creates the second veth pair", func() {
+                       _ = containerNetNS.Do(func(ns.NetNS) error {
+                               defer GinkgoRecover()
+
+                               hostVeth, _, err := ip.SetupVeth(containerVethName, mtu, hostNetNS)
+                               Expect(err).NotTo(HaveOccurred())
+                               hostVethName = hostVeth.Attrs().Name
+                               return nil
+                       })
+
+                       //verify veths are in different namespaces
+                       _ = containerNetNS.Do(func(ns.NetNS) error {
+                               defer GinkgoRecover()
+
+                               _, err := netlink.LinkByName(containerVethName)
+                               Expect(err).NotTo(HaveOccurred())
+
+                               return nil
+                       })
+
+                       _ = hostNetNS.Do(func(ns.NetNS) error {
+                               defer GinkgoRecover()
+
+                               _, err := netlink.LinkByName(hostVethName)
+                               Expect(err).NotTo(HaveOccurred())
+
+                               return nil
+                       })
+               })
+
+       })
+
        It("DelLinkByName must delete the veth endpoints", func() {
                _ = containerNetNS.Do(func(ns.NetNS) error {
                        defer GinkgoRecover()