From bb4c0f5247bed173c8ff5eae45f78ebadf1cee79 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Tue, 9 Feb 2016 09:41:42 +0000 Subject: [PATCH 1/5] Start existing container, if its image and arguments match --- weave | 96 +++++++++++++++++++++++++++++++++-------------------------- 1 file changed, 54 insertions(+), 42 deletions(-) diff --git a/weave b/weave index b16131db..8d83aaf3 100755 --- a/weave +++ b/weave @@ -936,28 +936,56 @@ detach() { # functions for interacting with containers ###################################################################### -# Check that a container for component $1 named $2 with image $3 is not running -check_not_running() { - RUN_STATUS=$(docker inspect --format='{{.State.Running}} {{.State.Status}} {{.Config.Image}}' $2 2>/dev/null) || true +args_match() { + prev_args="$(docker inspect -f '{{.Args}}' $1)" + shift + dummy_container=$(docker create "$@" 2>/dev/null) || return 1 + dummy_args="$(docker inspect -f '{{.Args}}' $dummy_container)" + docker rm $dummy_container >/dev/null + [ "$prev_args" = "$dummy_args" ] +} + +# Check if a container for component $1 named $2 with image $3 and args $4... is running, +# and start it if necessary +start_if_necessary() { + component=$1 + name=$2 + image=$3 + shift 3 + RUN_STATUS=$(docker inspect --format='{{.State.Running}} {{.State.Status}} {{.Config.Image}} {{.Id}}' $name 2>/dev/null) || true + CONTAINER_ID=${RUN_STATUS##* } case ${RUN_STATUS%:*} in - "true restarting $3") - echo "$2 is restarting; you can stop it with 'weave stop-$1'." >&2 + "true restarting $image") + echo "$name is restarting; you can stop it with 'weave stop-$component'." >&2 return 3 ;; - "true "*" $3") - echo "$2 is already running; you can stop it with 'weave stop-$1'." >&2 - return 1 + "true "*" $image") + if args_match $CONTAINER_ID "$@" ; then + ALREADY_RUNNING_ID=$CONTAINER_ID + echo $CONTAINER_ID + else + echo "$name is already running with different arguments; you can stop it with 'weave stop-$component'." >&2 + return 1 + fi ;; - "false "*" $3") - docker rm $2 >/dev/null + "false "*" $image") + if args_match $CONTAINER_ID "$@" ; then + docker start $CONTAINER_ID + else + docker rm $name >/dev/null + docker run -d --name=$name "$@" + fi ;; true*) - echo "Found another running container named '$2'. Aborting." >&2 + echo "Found another running container named '$name'. Aborting." >&2 return 2 ;; false*) - echo "Found another container named '$2'. Aborting." >&2 - return 2 + docker rm $name >/dev/null + docker run -d --name=$name "$@" + ;; + "") + docker run -d --name=$name "$@" ;; esac } @@ -1590,8 +1618,8 @@ launch_router() { # Set WEAVE_DOCKER_ARGS in the environment in order to supply # additional parameters, such as resource limits, to docker # when launching the weave container. - ROUTER_CONTAINER=$(docker run --name=$CONTAINER_NAME \ - -d --privileged --net=host $(docker_sock_options) \ + start_if_necessary router $CONTAINER_NAME $BASE_IMAGE \ + --privileged --net=host $(docker_sock_options) \ -e WEAVE_PASSWORD \ -e CHECKPOINT_DISABLE \ $WEAVE_DOCKER_ARGS $IMAGE $COVERAGE_ARGS \ @@ -1601,7 +1629,8 @@ launch_router() { --dns-effective-listen-address $DOCKER_BRIDGE_IP \ $DNS_ROUTER_OPTS $NO_DNS_OPT \ --http-addr $HTTP_ADDR \ - "$@") + "$@" + [ -n "$ALREADY_RUNNING_ID" ] && return 0 setup_router_iface_$BRIDGE_TYPE attach_router } @@ -1646,8 +1675,8 @@ launch_proxy() { docker create -v /w -v /w-noop -v /w-nomcast --name=$VOLUMES_CONTAINER_NAME \ --label=weavevolumes --entrypoint=/bin/false $EXEC_IMAGE >/dev/null fi - PROXY_CONTAINER=$(docker run --name=$PROXY_CONTAINER_NAME \ - -d --privileged --net=host $(docker_sock_options) \ + start_if_necessary proxy $PROXY_CONTAINER_NAME $BASE_EXEC_IMAGE \ + --privileged --net=host $(docker_sock_options) \ $PROXY_VOLUMES \ --volumes-from $VOLUMES_CONTAINER_NAME \ -v /var/run/weave:/var/run/weave \ @@ -1659,7 +1688,7 @@ launch_proxy() { -e WEAVE_HTTP_ADDR \ -e EXEC_IMAGE=$EXEC_IMAGE \ --entrypoint=/home/weave/weaveproxy \ - $WEAVEPROXY_DOCKER_ARGS $EXEC_IMAGE $COVERAGE_ARGS $PROXY_ARGS) + $WEAVEPROXY_DOCKER_ARGS $EXEC_IMAGE $COVERAGE_ARGS $PROXY_ARGS wait_for_status $PROXY_CONTAINER_NAME http_call_unix $PROXY_CONTAINER_NAME status.sock } @@ -1669,21 +1698,12 @@ stop_proxy() { } launch_plugin_if_not_running() { - retval=0 - check_not_running plugin $PLUGIN_CONTAINER_NAME $BASE_PLUGIN_IMAGE || retval=$? - # If an existing plugin is running (we start it with restart=always), return its ID - [ $retval = 1 ] && PLUGIN_CONTAINER=$(container_id $PLUGIN_CONTAINER_NAME) && return 0 - # Any other kind of error code from check_not_running is a failure. - [ $retval -gt 0 ] && return $retval - - if ! PLUGIN_CONTAINER=$(docker run --name=$PLUGIN_CONTAINER_NAME \ - -d --privileged --net=host $(docker_sock_options) \ + start_if_necessary plugin $PLUGIN_CONTAINER_NAME $BASE_PLUGIN_IMAGE \ + --privileged --net=host $(docker_sock_options) \ --restart=always \ -v /run/docker/plugins:/run/docker/plugins \ -e WEAVE_HTTP_ADDR \ - $PLUGIN_IMAGE "$@") ; then - return 1 - fi + $PLUGIN_IMAGE "$@" wait_for_status $PLUGIN_CONTAINER_NAME http_call_unix $PLUGIN_CONTAINER_NAME status.sock util_op create-plugin-network weave weavemesh } @@ -1848,19 +1868,14 @@ EOF ;; launch) deprecation_warnings "$@" - check_not_running router $CONTAINER_NAME $BASE_IMAGE - check_not_running proxy $PROXY_CONTAINER_NAME $BASE_EXEC_IMAGE - # Don't check the plugin here, since we start it with `--restart=always` COMMON_ARGS=$(common_launch_args "$@") - launch_router "$@" - launch_proxy $COMMON_ARGS - plugin_disabled || launch_plugin_if_not_running $COMMON_ARGS + launch_router "$@" >/dev/null + launch_proxy $COMMON_ARGS >/dev/null + plugin_disabled || launch_plugin_if_not_running $COMMON_ARGS >/dev/null ;; launch-router) deprecation_warnings "$@" - check_not_running router $CONTAINER_NAME $BASE_IMAGE launch_router "$@" - echo $ROUTER_CONTAINER ;; attach-router) check_running $CONTAINER_NAME @@ -1872,13 +1887,10 @@ EOF ;; launch-proxy) deprecation_warnings "$@" - check_not_running proxy $PROXY_CONTAINER_NAME $BASE_EXEC_IMAGE launch_proxy "$@" - echo $PROXY_CONTAINER ;; launch-plugin) launch_plugin_if_not_running "$@" - echo $PLUGIN_CONTAINER ;; env|proxy-env) [ "$COMMAND" = "env" ] || deprecation_warning "$COMMAND" "'weave env'" From 561c8d012ca7883b299c9c802b334353a84c2702 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Feb 2016 12:23:44 +0000 Subject: [PATCH 2/5] Treat a restarting container like a running one, for idempotency --- weave | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/weave b/weave index 8d83aaf3..3cfd1063 100755 --- a/weave +++ b/weave @@ -952,23 +952,19 @@ start_if_necessary() { name=$2 image=$3 shift 3 - RUN_STATUS=$(docker inspect --format='{{.State.Running}} {{.State.Status}} {{.Config.Image}} {{.Id}}' $name 2>/dev/null) || true + RUN_STATUS=$(docker inspect --format='{{.State.Running}} {{.Config.Image}} {{.Id}}' $name 2>/dev/null) || true CONTAINER_ID=${RUN_STATUS##* } case ${RUN_STATUS%:*} in - "true restarting $image") - echo "$name is restarting; you can stop it with 'weave stop-$component'." >&2 - return 3 - ;; - "true "*" $image") + "true $image") if args_match $CONTAINER_ID "$@" ; then - ALREADY_RUNNING_ID=$CONTAINER_ID + ALREADY_RUNNING_CONTAINER=$CONTAINER_ID echo $CONTAINER_ID else echo "$name is already running with different arguments; you can stop it with 'weave stop-$component'." >&2 return 1 fi ;; - "false "*" $image") + "false $image") if args_match $CONTAINER_ID "$@" ; then docker start $CONTAINER_ID else @@ -1205,6 +1201,7 @@ check_running() { echo "$1 container is not present. Have you launched it?" >&2 return 1 elif [ "$STATUS" = "true true" ] ; then + [ -n "$ALREADY_RUNNING_CONTAINER" ] && return 0 echo "$1 container is restarting." >&2 return 2 elif [ "$STATUS" != "true false" ] ; then @@ -1630,9 +1627,9 @@ launch_router() { $DNS_ROUTER_OPTS $NO_DNS_OPT \ --http-addr $HTTP_ADDR \ "$@" - [ -n "$ALREADY_RUNNING_ID" ] && return 0 setup_router_iface_$BRIDGE_TYPE - attach_router + wait_for_status $CONTAINER_NAME http_call $HTTP_ADDR + [ -n "$ALREADY_RUNNING_CONTAINER" ] || attach_router } # Recreate the parameter values that are set when the router is first launched @@ -1643,7 +1640,6 @@ fetch_router_args() { } attach_router() { - wait_for_status $CONTAINER_NAME http_call $HTTP_ADDR if [ -n "$IPRANGE" ] ; then # Tell the newly-started weave IP allocator about existing weave IPs with_container_addresses ipam_reclaim weave:expose $(docker ps -q --no-trunc) @@ -1883,6 +1879,7 @@ EOF create_bridge fetch_router_args with_container_netns $CONTAINER_NAME setup_router_iface_$BRIDGE_TYPE + wait_for_status $CONTAINER_NAME http_call $HTTP_ADDR attach_router ;; launch-proxy) From 65f435f042ac808dc7f32a88c1d88eaa16a412d9 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Feb 2016 16:42:30 +0000 Subject: [PATCH 3/5] Smoke-test for restarts --- test/175_stop_start_test.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) create mode 100755 test/175_stop_start_test.sh diff --git a/test/175_stop_start_test.sh b/test/175_stop_start_test.sh new file mode 100755 index 00000000..56df0409 --- /dev/null +++ b/test/175_stop_start_test.sh @@ -0,0 +1,21 @@ +#! /bin/bash + +. ./config.sh + +start_suite "Check restart re-uses same container when possible" + +ID=$(weave_on $HOST1 launch-router) +weave_on $HOST1 stop-router +assert "weave_on $HOST1 launch-router" "$ID" +# Can't start if already running with different args +assert_raises "weave_on $HOST1 launch-router --log-level=debug" 1 +# Stop then start with different arg +weave_on $HOST1 stop-router +ID2=$(weave_on $HOST1 launch-router --log-level=debug) +assert "[ $ID != $ID2 ]" +# Fails on bad argument +assert_raises "weave_on $HOST1 launch-plugin --foo" 1 +# Fails again on bad argument, although there is a dead container +assert_raises "weave_on $HOST1 launch-plugin --foo" 1 + +end_suite From 436f8ef7c836ca14cec43be4da25b92f063fa9f3 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Feb 2016 17:37:26 +0000 Subject: [PATCH 4/5] Compare the environment so we can detect a change in password --- test/175_stop_start_test.sh | 9 +++++++++ weave | 4 ++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/test/175_stop_start_test.sh b/test/175_stop_start_test.sh index 56df0409..791d7a8c 100755 --- a/test/175_stop_start_test.sh +++ b/test/175_stop_start_test.sh @@ -13,6 +13,15 @@ assert_raises "weave_on $HOST1 launch-router --log-level=debug" 1 weave_on $HOST1 stop-router ID2=$(weave_on $HOST1 launch-router --log-level=debug) assert "[ $ID != $ID2 ]" + +# Stop then start with different password +weave_on $HOST1 stop-router +ID3=$(weave_on $HOST1 launch-router --password=xyzzy) +assert "[ $ID2 != $ID3 ]" +# Stop then start with same password +weave_on $HOST1 stop-router +assert "weave_on $HOST1 launch-router --password=xyzzy" "$ID3" + # Fails on bad argument assert_raises "weave_on $HOST1 launch-plugin --foo" 1 # Fails again on bad argument, although there is a dead container diff --git a/weave b/weave index 3cfd1063..80377906 100755 --- a/weave +++ b/weave @@ -937,10 +937,10 @@ detach() { ###################################################################### args_match() { - prev_args="$(docker inspect -f '{{.Args}}' $1)" + prev_args="$(docker inspect -f '{{.Args}} {{.Config.Env}}' $1)" shift dummy_container=$(docker create "$@" 2>/dev/null) || return 1 - dummy_args="$(docker inspect -f '{{.Args}}' $dummy_container)" + dummy_args="$(docker inspect -f '{{.Args}} {{.Config.Env}}' $dummy_container)" docker rm $dummy_container >/dev/null [ "$prev_args" = "$dummy_args" ] } From f17d50cc50eae6248b75ba601d5ba45ec067b856 Mon Sep 17 00:00:00 2001 From: Bryan Boreham Date: Wed, 10 Feb 2016 17:56:57 +0000 Subject: [PATCH 5/5] Check the exact version of the image, not the base --- weave | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/weave b/weave index 80377906..f899d5fc 100755 --- a/weave +++ b/weave @@ -952,10 +952,12 @@ start_if_necessary() { name=$2 image=$3 shift 3 - RUN_STATUS=$(docker inspect --format='{{.State.Running}} {{.Config.Image}} {{.Id}}' $name 2>/dev/null) || true + image_base=${image%:*} + IMAGE_ID=$(docker inspect --format='{{.Id}}' $image) + RUN_STATUS=$(docker inspect --format='{{.State.Running}} {{.Image}} {{.Config.Image}} {{.Id}}' $name 2>/dev/null) || true CONTAINER_ID=${RUN_STATUS##* } case ${RUN_STATUS%:*} in - "true $image") + "true $IMAGE_ID "*) if args_match $CONTAINER_ID "$@" ; then ALREADY_RUNNING_CONTAINER=$CONTAINER_ID echo $CONTAINER_ID @@ -964,7 +966,7 @@ start_if_necessary() { return 1 fi ;; - "false $image") + "false $IMAGE_ID "*) if args_match $CONTAINER_ID "$@" ; then docker start $CONTAINER_ID else @@ -972,6 +974,10 @@ start_if_necessary() { docker run -d --name=$name "$@" fi ;; + true*$image_base) + echo "Found another version of $name running. Aborting." >&2 + return 2 + ;; true*) echo "Found another running container named '$name'. Aborting." >&2 return 2 @@ -1615,7 +1621,7 @@ launch_router() { # Set WEAVE_DOCKER_ARGS in the environment in order to supply # additional parameters, such as resource limits, to docker # when launching the weave container. - start_if_necessary router $CONTAINER_NAME $BASE_IMAGE \ + start_if_necessary router $CONTAINER_NAME $IMAGE \ --privileged --net=host $(docker_sock_options) \ -e WEAVE_PASSWORD \ -e CHECKPOINT_DISABLE \ @@ -1671,7 +1677,7 @@ launch_proxy() { docker create -v /w -v /w-noop -v /w-nomcast --name=$VOLUMES_CONTAINER_NAME \ --label=weavevolumes --entrypoint=/bin/false $EXEC_IMAGE >/dev/null fi - start_if_necessary proxy $PROXY_CONTAINER_NAME $BASE_EXEC_IMAGE \ + start_if_necessary proxy $PROXY_CONTAINER_NAME $EXEC_IMAGE \ --privileged --net=host $(docker_sock_options) \ $PROXY_VOLUMES \ --volumes-from $VOLUMES_CONTAINER_NAME \ @@ -1694,7 +1700,7 @@ stop_proxy() { } launch_plugin_if_not_running() { - start_if_necessary plugin $PLUGIN_CONTAINER_NAME $BASE_PLUGIN_IMAGE \ + start_if_necessary plugin $PLUGIN_CONTAINER_NAME $PLUGIN_IMAGE \ --privileged --net=host $(docker_sock_options) \ --restart=always \ -v /run/docker/plugins:/run/docker/plugins \