make proxy attach network on docker start event #1210

bboreham · 2015-07-22T16:26:22Z

Do network attach on docker event rather than intercepting start and restart commands.

This means it also works when the docker daemon restarts a container. Fixes #401 and #1258.

bboreham · 2015-07-23T13:29:12Z

This change has a side-effect that docker run -d via the proxy now returns before the network is set up and the container is running.

rade · 2015-07-23T13:39:49Z

Presumably the container is running; just not the application process. Still i can imagine that might trip up some users. And the same is true of docker start.

rade · 2015-07-23T13:41:35Z

...and docker restart. i.e. it is probably not unreasonable to expect that when these command return successfully that the container is fully up and running.

The question is what might break for users.

bboreham · 2015-07-23T14:07:32Z

It sometimes breaks smoke-tests like 680. I made a fix.

prog/weavewait/main.go

@@ -26,6 +26,11 @@ func main() {
 		args = args[1:]
 	}

+	if len(args) > 0 && args[0] == "-c" { // tell pid 1 to continue
+		checkErr(syscall.Kill(1, syscall.SIGUSR2))


proxy/proxy.go

 	if err != nil {
 		return nil, err
 	}
-	p.client = client
+	p.client = client.Client
+	client.AddObserver(p)


errordeveloper · 2015-08-14T11:06:06Z

@paulbellamy if you have a minute, may be you could cherry-pick --rewrite-inspect changes on top of this? I got pretty confused last time I tried cherry-picking it myself 😕

weave

+    HOSTS_PATH=$(docker $DOCKER_CLIENT_ARGS run --rm --net=none \
+        -v /var/run/docker.sock:/var/run/docker.sock \
+        --entrypoint /bin/sh \
+        $EXEC_IMAGE -c 'docker inspect -f {{.HostsPath}} $(hostname)')


bboreham · 2015-08-17T10:05:53Z

OK, I think the coding is done; I'm going to rebase from latest master and take a look at splitting into two PRs: one for (#401 and #1258) and one for (#1209 and #1300).

test/640_proxy_restart_reattaches_test.sh

+# Restart docker itself
+# - disabled since the commands are different between our Vagrant VMs and GCE
+#run_on $HOST1 sudo systemctl restart docker # for systemd
+#run_on $HOST1 sudo service docker restart # for upstart


bboreham · 2015-08-18T16:06:09Z

I have addressed the two most recent review comments, rebased, and commented out the part of the test that will fail until #1209 is addressed.

paulbellamy · 2015-08-19T13:26:33Z

LGTM

awh · 2015-08-21T09:34:30Z

The plan of record is that we're waiting for #1314 before merging this.

…restart commands. This means it also works when the docker daemon restarts a container.

…uest

bboreham · 2015-09-01T17:09:21Z

Have rebased and adjusted following merge of #1314

proxy/proxy.go

+
+func (proxy *Proxy) waitForStart(r *http.Request) {
+	var ch chan struct{}
+	proxy.Lock()


make proxy attach network on docker start event

bboreham mentioned this pull request Jul 22, 2015

Don't delete record of IP allocation immediately after container death #1191

Merged

bboreham changed the title ~~In proxy, attach network on docker start event~~ make proxy attach network on docker start event Jul 23, 2015

bboreham self-assigned this Jul 23, 2015

bboreham force-pushed the 401-proxy-start-events branch from 84b4e9a to 2439ce4 Compare July 23, 2015 11:59

bboreham removed their assignment Jul 23, 2015

bboreham force-pushed the 401-proxy-start-events branch from 2439ce4 to 376b45e Compare July 23, 2015 12:57

bboreham self-assigned this Jul 23, 2015

bboreham force-pushed the 401-proxy-start-events branch 2 times, most recently from 427e5e1 to e9818d5 Compare July 23, 2015 18:34

bboreham removed their assignment Jul 23, 2015

bboreham force-pushed the 401-proxy-start-events branch 2 times, most recently from 90b61c8 to a4b0186 Compare August 12, 2015 17:15

rade reviewed Aug 12, 2015
View reviewed changes

bboreham mentioned this pull request Aug 13, 2015

containers fail to start correctly when networking options are passed on start #1258

Closed

paulbellamy reviewed Aug 13, 2015
View reviewed changes

proxy/proxy.go

if err != nil {

return nil, err

}

p.client = client

p.client = client.Client

client.AddObserver(p)

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

bboreham force-pushed the 401-proxy-start-events branch 2 times, most recently from c3c669e to 79b40ce Compare August 14, 2015 15:14

bboreham reviewed Aug 17, 2015
View reviewed changes

bboreham force-pushed the 401-proxy-start-events branch 4 times, most recently from f814287 to 5427bdd Compare August 17, 2015 13:00

paulbellamy reviewed Aug 17, 2015
View reviewed changes

bboreham force-pushed the 401-proxy-start-events branch 4 times, most recently from 2f1216b to 165373d Compare August 18, 2015 15:24

bboreham self-assigned this Aug 24, 2015

This was referenced Aug 24, 2015

make docker daemon initiated container (re)start work properly #401

Closed

Get proxy to re-attach weave router if it restarts #1359

Closed

rade mentioned this pull request Aug 27, 2015

Wait for ethwe via netlink, not via polling or signals #1314

Merged

bboreham force-pushed the 401-proxy-start-events branch from 165373d to 9fbb578 Compare September 1, 2015 16:16

bboreham added 4 commits September 1, 2015 17:48

Do network attach on docker event rather than intercepting start and …

81e8ba2

…restart commands. This means it also works when the docker daemon restarts a container.

Attach existing weave containers at startup

9ae6435

Extend test to cover containers restarting automatically

b712ba1

Wait for container to be attached before returning from {re}start req…

1e23443

…uest

bboreham force-pushed the 401-proxy-start-events branch from 9fbb578 to 1e23443 Compare September 1, 2015 16:49

bboreham removed their assignment Sep 1, 2015

paulbellamy reviewed Sep 14, 2015
View reviewed changes

proxy/proxy.go

func (proxy *Proxy) waitForStart(r *http.Request) {

var ch chan struct{}

proxy.Lock()

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

This comment was marked as abuse.

Sign in to view

Use Mutex instead of RWMutex, for simplicity

76895c7

paulbellamy added a commit that referenced this pull request Sep 15, 2015

Merge pull request #1210 from weaveworks/401-proxy-start-events

e39974e

make proxy attach network on docker start event

paulbellamy merged commit e39974e into master Sep 15, 2015

rade modified the milestone: 1.2.0 Sep 18, 2015

paulbellamy mentioned this pull request Sep 22, 2015

[proxy] fix race condition when waiting for containers to start #1456

Merged

This was referenced Oct 9, 2015

Get proxy to re-attach weave router if it restarts #1518

Merged

proxy shouldn't kill containers when it restarts #1547

Closed

awh deleted the 401-proxy-start-events branch November 9, 2015 16:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

make proxy attach network on docker start event #1210

make proxy attach network on docker start event #1210

bboreham commented Jul 22, 2015

bboreham commented Jul 23, 2015

rade commented Jul 23, 2015

rade commented Jul 23, 2015

bboreham commented Jul 23, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

errordeveloper commented Aug 14, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

bboreham commented Aug 17, 2015

This comment was marked as abuse.

This comment was marked as abuse.

bboreham commented Aug 18, 2015

paulbellamy commented Aug 19, 2015

awh commented Aug 21, 2015

bboreham commented Sep 1, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

make proxy attach network on docker start event #1210

make proxy attach network on docker start event #1210

Conversation

bboreham commented Jul 22, 2015

bboreham commented Jul 23, 2015

rade commented Jul 23, 2015

rade commented Jul 23, 2015

bboreham commented Jul 23, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

errordeveloper commented Aug 14, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.

bboreham commented Aug 17, 2015

This comment was marked as abuse.

This comment was marked as abuse.

bboreham commented Aug 18, 2015

paulbellamy commented Aug 19, 2015

awh commented Aug 21, 2015

bboreham commented Sep 1, 2015

This comment was marked as abuse.

This comment was marked as abuse.

This comment was marked as abuse.