You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2020/08/19 09:15:05 UTC

[GitHub] [airflow] feluelle commented on a change in pull request #10368: CI Images are now pre-build and stored in registry

feluelle commented on a change in pull request #10368:
URL: https://github.com/apache/airflow/pull/10368#discussion_r472848834



##########
File path: scripts/ci/libraries/_push_pull_remove_images.sh
##########
@@ -33,41 +33,43 @@ function pull_image_if_needed() {
         echo
         echo "Pulling the image ${IMAGE_TO_PULL}"
         echo
-        verbose_docker pull "${IMAGE_TO_PULL}" | tee -a "${OUTPUT_LOG}"
+        # need eto be explicitly verbose in order to have pre-commit spinner working

Review comment:
       ```suggestion
           # need to be explicitly verbose in order to have pre-commit spinner working
   ```

##########
File path: scripts/ci/in_container/_in_container_utils.sh
##########
@@ -39,17 +39,19 @@ function in_container_script_end() {
     #shellcheck disable=2181
     EXIT_CODE=$?
     if [[ ${EXIT_CODE} != 0 ]]; then
-        if [[ -n ${OUT_FILE:=} ]]; then
-            echo "  ERROR ENCOUNTERED!"
-            echo
-            echo "  Output:"
-            echo
-            cat "${OUT_FILE}"
+        if [[ "${PRINT_INFO_FROM_SCRIPTS=="ture"}" == "true" ]] ;then

Review comment:
       ```suggestion
           if [[ "${PRINT_INFO_FROM_SCRIPTS=="true"}" == "true" ]] ;then
   ```

##########
File path: scripts/ci/libraries/_push_pull_remove_images.sh
##########
@@ -103,75 +111,122 @@ function pull_prod_images_if_needed() {
             echo
             echo "Force pull base image ${PYTHON_BASE_IMAGE}"
             echo
-            if [[ ${PULL_PYTHON_BASE_IMAGES_FROM_CACHE:="true"} == "true" ]]; then
-                pull_image_possibly_from_cache "${PYTHON_BASE_IMAGE}" "${CACHED_PYTHON_BASE_IMAGE}"
+            if [[ ${USE_GITHUB_REGISTRY:="false"} == "true" ]]; then
+                PYTHON_TAG_SUFFIX=""
+                if [[ ${GITHUB_REGISTRY_PULL_IMAGE_TAG} != "latest" ]]; then
+                    PYTHON_TAG_SUFFIX="-${GITHUB_REGISTRY_PULL_IMAGE_TAG}"
+                fi
+                pull_image_github_dockerhub "${PYTHON_BASE_IMAGE}" "${GITHUB_REGISTRY_PYTHON_BASE_IMAGE}${PYTHON_TAG_SUFFIX}"
             else
-                verbose_docker pull "${PYTHON_BASE_IMAGE}" | tee -a "${OUTPUT_LOG}"
+                # need eto be explicitly verbose in order to have pre-commit spinner working
+                # No aliases in pre-commit non-interactive shells :(
+                verbose_docker pull "${PYTHON_BASE_IMAGE}"
             fi
             echo
         fi
         # "Build" segment of production image
-        pull_image_possibly_from_cache "${AIRFLOW_PROD_BUILD_IMAGE}" "${CACHED_AIRFLOW_PROD_BUILD_IMAGE}"
-        # we never pull the main segment of production image - we always build it locally = this is
-        # usually very fast this way and it is much nicer for rebuilds and development
+        pull_image_github_dockerhub "${AIRFLOW_PROD_BUILD_IMAGE}" "${GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE}:${GITHUB_REGISTRY_PULL_IMAGE_TAG}"
+        # "Main" segment of production image
+        pull_image_github_dockerhub "${AIRFLOW_PROD_IMAGE}" "${GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE}:${GITHUB_REGISTRY_PULL_IMAGE_TAG}"
     fi
 }
 
-# Pushes Ci image and it's manifest to the registry. In case the image was taken from cache registry
-# it is pushed to the cache, not to the main registry. Manifest is only pushed to the main registry
-function push_ci_image() {
-    if [[ ${CACHED_AIRFLOW_CI_IMAGE:=} != "" ]]; then
-        verbose_docker tag "${AIRFLOW_CI_IMAGE}" "${CACHED_AIRFLOW_CI_IMAGE}"
-        IMAGE_TO_PUSH="${CACHED_AIRFLOW_CI_IMAGE}"
-    else
-        IMAGE_TO_PUSH="${AIRFLOW_CI_IMAGE}"
+# Pushes Ci images and the manifest to the registry in DockerHub.
+function push_ci_images_to_dockerhub() {
+    docker push "${AIRFLOW_CI_IMAGE}"
+    docker tag "${AIRFLOW_CI_LOCAL_MANIFEST_IMAGE}" "${AIRFLOW_CI_REMOTE_MANIFEST_IMAGE}"
+    docker push "${AIRFLOW_CI_REMOTE_MANIFEST_IMAGE}"
+    if [[ -n ${DEFAULT_CI_IMAGE:=""} ]]; then
+        # Only push default image to DockerHub registry if it is defined and

Review comment:
       ```suggestion
           # Only push default image to DockerHub registry if it is defined
   ```

##########
File path: scripts/ci/libraries/_verbosity.sh
##########
@@ -28,58 +27,87 @@ 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" && ${VERBOSE_COMMANDS:=} != "true" ]]; then
-       # do not print echo if VERBOSE_COMMAND is set (set -x does it already)
-        echo "docker" "${@}"
+    if [[ ${VERBOSE:="false"} == "true" && \
+        # do not print echo if VERBOSE_COMMAND is set (set -x does it already)
+        ${VERBOSE_COMMANDS:=} != "true" && \
+        # And when generally printing info is disabled
+        ${PRINT_INFO_FROM_SCRIPTS} == "true" ]]; then
+        >&2 echo "docker" "${@}"
+    fi
+    if [[ ${PRINT_INFO_FROM_SCRIPTS} == "false" ]]; then
+        docker "${@}" >>"${OUTPUT_LOG}" 2>&1
+    else
+        docker "${@}" 2>&1 | tee -a "${OUTPUT_LOG}"
+    fi
+    EXIT_CODE="$?"
+    if [[ ${EXIT_CODE} == "0" ]]; then
+        # No matter if "set -e" is used the log will be removed on success.
+        # This way in the output log we only see the most recent failed command and what was echoed before
+        rm -f "${OUTPUT_LOG}"
     fi
-    docker "${@}"
+    return "${EXIT_CODE}"
 }
 
 # In case "VERBOSE" is set to "true" (--verbose flag in Breeze) all helm commands run will be
 # printed before execution
 function verbose_helm {
     if [[ ${VERBOSE:="false"} == "true" && ${VERBOSE_COMMANDS:=} != "true" ]]; then
        # do not print echo if VERBOSE_COMMAND is set (set -x does it already)
-        echo "helm" "${@}"
+        >&2 echo "helm" "${@}"
+    fi
+    helm "${@}" | tee -a "${OUTPUT_LOG}"
+    if [[ ${EXIT_CODE} == "0" ]]; then
+        # No matter if "set -e" is used the log will be removed on success.
+        rm -f "${OUTPUT_LOG}"
     fi
-    helm "${@}"
 }
 
 # In case "VERBOSE" is set to "true" (--verbose flag in Breeze) all kubectl commands run will be
 # printed before execution
 function verbose_kubectl {
     if [[ ${VERBOSE:="false"} == "true" && ${VERBOSE_COMMANDS:=} != "true" ]]; then
        # do not print echo if VERBOSE_COMMAND is set (set -x does it already)
-        echo "kubectl" "${@}"
+        >&2 echo "kubectl" "${@}"
+    fi
+    kubectl "${@}" | tee -a "${OUTPUT_LOG}"
+    if [[ ${EXIT_CODE} == "0" ]]; then
+        # No matter if "set -e" is used the log will be removed on success.
+        rm -f "${OUTPUT_LOG}"
     fi
-    kubectl "${@}"
 }
 
 # In case "VERBOSE" is set to "true" (--verbose flag in Breeze) all kind commands run will be
 # printed before execution
 function verbose_kind {
     if [[ ${VERBOSE:="false"} == "true" && ${VERBOSE_COMMANDS:=} != "true" ]]; then
        # do not print echo if VERBOSE_COMMAND is set (set -x does it already)
-        echo "kind" "${@}"
+        >&2 echo "kind" "${@}"
     fi
+    # kind outputs nice output on terminal.
     kind "${@}"
 }
 
-
-# In case "VERBOSE" is set to "true" (--verbose flag in Breeze) all docker commands run will be
-# printed before execution
-function verbose_docker_hide_output_on_success {
-    if [[ ${VERBOSE:="false"} == "true" && ${VERBOSE_COMMANDS:=} != "true" ]]; then
-       # do not print echo if VERBOSE_COMMAND is set (set -x does it already)
-        echo "docker" "${@}"
-    fi
-    docker "${@}" >>"${OUTPUT_LOG}" 2>&1
-}
-
-
 # Prints verbose information in case VERBOSE variable is set
 function print_info() {
-    if [[ ${VERBOSE:="false"} == "true" ]]; then
+    if [[ ${VERBOSE:="false"} == "true" && ${PRINT_INFO_FROM_SCRIPTS} == "true" ]]; then
         echo "$@"
     fi
 }
+
+function set_verbosity() {
+    # whether verbose output should be produced
+    export VERBOSE=${VERBOSE:="false"}
+
+    # whether every bash statement should be printed as they are executed
+    export VERBOSE_COMMANDS=${VERBOSE_COMMANDS:="false"}
+
+    # whether the output from script should be printed at all
+    export PRINT_INFO_FROM_SCRIPTS=${PRINT_INFO_FROM_SCRIPTS:="true"}
+}
+
+set_verbosity
+
+alias docker=verbose_docker
+alias kubectl=verbose_kubectl
+alias helm=verbose_helm
+alias kind=verbose_kind

Review comment:
       For how long do these alias' exist? I mean if you simply run docker commands after a breeze session. Does it still use the verbose version?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org