Skip to content

Commit 1bf8414

Browse files
jtattermuschjtattermusch
authored
build_and_run_docker.sh cleanup and simplification (grpc#29468)
* unify DOCKER_TTY_ARGS in docker scripts * improvements and cleanup in build_and_run_docker.sh * fix shellcheck * make sure python sdist artifact is readable
1 parent 65f510c commit 1bf8414

File tree

6 files changed

+169
-57
lines changed

6 files changed

+169
-57
lines changed

‎tools/internal_ci/linux/grpc_bloat_diff.sh

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,4 @@ source tools/internal_ci/helper_scripts/prepare_build_linux_rc
2222

2323
export DOCKERFILE_DIR=tools/dockerfile/test/cxx_debian11_x64
2424
export DOCKER_RUN_SCRIPT=tools/internal_ci/linux/grpc_bloat_diff_in_docker.sh
25-
# The check_on_pr.py needs access to the key to post status on github PRs,
26-
# so we mount the keystore dir to the docker container.
27-
export EXTRA_DOCKER_ARGS="-v ${KOKORO_KEYSTORE_DIR}:/kokoro_keystore -e KOKORO_KEYSTORE_DIR=/kokoro_keystore"
2825
exec tools/run_tests/dockerize/build_and_run_docker.sh

‎tools/internal_ci/linux/grpc_memory_diff.sh

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,4 @@ source tools/internal_ci/helper_scripts/prepare_build_linux_rc
2222

2323
export DOCKERFILE_DIR=tools/dockerfile/test/cxx_debian11_x64
2424
export DOCKER_RUN_SCRIPT=tools/internal_ci/linux/grpc_memory_diff_in_docker.sh
25-
# The check_on_pr.py needs access to the key to post status on github PRs,
26-
# so we mount the keystore dir to the docker container.
27-
export EXTRA_DOCKER_ARGS="-v ${KOKORO_KEYSTORE_DIR}:/kokoro_keystore -e KOKORO_KEYSTORE_DIR=/kokoro_keystore"
2825
exec tools/run_tests/dockerize/build_and_run_docker.sh

‎tools/run_tests/artifacts/build_artifact_python.sh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ tar xzf "${GRPCIO_TAR_GZ}" -C "${GRPCIO_STRIP_TEMPDIR}"
117117
clean_non_source_files "${dir}" || true
118118
done
119119
tar czf "${GRPCIO_STRIPPED_TAR_GZ}" -- *
120+
chmod ugo+r "${GRPCIO_STRIPPED_TAR_GZ}"
120121
)
121122
mv "${GRPCIO_STRIPPED_TAR_GZ}" "${GRPCIO_TAR_GZ}"
122123

‎tools/run_tests/dockerize/build_and_run_docker.sh

Lines changed: 157 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -26,10 +26,11 @@ cd -
2626

2727
# Inputs
2828
# DOCKERFILE_DIR - Directory in which Dockerfile file is located.
29-
# DOCKER_RUN_SCRIPT - Script to run under docker (relative to grpc repo root)
30-
# OUTPUT_DIR - Directory (relatively to git repo root) that will be copied from inside docker container after finishing.
31-
# DOCKERHUB_ORGANIZATION - If set, pull a prebuilt image from given dockerhub org.
32-
# $@ - Extra args to pass to the "docker run" command.
29+
# OUTPUT_DIR - (optional) Directory under the git repo root that will be copied from inside docker container after finishing.
30+
# DOCKERHUB_ORGANIZATION - (optional) If set, pull a prebuilt image from given dockerhub org (instead of with "docker build" locally)
31+
# DOCKER_RUN_SCRIPT - (optional) Script to run under docker (relative to grpc repo root). If specified, the cmdline args
32+
# passed on the commadline (a.k.a. $@) will be interpreted as extra args to pass to the "docker run" command.
33+
# If DOCKER_RUN_SCRIPT is not set, $@ will be interpreted as the command and args to run under the docker container.
3334

3435
# Use image name based on Dockerfile location checksum
3536
DOCKER_IMAGE_NAME=$(basename "$DOCKERFILE_DIR"):$(sha1sum "$DOCKERFILE_DIR/Dockerfile" | cut -f1 -d\ )
@@ -43,45 +44,175 @@ else
4344
docker build -t "$DOCKER_IMAGE_NAME" "$DOCKERFILE_DIR"
4445
fi
4546

47+
# If TTY is available, the running container can be conveniently terminated with Ctrl+C.
4648
if [[ -t 0 ]]; then
47-
DOCKER_TTY_ARGS="-it"
49+
DOCKER_TTY_ARGS=("-it")
4850
else
4951
# The input device on kokoro is not a TTY, so -it does not work.
50-
DOCKER_TTY_ARGS=
52+
DOCKER_TTY_ARGS=()
5153
fi
5254

55+
if [ "${DOCKER_RUN_SCRIPT}" != "" ]
56+
then
57+
# Execute a well-known script inside the docker container.
58+
# The script will act as a "wrapper" for the actual test command we want to run.
59+
# TODO(jtattermusch): is the -l flag still necessary?
60+
DOCKER_CMD_AND_ARGS=( bash -l "/var/local/jenkins/grpc/${DOCKER_RUN_SCRIPT}" )
61+
DOCKER_RUN_SCRIPT_ARGS=(
62+
# propagate the variable with command to be run by the DOCKER_RUN_SCRIPT
63+
"-e=DOCKER_RUN_SCRIPT_COMMAND=${DOCKER_RUN_SCRIPT_COMMAND}"
64+
# Interpret any cmdline args as extra docker args.
65+
# TODO(jtattermusch): remove this hack once there are no places where build_and_run_docker.sh is invoked
66+
# with args.
67+
"$@"
68+
)
69+
else
70+
# Interpret any cmdline args as the command to be run inside the docker container.
71+
DOCKER_CMD_AND_ARGS=( "$@" )
72+
DOCKER_RUN_SCRIPT_ARGS=(
73+
# TODO(jtattermusch): remove the hack
74+
"-w=/var/local/jenkins/grpc"
75+
)
76+
fi
77+
78+
# If available, make KOKORO_KEYSTORE_DIR accessible from the container (as readonly)
79+
if [ "${KOKORO_KEYSTORE_DIR}" != "" ]
80+
then
81+
MOUNT_KEYSTORE_DIR_ARGS=(
82+
"-v=${KOKORO_KEYSTORE_DIR}:/kokoro_keystore:ro"
83+
"-e=KOKORO_KEYSTORE_DIR=/kokoro_keystore"
84+
)
85+
else
86+
MOUNT_KEYSTORE_DIR_ARGS=()
87+
fi
88+
89+
# If available, make KOKORO_GFILE_DIR accessible from the container (as readonly)
90+
if [ "${KOKORO_GFILE_DIR}" != "" ]
91+
then
92+
MOUNT_GFILE_DIR_ARGS=(
93+
"-v=${KOKORO_GFILE_DIR}:/kokoro_gfile:ro"
94+
"-e=KOKORO_GFILE_DIR=/kokoro_gfile"
95+
)
96+
else
97+
MOUNT_GFILE_DIR_ARGS=()
98+
fi
99+
100+
# If available, make KOKORO_ARTIFACTS_DIR accessible from the container
101+
if [ "${KOKORO_ARTIFACTS_DIR}" != "" ]
102+
then
103+
MOUNT_ARTIFACTS_DIR_ARGS=(
104+
"-v=${KOKORO_ARTIFACTS_DIR}:/kokoro_artifacts"
105+
"-e=KOKORO_ARTIFACTS_DIR=/kokoro_artifacts"
106+
)
107+
else
108+
MOUNT_ARTIFACTS_DIR_ARGS=()
109+
fi
110+
111+
# args required to be able to run gdb/strace under the docker container
112+
DOCKER_PRIVILEGED_ARGS=(
113+
# TODO(jtattermusch): document why exactly is this option needed.
114+
"--cap-add=SYS_PTRACE"
115+
)
116+
117+
# propagate the well known variables to the docker container
118+
DOCKER_PROPAGATE_ENV_ARGS=(
119+
"--env-file=tools/run_tests/dockerize/docker_propagate_env.list"
120+
)
121+
122+
DOCKER_CLEANUP_ARGS=(
123+
# delete the container when the containers exits
124+
# (otherwise the container will not release the disk space it used)
125+
"--rm=true"
126+
)
127+
128+
DOCKER_NETWORK_ARGS=(
129+
# enable IPv6
130+
"--sysctl=net.ipv6.conf.all.disable_ipv6=0"
131+
)
132+
133+
DOCKER_IMAGE_IDENTITY_ARGS=(
134+
# make the info about docker image used available from inside the docker container
135+
"-e=GRPC_TEST_DOCKER_IMAGE_IDENTITY=${DOCKER_IMAGE_NAME}"
136+
)
137+
138+
# TODO: silence complaint about lack of quotes in some other way
139+
# shellcheck disable=SC2206
140+
DOCKER_EXTRA_ARGS_FROM_ENV=(
141+
# Not quoting the variable is intentional, since the env variable may contain
142+
# multiple arguments and we want to interpret it as such.
143+
# TODO: get rid of EXTRA_DOCKER_ARGS occurrences and replace with DOCKER_EXTRA_ARGS
144+
${EXTRA_DOCKER_ARGS}
145+
${DOCKER_EXTRA_ARGS}
146+
)
147+
53148
# Git root as seen by the docker instance
54149
# TODO(jtattermusch): rename to a more descriptive directory name
55150
# currently that's nontrivial because the name is hardcoded in many places.
56151
EXTERNAL_GIT_ROOT=/var/local/jenkins/grpc
57152

153+
MOUNT_GIT_ROOT_ARGS=(
154+
"-v=${git_root}:${EXTERNAL_GIT_ROOT}"
155+
"-e=EXTERNAL_GIT_ROOT=${EXTERNAL_GIT_ROOT}"
156+
)
157+
58158
# temporary directory that will be mounted to the docker container
59-
# as a way to persist output files.
60-
# use unique name for the output directory to prevent clash between concurrent
61-
# runs of multiple docker containers
159+
# as a way to persist report files.
62160
TEMP_REPORT_DIR="$(mktemp -d)"
63-
TEMP_OUTPUT_DIR="$(mktemp -d)"
161+
# make sure the "reports" dir exists and is owned by current user.
162+
mkdir -p "${TEMP_REPORT_DIR}/reports"
163+
mkdir -p "${git_root}/reports"
164+
165+
MOUNT_REPORT_DIR_ARGS=(
166+
# mount the temporary directory as the "report dir".
167+
"-v=${TEMP_REPORT_DIR}:/var/local/report_dir"
168+
# make the reports/ directory show up under the mounted git root
169+
"-v=${TEMP_REPORT_DIR}/reports:${EXTERNAL_GIT_ROOT}/reports"
170+
# set GRPC_TEST_REPORT_BASE_DIR to make sure that when XML test reports
171+
# are generated, they will be stored in the report dir.
172+
"-e=GRPC_TEST_REPORT_BASE_DIR=/var/local/report_dir"
173+
)
174+
175+
if [ "${OUTPUT_DIR}" != "" ]
176+
then
177+
# temporary directory that will be mounted to the docker container
178+
# as a way to persist output files.
179+
# use unique name for the output directory to prevent clash between concurrent
180+
# runs of multiple docker containers
181+
TEMP_OUTPUT_DIR="$(mktemp -d)"
182+
183+
# make sure the "${OUTPUT_DIR}" dir exists and is owned by current user.
184+
mkdir -p "${TEMP_OUTPUT_DIR}/${OUTPUT_DIR}"
185+
mkdir -p "${git_root}/${OUTPUT_DIR}"
186+
187+
MOUNT_OUTPUT_DIR_ARGS=(
188+
# the OUTPUT_DIR refers to a subdirectory of the git root.
189+
"-v=${TEMP_OUTPUT_DIR}/${OUTPUT_DIR}:${EXTERNAL_GIT_ROOT}/${OUTPUT_DIR}"
190+
"-e=OUTPUT_DIR=${OUTPUT_DIR}"
191+
)
192+
else
193+
MOUNT_OUTPUT_DIR_ARGS=()
194+
fi
64195

65196
# Run tests inside docker
66197
DOCKER_EXIT_CODE=0
67-
# TODO: silence complaint about $DOCKER_TTY_ARGS expansion in some other way
68-
# shellcheck disable=SC2086,SC2154
198+
69199
docker run \
70-
"$@" \
71-
${DOCKER_TTY_ARGS} \
72-
${EXTRA_DOCKER_ARGS} \
73-
--cap-add SYS_PTRACE \
74-
-e "DOCKER_RUN_SCRIPT_COMMAND=${DOCKER_RUN_SCRIPT_COMMAND}" \
75-
-e "EXTERNAL_GIT_ROOT=${EXTERNAL_GIT_ROOT}" \
76-
-e "OUTPUT_DIR=${OUTPUT_DIR}" \
77-
--env-file tools/run_tests/dockerize/docker_propagate_env.list \
78-
--rm \
79-
--sysctl net.ipv6.conf.all.disable_ipv6=0 \
80-
-v "${git_root}:${EXTERNAL_GIT_ROOT}" \
81-
-v "${TEMP_REPORT_DIR}:/var/local/report_dir" \
82-
-v "${TEMP_OUTPUT_DIR}:/var/local/output_dir" \
200+
"${DOCKER_TTY_ARGS[@]}" \
201+
"${DOCKER_RUN_SCRIPT_ARGS[@]}" \
202+
"${MOUNT_KEYSTORE_DIR_ARGS[@]}" \
203+
"${MOUNT_GFILE_DIR_ARGS[@]}" \
204+
"${MOUNT_ARTIFACTS_DIR_ARGS[@]}" \
205+
"${DOCKER_PRIVILEGED_ARGS[@]}" \
206+
"${DOCKER_PROPAGATE_ENV_ARGS[@]}" \
207+
"${DOCKER_CLEANUP_ARGS[@]}" \
208+
"${DOCKER_NETWORK_ARGS[@]}" \
209+
"${DOCKER_IMAGE_IDENTITY_ARGS[@]}" \
210+
"${MOUNT_GIT_ROOT_ARGS[@]}" \
211+
"${MOUNT_REPORT_DIR_ARGS[@]}" \
212+
"${MOUNT_OUTPUT_DIR_ARGS[@]}" \
213+
"${DOCKER_EXTRA_ARGS_FROM_ENV[@]}" \
83214
"${DOCKER_IMAGE_NAME}" \
84-
bash -l "/var/local/jenkins/grpc/${DOCKER_RUN_SCRIPT}" || DOCKER_EXIT_CODE=$?
215+
"${DOCKER_CMD_AND_ARGS[@]}" || DOCKER_EXIT_CODE=$?
85216

86217
# Copy reports stored by the container (if any)
87218
if [ "${GRPC_TEST_REPORT_BASE_DIR}" != "" ]
@@ -95,8 +226,6 @@ fi
95226
# Copy contents of OUTPUT_DIR back under the git repo root
96227
if [ "${OUTPUT_DIR}" != "" ]
97228
then
98-
# create the directory if it doesn't exist yet.
99-
mkdir -p "${TEMP_OUTPUT_DIR}/${OUTPUT_DIR}"
100229
cp -r "${TEMP_OUTPUT_DIR}/${OUTPUT_DIR}" "${git_root}" || DOCKER_EXIT_CODE=$?
101230
fi
102231

‎tools/run_tests/dockerize/build_interop_image.sh

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,11 +97,12 @@ else
9797
docker build -t "$BASE_IMAGE" --force-rm=true "tools/dockerfile/interoptest/$BASE_NAME" || exit $?
9898
fi
9999

100+
# If TTY is available, the running container can be conveniently terminated with Ctrl+C.
100101
if [[ -t 0 ]]; then
101-
DOCKER_TTY_ARGS="-it"
102+
DOCKER_TTY_ARGS=("-it")
102103
else
103104
# The input device on kokoro is not a TTY, so -it does not work.
104-
DOCKER_TTY_ARGS=
105+
DOCKER_TTY_ARGS=()
105106
fi
106107

107108
CONTAINER_NAME="build_${BASE_NAME}_$(uuidgen)"
@@ -114,7 +115,7 @@ CONTAINER_NAME="build_${BASE_NAME}_$(uuidgen)"
114115
(docker run \
115116
--cap-add SYS_PTRACE \
116117
--env-file "tools/run_tests/dockerize/docker_propagate_env.list" \
117-
$DOCKER_TTY_ARGS \
118+
"${DOCKER_TTY_ARGS[@]}" \
118119
$MOUNT_ARGS \
119120
$BUILD_INTEROP_DOCKER_EXTRA_ARGS \
120121
--name="$CONTAINER_NAME" \

‎tools/run_tests/dockerize/docker_run.sh

Lines changed: 7 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -34,30 +34,17 @@ fi
3434

3535
cd /var/local/git/grpc
3636

37-
# ensure the "reports" directory exists
38-
mkdir -p reports
39-
40-
exit_code=0
41-
${DOCKER_RUN_SCRIPT_COMMAND} || exit_code=$?
42-
43-
# copy reports/ dir and files matching one of the patterns to the well-known
44-
# location of report dir mounted to the docker container.
45-
# --parents preserves the directory structure for files matched by find.
46-
cp -r reports/ /var/local/report_dir
47-
find . -name report.xml -exec cp --parents {} /var/local/report_dir \;
48-
find . -name sponge_log.xml -exec cp --parents {} /var/local/report_dir \;
49-
find . -name 'report_*.xml' -exec cp --parents {} /var/local/report_dir \;
50-
chmod -R ugo+r /var/local/report_dir || true
51-
52-
# Move contents of OUTPUT_DIR from under the workspace to a directory that will be visible to the docker host.
37+
# whatever is written to the reports/ dir will be made available outside of the docker container.
38+
ln -s "${EXTERNAL_GIT_ROOT}/reports" reports
39+
# if OUTPUT_DIR is specified, whatever is written to ./${OUTPUT_DIR}/ will be made available outside of the docker container.
5340
if [ "${OUTPUT_DIR}" != "" ]
5441
then
55-
# create the directory if it doesn't exist yet.
56-
mkdir -p "${OUTPUT_DIR}"
57-
mv "${OUTPUT_DIR}" /var/local/output_dir || exit_code=$?
58-
chmod -R ugo+r /var/local/output_dir || true
42+
ln -s "${EXTERNAL_GIT_ROOT}/${OUTPUT_DIR}" "${OUTPUT_DIR}"
5943
fi
6044

45+
exit_code=0
46+
${DOCKER_RUN_SCRIPT_COMMAND} || exit_code=$?
47+
6148
if [ -x "$(command -v ccache)" ]
6249
then
6350
ccache --show-stats || true

0 commit comments

Comments
 (0)