Browse code

Fix daemon start/stop logic in hack/make/* scripts

From the Bash manual's `set -e` description:
(https://www.gnu.org/software/bash/manual/bashref.html#index-set)

> Exit immediately if a pipeline (see Pipelines), which may consist of a
> single simple command (see Simple Commands), a list (see Lists), or a
> compound command (see Compound Commands) returns a non-zero status.
> The shell does not exit if the command that fails is part of the
> command list immediately following a while or until keyword, part of
> the test in an if statement, part of any command executed in a && or
> || list except the command following the final && or ||, any command
> in a pipeline but the last, or if the command’s return status is being
> inverted with !. If a compound command other than a subshell returns a
> non-zero status because a command failed while -e was being ignored,
> the shell does not exit.

Additionally, further down:

> If a compound command or shell function executes in a context where -e
> is being ignored, none of the commands executed within the compound
> command or function body will be affected by the -e setting, even if
> -e is set and a command returns a failure status. If a compound
> command or shell function sets -e while executing in a context where
> -e is ignored, that setting will not have any effect until the
> compound command or the command containing the function call
> completes.

Thus, the only way to have our `.integration-daemon-stop` script
actually run appropriately to clean up our daemon on test/script failure
is to use `trap ... EXIT`, which we traditionally avoid because it does
not have any stacking capabilities, but in this case is a reasonable
compromise because it's going to be the only script using it (for now,
at least; we can evaluate more complex solutions in the future if they
actually become necessary).

The alternatives were much less reasonable. One is to have the entire
complex chains in any script wanting to use `.integration-daemon-start`
/ `.integration-daemon-stop` be chained together with `&&` in an `if`
block, which is untenable. The other I could think of was taking the
body of these scripts out into separate scripts, essentially meaning
we'd need two files for each of these, which further complicates the
maintenance.

Add to that the fact that our `trap ... EXIT` is scoped to the enclosing
subshell (`( ... )`) and we're in even more reasonable territory with
this pattern.

Signed-off-by: Andrew "Tianon" Page <admwiggin@gmail.com>

Tianon Gravi authored on 2015/04/18 07:19:34
Showing 5 changed files
... ...
@@ -25,6 +25,7 @@ if [ -z "$DOCKER_TEST_HOST" ]; then
25 25
 		--pidfile "$DEST/docker.pid" \
26 26
 			&> "$DEST/docker.log"
27 27
 	) &
28
+	trap "source '${MAKEDIR}/.integration-daemon-stop'" EXIT # make sure that if the script exits unexpectedly, we stop this daemon we just started
28 29
 else
29 30
 	export DOCKER_HOST="$DOCKER_TEST_HOST"
30 31
 fi
... ...
@@ -1,5 +1,7 @@
1 1
 #!/bin/bash
2 2
 
3
+trap - EXIT # reset EXIT trap applied in .integration-daemon-start
4
+
3 5
 for pidFile in $(find "$DEST" -name docker.pid); do
4 6
 	pid=$(set -x; cat "$pidFile")
5 7
 	( set -x; kill "$pid" )
... ...
@@ -7,76 +7,64 @@ DEST=$1
7 7
 (
8 8
 	source "${MAKEDIR}/.integration-daemon-start"
9 9
 
10
-	# we need to wrap up everything in between integration-daemon-start and
11
-	# integration-daemon-stop to make sure we kill the daemon and don't hang,
12
-	# even and especially on test failures
13
-	didFail=
14
-	if ! {
15
-		set -e
10
+	# TODO consider using frozen images for the dockercore/builder-deb tags
16 11
 
17
-		# TODO consider using frozen images for the dockercore/builder-deb tags
12
+	debVersion="${VERSION//-/'~'}"
13
+	# if we have a "-dev" suffix or have change in Git, let's make this package version more complex so it works better
14
+	if [[ "$VERSION" == *-dev ]] || [ -n "$(git status --porcelain)" ]; then
15
+		gitUnix="$(git log -1 --pretty='%at')"
16
+		gitDate="$(date --date "@$gitUnix" +'%Y%m%d.%H%M%S')"
17
+		gitCommit="$(git log -1 --pretty='%h')"
18
+		gitVersion="git${gitDate}.0.${gitCommit}"
19
+		# gitVersion is now something like 'git20150128.112847.0.17e840a'
20
+		debVersion="$debVersion~$gitVersion"
18 21
 
19
-		debVersion="${VERSION//-/'~'}"
20
-		# if we have a "-dev" suffix or have change in Git, let's make this package version more complex so it works better
21
-		if [[ "$VERSION" == *-dev ]] || [ -n "$(git status --porcelain)" ]; then
22
-			gitUnix="$(git log -1 --pretty='%at')"
23
-			gitDate="$(date --date "@$gitUnix" +'%Y%m%d.%H%M%S')"
24
-			gitCommit="$(git log -1 --pretty='%h')"
25
-			gitVersion="git${gitDate}.0.${gitCommit}"
26
-			# gitVersion is now something like 'git20150128.112847.0.17e840a'
27
-			debVersion="$debVersion~$gitVersion"
22
+		# $ dpkg --compare-versions 1.5.0 gt 1.5.0~rc1 && echo true || echo false
23
+		# true
24
+		# $ dpkg --compare-versions 1.5.0~rc1 gt 1.5.0~git20150128.112847.17e840a && echo true || echo false
25
+		# true
26
+		# $ dpkg --compare-versions 1.5.0~git20150128.112847.17e840a gt 1.5.0~dev~git20150128.112847.17e840a && echo true || echo false
27
+		# true
28 28
 
29
-			# $ dpkg --compare-versions 1.5.0 gt 1.5.0~rc1 && echo true || echo false
30
-			# true
31
-			# $ dpkg --compare-versions 1.5.0~rc1 gt 1.5.0~git20150128.112847.17e840a && echo true || echo false
32
-			# true
33
-			# $ dpkg --compare-versions 1.5.0~git20150128.112847.17e840a gt 1.5.0~dev~git20150128.112847.17e840a && echo true || echo false
34
-			# true
35
-
36
-			# ie, 1.5.0 > 1.5.0~rc1 > 1.5.0~git20150128.112847.17e840a > 1.5.0~dev~git20150128.112847.17e840a
37
-		fi
29
+		# ie, 1.5.0 > 1.5.0~rc1 > 1.5.0~git20150128.112847.17e840a > 1.5.0~dev~git20150128.112847.17e840a
30
+	fi
38 31
 
39
-		debSource="$(awk -F ': ' '$1 == "Source" { print $2; exit }' hack/make/.build-deb/control)"
40
-		debMaintainer="$(awk -F ': ' '$1 == "Maintainer" { print $2; exit }' hack/make/.build-deb/control)"
41
-		debDate="$(date --rfc-2822)"
32
+	debSource="$(awk -F ': ' '$1 == "Source" { print $2; exit }' hack/make/.build-deb/control)"
33
+	debMaintainer="$(awk -F ': ' '$1 == "Maintainer" { print $2; exit }' hack/make/.build-deb/control)"
34
+	debDate="$(date --rfc-2822)"
42 35
 
43
-		# if go-md2man is available, pre-generate the man pages
44
-		./docs/man/md2man-all.sh -q || true
45
-		# TODO decide if it's worth getting go-md2man in _each_ builder environment to avoid this
36
+	# if go-md2man is available, pre-generate the man pages
37
+	./docs/man/md2man-all.sh -q || true
38
+	# TODO decide if it's worth getting go-md2man in _each_ builder environment to avoid this
46 39
 
47
-		# TODO add a configurable knob for _which_ debs to build so we don't have to modify the file or build all of them every time we need to test
48
-		for dir in contrib/builder/deb/*/; do
49
-			version="$(basename "$dir")"
50
-			suite="${version##*-}"
40
+	# TODO add a configurable knob for _which_ debs to build so we don't have to modify the file or build all of them every time we need to test
41
+	for dir in contrib/builder/deb/*/; do
42
+		version="$(basename "$dir")"
43
+		suite="${version##*-}"
51 44
 
52
-			image="dockercore/builder-deb:$version"
53
-			if ! docker inspect "$image" &> /dev/null; then
54
-				( set -x && docker build -t "$image" "$dir" )
55
-			fi
45
+		image="dockercore/builder-deb:$version"
46
+		if ! docker inspect "$image" &> /dev/null; then
47
+			( set -x && docker build -t "$image" "$dir" )
48
+		fi
56 49
 
57
-			mkdir -p "$DEST/$version"
58
-			cat > "$DEST/$version/Dockerfile.build" <<-EOF
59
-				FROM $image
60
-				WORKDIR /usr/src/docker
61
-				COPY . /usr/src/docker
62
-				RUN ln -sfv hack/make/.build-deb debian
63
-				RUN { echo '$debSource (${debVersion}-0~${suite}) $suite; urgency=low'; echo; echo '  * Version: $VERSION'; echo; echo " -- $debMaintainer  $debDate"; } > debian/changelog && cat >&2 debian/changelog
64
-				RUN dpkg-buildpackage -uc -us
65
-			EOF
66
-			cp -a "$DEST/$version/Dockerfile.build" . # can't use $DEST because it's in .dockerignore...
67
-			tempImage="docker-temp/build-deb:$version"
68
-			( set -x && docker build -t "$tempImage" -f Dockerfile.build . )
69
-			docker run --rm "$tempImage" bash -c 'cd .. && tar -c *_*' | tar -xvC "$DEST/$version"
70
-			docker rmi "$tempImage"
71
-		done
72
-	}; then
73
-		didFail=1
74
-	fi
50
+		mkdir -p "$DEST/$version"
51
+		cat > "$DEST/$version/Dockerfile.build" <<-EOF
52
+			FROM $image
53
+			WORKDIR /usr/src/docker
54
+			COPY . /usr/src/docker
55
+			RUN ln -sfv hack/make/.build-deb debian
56
+			RUN { echo '$debSource (${debVersion}-0~${suite}) $suite; urgency=low'; echo; echo '  * Version: $VERSION'; echo; echo " -- $debMaintainer  $debDate"; } > debian/changelog && cat >&2 debian/changelog
57
+			RUN dpkg-buildpackage -uc -us
58
+		EOF
59
+		cp -a "$DEST/$version/Dockerfile.build" . # can't use $DEST because it's in .dockerignore...
60
+		tempImage="docker-temp/build-deb:$version"
61
+		( set -x && docker build -t "$tempImage" -f Dockerfile.build . )
62
+		docker run --rm "$tempImage" bash -c 'cd .. && tar -c *_*' | tar -xvC "$DEST/$version"
63
+		docker rmi "$tempImage"
64
+	done
75 65
 
76 66
 	# clean up after ourselves
77 67
 	rm -f Dockerfile.build
78 68
 
79 69
 	source "${MAKEDIR}/.integration-daemon-stop"
80
-
81
-	[ -z "$didFail" ] # "set -e" ftw
82
-) 2>&1 | tee -a $DEST/test.log
70
+) 2>&1 | tee -a "$DEST/test.log"
... ...
@@ -7,24 +7,14 @@ DEST=$1
7 7
 (
8 8
 	source "${MAKEDIR}/.integration-daemon-start"
9 9
 
10
-	# we need to wrap up everything in between integration-daemon-start and
11
-	# integration-daemon-stop to make sure we kill the daemon and don't hang,
12
-	# even and especially on test failures
13
-	didFail=
14
-	if ! {
15
-		dockerPy='/docker-py'
16
-		[ -d "$dockerPy" ] || {
17
-			dockerPy="$DEST/docker-py"
18
-			git clone https://github.com/docker/docker-py.git "$dockerPy"
19
-		}
10
+	dockerPy='/docker-py'
11
+	[ -d "$dockerPy" ] || {
12
+		dockerPy="$DEST/docker-py"
13
+		git clone https://github.com/docker/docker-py.git "$dockerPy"
14
+	}
20 15
 
21
-		# exporting PYTHONPATH to import "docker" from our local docker-py
22
-		test_env PYTHONPATH="$dockerPy" python "$dockerPy/tests/integration_test.py"
23
-	}; then
24
-		didFail=1
25
-	fi
16
+	# exporting PYTHONPATH to import "docker" from our local docker-py
17
+	test_env PYTHONPATH="$dockerPy" python "$dockerPy/tests/integration_test.py"
26 18
 
27 19
 	source "${MAKEDIR}/.integration-daemon-stop"
28
-
29
-	[ -z "$didFail" ] # "set -e" ftw
30
-) 2>&1 | tee -a $DEST/test.log
20
+) 2>&1 | tee -a "$DEST/test.log"
... ...
@@ -11,21 +11,11 @@ bundle_test_integration_cli() {
11 11
 (
12 12
 	source "${MAKEDIR}/.integration-daemon-start"
13 13
 
14
-	# we need to wrap up everything in between integration-daemon-start and
15
-	# integration-daemon-stop to make sure we kill the daemon and don't hang,
16
-	# even and especially on test failures
17
-	didFail=
18
-	if ! {
19
-		source "${MAKEDIR}/.ensure-frozen-images"
20
-		source "${MAKEDIR}/.ensure-httpserver"
21
-		source "${MAKEDIR}/.ensure-emptyfs"
14
+	source "${MAKEDIR}/.ensure-frozen-images"
15
+	source "${MAKEDIR}/.ensure-httpserver"
16
+	source "${MAKEDIR}/.ensure-emptyfs"
22 17
 
23
-		bundle_test_integration_cli
24
-	}; then
25
-		didFail=1
26
-	fi
18
+	bundle_test_integration_cli
27 19
 
28 20
 	source "${MAKEDIR}/.integration-daemon-stop"
29
-
30
-	[ -z "$didFail" ] # "set -e" ftw
31 21
 ) 2>&1 | tee -a "$DEST/test.log"