ns: fix reading net namespace in multi-threaded processes #176
Conversation
|
@steveej @jonboulle @rosenhouse took a while to debug these failures, but I think this is the fix. |
|
@dcbw thanks for this. One thing of note now, I think this means that this set of tests now can no longer be run in parallel due to locking to a particular thread. |
|
@zachgersh yeah, we've got to take that hit unfortunately :( |
|
Naiv question, why isn't it enough to put |
|
Also, could we just use https://github.com/vishvananda/netns instead of maintaining something similar here? |
Locking to an OS thread doesn't guarantee which thread you're locked to. /proc/self/ns/net only tells you what the main thread's namespace is, but the current thread may not be the main thread, depending on where Go schedules you and whatever Ginkgo does. |
Thanks, sounds reasonable to assume Ginkgo runs things in parallel from the beginning. 👍 |
| var err error | ||
| originalNetNS, err = os.Open(CurrentNetNS) | ||
| name := fmt.Sprintf("/proc/%d/task/%d/ns/net", os.Getpid(), syscall.Gettid()) |
There was a problem hiding this comment.
would you mind using a constant for this format string?
@steveej I looked into it yesterday and it is possible, but it makes a few things harder because it's not quite a match for the testing infrastructure PRs I've submitted. I'd like to explore doing that after this PR and the testing ones are merged if that's OK? |
|
@steveej pulled out the function and repushed, PTAL thanks! |
|
Another test flake? Bad response status from coveralls: 422 - {"message":"Couldn't find a repository matching this job.","error":true} |
Yeah, we've been getting those but I haven't found the reason. Pressing rebuild on travis has always lead to a successful build. |
|
LGTM @rosenhouse would you please take a look at this too? Since you wrote the initial test. |
|
@steveej Just seeing this now. Will take a look soon! |
|
hey @dcbw - you may want to have a look at the test-helpers I just merged from @rosenhouse. |
/proc/self/ns/net gives the main thread's namespace, not necessarily the namespace of the thread that's running the testcases. This causes sporadic failures of the tests. For example, with a testcase reading inodes after switching netns: /proc/27686/task/27689/ns/net 4026532565 /proc/self/ns/net 4026531969 /proc/27686/task/27689/ns/net 4026532565 See also: vishvananda/netns@008d17a Running Suite: pkg/ns Suite =========================== Random Seed: 1459953577 Will run 6 of 6 specs • Failure [0.028 seconds] Linux namespace operations /cni/gopath/src/github.com/appc/cni/pkg/ns/ns_test.go:167 WithNetNS /cni/gopath/src/github.com/appc/cni/pkg/ns/ns_test.go:166 executes the callback within the target network namespace [It] /cni/gopath/src/github.com/appc/cni/pkg/ns/ns_test.go:97 Expected <uint64>: 4026531969 to equal <uint64>: 4026532565 /cni/gopath/src/github.com/appc/cni/pkg/ns/ns_test.go:96 ------------------------------ ••••• Summarizing 1 Failure: [Fail] Linux namespace operations WithNetNS [It] executes the callback within the target network namespace /cni/gopath/src/github.com/appc/cni/pkg/ns/ns_test.go:96 Ran 6 of 6 Specs in 0.564 seconds FAIL! -- 5 Passed | 1 Failed | 0 Pending | 0 Skipped --- FAIL: TestNs (0.56s) FAIL
|
@rosenhouse @steveej @zachgersh rebased on top of master and moved a few things around. PTAL, thanks! |
/proc/self/ns/net gives the main thread's namespace, not necessarily
the namespace of the thread that's running the testcases. This causes
sporadic failures of the tests. For example, with a testcase reading
inodes after switching netns:
/proc/27686/task/27689/ns/net 4026532565
/proc/self/ns/net 4026531969
/proc/27686/task/27689/ns/net 4026532565
See also:
vishvananda/netns@008d17a