Skip to content

Commit 9e0ccde

Browse files
ashbashb
authored
Produce less verbose output when building docker mount options (#9103)
The previous method of generating this list had two "problems"/niggles that this PR solves, when running with VERBOSE=true - Firstly, LOCAL_MOUNTS was set at the top level, so running with `set -x` produced 30 extra lines of output. - Because of the `while read` used, it created 4 or 5 lines _per_ mount, resulting in a lot verbose output. Nothing I've changed here is "critical", it's just making it a bit easier to see with the debug output what is going on, by running fewer commands. I have also expanded the BATS test a little bit to check each pair (`-v` and its following option)
1 parent 87a4a0a commit 9e0ccde

File tree

3 files changed

+64
-78
lines changed

3 files changed

+64
-78
lines changed

‎scripts/ci/_utils.sh

Lines changed: 42 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -145,11 +145,7 @@ function initialize_common_environment {
145145
print_info "Mounting necessary host volumes to Docker"
146146
print_info
147147

148-
EXTRA_DOCKER_FLAGS=()
149-
150-
while IFS= read -r LINE; do
151-
EXTRA_DOCKER_FLAGS+=( "${LINE}")
152-
done < <(convert_local_mounts_to_docker_params)
148+
read -r -a EXTRA_DOCKER_FLAGS <<< "$(convert_local_mounts_to_docker_params)"
153149
else
154150
print_info
155151
print_info "Skip mounting host volumes to Docker"
@@ -244,53 +240,52 @@ function print_info() {
244240
# By default not the whole airflow sources directory is mounted because there are often
245241
# artifacts created there (for example .egg-info files) that are breaking the capability
246242
# of running different python versions in Breeze. So we only mount what is needed by default.
247-
LOCAL_MOUNTS="
248-
.bash_aliases /root/
249-
.bash_history /root/
250-
.coveragerc /opt/airflow/
251-
.dockerignore /opt/airflow/
252-
.flake8 /opt/airflow/
253-
.github /opt/airflow/
254-
.inputrc /root/
255-
.kube /root/
256-
.rat-excludes /opt/airflow/
257-
CHANGELOG.txt /opt/airflow/
258-
Dockerfile.ci /opt/airflow/
259-
LICENSE /opt/airflow/
260-
MANIFEST.in /opt/airflow/
261-
NOTICE /opt/airflow/
262-
airflow /opt/airflow/
263-
backport_packages /opt/airflow/
264-
common /opt/airflow/
265-
dags /opt/airflow/
266-
dev /opt/airflow/
267-
docs /opt/airflow/
268-
files /
269-
dist /
270-
hooks /opt/airflow/
271-
logs /root/airflow/
272-
pylintrc /opt/airflow/
273-
pytest.ini /opt/airflow/
274-
requirements /opt/airflow/
275-
scripts /opt/airflow/
276-
scripts/ci/in_container/entrypoint_ci.sh /
277-
setup.cfg /opt/airflow/
278-
setup.py /opt/airflow/
279-
tests /opt/airflow/
280-
tmp /opt/airflow/
281-
"
243+
function generate_local_mounts_list {
244+
local prefix="$1"
245+
LOCAL_MOUNTS=(
246+
"$prefix".bash_aliases:/root/.bash_aliases:cached
247+
"$prefix".bash_history:/root/.bash_history:cached
248+
"$prefix".coveragerc:/opt/airflow/.coveragerc:cached
249+
"$prefix".dockerignore:/opt/airflow/.dockerignore:cached
250+
"$prefix".flake8:/opt/airflow/.flake8:cached
251+
"$prefix".github:/opt/airflow/.github:cached
252+
"$prefix".inputrc:/root/.inputrc:cached
253+
"$prefix".kube:/root/.kube:cached
254+
"$prefix".rat-excludes:/opt/airflow/.rat-excludes:cached
255+
"$prefix"CHANGELOG.txt:/opt/airflow/CHANGELOG.txt:cached
256+
"$prefix"Dockerfile.ci:/opt/airflow/Dockerfile.ci:cached
257+
"$prefix"LICENSE:/opt/airflow/LICENSE:cached
258+
"$prefix"MANIFEST.in:/opt/airflow/MANIFEST.in:cached
259+
"$prefix"NOTICE:/opt/airflow/NOTICE:cached
260+
"$prefix"airflow:/opt/airflow/airflow:cached
261+
"$prefix"backport_packages:/opt/airflow/backport_packages:cached
262+
"$prefix"common:/opt/airflow/common:cached
263+
"$prefix"dags:/opt/airflow/dags:cached
264+
"$prefix"dev:/opt/airflow/dev:cached
265+
"$prefix"docs:/opt/airflow/docs:cached
266+
"$prefix"files:/files:cached
267+
"$prefix"dist:/dist:cached
268+
"$prefix"hooks:/opt/airflow/hooks:cached
269+
"$prefix"logs:/root/airflow/logs:cached
270+
"$prefix"pylintrc:/opt/airflow/pylintrc:cached
271+
"$prefix"pytest.ini:/opt/airflow/pytest.ini:cached
272+
"$prefix"requirements:/opt/airflow/requirements:cached
273+
"$prefix"scripts:/opt/airflow/scripts:cached
274+
"$prefix"scripts/ci/in_container/entrypoint_ci.sh:/entrypoint_ci.sh:cached
275+
"$prefix"setup.cfg:/opt/airflow/setup.cfg:cached
276+
"$prefix"setup.py:/opt/airflow/setup.py:cached
277+
"$prefix"tests:/opt/airflow/tests:cached
278+
"$prefix"tmp:/opt/airflow/tmp:cached
279+
)
280+
}
282281

283282
# Converts the local mounts that we defined above to the right set of -v
284283
# volume mappings in docker-compose file. This is needed so that we only
285284
# maintain the volumes in one place (above)
286285
function convert_local_mounts_to_docker_params() {
287-
echo "${LOCAL_MOUNTS}" |sed '/^$/d' | awk -v AIRFLOW_SOURCES="${AIRFLOW_SOURCES}" \
288-
'
289-
function basename(file) {
290-
sub(".*/", "", file)
291-
return file
292-
}
293-
{ print "-v"; print AIRFLOW_SOURCES "/" $1 ":" $2 basename($1) ":cached" }'
286+
generate_local_mounts_list "${AIRFLOW_SOURCES}/"
287+
# Bash can't "return" arrays, so we need to quote any special characters
288+
printf -- '-v %q ' "${LOCAL_MOUNTS[@]}"
294289
}
295290

296291
# Fixes a file that is expected to be a file. If - for whatever reason - the local file is not created

‎scripts/ci/pre_commit_local_yml_mounts.sh

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -23,29 +23,21 @@ MY_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
2323
# shellcheck source=scripts/ci/_script_init.sh
2424
. "$( dirname "${BASH_SOURCE[0]}" )/_script_init.sh"
2525

26-
TMP_FILE=$(mktemp)
2726
TMP_OUTPUT=$(mktemp)
2827

28+
# Remove temp file if it's hanging around
29+
trap 'rm -rf -- "${TMP_OUTPUT}" 2>/dev/null' EXIT
30+
2931
LOCAL_YML_FILE="${MY_DIR}/docker-compose/local.yml"
3032

3133
LEAD=' # START automatically generated volumes from LOCAL_MOUNTS in _utils.sh'
3234
TAIL=' # END automatically generated volumes from LOCAL_MOUNTS in _utils.sh'
3335

34-
echo "${LOCAL_MOUNTS}" |sed '/^$/d' | \
35-
awk '
36-
function basename(file) {
37-
sub(".*/", "", file)
38-
return file
39-
}
40-
{ print " - ../../../" $1 ":" $2 basename($1) ":cached"}
41-
' > "${TMP_FILE}"
42-
43-
44-
BEGIN_GEN=$(grep -n "${LEAD}" <"${LOCAL_YML_FILE}" | sed 's/\(.*\):.*/\1/g')
45-
END_GEN=$(grep -n "${TAIL}" <"${LOCAL_YML_FILE}" | sed 's/\(.*\):.*/\1/g')
46-
cat <(head -n "${BEGIN_GEN}" "${LOCAL_YML_FILE}") \
47-
"${TMP_FILE}" \
48-
<(tail -n +"${END_GEN}" "${LOCAL_YML_FILE}") \
49-
>"${TMP_OUTPUT}"
36+
generate_local_mounts_list " - ../../../"
37+
38+
sed -ne "0,/$LEAD/ p" "${LOCAL_YML_FILE}" > "${TMP_OUTPUT}"
39+
40+
printf '%s\n' "${LOCAL_MOUNTS[@]}" >> "${TMP_OUTPUT}"
41+
sed -ne "/$TAIL/,\$ p" "${LOCAL_YML_FILE}" >> "${TMP_OUTPUT}"
5042

5143
mv "${TMP_OUTPUT}" "${LOCAL_YML_FILE}"

‎tests/bats/test_local_mounts.bats

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -23,19 +23,18 @@
2323

2424
initialize_common_environment
2525

26-
run convert_local_mounts_to_docker_params
26+
read -r -a RES <<< "$(convert_local_mounts_to_docker_params)"
2727

28-
RES="${output}"
29-
COUNT_LINES=$(grep -c '' <(echo "${RES}"))
30-
COUNT_MINUS_V_LINES=$(grep -c '^-v$' <(echo "${RES}"))
31-
if (( COUNT_LINES != 2*COUNT_MINUS_V_LINES )); then
32-
echo "The output produced by the convert_local_mounts_to_docker_params function is wrong"
33-
echo "There are ${COUNT_LINES} lines in total and ${COUNT_MINUS_V_LINES} lines with -v"
34-
echo "They seconf number should be exactly 1/2 of the first."
35-
echo
36-
echo "The output below should contain interleaving -v / volume lines"
37-
echo
38-
echo "${RES}"
39-
exit 1
40-
fi
28+
[[ ${#RES[@]} -gt 0 ]] # Array should be non-zero length
29+
[[ $((${#RES[@]} % 2)) == 0 ]] # Array should be even length
30+
31+
for i in "${!RES[@]}"; do
32+
if [[ $((i % 2)) == 0 ]]; then
33+
# Every other value should be `-v`
34+
[[ ${RES[$i]} == "-v" ]]
35+
else
36+
# And the options should be of form <src>:<dest>:cached
37+
[[ ${RES[$i]} = *:*:cached ]]
38+
fi
39+
done
4140
}

0 commit comments

Comments
 (0)