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

fix create_bridge() bug, patch README install recipe, WEAVE_DOCKER_ARGS=... #103

Closed
wants to merge 2 commits into from

Conversation

hesco
Copy link

@hesco hesco commented Oct 7, 2014

...''

I set WEAVE_DOCKER_ARGS to a null string to facilitate patching it on install with an inline sed -i string substitution so that I (and others) can add a --memory=1gb argument to the build of the weave container, or whatever other arguments folks want to feed it.

I'm pretty sure that create_bridge() wants to:

  • ip link add name $BRIDGE type bridge

not:

  • ip link add dev $BRIDGE type bridge

at least I could see no joy in trying it the other way.

I found that the weave shell script failed to docker pull zettio/weave, and that I made progress only when I explicitly performed that step manually. So I added that to the install recipe in the README.md.

I also did a global replace of 'zettio/weave' with $WEAVE_IMAGE and defined that variable at the top of the script, to facilitate maintainance.

So far I have been able to pull and build an image and run a weave container, and to use weave to wrap docker to run other containers assigning them a fixed IP. I'm still sorting out how to set the peer for a weave bridge on one docker host to the weave bridge on another docker host. The documentation seems unclear as to what is expected in that $HOST1 variable, and reading the weaver/weave code reveals no use of a peer attribute in the code, leaving me in doubt as to how and where that argument would be used.

@rade
Copy link
Member

rade commented Oct 7, 2014

I set WEAVE_DOCKER_ARGS to a null string to facilitate patching it on install with an inline sed -i string substitution

Then it is no longer possible to set WEAVE_DOCKER_ARGS in the environment of the script execution, which is its original intended use.

I'm pretty sure that create_bridge() wants to:

  • ip link add name $BRIDGE type bridge

not:

  • ip link add dev $BRIDGE type bridge

at least I could see no joy in trying it the other way.

Reading the ip command docs does indeed suggest the former, so I've just pushed that change. I am curious what OS and kernel version you are running though, since the latter certainly works for me and, evidently, for everybody that has tried weave so far.

I found that the weave shell script failed to docker pull zettio/weave

Failed in what way? Was it perhaps just taking a long time? Note that there is no progress output due to issue #8.

I also did a global replace of 'zettio/weave' with $WEAVE_IMAGE and defined that variable at the top of the script, to facilitate maintenance.

Good idea. I've just pushed that change.

The documentation seems unclear as to what is expected in that $HOST1 variable.

The docs say "Say you have docker running on two hosts, accessible to each other as $HOST1 and $HOST2". So $HOST1 is expected to contain the IP address by which $HOST2 can reach it.

Any suggestions for making this clearer in the README?

@hesco
Copy link
Author

hesco commented Oct 7, 2014

That commit message ought to reference PR #103.

# uname -a 
Linux dessalines001 3.14-2-amd64 #1 SMP Debian 3.14.15-2 (2014-08-09) x86_64 GNU/Linux

# cat /etc/debian_version 
jessie/sid

# facter operatingsystem
Debian

though I had thought I was on Ubuntu 14.04 LTS, but perhaps those are my docker containers.

@rade
Copy link
Member

rade commented Oct 7, 2014

I've tweaked the README based on your suggestions. Couldn't find a good place to talk about WEAVE_DOCKER_ARGS - it is very much and 'advanced usage' topic, so doesn't belong in the introductory example. So I've just put a comment in the script.

Re OS/kernel version... I'm actually on a slightly older version. Weird.

Thanks for submitting this PR; weave has been improved as a result. Let us know how you are getting on with your weave experiments.

@rade rade closed this Oct 7, 2014
@awh awh modified the milestone: n/a Jan 14, 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.

3 participants