Merge pull request #211 from dcbw/e2e-testing

Fix namespace switch issues and add ipvlan, macvlan, and bridge e2e testing

pkg/ip/link.go

@@ -81,7 +81,7 @@ func RandomVethName() (string, error) {
 // SetupVeth sets up a virtual ethernet link.
 // Should be in container netns, and will switch back to hostNS to set the host
 // veth end up.
-func SetupVeth(contVethName string, mtu int, hostNS *os.File) (hostVeth, contVeth netlink.Link, err error) {
+func SetupVeth(contVethName string, mtu int, hostNS ns.NetNS) (hostVeth, contVeth netlink.Link, err error) {
 	var hostVethName string
 	hostVethName, contVeth, err = makeVeth(contVethName, mtu)
 	if err != nil {
@@ -104,10 +104,10 @@ func SetupVeth(contVethName string, mtu int, hostNS *os.File) (hostVeth, contVet
 		return
 	}
 
-	err = ns.WithNetNS(hostNS, false, func(_ *os.File) error {
+	err = hostNS.Do(func(_ ns.NetNS) error {
 		hostVeth, err := netlink.LinkByName(hostVethName)
 		if err != nil {
-			return fmt.Errorf("failed to lookup %q in %q: %v", hostVethName, hostNS.Name(), err)
+			return fmt.Errorf("failed to lookup %q in %q: %v", hostVethName, hostNS.Path(), err)
 		}
 
 		if err = netlink.LinkSetUp(hostVeth); err != nil {

pkg/ns/README.md

@@ -0,0 +1,31 @@
+### Namespaces, Threads, and Go
+On Linux each OS thread can have a different network namespace.  Go's thread scheduling model switches goroutines between OS threads based on OS thread load and whether the goroutine would block other goroutines.  This can result in a goroutine switching network namespaces without notice and lead to errors in your code.
+
+### Namespace Switching
+Switching namespaces with the `ns.Set()` method is not recommended without additional strategies to prevent unexpected namespace changes when your goroutines switch OS threads.
+
+Go provides the `runtime.LockOSThread()` function to ensure a specific goroutine executes on its current OS thread and prevents any other goroutine from running in that thread until the locked one exits.  Careful usage of `LockOSThread()` and goroutines can provide good control over which network namespace a given goroutine executes in.
+
+For example, you cannot rely on the `ns.Set()` namespace being the current namespace after the `Set()` call unless you do two things.  First, the goroutine calling `Set()` must have previously called `LockOSThread()`.  Second, you must ensure `runtime.UnlockOSThread()` is not called somewhere in-between.  You also cannot rely on the initial network namespace remaining the current network namespace if any other code in your program switches namespaces, unless you have already called `LockOSThread()` in that goroutine.  Note that `LockOSThread()` prevents the Go scheduler from optimally scheduling goroutines for best performance, so `LockOSThread()` should only be used in small, isolated goroutines that release the lock quickly.
+
+### Do() The Recommended Thing
+The `ns.Do()` method provides control over network namespaces for you by implementing these strategies. All code dependent on a particular network namespace should be wrapped in the `ns.Do()` method to ensure the correct namespace is selected for the duration of your code.  For example:
+
+```go
+targetNs, err := ns.NewNS()
+if err != nil {
+    return err
+}
+err = targetNs.Do(func(hostNs ns.NetNS) error {
+	dummy := &netlink.Dummy{
+		LinkAttrs: netlink.LinkAttrs{
+			Name: "dummy0",
+		},
+	}
+	return netlink.LinkAdd(dummy)
+})
+```
+
+### Further Reading
+ - https://github.com/golang/go/wiki/LockOSThread
+ - http://morsmachine.dk/go-scheduler

pkg/ns/consts_linux_arm.go

@@ -1,17 +0,0 @@
-// Copyright 2015 CNI authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-package ns
-
-const setNsNr = 375

pkg/ns/ns.go

@@ -15,17 +15,211 @@
 package ns
 
 import (
+	"crypto/rand"
 	"fmt"
 	"os"
+	"path"
 	"runtime"
-	"syscall"
+	"sync"
+
+	"golang.org/x/sys/unix"
 )
 
-// SetNS sets the network namespace on a target file.
+type NetNS interface {
-func SetNS(f *os.File, flags uintptr) error {
+	// Executes the passed closure in this object's network namespace,
-	_, _, err := syscall.RawSyscall(setNsNr, f.Fd(), flags, 0)
+	// attemtping to restore the original namespace before returning.
-	if err != 0 {
+	// However, since each OS thread can have a different network namespace,
-		return err
+	// and Go's thread scheduling is highly variable, callers cannot
+	// guarantee any specific namespace is set unless operations that
+	// require that namespace are wrapped with Do().  Also, no code called
+	// from Do() should call runtime.UnlockOSThread(), or the risk
+	// of executing code in an incorrect namespace will be greater.  See
+	// https://github.com/golang/go/wiki/LockOSThread for further details.
+	Do(toRun func(NetNS) error) error
+
+	// Sets the current network namespace to this object's network namespace.
+	// Note that since Go's thread scheduling is highly variable, callers
+	// cannot guarantee the requested namespace will be the current namespace
+	// after this function is called; to ensure this wrap operations that
+	// require the namespace with Do() instead.
+	Set() error
+
+	// Returns the filesystem path representing this object's network namespace
+	Path() string
+
+	// Returns a file descriptor representing this object's network namespace
+	Fd() uintptr
+
+	// Cleans up this instance of the network namespace; if this instance
+	// is the last user the namespace will be destroyed
+	Close() error
+}
+
+type netNS struct {
+	file    *os.File
+	mounted bool
+}
+
+func getCurrentThreadNetNSPath() string {
+	// /proc/self/ns/net returns the namespace of the main thread, not
+	// of whatever thread this goroutine is running on.  Make sure we
+	// use the thread's net namespace since the thread is switching around
+	return fmt.Sprintf("/proc/%d/task/%d/ns/net", os.Getpid(), unix.Gettid())
+}
+
+// Returns an object representing the current OS thread's network namespace
+func GetCurrentNS() (NetNS, error) {
+	return GetNS(getCurrentThreadNetNSPath())
+}
+
+// Returns an object representing the namespace referred to by @path
+func GetNS(nspath string) (NetNS, error) {
+	fd, err := os.Open(nspath)
+	if err != nil {
+		return nil, err
+	}
+	return &netNS{file: fd}, nil
+}
+
+// Creates a new persistent network namespace and returns an object
+// representing that namespace, without switching to it
+func NewNS() (NetNS, error) {
+	const nsRunDir = "/var/run/netns"
+
+	b := make([]byte, 16)
+	_, err := rand.Reader.Read(b)
+	if err != nil {
+		return nil, fmt.Errorf("failed to generate random netns name: %v", err)
+	}
+
+	err = os.MkdirAll(nsRunDir, 0755)
+	if err != nil {
+		return nil, err
+	}
+
+	// create an empty file at the mount point
+	nsName := fmt.Sprintf("cni-%x-%x-%x-%x-%x", b[0:4], b[4:6], b[6:8], b[8:10], b[10:])
+	nsPath := path.Join(nsRunDir, nsName)
+	mountPointFd, err := os.Create(nsPath)
+	if err != nil {
+		return nil, err
+	}
+	mountPointFd.Close()
+
+	// Ensure the mount point is cleaned up on errors; if the namespace
+	// was successfully mounted this will have no effect because the file
+	// is in-use
+	defer os.RemoveAll(nsPath)
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+
+	// do namespace work in a dedicated goroutine, so that we can safely
+	// Lock/Unlock OSThread without upsetting the lock/unlock state of
+	// the caller of this function
+	var fd *os.File
+	go (func() {
+		defer wg.Done()
+		runtime.LockOSThread()
+
+		var origNS NetNS
+		origNS, err = GetNS(getCurrentThreadNetNSPath())
+		if err != nil {
+			return
+		}
+		defer origNS.Close()
+
+		// create a new netns on the current thread
+		err = unix.Unshare(unix.CLONE_NEWNET)
+		if err != nil {
+			return
+		}
+		defer origNS.Set()
+
+		// bind mount the new netns from the current thread onto the mount point
+		err = unix.Mount(getCurrentThreadNetNSPath(), nsPath, "none", unix.MS_BIND, "")
+		if err != nil {
+			return
+		}
+
+		fd, err = os.Open(nsPath)
+		if err != nil {
+			return
+		}
+	})()
+	wg.Wait()
+
+	if err != nil {
+		unix.Unmount(nsPath, unix.MNT_DETACH)
+		return nil, fmt.Errorf("failed to create namespace: %v", err)
+	}
+
+	return &netNS{file: fd, mounted: true}, nil
+}
+
+func (ns *netNS) Path() string {
+	return ns.file.Name()
+}
+
+func (ns *netNS) Fd() uintptr {
+	return ns.file.Fd()
+}
+
+func (ns *netNS) Close() error {
+	ns.file.Close()
+
+	if ns.mounted {
+		if err := unix.Unmount(ns.file.Name(), unix.MNT_DETACH); err != nil {
+			return fmt.Errorf("Failed to unmount namespace %s: %v", ns.file.Name(), err)
+		}
+		if err := os.RemoveAll(ns.file.Name()); err != nil {
+			return fmt.Errorf("Failed to clean up namespace %s: %v", ns.file.Name(), err)
+		}
+	}
+	return nil
+}
+
+func (ns *netNS) Do(toRun func(NetNS) error) error {
+	containedCall := func(hostNS NetNS) error {
+		threadNS, err := GetNS(getCurrentThreadNetNSPath())
+		if err != nil {
+			return fmt.Errorf("failed to open current netns: %v", err)
+		}
+		defer threadNS.Close()
+
+		// switch to target namespace
+		if err = ns.Set(); err != nil {
+			return fmt.Errorf("error switching to ns %v: %v", ns.file.Name(), err)
+		}
+		defer threadNS.Set() // switch back
+
+		return toRun(hostNS)
+	}
+
+	// save a handle to current network namespace
+	hostNS, err := GetNS(getCurrentThreadNetNSPath())
+	if err != nil {
+		return fmt.Errorf("Failed to open current namespace: %v", err)
+	}
+	defer hostNS.Close()
+
+	var wg sync.WaitGroup
+	wg.Add(1)
+
+	var innerError error
+	go func() {
+		defer wg.Done()
+		runtime.LockOSThread()
+		innerError = containedCall(hostNS)
+	}()
+	wg.Wait()
+
+	return innerError
+}
+
+func (ns *netNS) Set() error {
+	if _, _, err := unix.Syscall(unix.SYS_SETNS, ns.Fd(), uintptr(unix.CLONE_NEWNET), 0); err != 0 {
+		return fmt.Errorf("Error switching to ns %v: %v", ns.file.Name(), err)
 	}
 
 	return nil
@@ -33,46 +227,11 @@ func SetNS(f *os.File, flags uintptr) error {
 
 // WithNetNSPath executes the passed closure under the given network
 // namespace, restoring the original namespace afterwards.
-// Changing namespaces must be done on a goroutine that has been
+func WithNetNSPath(nspath string, toRun func(NetNS) error) error {
-// locked to an OS thread. If lockThread arg is true, this function
+	ns, err := GetNS(nspath)
-// locks the goroutine prior to change namespace and unlocks before
-// returning
-func WithNetNSPath(nspath string, lockThread bool, f func(*os.File) error) error {
-	ns, err := os.Open(nspath)
 	if err != nil {
 		return fmt.Errorf("Failed to open %v: %v", nspath, err)
 	}
 	defer ns.Close()
-	return WithNetNS(ns, lockThread, f)
+	return ns.Do(toRun)
-}
-
-// WithNetNS executes the passed closure under the given network
-// namespace, restoring the original namespace afterwards.
-// Changing namespaces must be done on a goroutine that has been
-// locked to an OS thread. If lockThread arg is true, this function
-// locks the goroutine prior to change namespace and unlocks before
-// returning.  If the closure returns an error, WithNetNS attempts to
-// restore the original namespace before returning.
-func WithNetNS(ns *os.File, lockThread bool, f func(*os.File) error) error {
-	if lockThread {
-		runtime.LockOSThread()
-		defer runtime.UnlockOSThread()
-	}
-	// save a handle to current (host) network namespace
-	thisNS, err := os.Open("/proc/self/ns/net")
-	if err != nil {
-		return fmt.Errorf("Failed to open /proc/self/ns/net: %v", err)
-	}
-	defer thisNS.Close()
-
-	if err = SetNS(ns, syscall.CLONE_NEWNET); err != nil {
-		return fmt.Errorf("Error switching to ns %v: %v", ns.Name(), err)
-	}
-	defer SetNS(thisNS, syscall.CLONE_NEWNET) // switch back
-
-	if err = f(thisNS); err != nil {
-		return err
-	}
-
-	return nil
 }

pkg/ns/ns_test.go

@@ -17,109 +17,123 @@ package ns_test
 import (
 	"errors"
 	"fmt"
-	"math/rand"
 	"os"
-	"os/exec"
 	"path/filepath"
 
 	"github.com/containernetworking/cni/pkg/ns"
-	"github.com/containernetworking/cni/pkg/testhelpers"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
+	"golang.org/x/sys/unix"
 )
 
+func getInodeCurNetNS() (uint64, error) {
+	curNS, err := ns.GetCurrentNS()
+	if err != nil {
+		return 0, err
+	}
+	defer curNS.Close()
+	return getInodeNS(curNS)
+}
+
+func getInodeNS(netns ns.NetNS) (uint64, error) {
+	return getInodeFd(int(netns.Fd()))
+}
+
+func getInode(path string) (uint64, error) {
+	file, err := os.Open(path)
+	if err != nil {
+		return 0, err
+	}
+	defer file.Close()
+	return getInodeFd(int(file.Fd()))
+}
+
+func getInodeFd(fd int) (uint64, error) {
+	stat := &unix.Stat_t{}
+	err := unix.Fstat(fd, stat)
+	return stat.Ino, err
+}
+
 var _ = Describe("Linux namespace operations", func() {
 	Describe("WithNetNS", func() {
 		var (
-			targetNetNSName string
+			originalNetNS ns.NetNS
-			targetNetNSPath string
+			targetNetNS   ns.NetNS
-			targetNetNS     *os.File
 		)
 
 		BeforeEach(func() {
 			var err error
 
-			targetNetNSName = fmt.Sprintf("test-netns-%d", rand.Int())
+			originalNetNS, err = ns.NewNS()
-
-			err = exec.Command("ip", "netns", "add", targetNetNSName).Run()
 			Expect(err).NotTo(HaveOccurred())
 
-			targetNetNSPath = filepath.Join("/var/run/netns/", targetNetNSName)
+			targetNetNS, err = ns.NewNS()
-			targetNetNS, err = os.Open(targetNetNSPath)
 			Expect(err).NotTo(HaveOccurred())
 		})
 
 		AfterEach(func() {
 			Expect(targetNetNS.Close()).To(Succeed())
-
+			Expect(originalNetNS.Close()).To(Succeed())
-			err := exec.Command("ip", "netns", "del", targetNetNSName).Run()
-			Expect(err).NotTo(HaveOccurred())
 		})
 
 		It("executes the callback within the target network namespace", func() {
-			expectedInode, err := testhelpers.GetInode(targetNetNSPath)
+			expectedInode, err := getInodeNS(targetNetNS)
 			Expect(err).NotTo(HaveOccurred())
 
-			var actualInode uint64
+			err = targetNetNS.Do(func(ns.NetNS) error {
-			var innerErr error
+				defer GinkgoRecover()
-			err = ns.WithNetNS(targetNetNS, false, func(*os.File) error {
+
-				actualInode, innerErr = testhelpers.GetInodeCurNetNS()
+				actualInode, err := getInodeCurNetNS()
+				Expect(err).NotTo(HaveOccurred())
+				Expect(actualInode).To(Equal(expectedInode))
 				return nil
 			})
 			Expect(err).NotTo(HaveOccurred())
-
-			Expect(innerErr).NotTo(HaveOccurred())
-			Expect(actualInode).To(Equal(expectedInode))
 		})
 
 		It("provides the original namespace as the argument to the callback", func() {
-			hostNSInode, err := testhelpers.GetInodeCurNetNS()
+			// Ensure we start in originalNetNS
-			Expect(err).NotTo(HaveOccurred())
+			err := originalNetNS.Do(func(ns.NetNS) error {
+				defer GinkgoRecover()
 
-			var inputNSInode uint64
+				origNSInode, err := getInodeNS(originalNetNS)
-			var innerErr error
+				Expect(err).NotTo(HaveOccurred())
-			err = ns.WithNetNS(targetNetNS, false, func(inputNS *os.File) error {
+
-				inputNSInode, err = testhelpers.GetInodeF(inputNS)
+				err = targetNetNS.Do(func(hostNS ns.NetNS) error {
+					defer GinkgoRecover()
+
+					hostNSInode, err := getInodeNS(hostNS)
+					Expect(err).NotTo(HaveOccurred())
+					Expect(hostNSInode).To(Equal(origNSInode))
+					return nil
+				})
 				return nil
 			})
 			Expect(err).NotTo(HaveOccurred())
-
-			Expect(innerErr).NotTo(HaveOccurred())
-			Expect(inputNSInode).To(Equal(hostNSInode))
-		})
-
-		It("restores the calling thread to the original network namespace", func() {
-			preTestInode, err := testhelpers.GetInodeCurNetNS()
-			Expect(err).NotTo(HaveOccurred())
-
-			err = ns.WithNetNS(targetNetNS, false, func(*os.File) error {
-				return nil
-			})
-			Expect(err).NotTo(HaveOccurred())
-
-			postTestInode, err := testhelpers.GetInodeCurNetNS()
-			Expect(err).NotTo(HaveOccurred())
-
-			Expect(postTestInode).To(Equal(preTestInode))
 		})
 
 		Context("when the callback returns an error", func() {
 			It("restores the calling thread to the original namespace before returning", func() {
-				preTestInode, err := testhelpers.GetInodeCurNetNS()
+				err := originalNetNS.Do(func(ns.NetNS) error {
-				Expect(err).NotTo(HaveOccurred())
+					defer GinkgoRecover()
 
-				_ = ns.WithNetNS(targetNetNS, false, func(*os.File) error {
+					preTestInode, err := getInodeCurNetNS()
-					return errors.New("potato")
+					Expect(err).NotTo(HaveOccurred())
+
+					_ = targetNetNS.Do(func(ns.NetNS) error {
+						return errors.New("potato")
+					})
+
+					postTestInode, err := getInodeCurNetNS()
+					Expect(err).NotTo(HaveOccurred())
+					Expect(postTestInode).To(Equal(preTestInode))
+					return nil
 				})
-
-				postTestInode, err := testhelpers.GetInodeCurNetNS()
 				Expect(err).NotTo(HaveOccurred())
-
-				Expect(postTestInode).To(Equal(preTestInode))
 			})
 
 			It("returns the error from the callback", func() {
-				err := ns.WithNetNS(targetNetNS, false, func(*os.File) error {
+				err := targetNetNS.Do(func(ns.NetNS) error {
 					return errors.New("potato")
 				})
 				Expect(err).To(MatchError("potato"))
@@ -128,16 +142,40 @@ var _ = Describe("Linux namespace operations", func() {
 
 		Describe("validating inode mapping to namespaces", func() {
 			It("checks that different namespaces have different inodes", func() {
-				hostNSInode, err := testhelpers.GetInodeCurNetNS()
+				origNSInode, err := getInodeNS(originalNetNS)
 				Expect(err).NotTo(HaveOccurred())
 
-				testNsInode, err := testhelpers.GetInode(targetNetNSPath)
+				testNsInode, err := getInodeNS(targetNetNS)
 				Expect(err).NotTo(HaveOccurred())
 
-				Expect(hostNSInode).NotTo(Equal(0))
 				Expect(testNsInode).NotTo(Equal(0))
-				Expect(testNsInode).NotTo(Equal(hostNSInode))
+				Expect(testNsInode).NotTo(Equal(origNSInode))
+			})
+
+			It("should not leak a closed netns onto any threads in the process", func() {
+				By("creating a new netns")
+				createdNetNS, err := ns.NewNS()
+				Expect(err).NotTo(HaveOccurred())
+
+				By("discovering the inode of the created netns")
+				createdNetNSInode, err := getInodeNS(createdNetNS)
+				Expect(err).NotTo(HaveOccurred())
+				createdNetNS.Close()
+
+				By("comparing against the netns inode of every thread in the process")
+				for _, netnsPath := range allNetNSInCurrentProcess() {
+					netnsInode, err := getInode(netnsPath)
+					Expect(err).NotTo(HaveOccurred())
+					Expect(netnsInode).NotTo(Equal(createdNetNSInode))
+				}
 			})
 		})
 	})
 })
+
+func allNetNSInCurrentProcess() []string {
+	pid := unix.Getpid()
+	paths, err := filepath.Glob(fmt.Sprintf("/proc/%d/task/*/ns/net", pid))
+	Expect(err).NotTo(HaveOccurred())
+	return paths
+}

pkg/testhelpers/testhelpers.go

@@ -1,136 +0,0 @@
-// Copyright 2016 CNI authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// Package testhelpers provides common support behavior for tests
-package testhelpers
-
-import (
-	"fmt"
-	"os"
-	"runtime"
-	"sync"
-
-	"golang.org/x/sys/unix"
-
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
-)
-
-func getCurrentThreadNetNSPath() string {
-	pid := unix.Getpid()
-	tid := unix.Gettid()
-	return fmt.Sprintf("/proc/%d/task/%d/ns/net", pid, tid)
-}
-
-func GetInodeCurNetNS() (uint64, error) {
-	return GetInode(getCurrentThreadNetNSPath())
-}
-
-func GetInode(path string) (uint64, error) {
-	file, err := os.Open(path)
-	if err != nil {
-		return 0, err
-	}
-	defer file.Close()
-	return GetInodeF(file)
-}
-
-func GetInodeF(file *os.File) (uint64, error) {
-	stat := &unix.Stat_t{}
-	err := unix.Fstat(int(file.Fd()), stat)
-	return stat.Ino, err
-}
-
-/*
-A note about goroutines, Linux namespaces and runtime.LockOSThread
-
-In Linux, network namespaces have thread affinity.
-
-In the Go language runtime, goroutines do not have affinity for OS threads.
-The Go runtime scheduler moves goroutines around amongst OS threads.  It
-is supposed to be transparent to the Go programmer.
-
-In order to address cases where the programmer needs thread affinity, Go
-provides runtime.LockOSThread and runtime.UnlockOSThread()
-
-However, the Go runtime does not reference count the Lock and Unlock calls.
-Repeated calls to Lock will succeed, but the first call to Unlock will unlock
-everything.  Therefore, it is dangerous to hide a Lock/Unlock in a library
-function, such as in this package.
-
-The code below, in MakeNetworkNS, avoids this problem by spinning up a new
-Go routine specifically so that LockOSThread can be called on it.  Thus
-goroutine-thread affinity is maintained long enough to perform all the required
-namespace operations.
-
-Because the LockOSThread call is performed inside this short-lived goroutine,
-there is no effect either way on the caller's goroutine-thread affinity.
-
-* */
-
-func MakeNetworkNS(containerID string) string {
-	namespace := "/var/run/netns/" + containerID
-
-	err := os.MkdirAll("/var/run/netns", 0600)
-	Expect(err).NotTo(HaveOccurred())
-
-	// create an empty file at the mount point
-	mountPointFd, err := os.Create(namespace)
-	Expect(err).NotTo(HaveOccurred())
-	mountPointFd.Close()
-
-	var wg sync.WaitGroup
-	wg.Add(1)
-
-	// do namespace work in a dedicated goroutine, so that we can safely
-	// Lock/Unlock OSThread without upsetting the lock/unlock state of
-	// the caller of this function.  See block comment above.
-	go (func() {
-		defer wg.Done()
-
-		runtime.LockOSThread()
-		defer runtime.UnlockOSThread()
-
-		defer GinkgoRecover()
-
-		// capture current thread's original netns
-		currentThreadNetNSPath := getCurrentThreadNetNSPath()
-		originalNetNS, err := unix.Open(currentThreadNetNSPath, unix.O_RDONLY, 0)
-		Expect(err).NotTo(HaveOccurred())
-		defer unix.Close(originalNetNS)
-
-		// create a new netns on the current thread
-		err = unix.Unshare(unix.CLONE_NEWNET)
-		Expect(err).NotTo(HaveOccurred())
-
-		// bind mount the new netns from the current thread onto the mount point
-		err = unix.Mount(currentThreadNetNSPath, namespace, "none", unix.MS_BIND, "")
-		Expect(err).NotTo(HaveOccurred())
-
-		// reset current thread's netns to the original
-		_, _, e1 := unix.Syscall(unix.SYS_SETNS, uintptr(originalNetNS), uintptr(unix.CLONE_NEWNET), 0)
-		Expect(e1).To(BeZero())
-	})()
-
-	wg.Wait()
-
-	return namespace
-}
-
-func RemoveNetworkNS(networkNS string) error {
-	err := unix.Unmount(networkNS, unix.MNT_DETACH)
-
-	err = os.RemoveAll(networkNS)
-	return err
-}

pkg/testhelpers/testhelpers_test.go

@@ -1,96 +0,0 @@
-// Copyright 2016 CNI authors
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-//     http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-
-// Package testhelpers_test contains unit tests of the testhelpers
-//
-// Some of this stuff is non-trivial and can interact in surprising ways
-// with the Go runtime.  Better be safe.
-package testhelpers_test
-
-import (
-	"fmt"
-	"math/rand"
-	"path/filepath"
-
-	"golang.org/x/sys/unix"
-
-	"github.com/containernetworking/cni/pkg/testhelpers"
-	. "github.com/onsi/ginkgo"
-	. "github.com/onsi/gomega"
-)
-
-var _ = Describe("Test helper functions", func() {
-	Describe("MakeNetworkNS", func() {
-		It("should return the filepath to a network namespace", func() {
-			containerID := fmt.Sprintf("c-%x", rand.Int31())
-			nsPath := testhelpers.MakeNetworkNS(containerID)
-
-			Expect(nsPath).To(BeAnExistingFile())
-
-			testhelpers.RemoveNetworkNS(containerID)
-		})
-
-		It("should return a network namespace different from that of the caller", func() {
-			containerID := fmt.Sprintf("c-%x", rand.Int31())
-
-			By("discovering the inode of the current netns")
-			originalNetNSPath := currentNetNSPath()
-			originalNetNSInode, err := testhelpers.GetInode(originalNetNSPath)
-			Expect(err).NotTo(HaveOccurred())
-
-			By("creating a new netns")
-			createdNetNSPath := testhelpers.MakeNetworkNS(containerID)
-			defer testhelpers.RemoveNetworkNS(createdNetNSPath)
-
-			By("discovering the inode of the created netns")
-			createdNetNSInode, err := testhelpers.GetInode(createdNetNSPath)
-			Expect(err).NotTo(HaveOccurred())
-
-			By("comparing the inodes")
-			Expect(createdNetNSInode).NotTo(Equal(originalNetNSInode))
-		})
-
-		It("should not leak the new netns onto any threads in the process", func() {
-			containerID := fmt.Sprintf("c-%x", rand.Int31())
-
-			By("creating a new netns")
-			createdNetNSPath := testhelpers.MakeNetworkNS(containerID)
-			defer testhelpers.RemoveNetworkNS(createdNetNSPath)
-
-			By("discovering the inode of the created netns")
-			createdNetNSInode, err := testhelpers.GetInode(createdNetNSPath)
-			Expect(err).NotTo(HaveOccurred())
-
-			By("comparing against the netns inode of every thread in the process")
-			for _, netnsPath := range allNetNSInCurrentProcess() {
-				netnsInode, err := testhelpers.GetInode(netnsPath)
-				Expect(err).NotTo(HaveOccurred())
-				Expect(netnsInode).NotTo(Equal(createdNetNSInode))
-			}
-		})
-	})
-})
-
-func currentNetNSPath() string {
-	pid := unix.Getpid()
-	tid := unix.Gettid()
-	return fmt.Sprintf("/proc/%d/task/%d/ns/net", pid, tid)
-}
-
-func allNetNSInCurrentProcess() []string {
-	pid := unix.Getpid()
-	paths, err := filepath.Glob(fmt.Sprintf("/proc/%d/task/*/ns/net", pid))
-	Expect(err).NotTo(HaveOccurred())
-	return paths
-}

pkg/testutils/cmd.go

@@ -0,0 +1,77 @@
+// Copyright 2016 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package testutils
+
+import (
+	"encoding/json"
+	"io/ioutil"
+	"os"
+
+	"github.com/containernetworking/cni/pkg/types"
+)
+
+func envCleanup() {
+	os.Unsetenv("CNI_COMMAND")
+	os.Unsetenv("CNI_PATH")
+	os.Unsetenv("CNI_NETNS")
+	os.Unsetenv("CNI_IFNAME")
+}
+
+func CmdAddWithResult(cniNetns, cniIfname string, f func() error) (*types.Result, error) {
+	os.Setenv("CNI_COMMAND", "ADD")
+	os.Setenv("CNI_PATH", os.Getenv("PATH"))
+	os.Setenv("CNI_NETNS", cniNetns)
+	os.Setenv("CNI_IFNAME", cniIfname)
+	defer envCleanup()
+
+	// Redirect stdout to capture plugin result
+	oldStdout := os.Stdout
+	r, w, err := os.Pipe()
+	if err != nil {
+		return nil, err
+	}
+
+	os.Stdout = w
+	err = f()
+	w.Close()
+	if err != nil {
+		return nil, err
+	}
+
+	// parse the result
+	out, err := ioutil.ReadAll(r)
+	os.Stdout = oldStdout
+	if err != nil {
+		return nil, err
+	}
+
+	result := types.Result{}
+	err = json.Unmarshal(out, &result)
+	if err != nil {
+		return nil, err
+	}
+
+	return &result, nil
+}
+
+func CmdDelWithResult(cniNetns, cniIfname string, f func() error) error {
+	os.Setenv("CNI_COMMAND", "DEL")
+	os.Setenv("CNI_PATH", os.Getenv("PATH"))
+	os.Setenv("CNI_NETNS", cniNetns)
+	os.Setenv("CNI_IFNAME", cniIfname)
+	defer envCleanup()
+
+	return f()
+}

plugins/ipam/dhcp/lease.go

@@ -19,7 +19,6 @@ import (
 	"log"
 	"math/rand"
 	"net"
-	"os"
 	"sync"
 	"time"
 
@@ -74,7 +73,7 @@ func AcquireLease(clientID, netns, ifName string) (*DHCPLease, error) {
 
 	l.wg.Add(1)
 	go func() {
-		errCh <- ns.WithNetNSPath(netns, true, func(_ *os.File) error {
+		errCh <- ns.WithNetNSPath(netns, func(_ ns.NetNS) error {
 			defer l.wg.Done()
 
 			link, err := netlink.LinkByName(ifName)

plugins/main/bridge/bridge.go

@@ -19,7 +19,6 @@ import (
 	"errors"
 	"fmt"
 	"net"
-	"os"
 	"runtime"
 	"syscall"
 
@@ -129,10 +128,10 @@ func ensureBridge(brName string, mtu int) (*netlink.Bridge, error) {
 	return br, nil
 }
 
-func setupVeth(netns string, br *netlink.Bridge, ifName string, mtu int, hairpinMode bool) error {
+func setupVeth(netns ns.NetNS, br *netlink.Bridge, ifName string, mtu int, hairpinMode bool) error {
 	var hostVethName string
 
-	err := ns.WithNetNSPath(netns, false, func(hostNS *os.File) error {
+	err := netns.Do(func(hostNS ns.NetNS) error {
 		// create the veth pair in the container and move host end into host netns
 		hostVeth, _, err := ip.SetupVeth(ifName, mtu, hostNS)
 		if err != nil {
@@ -191,7 +190,13 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
-	if err = setupVeth(args.Netns, br, args.IfName, n.MTU, n.HairpinMode); err != nil {
+	netns, err := ns.GetNS(args.Netns)
+	if err != nil {
+		return fmt.Errorf("failed to open netns %q: %v", args.Netns, err)
+	}
+	defer netns.Close()
+
+	if err = setupVeth(netns, br, args.IfName, n.MTU, n.HairpinMode); err != nil {
 		return err
 	}
 
@@ -209,7 +214,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		result.IP4.Gateway = calcGatewayIP(&result.IP4.IP)
 	}
 
-	err = ns.WithNetNSPath(args.Netns, false, func(hostNS *os.File) error {
+	err = netns.Do(func(_ ns.NetNS) error {
 		return ipam.ConfigureIface(args.IfName, result)
 	})
 	if err != nil {
@@ -254,7 +259,7 @@ func cmdDel(args *skel.CmdArgs) error {
 	}
 
 	var ipn *net.IPNet
-	err = ns.WithNetNSPath(args.Netns, false, func(hostNS *os.File) error {
+	err = ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		var err error
 		ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4)
 		return err

plugins/main/bridge/bridge_suite_test.go

@@ -12,20 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package testhelpers_test
+package main
 
 import (
-	"math/rand"
-
 	. "github.com/onsi/ginkgo"
-	"github.com/onsi/ginkgo/config"
 	. "github.com/onsi/gomega"
 
 	"testing"
 )
 
-func TestTesthelpers(t *testing.T) {
+func TestBridge(t *testing.T) {
-	rand.Seed(config.GinkgoConfig.RandomSeed)
 	RegisterFailHandler(Fail)
-	RunSpecs(t, "Testhelpers Suite")
+	RunSpecs(t, "bridge Suite")
 }

plugins/main/bridge/bridge_test.go

@@ -0,0 +1,225 @@
+// Copyright 2015 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"fmt"
+	"net"
+	"syscall"
+
+	"github.com/containernetworking/cni/pkg/ns"
+	"github.com/containernetworking/cni/pkg/skel"
+	"github.com/containernetworking/cni/pkg/testutils"
+	"github.com/containernetworking/cni/pkg/types"
+
+	"github.com/vishvananda/netlink"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+var _ = Describe("bridge Operations", func() {
+	var originalNS ns.NetNS
+
+	BeforeEach(func() {
+		// Create a new NetNS so we don't modify the host
+		var err error
+		originalNS, err = ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+		Expect(originalNS.Close()).To(Succeed())
+	})
+
+	It("creates a bridge", func() {
+		const IFNAME = "bridge0"
+
+		conf := &NetConf{
+			NetConf: types.NetConf{
+				Name: "testConfig",
+				Type: "bridge",
+			},
+			BrName: IFNAME,
+			IsGW:   false,
+			IPMasq: false,
+			MTU:    5000,
+		}
+
+		err := originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			bridge, err := setupBridge(conf)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(bridge.Attrs().Name).To(Equal(IFNAME))
+
+			// Double check that the link was added
+			link, err := netlink.LinkByName(IFNAME)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().Name).To(Equal(IFNAME))
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("handles an existing bridge", func() {
+		const IFNAME = "bridge0"
+
+		err := originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			err := netlink.LinkAdd(&netlink.Bridge{
+				LinkAttrs: netlink.LinkAttrs{
+					Name: IFNAME,
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+			link, err := netlink.LinkByName(IFNAME)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().Name).To(Equal(IFNAME))
+			ifindex := link.Attrs().Index
+
+			conf := &NetConf{
+				NetConf: types.NetConf{
+					Name: "testConfig",
+					Type: "bridge",
+				},
+				BrName: IFNAME,
+				IsGW:   false,
+				IPMasq: false,
+			}
+
+			bridge, err := setupBridge(conf)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(bridge.Attrs().Name).To(Equal(IFNAME))
+			Expect(bridge.Attrs().Index).To(Equal(ifindex))
+
+			// Double check that the link has the same ifindex
+			link, err = netlink.LinkByName(IFNAME)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().Name).To(Equal(IFNAME))
+			Expect(link.Attrs().Index).To(Equal(ifindex))
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("configures and deconfigures a bridge and veth with ADD/DEL", func() {
+		const BRNAME = "cni0"
+		const IFNAME = "eth0"
+
+		gwaddr, subnet, err := net.ParseCIDR("10.1.2.1/24")
+		Expect(err).NotTo(HaveOccurred())
+
+		conf := fmt.Sprintf(`{
+    "name": "mynet",
+    "type": "bridge",
+    "bridge": "%s",
+    "isGateway": true,
+    "ipMasq": false,
+    "ipam": {
+        "type": "host-local",
+        "subnet": "%s"
+    }
+}`, BRNAME, subnet.String())
+
+		targetNs, err := ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+		defer targetNs.Close()
+
+		args := &skel.CmdArgs{
+			ContainerID: "dummy",
+			Netns:       targetNs.Path(),
+			IfName:      IFNAME,
+			StdinData:   []byte(conf),
+		}
+
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			_, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, func() error {
+				return cmdAdd(args)
+			})
+			Expect(err).NotTo(HaveOccurred())
+
+			// Make sure bridge link exists
+			link, err := netlink.LinkByName(BRNAME)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().Name).To(Equal(BRNAME))
+
+			// Ensure bridge has gateway address
+			addrs, err := netlink.AddrList(link, syscall.AF_INET)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(len(addrs)).To(BeNumerically(">", 0))
+			found := false
+			subnetPrefix, subnetBits := subnet.Mask.Size()
+			for _, a := range addrs {
+				aPrefix, aBits := a.IPNet.Mask.Size()
+				if a.IPNet.IP.Equal(gwaddr) && aPrefix == subnetPrefix && aBits == subnetBits {
+					found = true
+					break
+				}
+			}
+			Expect(found).To(Equal(true))
+
+			// Check for the veth link in the main namespace
+			links, err := netlink.LinkList()
+			Expect(err).NotTo(HaveOccurred())
+			Expect(len(links)).To(Equal(3)) // Bridge, veth, and loopback
+			for _, l := range links {
+				if l.Attrs().Name != BRNAME && l.Attrs().Name != "lo" {
+					_, isVeth := l.(*netlink.Veth)
+					Expect(isVeth).To(Equal(true))
+				}
+			}
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Find the veth peer in the container namespace
+		err = targetNs.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			link, err := netlink.LinkByName(IFNAME)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().Name).To(Equal(IFNAME))
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			err := testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error {
+				return cmdDel(args)
+			})
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Make sure macvlan link has been deleted
+		err = targetNs.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			link, err := netlink.LinkByName(IFNAME)
+			Expect(err).To(HaveOccurred())
+			Expect(link).To(BeNil())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+})

plugins/main/ipvlan/ipvlan.go

@@ -18,7 +18,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"os"
 	"runtime"
 
 	"github.com/containernetworking/cni/pkg/ip"
@@ -65,7 +64,7 @@ func modeFromString(s string) (netlink.IPVlanMode, error) {
 	}
 }
 
-func createIpvlan(conf *NetConf, ifName string, netns *os.File) error {
+func createIpvlan(conf *NetConf, ifName string, netns ns.NetNS) error {
 	mode, err := modeFromString(conf.Mode)
 	if err != nil {
 		return err
@@ -97,7 +96,7 @@ func createIpvlan(conf *NetConf, ifName string, netns *os.File) error {
 		return fmt.Errorf("failed to create ipvlan: %v", err)
 	}
 
-	return ns.WithNetNS(netns, false, func(_ *os.File) error {
+	return netns.Do(func(_ ns.NetNS) error {
 		err := renameLink(tmpName, ifName)
 		if err != nil {
 			return fmt.Errorf("failed to rename ipvlan to %q: %v", ifName, err)
@@ -112,9 +111,9 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
-	netns, err := os.Open(args.Netns)
+	netns, err := ns.GetNS(args.Netns)
 	if err != nil {
-		return fmt.Errorf("failed to open netns %q: %v", netns, err)
+		return fmt.Errorf("failed to open netns %q: %v", args.Netns, err)
 	}
 	defer netns.Close()
 
@@ -131,7 +130,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return errors.New("IPAM plugin returned missing IPv4 config")
 	}
 
-	err = ns.WithNetNS(netns, false, func(_ *os.File) error {
+	err = netns.Do(func(_ ns.NetNS) error {
 		return ipam.ConfigureIface(args.IfName, result)
 	})
 	if err != nil {
@@ -153,7 +152,7 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
-	return ns.WithNetNSPath(args.Netns, false, func(hostNS *os.File) error {
+	return ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		return ip.DelLinkByName(args.IfName)
 	})
 }

plugins/main/ipvlan/ipvlan_suite_test.go

@@ -1,4 +1,4 @@
-// Copyright 2015 CNI authors
+// Copyright 2016 CNI authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,6 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package ns
+package main
 
-const setNsNr = 346
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"testing"
+)
+
+func TestIpvlan(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "ipvlan Suite")
+}

plugins/main/ipvlan/ipvlan_test.go

@@ -0,0 +1,168 @@
+// Copyright 2015 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"fmt"
+
+	"github.com/containernetworking/cni/pkg/ns"
+	"github.com/containernetworking/cni/pkg/skel"
+	"github.com/containernetworking/cni/pkg/testutils"
+	"github.com/containernetworking/cni/pkg/types"
+
+	"github.com/vishvananda/netlink"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+const MASTER_NAME = "eth0"
+
+var _ = Describe("ipvlan Operations", func() {
+	var originalNS ns.NetNS
+
+	BeforeEach(func() {
+		// Create a new NetNS so we don't modify the host
+		var err error
+		originalNS, err = ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			// Add master
+			err = netlink.LinkAdd(&netlink.Dummy{
+				LinkAttrs: netlink.LinkAttrs{
+					Name: MASTER_NAME,
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+			_, err = netlink.LinkByName(MASTER_NAME)
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+		Expect(originalNS.Close()).To(Succeed())
+	})
+
+	It("creates an ipvlan link in a non-default namespace", func() {
+		conf := &NetConf{
+			NetConf: types.NetConf{
+				Name: "testConfig",
+				Type: "ipvlan",
+			},
+			Master: MASTER_NAME,
+			Mode:   "l2",
+			MTU:    1500,
+		}
+
+		// Create ipvlan in other namespace
+		targetNs, err := ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+		defer targetNs.Close()
+
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			err := createIpvlan(conf, "foobar0", targetNs)
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Make sure ipvlan link exists in the target namespace
+		err = targetNs.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			link, err := netlink.LinkByName("foobar0")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().Name).To(Equal("foobar0"))
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("configures and deconfigures an iplvan link with ADD/DEL", func() {
+		const IFNAME = "ipvl0"
+
+		conf := fmt.Sprintf(`{
+    "name": "mynet",
+    "type": "ipvlan",
+    "master": "%s",
+    "ipam": {
+        "type": "host-local",
+        "subnet": "10.1.2.0/24"
+    }
+}`, MASTER_NAME)
+
+		targetNs, err := ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+		defer targetNs.Close()
+
+		args := &skel.CmdArgs{
+			ContainerID: "dummy",
+			Netns:       targetNs.Path(),
+			IfName:      IFNAME,
+			StdinData:   []byte(conf),
+		}
+
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			_, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, func() error {
+				return cmdAdd(args)
+			})
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Make sure ipvlan link exists in the target namespace
+		err = targetNs.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			link, err := netlink.LinkByName(IFNAME)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().Name).To(Equal(IFNAME))
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			err = testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error {
+				return cmdDel(args)
+			})
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Make sure ipvlan link has been deleted
+		err = targetNs.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			link, err := netlink.LinkByName(IFNAME)
+			Expect(err).To(HaveOccurred())
+			Expect(link).To(BeNil())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+})

plugins/main/loopback/loopback.go

@@ -15,8 +15,6 @@
 package main
 
 import (
-	"os"
-
 	"github.com/containernetworking/cni/pkg/ns"
 	"github.com/containernetworking/cni/pkg/skel"
 	"github.com/containernetworking/cni/pkg/types"
@@ -25,7 +23,7 @@ import (
 
 func cmdAdd(args *skel.CmdArgs) error {
 	args.IfName = "lo" // ignore config, this only works for loopback
-	err := ns.WithNetNSPath(args.Netns, false, func(hostNS *os.File) error {
+	err := ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		link, err := netlink.LinkByName(args.IfName)
 		if err != nil {
 			return err // not tested
@@ -48,7 +46,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 
 func cmdDel(args *skel.CmdArgs) error {
 	args.IfName = "lo" // ignore config, this only works for loopback
-	err := ns.WithNetNSPath(args.Netns, false, func(hostNS *os.File) error {
+	err := ns.WithNetNSPath(args.Netns, func(ns.NetNS) error {
 		link, err := netlink.LinkByName(args.IfName)
 		if err != nil {
 			return err // not tested

plugins/main/loopback/loopback_test.go

@@ -17,12 +17,10 @@ package main_test
 import (
 	"fmt"
 	"net"
-	"os"
 	"os/exec"
 	"strings"
 
 	"github.com/containernetworking/cni/pkg/ns"
-	"github.com/containernetworking/cni/pkg/testhelpers"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
@@ -31,7 +29,7 @@ import (
 
 var _ = Describe("Loopback", func() {
 	var (
-		networkNS   string
+		networkNS   ns.NetNS
 		containerID string
 		command     *exec.Cmd
 		environ     []string
@@ -39,12 +37,14 @@ var _ = Describe("Loopback", func() {
 
 	BeforeEach(func() {
 		command = exec.Command(pathToLoPlugin)
-		containerID = "some-container-id"
+
-		networkNS = testhelpers.MakeNetworkNS(containerID)
+		var err error
+		networkNS, err = ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
 
 		environ = []string{
 			fmt.Sprintf("CNI_CONTAINERID=%s", containerID),
-			fmt.Sprintf("CNI_NETNS=%s", networkNS),
+			fmt.Sprintf("CNI_NETNS=%s", networkNS.Path()),
 			fmt.Sprintf("CNI_IFNAME=%s", "this is ignored"),
 			fmt.Sprintf("CNI_ARGS=%s", "none"),
 			fmt.Sprintf("CNI_PATH=%s", "/some/test/path"),
@@ -53,7 +53,7 @@ var _ = Describe("Loopback", func() {
 	})
 
 	AfterEach(func() {
-		Expect(testhelpers.RemoveNetworkNS(networkNS)).To(Succeed())
+		Expect(networkNS.Close()).To(Succeed())
 	})
 
 	Context("when given a network namespace", func() {
@@ -67,7 +67,7 @@ var _ = Describe("Loopback", func() {
 			Eventually(session).Should(gexec.Exit(0))
 
 			var lo *net.Interface
-			err = ns.WithNetNSPath(networkNS, true, func(hostNS *os.File) error {
+			err = networkNS.Do(func(ns.NetNS) error {
 				var err error
 				lo, err = net.InterfaceByName("lo")
 				return err
@@ -87,7 +87,7 @@ var _ = Describe("Loopback", func() {
 			Eventually(session).Should(gexec.Exit(0))
 
 			var lo *net.Interface
-			err = ns.WithNetNSPath(networkNS, true, func(hostNS *os.File) error {
+			err = networkNS.Do(func(ns.NetNS) error {
 				var err error
 				lo, err = net.InterfaceByName("lo")
 				return err

plugins/main/macvlan/macvlan.go

@@ -18,7 +18,6 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
-	"os"
 	"runtime"
 
 	"github.com/containernetworking/cni/pkg/ip"
@@ -74,7 +73,7 @@ func modeFromString(s string) (netlink.MacvlanMode, error) {
 	}
 }
 
-func createMacvlan(conf *NetConf, ifName string, netns *os.File) error {
+func createMacvlan(conf *NetConf, ifName string, netns ns.NetNS) error {
 	mode, err := modeFromString(conf.Mode)
 	if err != nil {
 		return err
@@ -106,7 +105,7 @@ func createMacvlan(conf *NetConf, ifName string, netns *os.File) error {
 		return fmt.Errorf("failed to create macvlan: %v", err)
 	}
 
-	return ns.WithNetNS(netns, false, func(_ *os.File) error {
+	return netns.Do(func(_ ns.NetNS) error {
 		// TODO: duplicate following lines for ipv6 support, when it will be added in other places
 		ipv4SysctlValueName := fmt.Sprintf(IPv4InterfaceArpProxySysctlTemplate, tmpName)
 		if _, err := sysctl.Sysctl(ipv4SysctlValueName, "1"); err != nil {
@@ -130,7 +129,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return err
 	}
 
-	netns, err := os.Open(args.Netns)
+	netns, err := ns.GetNS(args.Netns)
 	if err != nil {
 		return fmt.Errorf("failed to open netns %q: %v", netns, err)
 	}
@@ -149,7 +148,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 		return errors.New("IPAM plugin returned missing IPv4 config")
 	}
 
-	err = ns.WithNetNS(netns, false, func(_ *os.File) error {
+	err = netns.Do(func(_ ns.NetNS) error {
 		return ipam.ConfigureIface(args.IfName, result)
 	})
 	if err != nil {
@@ -171,7 +170,7 @@ func cmdDel(args *skel.CmdArgs) error {
 		return err
 	}
 
-	return ns.WithNetNSPath(args.Netns, false, func(hostNS *os.File) error {
+	return ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		return ip.DelLinkByName(args.IfName)
 	})
 }

plugins/main/macvlan/macvlan_suite_test.go

@@ -1,4 +1,4 @@
-// Copyright 2015 CNI authors
+// Copyright 2016 CNI authors
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -12,6 +12,16 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package ns
+package main
 
-const setNsNr = 308
+import (
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+
+	"testing"
+)
+
+func TestMacvlan(t *testing.T) {
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "macvlan Suite")
+}

plugins/main/macvlan/macvlan_test.go

@@ -0,0 +1,168 @@
+// Copyright 2015 CNI authors
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package main
+
+import (
+	"fmt"
+
+	"github.com/containernetworking/cni/pkg/ns"
+	"github.com/containernetworking/cni/pkg/skel"
+	"github.com/containernetworking/cni/pkg/testutils"
+	"github.com/containernetworking/cni/pkg/types"
+
+	"github.com/vishvananda/netlink"
+
+	. "github.com/onsi/ginkgo"
+	. "github.com/onsi/gomega"
+)
+
+const MASTER_NAME = "eth0"
+
+var _ = Describe("macvlan Operations", func() {
+	var originalNS ns.NetNS
+
+	BeforeEach(func() {
+		// Create a new NetNS so we don't modify the host
+		var err error
+		originalNS, err = ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			// Add master
+			err = netlink.LinkAdd(&netlink.Dummy{
+				LinkAttrs: netlink.LinkAttrs{
+					Name: MASTER_NAME,
+				},
+			})
+			Expect(err).NotTo(HaveOccurred())
+			_, err = netlink.LinkByName(MASTER_NAME)
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	AfterEach(func() {
+		Expect(originalNS.Close()).To(Succeed())
+	})
+
+	It("creates an macvlan link in a non-default namespace", func() {
+		conf := &NetConf{
+			NetConf: types.NetConf{
+				Name: "testConfig",
+				Type: "macvlan",
+			},
+			Master: MASTER_NAME,
+			Mode:   "bridge",
+			MTU:    1500,
+		}
+
+		targetNs, err := ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+		defer targetNs.Close()
+
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			err = createMacvlan(conf, "foobar0", targetNs)
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Make sure macvlan link exists in the target namespace
+		err = targetNs.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			link, err := netlink.LinkByName("foobar0")
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().Name).To(Equal("foobar0"))
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+
+	It("configures and deconfigures a macvlan link with ADD/DEL", func() {
+		const IFNAME = "macvl0"
+
+		conf := fmt.Sprintf(`{
+    "name": "mynet",
+    "type": "macvlan",
+    "master": "%s",
+    "ipam": {
+        "type": "host-local",
+        "subnet": "10.1.2.0/24"
+    }
+}`, MASTER_NAME)
+
+		targetNs, err := ns.NewNS()
+		Expect(err).NotTo(HaveOccurred())
+		defer targetNs.Close()
+
+		args := &skel.CmdArgs{
+			ContainerID: "dummy",
+			Netns:       targetNs.Path(),
+			IfName:      IFNAME,
+			StdinData:   []byte(conf),
+		}
+
+		// Make sure macvlan link exists in the target namespace
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			_, err := testutils.CmdAddWithResult(targetNs.Path(), IFNAME, func() error {
+				return cmdAdd(args)
+			})
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Make sure macvlan link exists in the target namespace
+		err = targetNs.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			link, err := netlink.LinkByName(IFNAME)
+			Expect(err).NotTo(HaveOccurred())
+			Expect(link.Attrs().Name).To(Equal(IFNAME))
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		err = originalNS.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			err := testutils.CmdDelWithResult(targetNs.Path(), IFNAME, func() error {
+				return cmdDel(args)
+			})
+			Expect(err).NotTo(HaveOccurred())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+
+		// Make sure macvlan link has been deleted
+		err = targetNs.Do(func(ns.NetNS) error {
+			defer GinkgoRecover()
+
+			link, err := netlink.LinkByName(IFNAME)
+			Expect(err).To(HaveOccurred())
+			Expect(link).To(BeNil())
+			return nil
+		})
+		Expect(err).NotTo(HaveOccurred())
+	})
+})

plugins/main/ptp/ptp.go

@@ -58,7 +58,7 @@ func setupContainerVeth(netns, ifName string, mtu int, pr *types.Result) (string
 	// In other words we force all traffic to ARP via the gateway except for GW itself.
 
 	var hostVethName string
-	err := ns.WithNetNSPath(netns, false, func(hostNS *os.File) error {
+	err := ns.WithNetNSPath(netns, func(hostNS ns.NetNS) error {
 		hostVeth, _, err := ip.SetupVeth(ifName, mtu, hostNS)
 		if err != nil {
 			return err
@@ -200,7 +200,7 @@ func cmdDel(args *skel.CmdArgs) error {
 	}
 
 	var ipn *net.IPNet
-	err := ns.WithNetNSPath(args.Netns, false, func(hostNS *os.File) error {
+	err := ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		var err error
 		ipn, err = ip.DelLinkByNameAddr(args.IfName, netlink.FAMILY_V4)
 		return err

plugins/meta/tuning/tuning.go

@@ -21,7 +21,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"io/ioutil"
-	"os"
 	"path/filepath"
 	"strings"
 
@@ -45,7 +44,7 @@ func cmdAdd(args *skel.CmdArgs) error {
 	// The directory /proc/sys/net is per network namespace. Enter in the
 	// network namespace before writing on it.
 
-	err := ns.WithNetNSPath(args.Netns, false, func(hostNS *os.File) error {
+	err := ns.WithNetNSPath(args.Netns, func(_ ns.NetNS) error {
 		for key, value := range tuningConf.SysCtl {
 			fileName := filepath.Join("/proc/sys", strings.Replace(key, ".", "/", -1))
 			fileName = filepath.Clean(fileName)

test

@@ -11,8 +11,8 @@ set -e
 
 source ./build
 
-TESTABLE="plugins/ipam/dhcp plugins/main/loopback pkg/invoke pkg/ns pkg/skel pkg/types pkg/utils pkg/testhelpers"
+TESTABLE="plugins/ipam/dhcp plugins/main/loopback pkg/invoke pkg/ns pkg/skel pkg/types pkg/utils plugins/main/ipvlan plugins/main/macvlan plugins/main/bridge"
-FORMATTABLE="$TESTABLE libcni pkg/ip pkg/ns pkg/types pkg/ipam plugins/ipam/host-local plugins/main/bridge plugins/meta/flannel plugins/meta/tuning"
+FORMATTABLE="$TESTABLE libcni pkg/ip pkg/ipam pkg/testutils plugins/ipam/host-local plugins/main/bridge plugins/meta/flannel plugins/meta/tuning"
 
 # user has not provided PKG override
 if [ -z "$PKG" ]; then
@@ -35,7 +35,7 @@ TEST=${split[@]/#/${REPO_PATH}/}
 
 echo -n "Running tests "
 function testrun {
-    sudo -E bash -c "umask 0; PATH=\$GOROOT/bin:\$PATH go test -covermode set $@"
+    sudo -E bash -c "umask 0; PATH=\$GOBIN:\$GOROOT/bin:\$PATH go test -covermode set $@"
 }
 if [ ! -z "${COVERALLS}" ]; then
     echo "with coverage profile generation..."