fix: linked/depends-on container restarting (#1103)

internal/actions/update.go

@@ -9,6 +9,7 @@ import (
 	"github.com/containrrr/watchtower/pkg/sorter"
 	"github.com/containrrr/watchtower/pkg/types"
 	log "github.com/sirupsen/logrus"
+	"strings"
 )
 
 // Update looks at the running Docker containers to see if any of the images
@@ -68,7 +69,7 @@ func Update(client container.Client, params types.UpdateParams) (types.Report, e
 		return nil, err
 	}
 
-	checkDependencies(containers)
+	UpdateImplicitRestart(containers)
 
 	var containersToUpdate []container.Container
 	if !params.MonitorOnly {
@@ -216,24 +217,41 @@ func restartStaleContainer(container container.Container, client container.Clien
 	return nil
 }
 
-func checkDependencies(containers []container.Container) {
+// UpdateImplicitRestart iterates through the passed containers, setting the
+// `LinkedToRestarting` flag if any of it's linked containers are marked for restart
+func UpdateImplicitRestart(containers []container.Container) {
 
-	for _, c := range containers {
+	for ci, c := range containers {
 		if c.ToRestart() {
+			// The container is already marked for restart, no need to check
 			continue
 		}
 
-	LinkLoop:
+		if link := linkedContainerMarkedForRestart(c.Links(), containers); link != "" {
-		for _, linkName := range c.Links() {
+			log.WithFields(log.Fields{
-			for _, candidate := range containers {
+				"restarting": link,
-				if candidate.Name() != linkName {
+				"linked":     c.Name(),
-					continue
+			}).Debug("container is linked to restarting")
-				}
+			// NOTE: To mutate the array, the `c` variable cannot be used as it's a copy
-				if candidate.ToRestart() {
+			containers[ci].LinkedToRestarting = true
-					c.LinkedToRestarting = true
+		}
-					break LinkLoop
+
-				}
+	}
+}
+
+// linkedContainerMarkedForRestart returns the name of the first link that matches a
+// container marked for restart
+func linkedContainerMarkedForRestart(links []string, containers []container.Container) string {
+	for _, linkName := range links {
+		// Since the container names need to start with '/', let's prepend it if it's missing
+		if !strings.HasPrefix(linkName, "/") {
+			linkName = "/" + linkName
+		}
+		for _, candidate := range containers {
+			if candidate.Name() == linkName && candidate.ToRestart() {
+				return linkName
 			}
 		}
 	}
+	return ""
 }

internal/actions/update_test.go

@@ -20,6 +20,7 @@ var _ = Describe("the update action", func() {
 		BeforeEach(func() {
 			pullImages := false
 			removeVolumes := false
+			//goland:noinspection GoBoolExpressions
 			client = CreateMockClient(
 				&TestData{
 					NameOfContainerToKeep: "test-container-02",
@@ -255,6 +256,54 @@ var _ = Describe("the update action", func() {
 			})
 		})
 
+		When("container is linked to restarting containers", func() {
+			It("should be marked for restart", func() {
+
+				provider := CreateMockContainerWithConfig(
+					"test-container-provider",
+					"/test-container-provider",
+					"fake-image2:latest",
+					true,
+					false,
+					time.Now(),
+					&dockerContainer.Config{
+						Labels:       map[string]string{},
+						ExposedPorts: map[nat.Port]struct{}{},
+					})
+
+				provider.Stale = true
+
+				consumer := CreateMockContainerWithConfig(
+					"test-container-consumer",
+					"/test-container-consumer",
+					"fake-image3:latest",
+					true,
+					false,
+					time.Now(),
+					&dockerContainer.Config{
+						Labels: map[string]string{
+							"com.centurylinklabs.watchtower.depends-on": "test-container-provider",
+						},
+						ExposedPorts: map[nat.Port]struct{}{},
+					})
+
+				containers := []container.Container{
+					provider,
+					consumer,
+				}
+
+				Expect(provider.ToRestart()).To(BeTrue())
+				Expect(consumer.ToRestart()).To(BeFalse())
+
+				actions.UpdateImplicitRestart(containers)
+
+				Expect(containers[0].ToRestart()).To(BeTrue())
+				Expect(containers[1].ToRestart()).To(BeTrue())
+
+			})
+
+		})
+
 		When("container is not running", func() {
 			BeforeEach(func() {
 				client = CreateMockClient(

scripts/dependency-test.sh

@@ -1,16 +1,103 @@
 #!/usr/bin/env bash
 
 # Simulates a container that will always be updated, checking whether it shuts down it's dependencies correctly.
+# Note that this test does not verify the results in any way
 
-docker rm -f parent || true
+set -e
-docker rm -f depending || true
+SCRIPT_ROOT=$(dirname "$(readlink -m "$(type -p "$0")")")
+source "$SCRIPT_ROOT/docker-util.sh"
 
-CHANGE=redis:latest
+DepArgs=""
-KEEP=tutum/hello-world
+if [ -z "$1" ] || [ "$1" == "depends-on" ]; then
+  DepArgs="--label com.centurylinklabs.watchtower.depends-on=parent"
+elif [ "$1" == "linked" ]; then
+  DepArgs="--link parent"
+else
+  DepArgs=$1
+fi
 
-docker tag tutum/hello-world:latest redis:latest
+WatchArgs="${*:2}"
+if [ -z "$WatchArgs" ]; then
+  WatchArgs="--debug"
+fi
 
-docker run -d --name parent $CHANGE
+try-remove-container parent
-docker run -d --name depending --link parent $KEEP
+try-remove-container depending
 
-go run . --run-once --debug $@
+REPO=$(registry-host)
+
+create-dummy-image deptest/parent
+create-dummy-image deptest/depending
+
+echo ""
+
+echo -en "Starting \e[94mparent\e[0m container... "
+CmdParent="docker run -d -p 9090 --name parent $REPO/deptest/parent"
+$CmdParent
+PARENT_REV_BEFORE=$(query-rev parent)
+PARENT_START_BEFORE=$(container-started parent)
+echo -e "Rev: \e[92m$PARENT_REV_BEFORE\e[0m"
+echo -e "Started: \e[96m$PARENT_START_BEFORE\e[0m"
+echo -e "Command: \e[37m$CmdParent\e[0m"
+
+echo ""
+
+echo -en "Starting \e[94mdepending\e[0m container... "
+CmdDepend="docker run -d -p 9090 --name depending $DepArgs $REPO/deptest/depending"
+$CmdDepend
+DEPEND_REV_BEFORE=$(query-rev depending)
+DEPEND_START_BEFORE=$(container-started depending)
+echo -e "Rev: \e[92m$DEPEND_REV_BEFORE\e[0m"
+echo -e "Started: \e[96m$DEPEND_START_BEFORE\e[0m"
+echo -e "Command: \e[37m$CmdDepend\e[0m"
+
+echo -e ""
+
+create-dummy-image deptest/parent
+
+echo -e "\nRunning watchtower..."
+
+if [ -z "$WATCHTOWER_TAG" ]; then
+  ## Windows support:
+  #export DOCKER_HOST=tcp://localhost:2375
+  #export CLICOLOR=1
+  go run . --run-once $WatchArgs
+else
+  docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock containrrr/watchtower:"$WATCHTOWER_TAG" --run-once $WatchArgs
+fi
+
+echo -e "\nSession results:"
+
+PARENT_REV_AFTER=$(query-rev parent)
+PARENT_START_AFTER=$(container-started parent)
+echo -en "  Parent image: \e[95m$PARENT_REV_BEFORE\e[0m => \e[94m$PARENT_REV_AFTER\e[0m "
+if [ "$PARENT_REV_AFTER" == "$PARENT_REV_BEFORE" ]; then
+  echo -e "(\e[91mSame\e[0m)"
+else
+  echo -e "(\e[92mUpdated\e[0m)"
+fi
+echo -en "  Parent container: \e[95m$PARENT_START_BEFORE\e[0m => \e[94m$PARENT_START_AFTER\e[0m "
+if [ "$PARENT_START_AFTER" == "$PARENT_START_BEFORE" ]; then
+  echo -e "(\e[91mSame\e[0m)"
+else
+  echo -e "(\e[92mRestarted\e[0m)"
+fi
+
+echo ""
+
+DEPEND_REV_AFTER=$(query-rev depending)
+DEPEND_START_AFTER=$(container-started depending)
+echo -en "  Depend image: \e[95m$DEPEND_REV_BEFORE\e[0m => \e[94m$DEPEND_REV_AFTER\e[0m "
+if [ "$DEPEND_REV_BEFORE" == "$DEPEND_REV_AFTER" ]; then
+  echo -e "(\e[92mSame\e[0m)"
+else
+  echo -e "(\e[91mUpdated\e[0m)"
+fi
+echo -en "  Depend container: \e[95m$DEPEND_START_BEFORE\e[0m => \e[94m$DEPEND_START_AFTER\e[0m "
+if [ "$DEPEND_START_BEFORE" == "$DEPEND_START_AFTER" ]; then
+  echo -e "(\e[91mSame\e[0m)"
+else
+  echo -e "(\e[92mRestarted\e[0m)"
+fi
+
+echo ""

scripts/docker-util.sh

@@ -0,0 +1,125 @@
+#!/usr/bin/env bash
+# This file is meant to be sourced into other scripts and contain some utility functions for docker e2e testing
+
+
+CONTAINER_PREFIX=${CONTAINER_PREFIX:-du}
+
+function get-port() {
+    Container=$1
+    Port=$2
+
+    if [ -z "$Container" ];  then
+      echo "CONTAINER missing" 1>&2
+      return 1
+    fi
+
+    if [ -z "$Port" ];  then
+      echo "PORT missing" 1>&2
+      return 1
+    fi
+
+    Query=".[].NetworkSettings.Ports[\"$Port/tcp\"] | .[0].HostPort"
+    docker container inspect "$Container" | jq -r "$Query"
+}
+
+function start-registry() {
+  local Name="$CONTAINER_PREFIX-registry"
+  echo -en "Starting \e[94m$Name\e[0m container... "
+  local Port="${1:-5000}"
+  docker run -d -p 5000:"$Port" --restart=unless-stopped --name "$Name" registry:2
+}
+
+function stop-registry() {
+  try-remove-container "$CONTAINER_PREFIX-registry"
+}
+
+function registry-host() {
+  echo "localhost:$(get-port "$CONTAINER_PREFIX"-registry 5000)"
+}
+
+function try-remove-container() {
+  echo -en "Looking for container \e[95m$1\e[0m... "
+  local Found
+  Found=$(container-id "$1")
+  if [ -n "$Found" ]; then
+    echo "$Found"
+    echo -n "  Stopping... "
+    docker stop "$1"
+    echo -n "  Removing... "
+    docker rm "$1"
+  else
+    echo "Not found"
+  fi
+}
+
+function create-dummy-image() {
+    if [ -z "$1" ];  then
+      echo "TAG missing"
+      return 1
+    fi
+    local Tag="$1"
+    local Repo
+    Repo="$(registry-host)"
+    local Revision=${2:-$(("$(date +%s)" - "$(date --date='2021-10-21' +%s)"))}
+
+    echo -e "Creating new image \e[95m$Tag\e[0m revision: \e[94m$Revision\e[0m"
+
+    local BuildDir="/tmp/docker-dummy-$Tag-$Revision"
+
+    mkdir -p "$BuildDir"
+
+    cat > "$BuildDir/Dockerfile" << END
+FROM alpine
+
+RUN echo "Tag: $Tag"
+RUN echo "Revision: $Revision"
+ENTRYPOINT ["nc", "-lk", "-v", "-l", "-p", "9090", "-e", "echo", "-e", "HTTP/1.1 200 OK\n\n$Tag $Revision"]
+END
+
+   docker build -t "$Repo/$Tag:latest" -t "$Repo/$Tag:r$Revision" "$BuildDir"
+
+   echo -e "Pushing images...\e[93m"
+   docker push -q "$Repo/$Tag:latest"
+   docker push -q "$Repo/$Tag:r$Revision"
+   echo -en "\e[0m"
+
+   rm -r "$BuildDir"
+}
+
+function query-rev() {
+  local Name=$1
+  if [ -z "$Name" ];  then
+    echo "NAME missing"
+    return 1
+  fi
+  curl -s "localhost:$(get-port "$Name" 9090)"
+}
+
+function latest-image-rev() {
+  local Tag=$1
+  if [ -z "$Tag" ];  then
+    echo "TAG missing"
+    return 1
+  fi
+  local ID
+  ID=$(docker image ls "$(registry-host)"/"$Tag":latest -q)
+  docker image inspect "$ID" | jq -r '.[].RepoTags | .[]' | grep  -v latest
+}
+
+function container-id() {
+  local Name=$1
+  if [ -z "$Name" ];  then
+    echo "NAME missing"
+    return 1
+  fi
+  docker container ls -f name="$Name" -q
+}
+
+function container-started() {
+  local Name=$1
+  if [ -z "$Name" ];  then
+    echo "NAME missing"
+    return 1
+  fi
+  docker container inspect "$Name" | jq -r .[].State.StartedAt
+}

scripts/du-cli.sh

@@ -0,0 +1,58 @@
+#!/usr/bin/env bash
+
+SCRIPT_ROOT=$(dirname "$(readlink -m "$(type -p "$0")")")
+source "$SCRIPT_ROOT/docker-util.sh"
+
+case $1 in
+  registry | reg | r)
+    case $2 in
+      start)
+        start-registry
+        ;;
+      stop)
+        stop-registry
+        ;;
+      host)
+        registry-host
+        ;;
+      *)
+        echo "Unknown keyword \"$2\""
+        ;;
+    esac
+    ;;
+  image | img | i)
+    case $2 in
+      rev)
+        create-dummy-image "${@:3:2}"
+        ;;
+      latest)
+        latest-image-rev "$3"
+        ;;
+      *)
+        echo "Unknown keyword \"$2\""
+        ;;
+    esac
+    ;;
+  container | cnt | c)
+    case $2 in
+      query)
+        query-rev "$3"
+        ;;
+      rm)
+        try-remove-container "$3"
+        ;;
+      id)
+        container-id "$3"
+        ;;
+      started)
+        container-started "$3"
+        ;;
+      *)
+        echo "Unknown keyword \"$2\""
+        ;;
+    esac
+    ;;
+  *)
+    echo "Unknown keyword \"$1\""
+    ;;
+esac