Minimal implementation of the proxy #655

paulbellamy · 2015-05-13T13:43:17Z

Supersedes #652

This adds a docker remote API proxy to the weaveexec image. The proxy is NOT
auto-launched when running a container with weave cli. Nor is it used at all by the weave CLI script.
It is merely provided/documented as a beta feature, so that we can gauge feedback.

The proxy currently has hooks for:

Hook	Action
Pre Container Create	Adjusts the entrypoint of a container to wait for the ethwe interface.
Post Container Start	Attaches a weave interface to a container given a WEAVE_CIDR environment entry.

paulbellamy · 2015-05-13T13:43:31Z

"Beta features" or "Experimental features" ?

rade · 2015-05-13T14:01:00Z

"Experimental Features". And I'd put that right after 'Features'.

squaremo · 2015-05-13T14:40:35Z

You checked in the wait-for-weave binary -- is that safe with respect to licensing?

paulbellamy · 2015-05-13T14:41:11Z

Documetation notes from discussion in person:

check first/third-person for consistency
export DOCKER_HOST instead of -H, and explain how you can forward all commands
explain how it connects to docker
explain more about advantages (mention foreground run)
mention args -H and -L to launch-proxy
show how to do DNS this way
mention no TLS support, yet

rade · 2015-05-13T14:42:53Z

You checked in the wait-for-weave binary

We shouldn't have that checked it. Only the source.

paulbellamy · 2015-05-13T14:43:22Z

@squaremo good call, will check.

rade · 2015-05-13T14:44:45Z

or, rather, we should fetch & build it.

rade · 2015-05-13T15:37:23Z

This kinda fixes #47 and #230. Though we should probably not declare victory on these until the proxy is more fully integrated. @paulbellamy please raise an issue for the latter. TLS is covered by #506.

test/600_proxy_docker_py_test.sh

+
+start_suite "Run docker-py test suite against the proxy"
+
+assert_raises "docker_on $HOST1 ps | grep weaveproxy" 1


rade · 2015-05-13T15:58:49Z

I think this also addresses #117, with the same caveats.

rade · 2015-05-13T16:02:48Z

Does this also solve #400?

rade · 2015-05-13T16:03:50Z

and #401?

test/610_proxy_wait_for_weave_test.sh

+
+start_suite "Proxy waits for weave to be ready before running container commands"
+weave_on $HOST1 launch-proxy
+assert_raises "docker_proxy_on $HOST1 run -e 'WEAVE_CIDR=10.2.1.1/24' --name=c1 gliderlabs/alpine ifconfig ethwe" 0


paulbellamy · 2015-05-13T16:11:44Z

re: licensing, it's Apache2, so since we are "distributing" it, we probably need attribution, and a license included? but I am not a lawyer...

test/config.sh

    remote $host $SSH $host $@
 }

 docker_on() {
    host=$1
    shift 1
-    greyly echo "Docker on $host: $@"
+    greyly echo "Docker on $host: $@" >&2


paulbellamy · 2015-05-13T16:17:11Z

re: licensing, will remove binocarlos/wait-for-weave, and reinvent the wheel.

weaveexec/main.go

+		InitDefaultLogging(true)
+	}
+
+	p, err := proxy.NewProxy(target)


rade · 2015-05-14T02:56:33Z

A few more things to figure out and document...

Does setting DOCKER_HOST to the proxy interfere with weave ... operations? Ideally it wouldn't.
atm the docs only mention docker run. Presumably the proxy also does its thing on create, i.e. a container created with a WEAVE_CIDR will a) have its entry point rewritten, and b) be attached to the weave network when started. Correct? That needs documenting.
We need to make this work with IPAM from IP Allocation Management #563. Strawman: all containers run or created+started via the proxy will receive a weave IP from IPAM if no WEAVE_CIDR was specified. Gotcha: it's going to be tricky to make this work in conjunction with the first point, i.e. DOCKER_HOST set to the proxy not interfering with weave ... operations, since the weavexec containers would presumably end up getting attached to the weave network. One way around that might be to do IPAM when an empty WEAVE_CIDR was specified, and leave containers w/o any WEAVE_CIDR env vars well alone.

rade · 2015-05-14T02:58:58Z

Perhaps there could be a flag on the proxy that controls whether it should act on all containers or only containers with a WEAVE_CIDR.

rade · 2015-05-14T08:47:43Z

Regarding DNS... the proxy could have a --with-dns flag that controls whether to configure container's DNS to point at weaveDNS, much like weave run --with-dns does.

stephanbuys · 2015-05-14T09:39:51Z

+1 This is a great feature, of note as ,mentioned in #629, WEAVE_CIDR should support multiple CIDR's (for containers connected to other northbound and southbound containers).

rade · 2015-05-14T09:41:06Z

It already does. Per the docs in this PR.

stephanbuys · 2015-05-14T09:42:00Z

@rade, apologies, fired off too fast. Excitement got to me :)

proxy/client.go

+
+	down, _, up, rem, err := hijack(w, client)
+	if err != nil {
+		Error.Fatal("Unable to hijack response stream for chunked response", http.StatusInternalServerError)


rade · 2015-05-18T21:54:12Z

The proxy tests take longer to run than the entire existing test suite! 4 mins vs 3min30s. Over half of that is in running the docker integration test suite. Not sure what, if anything, to do about this.

This adds a docker remote API proxy to the weaveexec image. The proxy has hooks for (post-) container start and (pre-) container create; the former attaches a weave interface to a container given a WEAVE_CIDR environment entry, and the latter adjusts the entrypoint of a container to wait for the ethwe interface. Notes: None of the hooks need to inspect/change large bodies. In order to avoid a crash when handling images larger than memory we need to ensure that the entire request body is never loaded into memory (i.e. No ioutils.ReadAll). When proxying a raw-stream response, we can't use http.WriteHeader. It sets the Transfer-Encoding to chunked, which will throw off the clients. Instead we have to hijack the socket, and write out http status code manually. For chunked transfer-encodings, clients (docker-py) depend on each http chunk containing exactly one JSON object. Therefore, we cannot re-chunk the body, and have to pass it across verbatim. Proxy needs to pass multi-cidrs separately when calling weave. Otherwise they get smashed together on the command line, and the call fails. When creating the image, if no entrypoint is specified on the container, it should use the entrypoint of the image. Code for this is based on powerstrip-weave. We handle WEAVE_CIDR='' by acting like it is not set. The reason being, that because WEAVE_CIDR is inherited from images (along with env vars), setting it to blank is equivalent to unsetting it.

Use curl /weave to wait for the proxy to boot before returning.

binocarlos/wait-for-weave is Apache2 licensed, so attribution/etc applies. Thus, we re-invent the wheel here.

In the proxy, I swapped the 'flag' library for 'code.google.com/p/getopt', which uses double-dash flags. proxy lets 'weave attach' do dns registration on container start proxy needs to set DNSSearch if not specified

- some inlining - compile regexps once - consistent error messages - always mention container id in startContainerInterceptor errors

…them

busybox is smaller. gliderlabs/alpine is even smaller, but is used by weaveexec, so we can't remove it.

paulbellamy · 2015-05-19T09:37:08Z

Yeah, I agree the docker-py tests are quite slow. They have several sleeps in them. I had tried, the go-dockerclient tests, but they just use a mock server.

Overall, I'm not sure what to do about it at this point.

paulbellamy · 2015-05-19T09:44:19Z

Notes after f2f with @rade, the docker-py tests are quite slow, and possibly a bit flakey, but have been useful in catching proxy bugs. If they prove troublesome, could check if there is a cli suite as part of the main docker repo.

Or put some work into fixing up/speeding up the docker-py tests...

Minimal implementation of the proxy closes #47, closes #230, closes #251, closes #400.

squaremo · 2015-05-19T10:02:53Z

100!

paulbellamy mentioned this pull request May 13, 2015

Add weave launch-proxy #652

Closed

awh self-assigned this May 13, 2015

paulbellamy force-pushed the proxy-minimal branch 2 times, most recently from 4cb52f1 to 89b7f4c Compare May 13, 2015 15:46

awh reviewed May 13, 2015
View reviewed changes

test/600_proxy_docker_py_test.sh

start_suite "Run docker-py test suite against the proxy"

assert_raises "docker_on $HOST1 ps | grep weaveproxy" 1

This comment was marked as abuse.

Sign in to view

awh reviewed May 13, 2015
View reviewed changes

test/config.sh

remote $host $SSH $host $@

}

docker_on() {

host=$1

shift 1

greyly echo "Docker on $host: $@"

greyly echo "Docker on $host: $@" >&2

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

awh reviewed May 13, 2015
View reviewed changes

weaveexec/main.go

InitDefaultLogging(true)

}

p, err := proxy.NewProxy(target)

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

errordeveloper mentioned this pull request May 14, 2015

Example to use docker-compose with weave #629

Closed

rade reviewed May 18, 2015
View reviewed changes

proxy/client.go

down, _, up, rem, err := hijack(w, client)

if err != nil {

Error.Fatal("Unable to hijack response stream for chunked response", http.StatusInternalServerError)

This comment was marked as abuse.

Sign in to view

paulbellamy and others added 3 commits May 19, 2015 09:48

replace test usage of ifconfig with ip link show

d8e1d07

Add launch-proxy command to cli

6c4271f

Use curl /weave to wait for the proxy to boot before returning.

paulbellamy force-pushed the proxy-minimal branch from 3cb4f95 to bcb2ffa Compare May 19, 2015 08:48

paulbellamy and others added 14 commits May 19, 2015 10:34

Add integration test for the proxy, using docker-py client

5a6a67f

[docs] Initial version of proxy docs

9389001

add weavewait

444b148

binocarlos/wait-for-weave is Apache2 licensed, so attribution/etc applies. Thus, we re-invent the wheel here.

Add dns support for the proxy

96a69d2

In the proxy, I swapped the 'flag' library for 'code.google.com/p/getopt', which uses double-dash flags. proxy lets 'weave attach' do dns registration on container start proxy needs to set DNSSearch if not specified

extract docker-bridge-ip fetching from proxy into bash script

bedc20c

separate proxy package into weaveproxy and proxy

55d17ba

moving weaveproxy docs off 'experimental-features'

6fa0a9d

refactor proxy rawStream handler

2a120e5

add proxy support for 'docker restart'

ca4b8e6

some refactoring and tidying

126d4f4

- some inlining - compile regexps once - consistent error messages - always mention container id in startContainerInterceptor errors

proxy interceptors implicitly modify arguments, instead of returning …

2f8c5d7

…them

use busybox instead of ubuntu for proxy image pulling test

1e7c82b

busybox is smaller. gliderlabs/alpine is even smaller, but is used by weaveexec, so we can't remove it.

updating proxy with-dns description to match docs

6911dc8

proxy should be sending error code to client, instead of exiting

b71a428

paulbellamy force-pushed the proxy-minimal branch from bcb2ffa to b71a428 Compare May 19, 2015 09:34

golint cleanup of else block

ee07d64

paulbellamy force-pushed the proxy-minimal branch from f35db0d to ee07d64 Compare May 19, 2015 09:48

rade added a commit that referenced this pull request May 19, 2015

Merge pull request #655 from paulbellamy/proxy-minimal

8b66b8a

Minimal implementation of the proxy closes #47, closes #230, closes #251, closes #400.

rade merged commit 8b66b8a into weaveworks:master May 19, 2015

paulbellamy deleted the proxy-minimal branch May 19, 2015 10:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minimal implementation of the proxy #655

Minimal implementation of the proxy #655

paulbellamy commented May 13, 2015

paulbellamy commented May 13, 2015

rade commented May 13, 2015

squaremo commented May 13, 2015

paulbellamy commented May 13, 2015

rade commented May 13, 2015

paulbellamy commented May 13, 2015

rade commented May 13, 2015

rade commented May 13, 2015

This comment was marked as abuse.

rade commented May 13, 2015

rade commented May 13, 2015

rade commented May 13, 2015

This comment was marked as abuse.

This comment was marked as abuse.

paulbellamy commented May 13, 2015

This comment was marked as abuse.

This comment was marked as abuse.

paulbellamy commented May 13, 2015

This comment was marked as abuse.

This comment was marked as abuse.

rade commented May 14, 2015

rade commented May 14, 2015

rade commented May 14, 2015

stephanbuys commented May 14, 2015

rade commented May 14, 2015

stephanbuys commented May 14, 2015

This comment was marked as abuse.

rade commented May 18, 2015

paulbellamy commented May 19, 2015

paulbellamy commented May 19, 2015

squaremo commented May 19, 2015


		start_suite "Run docker-py test suite against the proxy"

		assert_raises "docker_on $HOST1 ps \| grep weaveproxy" 1

Minimal implementation of the proxy #655

Minimal implementation of the proxy #655

Conversation

paulbellamy commented May 13, 2015

paulbellamy commented May 13, 2015

rade commented May 13, 2015

squaremo commented May 13, 2015

paulbellamy commented May 13, 2015

rade commented May 13, 2015

paulbellamy commented May 13, 2015

rade commented May 13, 2015

rade commented May 13, 2015

This comment was marked as abuse.

rade commented May 13, 2015

rade commented May 13, 2015

rade commented May 13, 2015

This comment was marked as abuse.

This comment was marked as abuse.

paulbellamy commented May 13, 2015

This comment was marked as abuse.

This comment was marked as abuse.

paulbellamy commented May 13, 2015

This comment was marked as abuse.

This comment was marked as abuse.

rade commented May 14, 2015

rade commented May 14, 2015

rade commented May 14, 2015

stephanbuys commented May 14, 2015

rade commented May 14, 2015

stephanbuys commented May 14, 2015

This comment was marked as abuse.

rade commented May 18, 2015

paulbellamy commented May 19, 2015

paulbellamy commented May 19, 2015

squaremo commented May 19, 2015