From 4af3e30f641ca798c41b032941425c69ec6d5ef5 Mon Sep 17 00:00:00 2001 From: Xian Chaobo Date: Fri, 15 May 2015 04:46:02 -0400 Subject: [PATCH 1/3] Fix:#748 add refresh container Signed-off-by: Xian Chaobo --- api/handlers.go | 21 +++++++++++++++++++++ api/router.go | 14 +++++++------- cluster/container.go | 5 +++++ 3 files changed, 33 insertions(+), 7 deletions(-) diff --git a/api/handlers.go b/api/handlers.go index 0322d3f50b..f95937e209 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -532,6 +532,27 @@ func proxyContainer(c *context, w http.ResponseWriter, r *http.Request) { } } +// Proxy a request to the right node and force refresh container +func proxyContainerAndForceRefresh(c *context, w http.ResponseWriter, r *http.Request) { + name, container, err := getContainerFromVars(c, mux.Vars(r)) + if err != nil { + httpError(w, err.Error(), http.StatusNotFound) + return + } + + // Set the full container ID in the proxied URL path. + if name != "" { + r.URL.Path = strings.Replace(r.URL.Path, name, container.Id, 1) + } + + if err := proxy(c.tlsConfig, container.Engine.Addr, w, r); err != nil { + httpError(w, err.Error(), http.StatusInternalServerError) + } + + // force fresh container + container.Refresh() +} + // Proxy a request to the right node func proxyImage(c *context, w http.ResponseWriter, r *http.Request) { name := mux.Vars(r)["name"] diff --git a/api/router.go b/api/router.go index fbf95025e0..55d162b0d3 100644 --- a/api/router.go +++ b/api/router.go @@ -52,14 +52,14 @@ var routes = map[string]map[string]handler{ "/images/{name:.*}/push": proxyImage, "/images/{name:.*}/tag": proxyImageAndForceRefresh, "/containers/create": postContainersCreate, - "/containers/{name:.*}/kill": proxyContainer, - "/containers/{name:.*}/pause": proxyContainer, - "/containers/{name:.*}/unpause": proxyContainer, + "/containers/{name:.*}/kill": proxyContainerAndForceRefresh, + "/containers/{name:.*}/pause": proxyContainerAndForceRefresh, + "/containers/{name:.*}/unpause": proxyContainerAndForceRefresh, "/containers/{name:.*}/rename": postRenameContainer, - "/containers/{name:.*}/restart": proxyContainer, - "/containers/{name:.*}/start": proxyContainer, - "/containers/{name:.*}/stop": proxyContainer, - "/containers/{name:.*}/wait": proxyContainer, + "/containers/{name:.*}/restart": proxyContainerAndForceRefresh, + "/containers/{name:.*}/start": proxyContainerAndForceRefresh, + "/containers/{name:.*}/stop": proxyContainerAndForceRefresh, + "/containers/{name:.*}/wait": proxyContainerAndForceRefresh, "/containers/{name:.*}/resize": proxyContainer, "/containers/{name:.*}/attach": proxyHijack, "/containers/{name:.*}/copy": proxyContainer, diff --git a/cluster/container.go b/cluster/container.go index ad8004f912..37407a96a0 100644 --- a/cluster/container.go +++ b/cluster/container.go @@ -10,3 +10,8 @@ type Container struct { Info dockerclient.ContainerInfo Engine *Engine } + +// Refresh container +func (c *Container) Refresh() error { + return c.Engine.refreshContainer(c.Id, true) +} From ada820815788d9682e49209a5585b337ceb22393 Mon Sep 17 00:00:00 2001 From: Xian Chaobo Date: Fri, 15 May 2015 05:12:22 -0400 Subject: [PATCH 2/3] remove retry in test Signed-off-by: Xian Chaobo --- test/integration/api/cp.bats | 3 +-- test/integration/api/diff.bats | 3 +-- test/integration/api/exec.bats | 3 +-- test/integration/api/kill.bats | 6 ++---- test/integration/api/pause.bats | 6 ++---- test/integration/api/port.bats | 3 +-- test/integration/api/restart.bats | 6 ++---- test/integration/api/rm.bats | 3 +-- test/integration/api/run.bats | 3 +-- test/integration/api/start.bats | 3 +-- test/integration/api/stats.bats | 3 +-- test/integration/api/stop.bats | 6 ++---- test/integration/api/top.bats | 3 +-- test/integration/api/unpause.bats | 9 +++------ 14 files changed, 20 insertions(+), 40 deletions(-) diff --git a/test/integration/api/cp.bats b/test/integration/api/cp.bats index 569ad10e0d..9f91c59d07 100644 --- a/test/integration/api/cp.bats +++ b/test/integration/api/cp.bats @@ -19,8 +19,7 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 500 # make sure container is up - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] # grab the checksum of the test file inside the container. run docker_swarm exec test_container md5sum $test_file diff --git a/test/integration/api/diff.bats b/test/integration/api/diff.bats index 26f518ad59..0b1d8b0763 100644 --- a/test/integration/api/diff.bats +++ b/test/integration/api/diff.bats @@ -13,8 +13,7 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 500 # make sure container is up - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] # no changes run docker_swarm diff test_container diff --git a/test/integration/api/exec.bats b/test/integration/api/exec.bats index f6b1b4a18a..c7fbb79f1c 100644 --- a/test/integration/api/exec.bats +++ b/test/integration/api/exec.bats @@ -20,8 +20,7 @@ function teardown() { docker_swarm start test_container # make sure container is up and not paused - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] run docker_swarm exec test_container echo foobar [ "$status" -eq 0 ] diff --git a/test/integration/api/kill.bats b/test/integration/api/kill.bats index 395f86aded..4771987706 100644 --- a/test/integration/api/kill.bats +++ b/test/integration/api/kill.bats @@ -14,14 +14,12 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 1000 # make sure container is up before killing - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] # kill docker_swarm kill test_container # verify - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=exited) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=exited) ] [ $(docker_swarm inspect -f '{{ .State.ExitCode }}' test_container) -eq 137 ] } diff --git a/test/integration/api/pause.bats b/test/integration/api/pause.bats index 6e0ab45107..cc451a29fe 100644 --- a/test/integration/api/pause.bats +++ b/test/integration/api/pause.bats @@ -16,12 +16,10 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 1000 # make sure container is up - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] docker_swarm pause test_container # verify - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=paused) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=paused) ] } diff --git a/test/integration/api/port.bats b/test/integration/api/port.bats index b1f404eb53..0c87b30d09 100644 --- a/test/integration/api/port.bats +++ b/test/integration/api/port.bats @@ -13,8 +13,7 @@ function teardown() { docker_swarm run -d -p 8000 --name test_container busybox sleep 500 # make sure container is up - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] # port verify run docker_swarm port test_container diff --git a/test/integration/api/restart.bats b/test/integration/api/restart.bats index f11fce3cbb..83cb649bc5 100644 --- a/test/integration/api/restart.bats +++ b/test/integration/api/restart.bats @@ -14,8 +14,7 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 1000 # make sure container is up - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] # Keep track of when the container was started. local started_at=$(docker_swarm inspect -f '{{ .State.StartedAt }}' test_container) @@ -25,7 +24,6 @@ function teardown() { # verify run docker_swarm ps -l - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] [ "$(docker_swarm inspect -f '{{ .State.StartedAt }}' test_container)" != "$started_at" ] } diff --git a/test/integration/api/rm.bats b/test/integration/api/rm.bats index 76a25bee25..168798178b 100644 --- a/test/integration/api/rm.bats +++ b/test/integration/api/rm.bats @@ -32,8 +32,7 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 500 # make sure container exsists and is up - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] # rm, remove a running container, return error run docker_swarm rm test_container diff --git a/test/integration/api/run.bats b/test/integration/api/run.bats index e49560ffd8..2cb038eb87 100644 --- a/test/integration/api/run.bats +++ b/test/integration/api/run.bats @@ -19,6 +19,5 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 100 # verify, container is running - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] } diff --git a/test/integration/api/start.bats b/test/integration/api/start.bats index 2efe977f0e..4f46ad2737 100644 --- a/test/integration/api/start.bats +++ b/test/integration/api/start.bats @@ -23,6 +23,5 @@ function teardown() { docker_swarm start test_container # Verify - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] } diff --git a/test/integration/api/stats.bats b/test/integration/api/stats.bats index 47392f0c0b..01827b11d0 100644 --- a/test/integration/api/stats.bats +++ b/test/integration/api/stats.bats @@ -16,8 +16,7 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 50 # make sure container is up - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] # storage the stats output in TEMP_FILE # it will stop automatically when manager stop diff --git a/test/integration/api/stop.bats b/test/integration/api/stop.bats index 188a984e9c..9a8b64e5a2 100644 --- a/test/integration/api/stop.bats +++ b/test/integration/api/stop.bats @@ -14,13 +14,11 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 500 # make sure container is up before stop - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] # stop docker_swarm stop test_container # verify - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=exited) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=exited) ] } diff --git a/test/integration/api/top.bats b/test/integration/api/top.bats index 65b3888ef5..ea0054e974 100644 --- a/test/integration/api/top.bats +++ b/test/integration/api/top.bats @@ -14,8 +14,7 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 500 # make sure container is running - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] run docker_swarm top test_container [ "$status" -eq 0 ] diff --git a/test/integration/api/unpause.bats b/test/integration/api/unpause.bats index 647c1774a7..0093e71fed 100644 --- a/test/integration/api/unpause.bats +++ b/test/integration/api/unpause.bats @@ -14,19 +14,16 @@ function teardown() { docker_swarm run -d --name test_container busybox sleep 1000 # make sure container is up - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] # pause docker_swarm pause test_container - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=paused) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=paused) ] # unpause docker_swarm unpause test_container # verify - # FIXME(#748): Retry required because of race condition. - retry 5 0.5 eval "[ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ]" + [ -n $(docker_swarm ps -q --filter=name=test_container --filter=status=running) ] } From 014947daad277d9f4044def95d623a751c71e79f Mon Sep 17 00:00:00 2001 From: Xian Chaobo Date: Mon, 18 May 2015 04:52:37 -0400 Subject: [PATCH 3/3] refresh before closing the connection Signed-off-by: Xian Chaobo --- api/handlers.go | 10 ++++++---- api/utils.go | 8 ++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/api/handlers.go b/api/handlers.go index f95937e209..288f988173 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -545,12 +545,14 @@ func proxyContainerAndForceRefresh(c *context, w http.ResponseWriter, r *http.Re r.URL.Path = strings.Replace(r.URL.Path, name, container.Id, 1) } - if err := proxy(c.tlsConfig, container.Engine.Addr, w, r); err != nil { - httpError(w, err.Error(), http.StatusInternalServerError) + cb := func(resp *http.Response) { + // force fresh container + container.Refresh() } - // force fresh container - container.Refresh() + if err := proxyAsync(c.tlsConfig, container.Engine.Addr, w, r, cb); err != nil { + httpError(w, err.Error(), http.StatusInternalServerError) + } } // Proxy a request to the right node diff --git a/api/utils.go b/api/utils.go index 8041616c80..f5877f8979 100644 --- a/api/utils.go +++ b/api/utils.go @@ -81,14 +81,14 @@ func proxyAsync(tlsConfig *tls.Config, addr string, w http.ResponseWriter, r *ht w.WriteHeader(resp.StatusCode) io.Copy(NewWriteFlusher(w), resp.Body) - // cleanup - resp.Body.Close() - closeIdleConnections(client) - if callback != nil { callback(resp) } + // cleanup + resp.Body.Close() + closeIdleConnections(client) + return nil }