Merge branch '1.5'

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)

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"

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
                     ;;