Need to move pod network setup out of process #15991
Comments
|
@danwinship summary of our discussion on IRC... Using netns stuff in the same process is OK as long as all the code is properly using Do() or WithNetNSPath(). All the pod* stuff does that, but the concern is other code in Kubernetes/OpenShift that always expects to be run in the host's netns. If an OS thread is created by the Go runtime while some goroutine is in a container's netns, that OS thread will be created in the container's netns even though it is completely unrelated to netns-manipulating code. Later, any code that runs in that new OS thread will not be running in the netns it probably expects. Any code that does something network related should be setting the netns before running that code. Obviously that's somewhat hard as we don't/can't control all the code, nor vendor code. So yes, to completely ensure that this won't be a problem, the netns-using code should get moved. That will be somewhat complicated though, and given we haven't run into this issue much (if at all?) in OpenShift yet, perhaps this could be an RFE for now. |
|
@dcbw it looks like this does happen sometimes: #14385 (comment) |
|
Ugh. It looks like this happens a lot. In the last few weeks of post-commit extended networking tests:
In the last one week of extended_networking_minimal tests running against PRs:
|
Automatic merge from submit-queue (batch tested with PRs 18376, 18355). Move pod-namespace calls out of process As discussed in #15991, we need to move all operations in the pod's network namespace out of process, due to a golang issue that allows setns() calls in a locked thread to leak into other threads, causing random lossage as operations intended for the main network namespace end up running in other namespaces instead. (This is fixed in golang 1.10 but we need a fix before then.) Fixes #15991 Fixes #14385 Fixes #13108 Fixes #18317
Apparently, it is impossible to reliably work with network namespaces in a long-running golang process because the goroutine scheduler's shenanigans are incompatible with APIs that set current-thread state; calling ns.Set() or ns.Do() may cause other goroutines to change their current namespace as well. (eg, containernetworking/cni#262, https://www.weave.works/blog/linux-namespaces-and-go-don-t-mix)
I don't know of any bugs we've seen that would be explained by our getting this wrong, but it seems likely that there must be some, and if not there eventually will be.
So we need to move at least half of pod_linux.go over to sdn-cni-plugin... Basically, we probably don't want to import any of the CNI packages into the openshift binary. The OVS-manipulating parts of pod_linux.go would probably stay in the openshift binary.
The text was updated successfully, but these errors were encountered: