Merge pull request #1551 from weaveworks/1547-proxy-restart-no-kill-containers

Proxy restart shouldn't kill containers

Fixes #1547.

proxy/proxy.go

@@ -116,9 +116,16 @@ func NewProxy(c Config) (*Proxy, error) {
 
 func (proxy *Proxy) AttachExistingContainers() {
 	containers, _ := proxy.client.ListContainers(docker.ListContainersOptions{})
-	for _, cont := range containers {
-		if strings.HasPrefix(cont.Command, weaveWaitEntrypoint[0]) {
-			proxy.ContainerStarted(cont.ID)
+	for _, c := range containers {
+		container, err := proxy.client.InspectContainer(c.ID)
+		if err != nil {
+			if _, ok := err.(*docker.NoSuchContainer); !ok {
+				Log.Warningf("unable to attach existing container %s since inspecting it failed: %v", c.ID, err)
+			}
+			continue
+		}
+		if containerShouldAttach(container) && (container.State.Running || container.State.Paused) {
+			proxy.attach(container, false)
 		}
 	}
 }
@@ -290,12 +297,14 @@ func (proxy *Proxy) listen(protoAndAddr string) (net.Listener, string, error) {
 func (proxy *Proxy) ContainerStarted(ident string) {
 	container, err := proxy.client.InspectContainer(ident)
 	if err != nil {
-		Log.Warningf("Error inspecting container %s: %v", ident, err)
+		if _, ok := err.(*docker.NoSuchContainer); !ok {
+			Log.Warningf("unable to attach new container %s since inspecting it failed: %v", ident, err)
+		}
 		return
 	}
 	// If this was a container we modified the entrypoint for, attach it to the network
 	if containerShouldAttach(container) {
-		proxy.attach(container)
+		proxy.attach(container, true)
 	}
 	proxy.notifyWaiters(container.ID)
 }
@@ -345,7 +354,7 @@ func (proxy *Proxy) waitForStart(r *http.Request) {
 func (proxy *Proxy) ContainerDied(ident string) {
 }
 
-func (proxy *Proxy) attach(container *docker.Container) error {
+func (proxy *Proxy) attach(container *docker.Container, orDie bool) error {
 	cidrs, err := proxy.weaveCIDRs(container.HostConfig.NetworkMode, container.Config.Env)
 	if err != nil {
 		Log.Infof("Leaving container %s alone because %s", container.ID, err)
@@ -363,7 +372,10 @@ func (proxy *Proxy) attach(container *docker.Container) error {
 			}
 		}
 	}
-	args = append(args, "--or-die", container.ID)
+	if orDie {
+		args = append(args, "--or-die")
+	}
+	args = append(args, container.ID)
 	if _, stderr, err := callWeave(args...); err != nil {
 		Log.Warningf("Attaching container %s to weave network failed: %s", container.ID, string(stderr))
 		return errors.New(string(stderr))

test/640_proxy_restart_reattaches_test.sh

@@ -25,6 +25,12 @@ run_on $HOST1 sudo kill -KILL $(docker_on $HOST1 inspect --format='{{.State.Pid}
 sleep 1
 check_attached
 
+# 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 "proxy exec_on $HOST1 c3 $CHECK_ETHWE_UP"
+
 # Restart docker itself, using different commands for systemd- and upstart-managed.
 run_on $HOST1 sh -c "command -v systemctl >/dev/null && sudo systemctl restart docker || sudo service docker restart"
 sleep 1