ITQC/containernetworking-plugins
24
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 34 Wiki Activity

ns: add interface, use it, and fix thread-related namespace switch issues

Browse Source 
Add a namespace object interface for somewhat cleaner code when
creating and switching between network namespaces.  All created
namespaces are now mounted in /var/run/netns to ensure they
have persistent inodes and paths that can be passed around
between plugin components without relying on the current namespace
being correct.

Also remove the thread-locking arguments from the ns package
per https://github.com/appc/cni/issues/183 by doing all the namespace
changes in a separate goroutine that locks/unlocks itself, instead of
the caller having to track OS thread locking.
This commit is contained in:
Dan Williams
2016-04-05 11:10:31 -05:00
parent 3e1c3c60da
commit c0d34c692f
19 changed files with 373 additions and 460 deletions
Download Patch File Download Diff File Expand all files Collapse all files

156
pkg/ns/ns_test.go
View File

@ -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
targetNetNSPath string
targetNetNS *os.File
originalNetNS ns.NetNS
targetNetNS ns.NetNS
)
BeforeEach(func() {
var err error
targetNetNSName = fmt.Sprintf("test-netns-%d", rand.Int())
err = exec.Command("ip", "netns", "add", targetNetNSName).Run()
originalNetNS, err = ns.NewNS()
Expect(err).NotTo(HaveOccurred())
targetNetNSPath = filepath.Join("/var/run/netns/", targetNetNSName)
targetNetNS, err = os.Open(targetNetNSPath)
targetNetNS, err = ns.NewNS()
Expect(err).NotTo(HaveOccurred())
})
AfterEach(func() {
Expect(targetNetNS.Close()).To(Succeed())
err := exec.Command("ip", "netns", "del", targetNetNSName).Run()
Expect(err).NotTo(HaveOccurred())
Expect(originalNetNS.Close()).To(Succeed())
})
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
var innerErr error
err = ns.WithNetNS(targetNetNS, false, func(*os.File) error {
actualInode, innerErr = testhelpers.GetInodeCurNetNS()
err = targetNetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
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()
Expect(err).NotTo(HaveOccurred())
// Ensure we start in originalNetNS
err := originalNetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
var inputNSInode uint64
var innerErr error
err = ns.WithNetNS(targetNetNS, false, func(inputNS *os.File) error {
inputNSInode, err = testhelpers.GetInodeF(inputNS)
origNSInode, err := getInodeNS(originalNetNS)
Expect(err).NotTo(HaveOccurred())
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()
Expect(err).NotTo(HaveOccurred())
err := originalNetNS.Do(func(ns.NetNS) error {
defer GinkgoRecover()
_ = ns.WithNetNS(targetNetNS, false, func(*os.File) error {
return errors.New("potato")
preTestInode, err := getInodeCurNetNS()
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
}