Skip to content
This repository has been archived by the owner on Jun 20, 2024. It is now read-only.

Always use host netns #1930

Merged
merged 9 commits into from
Feb 4, 2016
Merged

Always use host netns #1930

merged 9 commits into from
Feb 4, 2016

Conversation

awh
Copy link
Contributor

@awh awh commented Jan 27, 2016

Fixes #1589,#1746.

This PR currently depends on an as yet unmerged change in mesh (weaveworks/mesh#5); that should be merged first and this PR rebased accordingly.

@rade rade force-pushed the issues/1746-always-use-host-netns branch from 7bb70b7 to 7a1a191 Compare January 28, 2016 12:24
@rade rade self-assigned this Jan 28, 2016
if ! ip link add name $BRIDGE_IFNAME mtu $MTU type veth peer name $PCAP_IFNAME mtu $MTU ||
! ip link set $BRIDGE_IFNAME up ||
! add_iface_bridge $BRIDGE_IFNAME ||
! ip link set $PCAP_IFNAME up ; then

This comment was marked as abuse.

@rade rade assigned awh and unassigned rade Jan 28, 2016
@awh awh force-pushed the issues/1746-always-use-host-netns branch 2 times, most recently from c6903da to 1f501f3 Compare January 29, 2016 11:47
@awh
Copy link
Contributor Author

awh commented Jan 29, 2016

Addressed comments and updated mesh dependency.

@awh awh assigned rade and unassigned awh Jan 29, 2016
@rade
Copy link
Member

rade commented Feb 1, 2016

I think connect_container_to_bridge can be simplified now that it only gets called for application containers and hence always operates on a proper netns. In particular the interface 'down'ing (and elaborate explanatory comment) should no longer be necessary.

@awh awh assigned awh and unassigned rade Feb 1, 2016
@awh awh force-pushed the issues/1746-always-use-host-netns branch from 1f501f3 to 9d0ad01 Compare February 1, 2016 16:12
@rade
Copy link
Member

rade commented Feb 1, 2016

The old multiweave lets one launch additional peers, and also stop just some of the peers, by setting FIRST_WEAVE, e.g.

NUM_WEAVES=10 bin/multiweave launch
FIRST_WEAVE=11 NUM_WEAVES=10 bin/multiweave launch
FIRST_WEAVE=11 NUM_WEAVES=10 bin/multiweave stop

This is no longer possible since a) 'launch' will trip over the multiweave bridge creation, and b) 'stop' will remove the bridge even though there are still weaves connected to it.

Suggested fix:

  • only create multiweave bridge if it doesn't exist already
  • remove ip addresses from multiweave when stopping individual weaves, and only delete multweave if there are no more addresses left

@rade rade assigned rade and unassigned awh Feb 2, 2016
@rade rade force-pushed the issues/1746-always-use-host-netns branch 6 times, most recently from 0e0d97b to 0946417 Compare February 2, 2016 20:46
rade and others added 5 commits February 2, 2016 20:48
which allows us to get rid of port mappings and a whole bunch of other
code.
We already know what the weave container IP is, since now it runs in
the host netns.
Allow the weave router bind to a specific host IP address instead of
INADDR_ANY.
@rade rade force-pushed the issues/1746-always-use-host-netns branch 2 times, most recently from cfd5136 to 6484d20 Compare February 2, 2016 22:45
@rade
Copy link
Member

rade commented Feb 2, 2016

The old multiweave lets one launch additional peers, and also stop just some of the peers, by setting FIRST_WEAVE

...and now so does the new multiweave.

rade and others added 4 commits February 3, 2016 11:28
Multiweave now creates a temporary bridge with an IP alias per router
in the host netns, and overrides the router to bind to that specific
IP for weave protocol and data traffic and http control.
@rade rade force-pushed the issues/1746-always-use-host-netns branch from 6484d20 to 034d893 Compare February 3, 2016 11:58
@rade
Copy link
Member

rade commented Feb 3, 2016

For posterity, here is a patch to multiweave that makes it use just distinct ports instead of IPs. This does work but results in some "connection shutting down due to error: cannot connect to ourself" noise.

diff --git a/bin/multiweave b/bin/multiweave
index 40654b4..090776e 100755
--- a/bin/multiweave
+++ b/bin/multiweave
@@ -15,8 +15,6 @@ START=${FIRST_WEAVE:-1}
 COUNT=${NUM_WEAVES:-28}
 FINISH=$((START+COUNT-1))

-SUBNET=192.168.7
-
 weavedir=$(dirname $0)/..

 # Generate a random MAC value - copied from 'weave' script
@@ -30,24 +28,21 @@ random_mac() {
 weave() {
     i=$1
     shift
-    WEAVE_CONTAINER_NAME=weave$i WEAVE_HTTP_ADDR=$SUBNET.$i:6784 $weavedir/weave "$@"
+    WEAVE_CONTAINER_NAME=weave$i WEAVE_HTTP_ADDR=127.0.0.1:$((6000+i)) WEAVE_PORT=$((5000+i)) $weavedir/weave "$@"
 }

 case "$1" in
     launch)
         shift 1
         echo $(date -R) launching
-        ip link show multiweave >/dev/null 2>&1 || sudo ip link add name multiweave type bridge
         for i in $(seq $START $FINISH); do
-            IP=$SUBNET.$i
-            sudo ip addr add $IP/24 dev multiweave
-            weave $i launch-router --datapath '' --iface '' --no-dns --host $IP \
+            weave $i launch-router --datapath '' --iface '' --no-dns --port $((5000+i)) \
                 --name $(random_mac) --nickname weave$i "$@"
         done
         echo $(date -R) connecting
         [ $START -gt 1 ] || START=2
         for i in $(seq $START $FINISH); do
-            TO=$SUBNET.$((i-1))
+            TO=localhost:$((5000+i-1))
             echo connecting weave$i to $TO
             weave $i connect $TO
         done
@@ -55,12 +50,9 @@ case "$1" in
         ;;
     stop)
         for i in $(seq $FINISH -1 $START); do
-            IP=$SUBNET.$i
             echo Stopping weave$i
             weave $i stop-router
-            sudo ip addr del $IP/24 dev multiweave || true
         done
-        [ -z "$(ip -4 -o addr show multiweave)" ] && sudo ip link del multiweave
         ;;
     *)
             weave "$@"

@rade rade assigned awh and unassigned rade Feb 3, 2016
awh added a commit that referenced this pull request Feb 4, 2016
@awh awh merged commit 415e550 into master Feb 4, 2016
@awh awh deleted the issues/1746-always-use-host-netns branch February 4, 2016 12:50
@rade rade added this to the 1.5.0 milestone Feb 4, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants