Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace ip netns exec with nsenter #580

Merged
merged 3 commits into from
Apr 21, 2015
Merged

Replace ip netns exec with nsenter #580

merged 3 commits into from
Apr 21, 2015

Conversation

awh
Copy link
Contributor

@awh awh commented Apr 20, 2015

Fixes #458. All smoke tests pass (400_weave_local_test.sh necessitated updating the Vagrantfiles to Ubuntu 14.10) but nevertheless quite invasive - suggest merging after 0.10.0?

@rade
Copy link
Member

rade commented Apr 20, 2015

  • The repetition of nsenter --net=$PROCFS/$NETNS/ns/net is a bit tiresome. Yes, the same was true for ip netns exec $NETNS, but at least that was shorter.
  • $NETNS worked as a var name for ip netns exec $NETNS, but is somewhat misleading for use in $PROCFS/$NETNS. Could we just use $CONTAINER_PID instead?
  • We need to check what, if any, new version constraints this imposes on the docker host's kernel. While nsenter itself is only a user space tool, it does depend on kernel features, so the questions is whether it depends on more recent features than ip netns exec.

@awh
Copy link
Contributor Author

awh commented Apr 21, 2015

The repetition of nsenter --net=$PROCFS/$NETNS/ns/net is a bit tiresome. Yes, the same was true for ip netns exec $NETNS, but at least that was shorter.

Abstracted out to a function.

$NETNS worked as a var name for ip netns exec $NETNS, but is somewhat misleading for use in $PROCFS/$NETNS. Could we just use $CONTAINER_PID instead?

Done.

We need to check what, if any, new version constraints this imposes on the docker host's kernel. While nsenter itself is only a user space tool, it does depend on kernel features, so the questions is whether it depends on more recent features than ip netns exec.

I've smoke tested this on Ubuntu Precise 12.04/3.5.0-54-generic and it works (modulo #585 and 400_weave_local_test.sh).

@awh awh assigned rade and unassigned awh Apr 21, 2015
# Set $LOCAL_IFNAME and $GUEST_IFNAME to suitable names for two ends
# of a veth pair, specific to the container, and execute args $2 $3 ...
# as a command. If an error is caused by container dying, swallow output
# from error.

This comment was marked as abuse.

@rade rade assigned awh and unassigned rade Apr 21, 2015
@awh
Copy link
Contributor Author

awh commented Apr 21, 2015

Fixed with_container_netns comment.

@awh awh assigned rade and unassigned awh Apr 21, 2015
rade added a commit that referenced this pull request Apr 21, 2015
Replace `ip netns exec` with `nsenter`

Closes #458.
@rade rade merged commit 5178e1a into weaveworks:master Apr 21, 2015
@awh awh deleted the use_nsenter branch April 28, 2015 14:17
@rade rade modified the milestone: 0.11.0 May 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

use nsenter instead of ip netns exec
2 participants