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

Move weave script into tools image with its deps #388

Merged
merged 10 commits into from
Mar 20, 2015

Conversation

squaremo
Copy link
Contributor

@squaremo squaremo commented Feb 5, 2015

This rearrangement means that the weave command can be used
remotely; and, as a side-effect, the script that does all the work is
using executables in its own filesystem, rather than invoking them via
docker.

A choice here is whether to base the tools image on scratch and add
all the deps, or base it on something else. I've used alpine linux,
which itself is based on busybox. This gives us the usual tools (sed
etc.) while keeping the image size small.

Adding the docker binary adds substantially to the image size, but it
cannot always be e.g., bind-mounted. To mitigate this image bloat, we
might build or otherwise get a client-only docker executable, or write
a special-purpose client, or indeed rewrite the driver script and use
a client library.

Closes #312, improves on #321

@rade
Copy link
Member

rade commented Feb 5, 2015

  • The definition of TOOLS_IMAGE has been removed from the driver script, but it is still being used - by setup and version. This also raises the question of what these two commands should do now. setup is meant to download all required images for the version corresponding to the script. But the driver script is part of an image. So do we need a version on the new weave script, and run the setup command there?
  • Surely the release script needs updating, to 'sed' the version number into the 'driver' script. Or we should get rid of that code and only have the tools image version.
  • We need to update the docs to explain that the weave script and docker daemon do not need to be co-located (our installation instructions do kinda imply that) when DOCKER_HOST is set.
  • I can think of one thing that will break for sure: the kind of scenario for which we introduced create-bridge and made hide and expose not depend on docker. See Add create-bridge command #357 and Also avoid calling ethtool in expose and hide #377. It seems to me that the only way to solve that would be to have alternate mode of operation where the the weave script executes everything locally, as before. Or at least those three commands (w/o duplicating the code).

@errordeveloper
Copy link
Contributor

I can think of one thing that will break for sure: the kind of scenario for which we introduced create-bridge and made hide and expose not depend on docker. See #357 and #377. It seems to me that the only way to solve that would be to have alternate mode of operation where the the weave script executes everything locally, as before. Or at least those three commands (w/o duplicating the code).

In terms of create-bridge, one can install the driver script on the host...

Looking the diff between weave script in master and driver script here:

47d46
< BASE_TOOLS_IMAGE=zettio/weavetools
50d48
< TOOLS_IMAGE=$BASE_TOOLS_IMAGE:$IMAGE_VERSION
72,78d69
< run_tool() {
<     TOOL_NET=$1
<     TOOL_COMMAND=$2
<     shift 2
<     docker run --rm --privileged --net=$TOOL_NET $TOOLS_IMAGE /bin/$TOOL_COMMAND "$@"
< }
< 
143,147c134
<         # disable offloading - we do this even when $BRIDGE already exists
<         # simply because it has the desirable side-effect of ensuring we
<         # have the $TOOLS_IMAGE and thus do not incur downloading-induced
<         # delays in more time-sensitive sections elsewhere.
<         run_tool host ethtool -K $BRIDGE tx off >/dev/null
---
>         ethtool -K $BRIDGE tx off >/dev/null
228c215
<     if run_tool host ethtool -K $GUEST_IFNAME tx off >/dev/null &&
---
>     if ethtool -K $GUEST_IFNAME tx off >/dev/null &&
247c234
<         run_tool container:$CONTAINER ethtool -K eth0 tx off >/dev/null &&
---
>         ip netns exec $NETNS ethtool -K eth0 tx off >/dev/null &&
327c314
<     run_tool host curl -s -X $http_verb "$@" http://$ip:$port$url
---
>     curl -s -X $http_verb "$@" http://$ip:$port$url
622c609
<         run_tool host conntrack -D -p udp --dport $PORT >/dev/null 2>&1 || true
---
>         conntrack -D -p udp --dport $PORT >/dev/null 2>&1 || true
637c624
<         run_tool host conntrack -D -p udp --dport $PORT >/dev/null 2>&1 || true
---
>         conntrack -D -p udp --dport $PORT >/dev/null 2>&1 || true

All I can see is run_tool being removed. So dual-mode driver script can be done by means of re-introducing run_tool it with a bypass mode. The only somewhat tricky part seems to be:

247c234
<         run_tool container:$CONTAINER ethtool -K eth0 tx off >/dev/null &&
---
>         ip netns exec $NETNS ethtool -K eth0 tx off >/dev/null &&

@rade
Copy link
Member

rade commented Feb 5, 2015

Another thing that breaks: env vars, specifically any env vars that the current weave script pays attention to. Which are:

  • VERSION
  • WEAVE_DOCKER_ARGS
  • WEAVE_PASSWORD
  • DOCKER_BRIDGE
  • PROC_FS

@squaremo squaremo self-assigned this Feb 10, 2015
@squaremo
Copy link
Contributor Author

The definition of TOOLS_IMAGE has been removed from the driver script, but it is still being used - by setup and version. This also raises the question of what these two commands should do now. setup is meant to download all required images for the version corresponding to the script. But the driver script is part of an image. So do we need a version on the new weave script, and run the setup command there?

The weave script is the entry point, so I think that is what has to be versioned ultimately. Then the images are locked to that version -- i.e., what happens now. The version is then in two places: in the driver script, so it asks for the correct images, and in the weave script. The weave script simply asks for the versioned tools image, which has its version built-in.

We could alter things so that the outer script knew about versioning, so that you could do e.g., weave setup 0.9.0 and have it install the correct images. But that's a bunch more work and I'm not sure we want to second-guess packaging.

Surely the release script needs updating, to 'sed' the version number into the 'driver' script. Or we should get rid of that code and only have the tools image version.

Yes, it needs updating.

We need to update the docs to explain that the weave script and docker daemon do not need to be co-located (our installation instructions do kinda imply that) when DOCKER_HOST is set.

Yes.

I can think of one thing that will break for sure: the kind of scenario for which we introduced create-bridge and made hide and expose not depend on docker. See #357 and #377. It seems to me that the only way to solve that would be to have alternate mode of operation where the the weave script executes everything locally, as before. Or at least those three commands (w/o duplicating the code).

I have to admit I don't see how that can work without some contortions (e.g., put those commands in the weave script and include it in the image too, then call it from the driver when necessary).

@squaremo
Copy link
Contributor Author

Another thing that breaks: env vars, specifically any env vars that the current weave script pays attention to. Which are:

VERSION
WEAVE_DOCKER_ARGS
WEAVE_PASSWORD
DOCKER_BRIDGE
PROC_FS

These would need to be passed as -e arguments if present. The PROC_FS already gets passed, since it's tied to how the tools container is started. I'll have to look at the rationale for the password being in the environment -- it may not be acceptable to pass it over to docker as a command line arg.

@squaremo
Copy link
Contributor Author

It seems to me that the only way to solve that would be to have alternate mode of operation where the the weave script executes everything locally, as before.

The logic in weave is to always forward commands to a docker container, but it could check if there's a docker URL and assume it can effect things locally if it's absent or a unix domain socket (or have a special --local option).

If these must be special-cased anyway, it's then not such a big deal to include the weave script in the tools image and use it for these particular commands.

@squaremo
Copy link
Contributor Author

WEAVE_PASSWORD

The aim of moving this into an environment variable is to give people a way to supply it without putting it on the command line. To achieve the same thing with the remote-actuation weave script, the password option parsing should be moved from the driver script to the weave script, and the driver script should receive it in an environment variable which is passes on to the container.

@rade
Copy link
Member

rade commented Feb 10, 2015

it could check if there's a docker URL and assume it can effect things locally if it's absent

That's what I was thinking. Doesn't mean it's the best approach though ;)

@squaremo
Copy link
Contributor Author

it could check if there's a docker URL and assume it can effect things locally if it's absent

That's what I was thinking. Doesn't mean it's the best approach though ;)

I favour the --local alternative, since these are niche uses.

@squaremo
Copy link
Contributor Author

These would need to be passed as -e arguments if present.

VERSION, WEAVE_DOCKER_ARGS, DOCKER_BRIDGE, and WEAVE_PASSWORD can all get passed directly along.

In the case of WEAVE_PASSWORD, there's no need to do the arg parsing first as described above, since it will pass along any arguments as well as the environment variable if present.

The PROC_FS already gets passed, since it's tied to how the tools container is started.

An elaboration: when acting as a remote, PROCFS refers to the host filesystem that should be mapped as a volume to /hostproc; when acting locally, it refers to the proc filesystem with namespaces etc. So when running the tools container remotely, the host proc filesystem is mounted as /hostproc, and /hostproc is passed as the value of PROCFS.

@squaremo
Copy link
Contributor Author

VERSION is a tricky one.

If it determines the weavetools image to run when acting remotely, it will mostly fail since earlier versions aren't set up to be called remotely -- for one thing they don't have the script in the container image.

If the latest weavetools image is always invoked remotely, it seems against the spirit of using VERSION in the first place, although it will do the right thing mostly, which is to use weave and weavedns images of the given version.

@bboreham
Copy link
Contributor

How does a user run this? Like docker run zettio/weave weave run ... ?

@squaremo
Copy link
Contributor Author

How does a user run this? Like docker run zettio/weave weave run ... ?

As before, the user is expected to download the weave script and call weave whatever. The difference is that it will forward the call to the weavetools image on a remote docker instance, if a DOCKER_HOST is given.

(There's also the option of driving weavetools directly with the docker CLI, but that needs you to get a bunch of docker options exactly right)

@squaremo squaremo force-pushed the remote_weaving branch 2 times, most recently from 522aa2a to 4a28f4c Compare February 24, 2015 13:40
@squaremo
Copy link
Contributor Author

This also raises the question of what these two commands should do now. setup is meant to download all required images for the version corresponding to the script. But the driver script is part of an image. So do we need a version on the new weave script, and run the setup command there?

I am trying to figure out what we want to happen, and I think it's this:

If the invoked weave script has a version, or is given a version, it should try to call that remotely; this means

  • That version of weavetools will be started (downloaded, possibly) to run the command
  • It will setup or start that version of weave and weavedns

A problem is that if you want to remote control a version 0.8 weave, it'll fail because weavetools v0.8 doesn't have the weave script and therefore cannot be called remotely.

This could be worked around by breaking with the name weavetools and using e.g., weaveremote. Then all published versions would work remotely.

@rade
Copy link
Member

rade commented Feb 24, 2015

Making remoting work with old weaves is a nice-to-have at best.

@squaremo
Copy link
Contributor Author

This is ready for another go-round. I'm going to test the release procedure, since I've changed the image name and so on.

@squaremo squaremo removed their assignment Feb 26, 2015
@squaremo
Copy link
Contributor Author

This is ready for another go-round.

Oh except now you merged #387, I'll have to rebase.

@squaremo
Copy link
Contributor Author

Oh except now you merged #387, I'll have to rebase.

Rebased. Ready for another look.

EXEC_IMAGE=$BASE_EXEC_IMAGE:$IMAGE_VERSION

exec_remote() {
docker run --rm \

This comment was marked as abuse.

This rearrangement means that the `weave` command can be used
remotely; and, as a side-effect, the script that does all the work is
using executables in its own filesystem, rather than invoking them via
docker.

The remote weavetools is given a terminal so it can be interrupted.

With the flag `--local`, the script will assume all its dependencies
are available and run then and there. Without the flag, it will run
via the weavetools image (adding the flag to the invocation).

A choice here is whether to base the tools image on scratch and add
all the deps, or base it on something else. I've used alpine linux,
which itself is based on busybox. This gives us the usual tools (sed
etc.) while keeping the image size small.

Adding the docker binary adds substantially to the image size, but it
cannot always be e.g., bind-mounted. To mitigate this image bloat, we
might build or otherwise get a client-only docker executable, or write
a special-purpose client, or indeed rewrite the driver script and use
a client library.

This also makes the dockerhub user part of the image names
parameterised, to make it easier to test the release process in full.
`apk-install` is a script that runs apk then tidies up. The problem is
that it doesn't exit with an error if the install fails, which means
that if some packages are unavailable at the time of building, it will
still all go ahead.
This is to make a visible break with previous releases, such that a
newly downloaded weave script will definitely work.

Without this change in name, an existing weavetools image that doesn't
include the script might be invoked, resulting in a confusing message
for the user.
squaremo added a commit to squaremo/weave that referenced this pull request Mar 18, 2015
As noted in weaveworks#388 comment 76958961, we need the docker client to be as
old as possible while supporting what weave needs to do. At the minute
that means version 1.3.1.

The `apk` package index does not have old versions of docker, so the
most direct way of getting a docker executable is to download the
specific version from https://get.docker.com.
@squaremo
Copy link
Contributor Author

  • Now uses a docker 1.3.1 from get.docker.com
  • weavetools -> weaveexec in .gitignore and site/building.md
  • rebased to master

The way I've changed the script to use the "local" docker for weave run needs looking at.

run_on $HOST mkdir -p `dirname $WEAVE`
cat ../weave | run_on $HOST sh -c "cat > $WEAVE"
docker_on $HOST load -i ../weaveexec.tar
cp ../weave ./

This comment was marked as abuse.

This comment was marked as abuse.

@rade
Copy link
Member

rade commented Mar 18, 2015

rest looks good.

The form of `docker -e` without an equals sign passes them along from
the environment without the value appearing in the command.
exit $?
fi

[ $(id -u) = 0 ] || [ "$1" = "run" ] || {

This comment was marked as abuse.

We want to be compatible with whatever docker the user has; `weave
run` is the only command that passes on arbitrary args to docker
(i.e., that doesn't have a fixed set of docker features that it
needs).

To make this workable we still need to do some things on the remote
host: the IP attachment and figuring out the DNS settings (since those
may depend on the bridge IP).
As noted in weaveworks#388 comment 76958961, we need the docker client to be as
old as possible while supporting what weave needs to do. At the minute
that means version 1.3.1.

The `apk` package index does not have old versions of docker, so the
most direct way of getting a docker executable is to download the
specific version from https://get.docker.com.
rade added a commit that referenced this pull request Mar 20, 2015
Move weave script into tools image with its deps
and thus enable weaving on a remote docker

Closes #312.
@rade rade merged commit 8e65c71 into weaveworks:master Mar 20, 2015
@rade rade removed the in progress label Mar 20, 2015
@rade rade modified the milestone: 0.10.0 Apr 18, 2015
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.

run weave on remote docker
4 participants