Skip to content

Commit

Permalink
Handle namespaces with care
Browse files Browse the repository at this point in the history
- After creating new netns, switch back to main netns
- Lock thread during test and test setup
  • Loading branch information
zachgersh committed Feb 29, 2016
1 parent 2708bdf commit 1e3d680
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 8 deletions.
33 changes: 26 additions & 7 deletions plugins/main/loopback/loopback_suite_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
package main_test

import (
"fmt"
"os"
"runtime"

"github.com/onsi/gomega/gexec"

Expand Down Expand Up @@ -32,21 +34,38 @@ var _ = AfterSuite(func() {

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())

err = unix.Unshare(unix.CLONE_NEWNET)
Expect(err).NotTo(HaveOccurred())
runtime.LockOSThread()
defer runtime.UnlockOSThread()
go (func() {
defer GinkgoRecover()

fd, err := os.Create(namespace)
Expect(err).NotTo(HaveOccurred())
defer fd.Close()
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())
})()

err = unix.Mount("/proc/self/ns/net", namespace, "none", unix.MS_BIND, "")
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())

Expect(namespace).To(BeAnExistingFile())
defer unix.Close(fd)

_, _, e1 := unix.Syscall(unix.SYS_SETNS, uintptr(fd), uintptr(unix.CLONE_NEWNET), 0)
Expect(e1).To(BeZero())

This comment has been minimized.

Copy link
@rosenhouse

rosenhouse Apr 17, 2016

Contributor

@zachgersh Do you have a reference you worked from for this code? I'm having trouble understanding it. Specifically, why are you opening the file on like 61, what is the syscall on line 66, and why do you expect e1 to be zero on like 67?

Context: I'm trying to extract this code to a testhelpers package, because I want to backfill more tests for other plugins. But my tests are behaving non-deterministically... so I'm digging into the namespace-switching stuff.

Thanks!

This comment has been minimized.

Copy link
@bboreham

bboreham Apr 17, 2016

Contributor

@rosenhouse does this help: http://linux.die.net/man/2/setns ?

There is a namespace package here already: github.com/appc/cni/pkg/ns, though some code uses github.com/vishvananda/netns.

This comment has been minimized.

Copy link
@zachgersh

zachgersh Apr 17, 2016

Author Contributor

Hey @rosenhouse, wanted to clarify, when you extracted this code did it start to behave non-deterministically or was some other code that you wrote behaving in such a manner?

This code is an amalgamation of talking to @sykesm, reading netns/setns/syscall man-pages, and looking at go source for people performing network namespace operations.

My unique addition, is executing this inside of a go-routine and locking that to a thread. I hadn't seen that anywhere and as you remember I was keen to try it.

Inside the go-routine I am grabbing a new netns by unsharing from within the go-routine and then re-mounting it so that when the go-routine exits I have access in my original function context to the netns that was created.

This function is attempting to attach a netns to the same process / thread that the function was called from. The file descriptor is opened based on process id pid and thread id tid I grabbed when the function was called (that happens at the top).

Then, the Syscall is setting the current netns onto the fd I opened up.

Syscall returns an errno instead of an error https://godoc.org/syscall#Errno so I expect zero to make sure that the operation was a success.

return namespace
}

Expand Down
2 changes: 1 addition & 1 deletion plugins/main/loopback/loopback_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ var _ = Describe("Loopback", func() {
Eventually(session).Should(gexec.Exit(0))

var lo *net.Interface
err = ns.WithNetNSPath(networkNS, false, func(hostNS *os.File) error {
err = ns.WithNetNSPath(networkNS, true, func(hostNS *os.File) error {
var err error
lo, err = net.InterfaceByName("lo")
return err
Expand Down

0 comments on commit 1e3d680

Please sign in to comment.