[airflow] branch master updated: Cope with multiple processes get_remote_image_info in parallel (#9105)

[as...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

ash pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/airflow.git


The following commit(s) were added to refs/heads/master by this push:
     new bb67a87  Cope with multiple processes get_remote_image_info in parallel (#9105)
bb67a87 is described below

commit bb67a87fa4a2893976fbdb8f0a95ba50653a15df
Author: Ash Berlin-Taylor <as...@firemirror.com>
AuthorDate: Thu Jun 4 11:05:51 2020 +0100

    Cope with multiple processes get_remote_image_info in parallel (#9105)
    
    When I'd made a change to a large number of python files, running the
    flake8 pre-commit hook would fail without obvious error (as in no error
    was printed, but exit code was 1).
    
    In debugging this I switch the pre-commit to `require_serial: true` and
    the problem went way - the fix for this is:
    
    - Don't redirect stderr to /dev/null (that silences both our VERBOSE
      trace output, and the errors from docker)
    - Use `--cidfile` option to docker to create a random name and write the
      created container ID to a file
---
 scripts/ci/_utils.sh | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/scripts/ci/_utils.sh b/scripts/ci/_utils.sh
index d11d2b3..024b819 100644
--- a/scripts/ci/_utils.sh
+++ b/scripts/ci/_utils.sh
@@ -26,6 +26,7 @@ declare -a EXTRA_DOCKER_PROD_BUILD_FLAGS
 export EXTRA_DOCKER_FLAGS
 export EXTRA_DOCKER_PROD_BUILD_FLAGS
 
+
 # In case "VERBOSE_COMMANDS" is set to "true" set -x is used to enable debugging
 function check_verbose_setup {
     if [[ ${VERBOSE_COMMANDS:="false"} == "true" ]]; then
@@ -39,7 +40,8 @@ function check_verbose_setup {
 # In case "VERBOSE" is set to "true" (--verbose flag in Breeze) all docker commands run will be
 # printed before execution
 function verbose_docker {
-    if [[ ${VERBOSE:="false"} == "true" ]]; then
+    if [[ ${VERBOSE:="false"} == "true" && ${VERBOSE_COMMANDS:=} != "true" ]]; then
+       # do not print echo if VERSBOSE_COMMAND is set (set -x does it already)
         echo "docker" "${@}"
     fi
     docker "${@}"
@@ -50,6 +52,8 @@ function initialize_common_environment {
     # default python Major/Minor version
     PYTHON_MAJOR_MINOR_VERSION=${PYTHON_MAJOR_MINOR_VERSION:="3.6"}
 
+    FILES_TO_CLEANUP_ON_EXIT=()
+
     # Sets to where airflow sources are located
     AIRFLOW_SOURCES=${AIRFLOW_SOURCES:=$(cd "${MY_DIR}/../../" && pwd)}
     export AIRFLOW_SOURCES
@@ -720,25 +724,28 @@ function get_local_image_info() {
 function get_remote_image_info() {
     set +e
     # Pull remote manifest image
-    if ! verbose_docker pull "${AIRFLOW_CI_REMOTE_MANIFEST_IMAGE}" >/dev/null 2>&1 ; then
+    if ! verbose_docker pull "${AIRFLOW_CI_REMOTE_MANIFEST_IMAGE}" >/dev/null; then
         echo
         echo "Remote docker registry unreachable"
         echo
         REMOTE_DOCKER_REGISTRY_UNREACHABLE="true"
         return
     fi
+    set -e
+
+    # Docker needs the file passed to --cidfile to not exist, so we can't use mktemp
+    TMP_CONTAINER_ID="remote-airflow-manifest-$$.container_id"
+    FILES_TO_CLEANUP_ON_EXIT+=("$TMP_CONTAINER_ID")
+
     TMP_MANIFEST_REMOTE_JSON=$(mktemp)
     TMP_MANIFEST_REMOTE_SHA=$(mktemp)
-    # delete container just in case
-    verbose_docker rm --force "remote-airflow-manifest" >/dev/null 2>&1
-    set -e
     # Create container out of the manifest image without running it
-    verbose_docker create --name "remote-airflow-manifest" "${AIRFLOW_CI_REMOTE_MANIFEST_IMAGE}" >/dev/null 2>&1
+    verbose_docker create --cidfile "${TMP_CONTAINER_ID}" "${AIRFLOW_CI_REMOTE_MANIFEST_IMAGE}"
     # Extract manifest and store it in local file
-    verbose_docker cp "remote-airflow-manifest:${AIRFLOW_CI_BASE_TAG}.json" "${TMP_MANIFEST_REMOTE_JSON}" >/dev/null 2>&1
+    verbose_docker cp "$(cat "${TMP_CONTAINER_ID}"):${AIRFLOW_CI_BASE_TAG}.json" "${TMP_MANIFEST_REMOTE_JSON}"
     # Filter everything except SHAs of image layers
     sed 's/ *//g' "${TMP_MANIFEST_REMOTE_JSON}" | grep '^"sha256:' >"${TMP_MANIFEST_REMOTE_SHA}"
-    verbose_docker rm --force "remote-airflow-manifest" >/dev/null 2>&1
+    verbose_docker rm --force "$( cat "${TMP_CONTAINER_ID}")"
 }
 
 # The Number determines the cut-off between local building time and pull + build time.
@@ -904,6 +911,11 @@ function script_end {
     if [[ ${VERBOSE_COMMANDS:="false"} == "true" ]]; then
         set +x
     fi
+
+    if [[ ${#FILES_TO_CLEANUP_ON_EXIT[@]} -gt 0 ]]; then
+      rm -rf -- "${FILES_TO_CLEANUP_ON_EXIT[@]}"
+    fi
+
     END_SCRIPT_TIME=$(date +%s)
     RUN_SCRIPT_TIME=$((END_SCRIPT_TIME-START_SCRIPT_TIME))
     if [[ ${BREEZE:=} != "true" ]]; then