From d8f9239a7fff1f8f0a62969c4d43435e97a38495 Mon Sep 17 00:00:00 2001
From: Jarek Potiuk <jarek@potiuk.com>
Date: Tue, 12 Oct 2021 19:21:29 +0200
Subject: [PATCH] Fix --github-image-id flag for Breeze (#18882)

When we moved to github registry, the --github-image-id flag was
broken as it had pulled the "latest" image when run right after
pulling the tagged image (and it run that image instead).

This change fixes it and uses GITHUB_PULL_IMAGE_TAG (latest if not
specified) everywhere where the image is used for running.
This flag is not persistent so it is not persistent.
---
 breeze                                        |  3 +-
 scripts/ci/docker-compose/base.yml            |  2 +-
 scripts/ci/libraries/_build_images.sh         | 10 ++++-
 .../ci/libraries/_docker_engine_resources.sh  |  2 +-
 .../ci/libraries/_push_pull_remove_images.sh  | 39 +++++++------------
 scripts/ci/libraries/_runs.sh                 | 10 ++---
 .../ci_install_and_test_provider_packages.sh  |  2 +-
 .../static_checks/in_container_bats_tests.sh  |  4 +-
 scripts/ci/static_checks/mypy.sh              |  2 +-
 scripts/ci/static_checks/ui_lint.sh           |  2 +-
 scripts/ci/static_checks/www_lint.sh          |  2 +-
 .../ci_run_single_airflow_test_in_docker.sh   |  2 +-
 scripts/ci/tools/fix_ownership.sh             |  6 +--
 13 files changed, 41 insertions(+), 45 deletions(-)

diff --git a/breeze b/breeze
index 09b7ab2b5684a..4a2c80d0b27f3 100755
--- a/breeze
+++ b/breeze
@@ -502,7 +502,7 @@ EOF
                                Use CI image.
 
                                Branch name:            ${BRANCH_NAME}
-                               Docker image:           ${AIRFLOW_CI_IMAGE}
+                               Docker image:           ${AIRFLOW_CI_IMAGE_WITH_TAG}
                                Airflow source version: ${AIRFLOW_VERSION}
 EOF
         fi
@@ -635,6 +635,7 @@ export MSSQL_HOST_PORT="${MSSQL_HOST_PORT}"
 export MSSQL_VERSION="${MSSQL_VERSION}"
 export AIRFLOW_SOURCES="${AIRFLOW_SOURCES}"
 export AIRFLOW_CI_IMAGE="${AIRFLOW_CI_IMAGE}"
+export AIRFLOW_CI_IMAGE_WITH_TAG="${AIRFLOW_CI_IMAGE_WITH_TAG}"
 export AIRFLOW_PROD_IMAGE="${AIRFLOW_PROD_IMAGE}"
 export AIRFLOW_IMAGE_KUBERNETES="${AIRFLOW_IMAGE_KUBERNETES}"
 export SQLITE_URL="${SQLITE_URL}"
diff --git a/scripts/ci/docker-compose/base.yml b/scripts/ci/docker-compose/base.yml
index 809ba0887c9c4..9c68c18fac16c 100644
--- a/scripts/ci/docker-compose/base.yml
+++ b/scripts/ci/docker-compose/base.yml
@@ -18,7 +18,7 @@
 version: "3.7"
 services:
   airflow:
-    image: ${AIRFLOW_CI_IMAGE}
+    image: ${AIRFLOW_CI_IMAGE_WITH_TAG}
     pull_policy: never
     environment:
       - USER=root
diff --git a/scripts/ci/libraries/_build_images.sh b/scripts/ci/libraries/_build_images.sh
index a9754d147a7ac..e9572b0979d76 100644
--- a/scripts/ci/libraries/_build_images.sh
+++ b/scripts/ci/libraries/_build_images.sh
@@ -275,7 +275,7 @@ function build_images::get_local_build_cache_hash() {
     local_image_build_cache_file="${AIRFLOW_SOURCES}/manifests/local-build-cache-hash-${PYTHON_MAJOR_MINOR_VERSION}"
     # Remove the container just in case
     docker_v rm --force "local-airflow-ci-container" 2>/dev/null >/dev/null
-    if ! docker_v inspect "${AIRFLOW_CI_IMAGE}" 2>/dev/null >/dev/null; then
+    if ! docker_v inspect "${AIRFLOW_CI_IMAGE_WITH_TAG}" 2>/dev/null >/dev/null; then
         verbosity::print_info
         verbosity::print_info "Local airflow CI image not available"
         verbosity::print_info
@@ -286,7 +286,7 @@ function build_images::get_local_build_cache_hash() {
         return
 
     fi
-    docker_v create --name "local-airflow-ci-container" "${AIRFLOW_CI_IMAGE}" 2>/dev/null >/dev/null
+    docker_v create --name "local-airflow-ci-container" "${AIRFLOW_CI_IMAGE_WITH_TAG}" 2>/dev/null >/dev/null
     docker_v cp "local-airflow-ci-container:/build-cache-hash" \
         "${local_image_build_cache_file}" 2>/dev/null ||
         touch "${local_image_build_cache_file}"
@@ -403,6 +403,11 @@ function build_images::get_docker_cache_image_names() {
     #  ghcr.io/apache/airflow/main/ci/python3.8
     export AIRFLOW_CI_IMAGE="${image_name}/${BRANCH_NAME}/ci/python${PYTHON_MAJOR_MINOR_VERSION}"
 
+    # Example:
+    #  ghcr.io/apache/airflow/main/ci/python3.8:latest
+    #  ghcr.io/apache/airflow/main/ci/python3.8:<COMMIT_SHA>
+    export AIRFLOW_CI_IMAGE_WITH_TAG="${image_name}/${BRANCH_NAME}/ci/python${PYTHON_MAJOR_MINOR_VERSION}:${GITHUB_REGISTRY_PULL_IMAGE_TAG}"
+
     # Example:
     #  local-airflow-ci-manifest/main/python3.8
     export AIRFLOW_CI_LOCAL_MANIFEST_IMAGE="local-airflow-ci-manifest/${BRANCH_NAME}/python${PYTHON_MAJOR_MINOR_VERSION}"
@@ -459,6 +464,7 @@ function build_images::login_to_docker_registry() {
         else
             verbosity::print_info "Skip Login to GitHub Container Registry as token is missing"
         fi
+        start_end::group_end
     fi
 }
 
diff --git a/scripts/ci/libraries/_docker_engine_resources.sh b/scripts/ci/libraries/_docker_engine_resources.sh
index 7bcf427e40f64..75daf175e29f9 100644
--- a/scripts/ci/libraries/_docker_engine_resources.sh
+++ b/scripts/ci/libraries/_docker_engine_resources.sh
@@ -45,6 +45,6 @@ function docker_engine_resources::get_available_memory_in_docker() {
 function docker_engine_resources::check_all_resources() {
     docker_v run -t "${EXTRA_DOCKER_FLAGS[@]}" \
         --entrypoint "/bin/bash"  \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         -c "/opt/airflow/scripts/in_container/run_resource_check.sh"
 }
diff --git a/scripts/ci/libraries/_push_pull_remove_images.sh b/scripts/ci/libraries/_push_pull_remove_images.sh
index 51611ae235434..9741e32e81771 100644
--- a/scripts/ci/libraries/_push_pull_remove_images.sh
+++ b/scripts/ci/libraries/_push_pull_remove_images.sh
@@ -108,26 +108,15 @@ function push_pull_remove_images::pull_base_python_image() {
         echo -n "Docker pull base python image. Upgrade to newer deps: ${UPGRADE_TO_NEWER_DEPENDENCIES}
 " > "${DETECTED_TERMINAL}"
     fi
-    if [[ ${GITHUB_REGISTRY_PULL_IMAGE_TAG} != "latest" ]]; then
-        push_pull_remove_images::pull_image_if_not_present_or_forced \
-            "${AIRFLOW_PYTHON_BASE_IMAGE}${GITHUB_REGISTRY_PULL_IMAGE_TAG}"
-        if [[ ${CHECK_IF_BASE_PYTHON_IMAGE_UPDATED} == "true" ]] ; then
-            echo
-            echo  "${COLOR_RED}ERROR: You cannot check for base python image if you pull specific tag: ${GITHUB_REGISTRY_PULL_IMAGE_TAG}.${COLOR_RESET}"
-            echo
-            return 1
-        fi
-    else
-        set +e
-        push_pull_remove_images::pull_image_if_not_present_or_forced "${AIRFLOW_PYTHON_BASE_IMAGE}"
-        local res="$?"
-        set -e
-        if [[ ${CHECK_IF_BASE_PYTHON_IMAGE_UPDATED} == "true" || ${res} != "0" ]] ; then
-            # Rebuild the base python image using DockerHub - either when we explicitly want it
-            # or when there is no image available yet in ghcr.io (usually when you build it for the
-            # first time in your repository
-            push_pull_remove_images::check_and_rebuild_python_base_image_if_needed
-        fi
+    set +e
+    push_pull_remove_images::pull_image_if_not_present_or_forced "${AIRFLOW_PYTHON_BASE_IMAGE}"
+    local res="$?"
+    set -e
+    if [[ ${CHECK_IF_BASE_PYTHON_IMAGE_UPDATED} == "true" || ${res} != "0" ]] ; then
+        # Rebuild the base python image using DockerHub - either when we explicitly want it
+        # or when there is no image available yet in ghcr.io (usually when you build it for the
+        # first time in your repository
+        push_pull_remove_images::check_and_rebuild_python_base_image_if_needed
     fi
 }
 
@@ -144,8 +133,7 @@ function push_pull_remove_images::pull_ci_images_if_needed() {
     fi
     if [[ "${DOCKER_CACHE}" == "pulled" ]]; then
         set +e
-        push_pull_remove_images::pull_image_if_not_present_or_forced \
-            "${AIRFLOW_CI_IMAGE}:${GITHUB_REGISTRY_PULL_IMAGE_TAG}"
+        push_pull_remove_images::pull_image_if_not_present_or_forced "${AIRFLOW_CI_IMAGE_WITH_TAG}"
         local res="$?"
         set -e
         if [[ ${res} != "0" ]]; then
@@ -228,18 +216,19 @@ function push_pull_remove_images::push_python_image_to_github() {
 
 # Pushes Ci images and their tags to registry in GitHub
 function push_pull_remove_images::push_ci_images_to_github() {
+    start_end::group_start "Push image"
     if [[ "${PUSH_PYTHON_BASE_IMAGE=}" != "false" ]]; then
         push_pull_remove_images::push_python_image_to_github
     fi
-    local airflow_ci_tagged_image="${AIRFLOW_CI_IMAGE}:${GITHUB_REGISTRY_PUSH_IMAGE_TAG}"
-    docker_v tag "${AIRFLOW_CI_IMAGE}" "${airflow_ci_tagged_image}"
-    push_pull_remove_images::push_image_with_retries "${airflow_ci_tagged_image}"
+    docker_v tag "${AIRFLOW_CI_IMAGE}" "${AIRFLOW_CI_IMAGE}:${GITHUB_REGISTRY_PUSH_IMAGE_TAG}"
+    push_pull_remove_images::push_image_with_retries "${AIRFLOW_CI_IMAGE}:${GITHUB_REGISTRY_PUSH_IMAGE_TAG}"
     # Also push ci manifest image if GITHUB_REGISTRY_PUSH_IMAGE_TAG is "latest"
     if [[ ${GITHUB_REGISTRY_PUSH_IMAGE_TAG} == "latest" ]]; then
         local airflow_ci_manifest_tagged_image="${AIRFLOW_CI_REMOTE_MANIFEST_IMAGE}:latest"
         docker_v tag "${AIRFLOW_CI_LOCAL_MANIFEST_IMAGE}" "${airflow_ci_manifest_tagged_image}"
         push_pull_remove_images::push_image_with_retries "${airflow_ci_manifest_tagged_image}"
     fi
+    start_end::group_end
 }
 
 # Pushes PROD image to registry in GitHub
diff --git a/scripts/ci/libraries/_runs.sh b/scripts/ci/libraries/_runs.sh
index 16cb1749fca77..84b31f4ef84ae 100644
--- a/scripts/ci/libraries/_runs.sh
+++ b/scripts/ci/libraries/_runs.sh
@@ -23,7 +23,7 @@ function runs::run_docs() {
         -e "GITHUB_ACTIONS=${GITHUB_ACTIONS="false"}" \
         --entrypoint "/usr/local/bin/dumb-init"  \
         --pull never \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         "--" "/opt/airflow/scripts/in_container/run_docs_build.sh" "${@}"
     start_end::group_end
 }
@@ -34,7 +34,7 @@ function runs::run_generate_constraints() {
     docker_v run "${EXTRA_DOCKER_FLAGS[@]}" \
         --entrypoint "/usr/local/bin/dumb-init"  \
         --pull never \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         "--" "/opt/airflow/scripts/in_container/run_generate_constraints.sh"
     start_end::group_end
 }
@@ -47,7 +47,7 @@ function runs::run_prepare_airflow_packages() {
         -t \
         -v "${AIRFLOW_SOURCES}:/opt/airflow" \
         --pull never \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         "--" "/opt/airflow/scripts/in_container/run_prepare_airflow_packages.sh"
     start_end::group_end
 }
@@ -61,7 +61,7 @@ function runs::run_prepare_provider_packages() {
         -t \
         -v "${AIRFLOW_SOURCES}:/opt/airflow" \
         --pull never \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         "--" "/opt/airflow/scripts/in_container/run_prepare_provider_packages.sh" "${@}"
 }
 
@@ -80,6 +80,6 @@ function runs::run_prepare_provider_documentation() {
         -e "GENERATE_PROVIDERS_ISSUE" \
         -e "GITHUB_TOKEN" \
         --pull never \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         "--" "/opt/airflow/scripts/in_container/run_prepare_provider_documentation.sh" "${@}"
 }
diff --git a/scripts/ci/provider_packages/ci_install_and_test_provider_packages.sh b/scripts/ci/provider_packages/ci_install_and_test_provider_packages.sh
index 51575ebe80cdf..b8228851f4dd9 100755
--- a/scripts/ci/provider_packages/ci_install_and_test_provider_packages.sh
+++ b/scripts/ci/provider_packages/ci_install_and_test_provider_packages.sh
@@ -38,7 +38,7 @@ function run_test_package_import_all_classes() {
         -v "${AIRFLOW_SOURCES}/empty:/opt/airflow/airflow:cached" \
         -v "${AIRFLOW_SOURCES}/scripts/in_container:/opt/airflow/scripts/in_container:cached" \
         -v "${AIRFLOW_SOURCES}/dev/import_all_classes.py:/opt/airflow/dev/import_all_classes.py:cached" \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         "--" "/opt/airflow/scripts/in_container/run_install_and_test_provider_packages.sh"
 }
 
diff --git a/scripts/ci/static_checks/in_container_bats_tests.sh b/scripts/ci/static_checks/in_container_bats_tests.sh
index fa4eacd86ab21..4778c6012f113 100644
--- a/scripts/ci/static_checks/in_container_bats_tests.sh
+++ b/scripts/ci/static_checks/in_container_bats_tests.sh
@@ -23,13 +23,13 @@ function run_in_container_bats_tests() {
         docker_v run "${EXTRA_DOCKER_FLAGS[@]}" \
         --entrypoint "/opt/bats/bin/bats"  \
         "-v" "$(pwd):/airflow" \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         --tap  "tests/bats/in_container/"
     else
         docker_v run "${EXTRA_DOCKER_FLAGS[@]}" \
         --entrypoint "/opt/bats/bin/bats"  \
         "-v" "$(pwd):/airflow" \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         --tap "${@}"
     fi
 }
diff --git a/scripts/ci/static_checks/mypy.sh b/scripts/ci/static_checks/mypy.sh
index 7ebbd6340ff06..7b5879e1e3d13 100755
--- a/scripts/ci/static_checks/mypy.sh
+++ b/scripts/ci/static_checks/mypy.sh
@@ -29,7 +29,7 @@ function run_mypy() {
     docker_v run "${EXTRA_DOCKER_FLAGS[@]}" \
         --entrypoint "/usr/local/bin/dumb-init"  \
         "-v" "${AIRFLOW_SOURCES}/.mypy_cache:/opt/airflow/.mypy_cache" \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         "--" "/opt/airflow/scripts/in_container/run_mypy.sh" "${files[@]}"
 }
 
diff --git a/scripts/ci/static_checks/ui_lint.sh b/scripts/ci/static_checks/ui_lint.sh
index 3722e54b93d42..d5f722c1354b5 100755
--- a/scripts/ci/static_checks/ui_lint.sh
+++ b/scripts/ci/static_checks/ui_lint.sh
@@ -27,5 +27,5 @@ build_images::rebuild_ci_image_if_needed
 
 docker run "${EXTRA_DOCKER_FLAGS[@]}" \
     --entrypoint "/bin/bash"  \
-    "${AIRFLOW_CI_IMAGE}" \
+    "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
     -c 'cd airflow/ui && yarn --frozen-lockfile --non-interactive && yarn run lint "${@}"' "${@#airflow/ui/}"
diff --git a/scripts/ci/static_checks/www_lint.sh b/scripts/ci/static_checks/www_lint.sh
index 7ae56204274a5..fec51516d5a24 100755
--- a/scripts/ci/static_checks/www_lint.sh
+++ b/scripts/ci/static_checks/www_lint.sh
@@ -27,5 +27,5 @@ build_images::rebuild_ci_image_if_needed
 
 docker run "${EXTRA_DOCKER_FLAGS[@]}" \
     --entrypoint "/bin/bash"  \
-    "${AIRFLOW_CI_IMAGE}" \
+    "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
     -c 'cd airflow/www && yarn --frozen-lockfile --non-interactive && yarn run lint "${@}"' "${@#airflow/www/static/js/}"
diff --git a/scripts/ci/testing/ci_run_single_airflow_test_in_docker.sh b/scripts/ci/testing/ci_run_single_airflow_test_in_docker.sh
index 615ce21437664..d97310ac7b260 100755
--- a/scripts/ci/testing/ci_run_single_airflow_test_in_docker.sh
+++ b/scripts/ci/testing/ci_run_single_airflow_test_in_docker.sh
@@ -170,7 +170,7 @@ function run_airflow_testing_in_docker() {
             echo "${COLOR_BLUE}*${COLOR_RESET}"
             echo "${COLOR_BLUE}***********************************************************************************************${COLOR_RESET}"
             echo
-            curl "${constraints_url}" | grep -ve "^#" | diff --color=always - <( docker run --entrypoint /bin/bash "${AIRFLOW_CI_IMAGE}"  -c 'pip freeze' \
+            curl "${constraints_url}" | grep -ve "^#" | diff --color=always - <( docker run --entrypoint /bin/bash "${AIRFLOW_CI_IMAGE_WITH_TAG}"  -c 'pip freeze' \
                 | sort | grep -v "apache_airflow" | grep -v "@" | grep -v "/opt/airflow" | grep -ve "^#")
             echo
         fi
diff --git a/scripts/ci/tools/fix_ownership.sh b/scripts/ci/tools/fix_ownership.sh
index de1562122a779..1dc7b92e4ec56 100755
--- a/scripts/ci/tools/fix_ownership.sh
+++ b/scripts/ci/tools/fix_ownership.sh
@@ -33,12 +33,12 @@ sanity_checks::sanitize_mounted_files
 
 read -r -a EXTRA_DOCKER_FLAGS <<<"$(local_mounts::convert_local_mounts_to_docker_params)"
 
-if docker image inspect "${AIRFLOW_CI_IMAGE}" >/dev/null 2>&1; then
+if docker image inspect "${AIRFLOW_CI_IMAGE_WITH_TAG}" >/dev/null 2>&1; then
     docker_v run --entrypoint /bin/bash "${EXTRA_DOCKER_FLAGS[@]}" \
         --rm \
         --env-file "${AIRFLOW_SOURCES}/scripts/ci/docker-compose/_docker.env" \
-        "${AIRFLOW_CI_IMAGE}" \
+        "${AIRFLOW_CI_IMAGE_WITH_TAG}" \
         -c /opt/airflow/scripts/in_container/run_fix_ownership.sh || true
 else
-    echo "Skip fixing ownership as seems that you do not have the ${AIRFLOW_CI_IMAGE} image yet"
+    echo "Skip fixing ownership as seems that you do not have the ${AIRFLOW_CI_IMAGE_WITH_TAG} image yet"
 fi
