Merge pull request #280 from prateekgogia/bug-fix-155

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

pkg/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)

pkg/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")
+		ip4one             = net.ParseIP("1.1.1.1")
-		ip4two = net.ParseIP("1.1.1.2")
+		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()