Merge pull request #181 from rosenhouse/testhelpers-extraction

Extract and improve test helpers

pkg/ns/ns_test.go

@@ -22,28 +22,12 @@ import (
 	"os/exec"
 	"path/filepath"
 
-	"golang.org/x/sys/unix"
-
 	"github.com/appc/cni/pkg/ns"
+	"github.com/appc/cni/pkg/testhelpers"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 )
 
-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
-}
-
 const CurrentNetNS = "/proc/self/ns/net"
 
 var _ = Describe("Linux namespace operations", func() {
@@ -81,13 +65,13 @@ var _ = Describe("Linux namespace operations", func() {
 		})
 
 		It("executes the callback within the target network namespace", func() {
-			expectedInode, err := getInode(targetNetNSPath)
+			expectedInode, err := testhelpers.GetInode(targetNetNSPath)
 			Expect(err).NotTo(HaveOccurred())
 
 			var actualInode uint64
 			var innerErr error
 			err = ns.WithNetNS(targetNetNS, false, func(*os.File) error {
-				actualInode, innerErr = getInode(CurrentNetNS)
+				actualInode, innerErr = testhelpers.GetInode(CurrentNetNS)
 				return nil
 			})
 			Expect(err).NotTo(HaveOccurred())
@@ -97,13 +81,13 @@ var _ = Describe("Linux namespace operations", func() {
 		})
 
 		It("provides the original namespace as the argument to the callback", func() {
-			hostNSInode, err := getInode(CurrentNetNS)
+			hostNSInode, err := testhelpers.GetInode(CurrentNetNS)
 			Expect(err).NotTo(HaveOccurred())
 
 			var inputNSInode uint64
 			var innerErr error
 			err = ns.WithNetNS(targetNetNS, false, func(inputNS *os.File) error {
-				inputNSInode, err = getInodeF(inputNS)
+				inputNSInode, err = testhelpers.GetInodeF(inputNS)
 				return nil
 			})
 			Expect(err).NotTo(HaveOccurred())
@@ -113,7 +97,7 @@ var _ = Describe("Linux namespace operations", func() {
 		})
 
 		It("restores the calling thread to the original network namespace", func() {
-			preTestInode, err := getInode(CurrentNetNS)
+			preTestInode, err := testhelpers.GetInode(CurrentNetNS)
 			Expect(err).NotTo(HaveOccurred())
 
 			err = ns.WithNetNS(targetNetNS, false, func(*os.File) error {
@@ -121,7 +105,7 @@ var _ = Describe("Linux namespace operations", func() {
 			})
 			Expect(err).NotTo(HaveOccurred())
 
-			postTestInode, err := getInode(CurrentNetNS)
+			postTestInode, err := testhelpers.GetInode(CurrentNetNS)
 			Expect(err).NotTo(HaveOccurred())
 
 			Expect(postTestInode).To(Equal(preTestInode))
@@ -129,14 +113,14 @@ var _ = Describe("Linux namespace operations", func() {
 
 		Context("when the callback returns an error", func() {
 			It("restores the calling thread to the original namespace before returning", func() {
-				preTestInode, err := getInode(CurrentNetNS)
+				preTestInode, err := testhelpers.GetInode(CurrentNetNS)
 				Expect(err).NotTo(HaveOccurred())
 
 				_ = ns.WithNetNS(targetNetNS, false, func(*os.File) error {
 					return errors.New("potato")
 				})
 
-				postTestInode, err := getInode(CurrentNetNS)
+				postTestInode, err := testhelpers.GetInode(CurrentNetNS)
 				Expect(err).NotTo(HaveOccurred())
 
 				Expect(postTestInode).To(Equal(preTestInode))
@@ -152,10 +136,10 @@ var _ = Describe("Linux namespace operations", func() {
 
 		Describe("validating inode mapping to namespaces", func() {
 			It("checks that different namespaces have different inodes", func() {
-				hostNSInode, err := getInode(CurrentNetNS)
+				hostNSInode, err := testhelpers.GetInode(CurrentNetNS)
 				Expect(err).NotTo(HaveOccurred())
 
-				testNsInode, err := getInode(targetNetNSPath)
+				testNsInode, err := testhelpers.GetInode(targetNetNSPath)
 				Expect(err).NotTo(HaveOccurred())
 
 				Expect(hostNSInode).NotTo(Equal(0))

pkg/testhelpers/testhelpers.go

@@ -0,0 +1,101 @@
+// 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 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
+}
+
+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
+	go (func() {
+		defer wg.Done()
+
+		runtime.LockOSThread()
+		defer runtime.UnlockOSThread()
+
+		defer GinkgoRecover()
+
+		// capture current thread's original netns
+		pid := unix.Getpid()
+		tid := unix.Gettid()
+		currentThreadNetNSPath := fmt.Sprintf("/proc/%d/task/%d/ns/net", pid, tid)
+		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_suite_test.go

@@ -0,0 +1,31 @@
+// 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
+
+import (
+	"math/rand"
+
+	. "github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/config"
+	. "github.com/onsi/gomega"
+
+	"testing"
+)
+
+func TestTesthelpers(t *testing.T) {
+	rand.Seed(config.GinkgoConfig.RandomSeed)
+	RegisterFailHandler(Fail)
+	RunSpecs(t, "Testhelpers Suite")
+}

pkg/testhelpers/testhelpers_test.go

@@ -0,0 +1,96 @@
+// 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/appc/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
+}

plugins/main/loopback/loopback_suite_test.go

@@ -15,18 +15,12 @@
 package main_test
 
 import (
-	"fmt"
-	"os"
-	"runtime"
-
 	"github.com/onsi/gomega/gexec"
 
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 
 	"testing"
-
-	"golang.org/x/sys/unix"
 )
 
 var pathToLoPlugin string
@@ -45,47 +39,3 @@ var _ = BeforeSuite(func() {
 var _ = AfterSuite(func() {
 	gexec.CleanupBuildArtifacts()
 })
-
-func makeNetworkNS(containerID string) string {
-	namespace := "/var/run/netns/" + containerID
-	pid := unix.Getpid()
-	tid := unix.Gettid()
-
-	err := os.MkdirAll("/var/run/netns", 0600)
-	Expect(err).NotTo(HaveOccurred())
-
-	runtime.LockOSThread()
-	defer runtime.UnlockOSThread()
-	go (func() {
-		defer GinkgoRecover()
-
-		err = unix.Unshare(unix.CLONE_NEWNET)
-		Expect(err).NotTo(HaveOccurred())
-
-		fd, err := os.Create(namespace)
-		Expect(err).NotTo(HaveOccurred())
-		defer fd.Close()
-
-		err = unix.Mount("/proc/self/ns/net", namespace, "none", unix.MS_BIND, "")
-		Expect(err).NotTo(HaveOccurred())
-	})()
-
-	Eventually(namespace).Should(BeAnExistingFile())
-
-	fd, err := unix.Open(fmt.Sprintf("/proc/%d/task/%d/ns/net", pid, tid), unix.O_RDONLY, 0)
-	Expect(err).NotTo(HaveOccurred())
-
-	defer unix.Close(fd)
-
-	_, _, e1 := unix.Syscall(unix.SYS_SETNS, uintptr(fd), uintptr(unix.CLONE_NEWNET), 0)
-	Expect(e1).To(BeZero())
-
-	return namespace
-}
-
-func removeNetworkNS(networkNS string) error {
-	err := unix.Unmount(networkNS, unix.MNT_DETACH)
-
-	err = os.RemoveAll(networkNS)
-	return err
-}

plugins/main/loopback/loopback_test.go

@@ -22,6 +22,7 @@ import (
 	"strings"
 
 	"github.com/appc/cni/pkg/ns"
+	"github.com/appc/cni/pkg/testhelpers"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/onsi/gomega/gbytes"
@@ -39,7 +40,7 @@ var _ = Describe("Loopback", func() {
 	BeforeEach(func() {
 		command = exec.Command(pathToLoPlugin)
 		containerID = "some-container-id"
-		networkNS = makeNetworkNS(containerID)
+		networkNS = testhelpers.MakeNetworkNS(containerID)
 
 		environ = []string{
 			fmt.Sprintf("CNI_CONTAINERID=%s", containerID),
@@ -52,7 +53,7 @@ var _ = Describe("Loopback", func() {
 	})
 
 	AfterEach(func() {
-		Expect(removeNetworkNS(networkNS)).To(Succeed())
+		Expect(testhelpers.RemoveNetworkNS(networkNS)).To(Succeed())
 	})
 
 	Context("when given a network namespace", func() {

test

@@ -11,7 +11,7 @@ set -e
 
 source ./build
 
-TESTABLE="plugins/ipam/dhcp plugins/main/loopback pkg/invoke pkg/ns pkg/skel pkg/types pkg/utils"
+TESTABLE="plugins/ipam/dhcp plugins/main/loopback pkg/invoke pkg/ns pkg/skel pkg/types pkg/utils pkg/testhelpers"
 FORMATTABLE="$TESTABLE libcni pkg/ip pkg/ns pkg/types pkg/ipam plugins/ipam/host-local plugins/main/bridge plugins/meta/flannel plugins/meta/tuning"
 
 # user has not provided PKG override