From cbf6d22b0cf9db5f8843c386e93f7dd8123867b1 Mon Sep 17 00:00:00 2001 From: Alexandre Menif Date: Wed, 19 Dec 2018 22:01:11 +0100 Subject: [PATCH] feat: do no execute pre/post update commands on linked containers --- actions/update.go | 16 +++++----- container/container.go | 7 +++++ test/commands-test.sh | 69 +++++++++++++++++++++++++++++++++++++++--- 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/actions/update.go b/actions/update.go index b321d0e..70755b6 100644 --- a/actions/update.go +++ b/actions/update.go @@ -48,9 +48,11 @@ func Update(client container.Client, filter container.Filter, cleanup bool, noRe continue } - if container.Stale { + if container.ToRestart() { - executePreUpdateCommand(enableUpdateCmd, client, container) + if container.Stale { + executePreUpdateCommand(enableUpdateCmd, client, container) + } if err := client.StopContainer(container, timeout); err != nil { log.Error(err) @@ -60,7 +62,7 @@ func Update(client container.Client, filter container.Filter, cleanup bool, noRe // Restart stale containers in sorted order for _, container := range containers { - if container.Stale { + if container.ToRestart() { // Since we can't shutdown a watchtower container immediately, we need to // start the new one while the old one is still running. This prevents us // from re-using the same container name so we first rename the current @@ -77,7 +79,7 @@ func Update(client container.Client, filter container.Filter, cleanup bool, noRe newContainerID, err := client.StartContainer(container) if err != nil { log.Error(err) - } else { + } else if container.Stale { executePostUpdateCommand(enableUpdateCmd, client, newContainerID) } } @@ -128,15 +130,15 @@ func executePostUpdateCommand(enabled bool, client container.Client, newContaine func checkDependencies(containers []container.Container) { for i, parent := range containers { - if parent.Stale { + if parent.ToRestart() { continue } LinkLoop: for _, linkName := range parent.Links() { for _, child := range containers { - if child.Name() == linkName && child.Stale { - containers[i].Stale = true + if child.Name() == linkName && child.ToRestart() { + containers[i].Linked = true break LinkLoop } } diff --git a/container/container.go b/container/container.go index 3173ac6..c321f13 100644 --- a/container/container.go +++ b/container/container.go @@ -29,6 +29,7 @@ func NewContainer(containerInfo *types.ContainerJSON, imageInfo *types.ImageInsp // Container represents a running Docker container. type Container struct { + Linked bool Stale bool containerInfo *types.ContainerJSON @@ -141,6 +142,12 @@ func (c Container) PostUpdateCommand() string { return "" } +// ToRestart return whether the container should be restarted, either because +// is stale or linked to another stale container. +func (c Container) ToRestart() bool { + return c.Stale || c.Linked +} + // Ideally, we'd just be able to take the ContainerConfig from the old container // and use it as the starting point for creating the new container; however, // the ContainerConfig that comes back from the Inspect call merges the default diff --git a/test/commands-test.sh b/test/commands-test.sh index b00e233..54dc18e 100755 --- a/test/commands-test.sh +++ b/test/commands-test.sh @@ -4,17 +4,20 @@ set -e IMAGE=server CONTAINER=server +LINKED_IMAGE=linked +LINKED_CONTAINER=linked WATCHTOWER_INTERVAL=2 function remove_container { - docker kill $CONTAINER >> /dev/null || true && docker rm -v $CONTAINER >> /dev/null || true + docker kill $1 >> /dev/null || true && docker rm -v $1 >> /dev/null || true } function cleanup { # Do cleanup on exit or error echo "Final cleanup" sleep 2 - remove_container + remove_container $CONTAINER + remove_container $LINKED_CONTAINER pkill -9 -f watchtower >> /dev/null || true } trap cleanup EXIT @@ -73,7 +76,7 @@ function builddocker { # Start watchtower echo "Starting watchtower" -$WATCHTOWER -i $WATCHTOWER_INTERVAL --no-pull --stop-timeout 2s --enable-update-commands $CONTAINER & +$WATCHTOWER -i $WATCHTOWER_INTERVAL --no-pull --stop-timeout 2s --enable-update-commands $CONTAINER $LINKED_CONTAINER & sleep 3 echo "#################################################################" @@ -109,7 +112,7 @@ if [[ $RESP != "image" ]]; then exit 1 fi -remove_container +remove_container $CONTAINER echo "#################################################################" echo "##### TEST CASE 2: Execute commands from container and base image" @@ -144,4 +147,62 @@ RESP=$(curl -s http://localhost:8888) if [[ $RESP != "container" ]]; then echo "Value of container response is invalid. Expected: container. Actual: $RESP" exit 1 +fi + +remove_container $CONTAINER + +echo "#################################################################" +echo "##### TEST CASE 3: Execute commands with a linked container" +echo "#################################################################" + +# Tag the current image to keep a version for the linked container +docker tag $IMAGE:latest $LINKED_IMAGE:latest + +# Build base image +builddocker + +# Run container +docker run -d -p 0.0.0.0:8888:8888 \ + --label=com.centurylinklabs.watchtower.post-update-command="echo container > /opt/test/value.txt" \ + --name $CONTAINER $IMAGE:latest >> /dev/null +docker run -d -p 0.0.0.0:8989:8888 \ + --label=com.centurylinklabs.watchtower.post-update-command="echo container > /opt/test/value.txt" \ + --link $CONTAINER \ + --name $LINKED_CONTAINER $LINKED_IMAGE:latest >> /dev/null +sleep 1 +echo "Container $CONTAINER and $LINKED_CONTAINER are runnning" + +# Test default value +RESP=$(curl -s http://localhost:8888) +if [ $RESP != "default" ]; then + echo "Default value of container response is invalid" 1>&2 + exit 1 +fi + +# Test default value for linked container +RESP=$(curl -s http://localhost:8989) +if [ $RESP != "default" ]; then + echo "Default value of linked container response is invalid" 1>&2 + exit 1 +fi + +# Build updated image to trigger watchtower update +builddocker + +WAIT_AMOUNT=$(($WATCHTOWER_INTERVAL * 3)) +echo "Wait for $WAIT_AMOUNT seconds" +sleep $WAIT_AMOUNT + +# Test value after post-update-command +RESP=$(curl -s http://localhost:8888) +if [[ $RESP != "container" ]]; then + echo "Value of container response is invalid. Expected: container. Actual: $RESP" + exit 1 +fi + +# Test that linked container did not execute pre/post-update-command +RESP=$(curl -s http://localhost:8989) +if [[ $RESP != "default" ]]; then + echo "Value of linked container response is invalid. Expected: default. Actual: $RESP" + exit 1 fi \ No newline at end of file