diff --git a/proxy/proxy.go b/proxy/proxy.go index 74eccfdf..54bedc65 100644 --- a/proxy/proxy.go +++ b/proxy/proxy.go @@ -103,7 +103,7 @@ func (proxy *Proxy) attachWithRetry(id string) { j := &attachJob{id: id, tryInterval: initialInterval} j.timer = time.AfterFunc(time.Duration(0), func() { - if err := proxy.attach(id, false); err != nil { + if err := proxy.attach(id, false, false); err != nil { // The delay at the nth retry is a random value in the range // [i-i/2,i+i/2], where i = initialInterval * 1.5^(n-1). j.timer.Reset(j.tryInterval) @@ -383,7 +383,10 @@ func (proxy *Proxy) listen(protoAndAddr string) (net.Listener, string, error) { // weavedocker.ContainerObserver interface func (proxy *Proxy) ContainerStarted(ident string) { - proxy.notifyWaiters(ident, proxy.attach(ident, true)) + // In case of attach failure, if we have a request waiting on the start, kill the container, + // otherwise assume it is a Docker-initated restart and kill the process inside. + err := proxy.attach(ident, true, proxy.waitChan(ident) == nil) + proxy.notifyWaiters(ident, err) } func containerShouldAttach(container *docker.Container) bool { @@ -438,18 +441,20 @@ func (proxy *Proxy) waitForStart(r *http.Request) error { return nil } -// If some other operation is waiting for a container to start, join in the wait -func (proxy *Proxy) waitForStartByIdent(ident string) error { - var ch chan error +func (proxy *Proxy) waitChan(ident string) chan error { proxy.Lock() + defer proxy.Unlock() for _, wait := range proxy.waiters { if ident == wait.ident && !wait.done { - ch = wait.ch - break + return wait.ch } } - proxy.Unlock() - if ch != nil { + return nil +} + +// If some other operation is waiting for a container to start, join in the wait +func (proxy *Proxy) waitForStartByIdent(ident string) error { + if ch := proxy.waitChan(ident); ch != nil { Log.Debugf("Wait for start of container %s", ident) return <-ch } @@ -461,7 +466,7 @@ func (proxy *Proxy) ContainerDestroyed(ident string) {} // Check if this container needs to be attached, if so then attach it, // and return nil on success or not needed. -func (proxy *Proxy) attach(containerID string, orDie bool) error { +func (proxy *Proxy) attach(containerID string, orDie, killProcess bool) error { container, err := proxy.client.InspectContainer(containerID) if err != nil { if _, ok := err.(*docker.NoSuchContainer); !ok { @@ -498,7 +503,11 @@ func (proxy *Proxy) attach(containerID string, orDie bool) error { args = append(args, "--no-multicast-route") } if orDie { - args = append(args, "--or-die") + if killProcess { + args = append(args, "--or-kill") + } else { + args = append(args, "--or-die") + } } args = append(args, container.ID) return callWeaveAttach(container, args) diff --git a/test/640_proxy_restart_reattaches_test.sh b/test/640_proxy_restart_reattaches_test.sh index 799a9542..4ea8e04b 100755 --- a/test/640_proxy_restart_reattaches_test.sh +++ b/test/640_proxy_restart_reattaches_test.sh @@ -60,8 +60,16 @@ check_attached assert_raises "timeout 90 cat <( run_many )" +# Start a container that needs IPAM, then restart it when the router is stopped +proxy_start_container $HOST1 -di --name=c3 --restart=always +assert_raises "exec_on $HOST1 c3 $CHECK_ETHWE_UP" +weave_on $HOST1 stop-router +docker_on $HOST1 restart c3 +weave_on $HOST1 launch-router +sleep 5 +assert_raises "exec_on $HOST1 c3 $CHECK_ETHWE_UP" + # Restarting proxy shouldn't kill unattachable containers -proxy_start_container $HOST1 -di --name=c3 --restart=always # Use ipam, so it won't be attachable w/o weave weave_on $HOST1 stop weave_on $HOST1 launch-proxy assert_raises "exec_on $HOST1 c3 $CHECK_ETHWE_UP" diff --git a/weave b/weave index 341d7c7c..76ebe589 100755 --- a/weave +++ b/weave @@ -828,9 +828,24 @@ with_container_netns() { return $STATUS } +kill_container() { + docker kill $1 >/dev/null 2>&1 || true +} + +kill_container_process() { + kill -9 $(docker inspect --format='{{.State.Pid}}' $1) >/dev/null 2>&1 || true +} + with_container_netns_or_die() { if ! with_container_netns "$@" >/dev/null ; then - docker kill $1 >/dev/null 2>&1 || true + kill_container $1 + exit 1 + fi +} + +with_container_netns_or_kill() { + if ! with_container_netns "$@" >/dev/null ; then + kill_container_process $1 exit 1 fi } @@ -1129,7 +1144,7 @@ wait_for_status() { while true ; do "$@" GET /status >/dev/null 2>&1 && return 0 if ! check_running $container >/dev/null 2>&1 ; then - docker kill $container >/dev/null 2>&1 || true # stop it restarting + kill_container $container # stop it restarting echo $(death_msg $container) >&2 return 1 fi @@ -1456,7 +1471,14 @@ ipam_cidrs() { ipam_cidrs_or_die() { if ! ipam_cidrs "$@" ; then - docker kill $2 >/dev/null 2>&1 || true + kill_container $2 + exit 1 + fi +} + +ipam_cidrs_or_kill() { + if ! ipam_cidrs "$@" ; then + kill_container_process $2 exit 1 fi } @@ -2137,6 +2159,9 @@ EOF --or-die) ATTACH_TYPE="_or_die" ;; + --or-kill) + ATTACH_TYPE="_or_kill" + ;; --rewrite-hosts) REWRITE_HOSTS=1 ;;