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 2021/01/17 11:21:40 UTC

[GitHub] [airflow] potiuk opened a new pull request #13726: Adds capability of switching to Github Container Registry

potiuk opened a new pull request #13726:
URL: https://github.com/apache/airflow/pull/13726


   Currently we are using GitHub Packages to cache images for the
   build. GitHub Packages are "legacy" storage of binary artifacts
   for GitHub and as of September 2020 they introduced Github
   Container Registry as more stable, easier to manage replacement
   for container storage. It includes complete self-management
   of the images including permission management, public access,
   retention management and many more.
   
   More about it here:
   
   https://github.blog/2020-09-01-introducing-github-container-registry/
   
   Recently we started to experience unstable behaviour of the
   Github Packages ('unknown blob' and manifest v1 vs. v2 when
   pushing images to it. So together with ASF we proposed to
   enable Github Container Registry and it happened as of
   January 2020.
   
   More about it in https://issues.apache.org/jira/browse/INFRA-20959
   
   We are currently in the testing phase, especially when it
   comes to management of permissions - the model of permission
   mangement is not the same for Container Registry as it was
   for GitHub Packages (it was per-repository in GitHub Packages,
   but it is organization-wide in the Container Registry.
   
   This PR introduces an option to use GitHub Container Registry
   rather than GitHub Packages. It is implemented in both - CI
   level and Breeze level allowing to seamlessly switch between
   those two solutions:
   
   In Breeze (which we use to test pushing/pulling the images)
   --github-registry option was added with `ghcr.io` (Github Container
   Registry) or `docker.pkg.github.com` (GitHub Packages).
   
   In CI the same can be achieved by setting GITHUB_REGISTRY value
   (same values possible as for --github-registry Breeze parameter)
   
   <!--
   Thank you for contributing! Please make sure that your code changes
   are covered with tests. And in case of new features or big changes
   remember to adjust the documentation.
   
   Feel free to ping committers for the review!
   
   In case of existing issue, reference it using one of the following:
   
   closes: #ISSUE
   related: #ISSUE
   
   How to write a good git commit message:
   http://chris.beams.io/posts/git-commit/
   -->
   
   ---
   **^ Add meaningful description above**
   
   Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/master/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
   In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
   In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
   In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/master/UPDATING.md).
   


----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559881055



##########
File path: scripts/ci/libraries/_push_pull_remove_images.sh
##########
@@ -294,6 +302,46 @@ function push_pull_remove_images::wait_for_github_registry_image() {
     verbosity::print_info "Found ${image_name_in_github_registry}:${image_tag_in_github_registry} image"
 }
 
+
+# waits for an image to be available in GitHub Container Registry
+function push_pull_remove_images::wait_for_image_in_github_container_registry() {
+    local image_name_in_github_registry="${1}"
+    local image_tag_in_github_registry=${2}
+
+    local image_to_wait_for="${GITHUB_REGISTRY}/${GITHUB_REPOSITORY}-${image_name_in_github_registry}:${image_tag_in_github_registry}"
+    echo
+    echo "Waiting for ${GITHUB_REGISTRY}/${GITHUB_REPOSITORY}-${image_name_in_github_registry}:${image_tag_in_github_registry} image"
+    echo
+    set +e
+    while true; do
+        docker manifest inspect "${image_to_wait_for}"

Review comment:
       Nah. I prefer to leave as it is and see the output /err of the command here. This is rather useful to see "Unknown manifest" as the response while waiting and then the manifest itself after it is retrieved. Good for debugging.




----------------------------------------------------------------
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



[GitHub] [airflow] github-actions[bot] commented on pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#issuecomment-764152824


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


----------------------------------------------------------------
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



[GitHub] [airflow] ashb commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559786576



##########
File path: IMAGES.rst
##########
@@ -247,29 +250,92 @@ significant changes are done in the Dockerfile.CI.
 The images are named differently (in Docker definition of image names - registry URL is part of the
 image name if DockerHub is not used as registry). Also GitHub has its own structure for registries
 each project has its own registry naming convention that should be followed. The name of
-images for GitHub registry are:
+images for GitHub registry are different as they must follow limitation of the registry used.
+
+We are still using Github Packages as registry, but we are in the process of testing and switching
+to GitHub Container Registry, and the naming conventions are slightly different (GitHub Packages
+required all packages to have "organization/repository/" URL prefix ("apache/airflow/",
+where in GitHub Container Registry, all images are in "organization" not in "repository" and they are all
+in organization wide "apache/" namespace rather than in "apache/airflow/" one).
+We are adding "airflow-" as prefix for image names of all Airflow images instead.
+The images are linked to the repository via ``org.opencontainers.image.source`` label in the image.
+
+Naming convention for GitHub Packages
+-------------------------------------
+
+Images built as "Run ID snapshot":
+
+.. code-block:: bash
+
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-ci-gcr-v1:<RUNID>    - for CI images
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-gcr-v1:<RUNID>       - for production images
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-build-gcr-v1:<RUNID> - for production build stage
+  ghcr.io/apache/airflow/pythonX.Y-<BRANCH>-gcr-v1:X.Y-slim-buster-<RUN_ID>  - for base python images
+
+Latest images (pushed when master merge succeeds):
 
 .. code-block:: bash
 
-  docker.pkg.github.com/apache/airflow/<BRANCH>-pythonX.Y       - for production images
-  docker.pkg.github.com/apache/airflow/<BRANCH>-pythonX.Y-ci    - for CI images
-  docker.pkg.github.com/apache/airflow/<BRANCH>-pythonX.Y-build - for production build state
-  docker.pkg.github.com/apache/airflow/pythonX.Y-<BRANCH>       - for base python images
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-ci-gcr-v1:latest    - for CI images

Review comment:
       We shouldn't need the `gcr` 5n the image name too.




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559876284



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"
+        local token=""
+        if [[ "${GITHUB_REGISTRY}" == "ghcr.io" ]]; then
+            # For now ghcr.io can only authenticate using Personal Access Token with package access scope.
+            # There are plans to implement GITHUB_TOKEN authentication but this is not implemented yet
+            token="${CONTAINER_REGISTRY_TOKEN=}"
+            echo
+            echo "Using CONTAINER_REGISTRY_TOKEN!"
+            echo
+        elif [[ "${GITHUB_REGISTRY}" == "docker.pkg.github.com" ]]; then
+            token="${GITHUB_TOKEN}"
+            echo
+            echo "Using GITHUB_TOKEN!"
+            echo
+        else
+            echo
+            echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+            echo
+            exit 1
+        fi
+        if [[ "${token}" == "" ]] ; then

Review comment:
       Surely can do. I sometimes prefer explicit comparision for people who are less familiar with some bash-isms . But yeah, why not.  It sometimes good to change your habits. I corrected it everywhere where I used != "" or == "" (there were not so many places overall.




----------------------------------------------------------------
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



[GitHub] [airflow] github-actions[bot] commented on pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#issuecomment-764152824


   The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest master at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.


----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#issuecomment-762566765


   Fixup added with changes @ashb 


----------------------------------------------------------------
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



[GitHub] [airflow] ashb commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559790496



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"
+        local token=""
+        if [[ "${GITHUB_REGISTRY}" == "ghcr.io" ]]; then
+            # For now ghcr.io can only authenticate using Personal Access Token with package access scope.
+            # There are plans to implement GITHUB_TOKEN authentication but this is not implemented yet
+            token="${CONTAINER_REGISTRY_TOKEN=}"
+            echo
+            echo "Using CONTAINER_REGISTRY_TOKEN!"
+            echo
+        elif [[ "${GITHUB_REGISTRY}" == "docker.pkg.github.com" ]]; then
+            token="${GITHUB_TOKEN}"
+            echo
+            echo "Using GITHUB_TOKEN!"
+            echo
+        else
+            echo
+            echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+            echo
+            exit 1
+        fi
+        if [[ "${token}" == "" ]] ; then
+            echo
+            echo "Skip logging in to Github Registry. No Token available!"
+            echo
+        fi
+        start_end::group_end
+        if [[ "${token}" != "" ]]; then
+            start_end::group_start "Login to GitHub Registry ${GITHUB_REGISTRY}"
+            echo "${token}" | docker login \
                 --username "${GITHUB_USERNAME:-apache}" \
                 --password-stdin \
                 "${GITHUB_REGISTRY}"
             start_end::group_end
+        else
+            start_end::group_start "Skip Login to GitHub Registry ${GITHUB_REGISTRY} as token is missing"
+            start_end::group_end
         fi
     fi
 }
 
+function build_image::enable_docker_experimental_features_if_github_registry_enabled() {

Review comment:
       This function always appears to be called together with `login_to_github_container_registry_if_github_registry_enabled` -- where one is called so is the other.
   
   I think it would be clearer at the call sites if you had a single `configure_github_container_registry` (or similarly named) function please.




----------------------------------------------------------------
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



[GitHub] [airflow] ashb commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559988352



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"
+        local token=""
+        if [[ "${GITHUB_REGISTRY}" == "ghcr.io" ]]; then
+            # For now ghcr.io can only authenticate using Personal Access Token with package access scope.
+            # There are plans to implement GITHUB_TOKEN authentication but this is not implemented yet
+            token="${CONTAINER_REGISTRY_TOKEN=}"
+            echo
+            echo "Using CONTAINER_REGISTRY_TOKEN!"
+            echo
+        elif [[ "${GITHUB_REGISTRY}" == "docker.pkg.github.com" ]]; then
+            token="${GITHUB_TOKEN}"
+            echo
+            echo "Using GITHUB_TOKEN!"
+            echo
+        else
+            echo
+            echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+            echo
+            exit 1
+        fi
+        if [[ "${token}" == "" ]] ; then
+            echo
+            echo "Skip logging in to Github Registry. No Token available!"
+            echo
+        fi
+        start_end::group_end
+        if [[ "${token}" != "" ]]; then
+            start_end::group_start "Login to GitHub Registry ${GITHUB_REGISTRY}"
+            echo "${token}" | docker login \
                 --username "${GITHUB_USERNAME:-apache}" \
                 --password-stdin \
                 "${GITHUB_REGISTRY}"
             start_end::group_end
+        else
+            start_end::group_start "Skip Login to GitHub Registry ${GITHUB_REGISTRY} as token is missing"
+            start_end::group_end
         fi
     fi
 }
 
+function build_image::enable_docker_experimental_features_if_github_registry_enabled() {
+    if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
+        start_end::group_start "Enable experimental features in Docker ('docker manifest' command)"
+        python <<EOF
+import json
+from os.path import expanduser, join
+
+DOCKER_CONFIG_FILE=join(expanduser("~"), ".docker", 'config.json')
+with open(DOCKER_CONFIG_FILE) as f:
+    config = json.load(f)
+print("\nBefore:\n")
+print(json.dumps(config, indent=4))
+print("\nAdding experimental flag to enable docker manifest command\n")
+config['experimental'] = "enabled"
+print("\nAfter:\n")
+print(json.dumps(config, indent=4))
+with open(DOCKER_CONFIG_FILE,"wt") as f:
+    json.dump(config, f)
+EOF
+        start_end::group_end

Review comment:
       Yeah, it's installed there too - some of the actions expect it, so we have to mirror the environment of hosted runners




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#issuecomment-761948314


   Only quarantined tests failed - it's good to go.


----------------------------------------------------------------
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



[GitHub] [airflow] ashb commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559800130



##########
File path: scripts/ci/libraries/_push_pull_remove_images.sh
##########
@@ -294,6 +302,46 @@ function push_pull_remove_images::wait_for_github_registry_image() {
     verbosity::print_info "Found ${image_name_in_github_registry}:${image_tag_in_github_registry} image"
 }
 
+
+# waits for an image to be available in GitHub Container Registry
+function push_pull_remove_images::wait_for_image_in_github_container_registry() {
+    local image_name_in_github_registry="${1}"
+    local image_tag_in_github_registry=${2}
+
+    local image_to_wait_for="${GITHUB_REGISTRY}/${GITHUB_REPOSITORY}-${image_name_in_github_registry}:${image_tag_in_github_registry}"
+    echo
+    echo "Waiting for ${GITHUB_REGISTRY}/${GITHUB_REPOSITORY}-${image_name_in_github_registry}:${image_tag_in_github_registry} image"
+    echo
+    set +e
+    while true; do
+        docker manifest inspect "${image_to_wait_for}"

Review comment:
       ```suggestion
           docker manifest inspect "${image_to_wait_for}" 2>/dev/null
   ```
   
   Maybe? Stdout too?




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk merged pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #13726:
URL: https://github.com/apache/airflow/pull/13726


   


----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559880099



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"
+        local token=""
+        if [[ "${GITHUB_REGISTRY}" == "ghcr.io" ]]; then
+            # For now ghcr.io can only authenticate using Personal Access Token with package access scope.
+            # There are plans to implement GITHUB_TOKEN authentication but this is not implemented yet
+            token="${CONTAINER_REGISTRY_TOKEN=}"
+            echo
+            echo "Using CONTAINER_REGISTRY_TOKEN!"
+            echo
+        elif [[ "${GITHUB_REGISTRY}" == "docker.pkg.github.com" ]]; then
+            token="${GITHUB_TOKEN}"
+            echo
+            echo "Using GITHUB_TOKEN!"
+            echo
+        else
+            echo
+            echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+            echo
+            exit 1
+        fi
+        if [[ "${token}" == "" ]] ; then
+            echo
+            echo "Skip logging in to Github Registry. No Token available!"
+            echo
+        fi
+        start_end::group_end
+        if [[ "${token}" != "" ]]; then
+            start_end::group_start "Login to GitHub Registry ${GITHUB_REGISTRY}"
+            echo "${token}" | docker login \
                 --username "${GITHUB_USERNAME:-apache}" \
                 --password-stdin \
                 "${GITHUB_REGISTRY}"
             start_end::group_end
+        else
+            start_end::group_start "Skip Login to GitHub Registry ${GITHUB_REGISTRY} as token is missing"
+            start_end::group_end
         fi
     fi
 }
 
+function build_image::enable_docker_experimental_features_if_github_registry_enabled() {
+    if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
+        start_end::group_start "Enable experimental features in Docker ('docker manifest' command)"
+        python <<EOF
+import json
+from os.path import expanduser, join
+
+DOCKER_CONFIG_FILE=join(expanduser("~"), ".docker", 'config.json')
+with open(DOCKER_CONFIG_FILE) as f:
+    config = json.load(f)
+print("\nBefore:\n")
+print(json.dumps(config, indent=4))
+print("\nAdding experimental flag to enable docker manifest command\n")
+config['experimental'] = "enabled"
+print("\nAfter:\n")
+print(json.dumps(config, indent=4))
+with open(DOCKER_CONFIG_FILE,"wt") as f:
+    json.dump(config, f)
+EOF
+        start_end::group_end

Review comment:
       We just have to make sure jqis installed.  But yeah. AFAIK is installed for GitHub Runner. Just make sure it is also there for the self-hosted run :)




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#issuecomment-761863267


   OK. Seems that this one is ready to go:
   
   1) By default Github Packages are used - no change vs. current behavior
   2) The "master" build after switching to GHCR works fine, including pushing image cache in "master" build: https://github.com/potiuk/airflow/runs/1717296858?check_suite_focus=true
   3) The PR from fork after switching to GHCR also works - fine builds from forks are also using GHCR properly.
   https://github.com/potiuk/airflow/runs/1717611971?check_suite_focus=true
   
   So I believe we can safely merge this one and as soon as I get the secret + public images in GCR (need infra support), we will be able to switch to Google Container Registry with this simple change: 
   
   https://github.com/potiuk/airflow/commit/dadba351fd59c723fe7cf5bf4a9620096fa31249
   
   
   


----------------------------------------------------------------
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



[GitHub] [airflow] ashb commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559787419



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"
+        local token=""
+        if [[ "${GITHUB_REGISTRY}" == "ghcr.io" ]]; then
+            # For now ghcr.io can only authenticate using Personal Access Token with package access scope.
+            # There are plans to implement GITHUB_TOKEN authentication but this is not implemented yet
+            token="${CONTAINER_REGISTRY_TOKEN=}"
+            echo
+            echo "Using CONTAINER_REGISTRY_TOKEN!"
+            echo
+        elif [[ "${GITHUB_REGISTRY}" == "docker.pkg.github.com" ]]; then
+            token="${GITHUB_TOKEN}"
+            echo
+            echo "Using GITHUB_TOKEN!"
+            echo
+        else
+            echo
+            echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+            echo
+            exit 1
+        fi
+        if [[ "${token}" == "" ]] ; then

Review comment:
       ```suggestion
           if [[ -z "${token}" ]] ; then
   ```




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559225071



##########
File path: Dockerfile
##########
@@ -477,8 +495,22 @@ LABEL org.apache.airflow.distro="debian" \
   org.apache.airflow.version="${AIRFLOW_VERSION}" \
   org.apache.airflow.uid="${AIRFLOW_UID}" \
   org.apache.airflow.gid="${AIRFLOW_GID}" \
-  org.apache.airflow.mainImage.buildId=${BUILD_ID} \
-  org.apache.airflow.mainImage.commitSha=${COMMIT_SHA}
+  org.apache.airflow.mainImage.buildId="${BUILD_ID}" \

Review comment:
       FYI. GCR uses standard OCI `org.opencontainers.image.source` label to link image to the repository. This is an interesting idea and seems to work well. I used the opportunity to add all predefined OCI labels so our image is following the standards. That includes human-readable descriptions, versions, URLs etc. I still leave the "org.apache." ones for backwards compatibility.




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk edited a comment on pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#issuecomment-761782451


   Note ! This change only adds capability to switching to GitHub Container Registry, but it does not enable it yet. 
   
   I am testing GCR in my fork  (for example here https://github.com/potiuk/airflow/actions/runs/491530166) where I changed the GITHUB_REGISTRY to `ghcr.io` as it needs cooperation with Infra, creating a token that can write to the registry, setting it as secret and setting our images to `Public` visibility - all of which might need (still testing) INFRA tickets. 
   
   This change should maintain compatibility with what we are using currently in GitHub packages.


----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559224615



##########
File path: scripts/ci/libraries/_push_pull_remove_images.sh
##########
@@ -294,6 +302,46 @@ function push_pull_remove_images::wait_for_github_registry_image() {
     verbosity::print_info "Found ${image_name_in_github_registry}:${image_tag_in_github_registry} image"
 }
 
+
+# waits for an image to be available in GitHub Container Registry
+function push_pull_remove_images::wait_for_image_in_github_container_registry() {
+    local image_name_in_github_registry="${1}"
+    local image_tag_in_github_registry=${2}
+
+    local image_to_wait_for="${GITHUB_REGISTRY}/${GITHUB_REPOSITORY}-${image_name_in_github_registry}:${image_tag_in_github_registry}"
+    echo
+    echo "Waiting for ${GITHUB_REGISTRY}/${GITHUB_REPOSITORY}-${image_name_in_github_registry}:${image_tag_in_github_registry} image"
+    echo
+    set +e
+    while true; do
+        docker manifest inspect "${image_to_wait_for}"

Review comment:
       FYI: GCR supports unauthenticated `docker manifest inspect` for public images, which is a big PLUS. 




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk edited a comment on pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk edited a comment on pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#issuecomment-761782451


   Note ! This change only adds capability to switching to GitHub Container Registry, but it does not enable it yet. 
   
   I am testing GCR in my fork  (for example here https://github.com/potiuk/airflow/actions/runs/491524892) where I changed the GITHUB_REGISTRY to `ghcr.io` as it needs cooperation with Infra, creating a token that can write to the registry, setting it as secret and setting our images to `Public` visibility - all of which might need (still testing) INFRA tickets. 
   
   This change should maintain compatibility with what we are using currently in GitHub packages.


----------------------------------------------------------------
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



[GitHub] [airflow] ashb commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559788194



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"

Review comment:
       Two groups for one function is excessive -- especially as there is only a few lines per group.
   
   At the very least limit it to at most one group per function.




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559876994



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"

Review comment:
       Sure. Why not.




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559880245



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"
+        local token=""
+        if [[ "${GITHUB_REGISTRY}" == "ghcr.io" ]]; then
+            # For now ghcr.io can only authenticate using Personal Access Token with package access scope.
+            # There are plans to implement GITHUB_TOKEN authentication but this is not implemented yet
+            token="${CONTAINER_REGISTRY_TOKEN=}"
+            echo
+            echo "Using CONTAINER_REGISTRY_TOKEN!"
+            echo
+        elif [[ "${GITHUB_REGISTRY}" == "docker.pkg.github.com" ]]; then
+            token="${GITHUB_TOKEN}"
+            echo
+            echo "Using GITHUB_TOKEN!"
+            echo
+        else
+            echo
+            echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+            echo
+            exit 1
+        fi
+        if [[ "${token}" == "" ]] ; then
+            echo
+            echo "Skip logging in to Github Registry. No Token available!"
+            echo
+        fi
+        start_end::group_end
+        if [[ "${token}" != "" ]]; then
+            start_end::group_start "Login to GitHub Registry ${GITHUB_REGISTRY}"
+            echo "${token}" | docker login \
                 --username "${GITHUB_USERNAME:-apache}" \
                 --password-stdin \
                 "${GITHUB_REGISTRY}"
             start_end::group_end
+        else
+            start_end::group_start "Skip Login to GitHub Registry ${GITHUB_REGISTRY} as token is missing"
+            start_end::group_end
         fi
     fi
 }
 
+function build_image::enable_docker_experimental_features_if_github_registry_enabled() {

Review comment:
       yeah. It is indeed everywhere. changing.




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#issuecomment-762901770


   @ashb, @kaxil - in case it passed the review we can safely merge this one. It still uses the old Github Packages - for now. But with this one we will be able to switch quickly between the two in case of problems.


----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559876284



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"
+        local token=""
+        if [[ "${GITHUB_REGISTRY}" == "ghcr.io" ]]; then
+            # For now ghcr.io can only authenticate using Personal Access Token with package access scope.
+            # There are plans to implement GITHUB_TOKEN authentication but this is not implemented yet
+            token="${CONTAINER_REGISTRY_TOKEN=}"
+            echo
+            echo "Using CONTAINER_REGISTRY_TOKEN!"
+            echo
+        elif [[ "${GITHUB_REGISTRY}" == "docker.pkg.github.com" ]]; then
+            token="${GITHUB_TOKEN}"
+            echo
+            echo "Using GITHUB_TOKEN!"
+            echo
+        else
+            echo
+            echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+            echo
+            exit 1
+        fi
+        if [[ "${token}" == "" ]] ; then

Review comment:
       Surely can do. I sometimes prefer explicit comparision for people who are less familiar with some bash-isms . But yeah, why not.  It sometimes good to change your habits. I corrected it everywhere where I used != "" or == "" (there were not so many places overall. Consistency is important.




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#issuecomment-761782451


   Note ! This change only adds capability to switching to GitHub Container Registry, but it does not enable it yet. 
   
   I am testing GCR in my fork  (for example here https://github.com/potiuk/airflow/actions/runs/491518232) where I changed the GITHUB_REGISTRY to `ghcr.io` as it needs cooperation with Infra, creating a token that can write to the registry, setting it as secret and setting our images to `Public` visibility - all of which might need (still testing) INFRA tickets. 
   
   This change should maintain compatibility with what we are using currently in GitHub packages.


----------------------------------------------------------------
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



[GitHub] [airflow] ashb commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559789756



##########
File path: scripts/ci/libraries/_build_images.sh
##########
@@ -394,30 +394,129 @@ function build_images::get_docker_image_names() {
     # File that is touched when the CI image is built for the first time locally
     export BUILT_CI_IMAGE_FLAG_FILE="${BUILD_CACHE_DIR}/${BRANCH_NAME}/.built_${PYTHON_MAJOR_MINOR_VERSION}"
 
-    # GitHub Registry names must be lowercase :(
-    github_repository_lowercase="$(echo "${GITHUB_REPOSITORY}" | tr '[:upper:]' '[:lower:]')"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}-build"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
-
-    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
-    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${GITHUB_REGISTRY}/${github_repository_lowercase}/python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+    # This is 1-1 mapping of image names of Apache Airflow stored in DockerHub vs. the same images stored
+    # in Github Registries (either Github Container Registry or Github Packages)
+    #
+    # We have to apply naming conventions used by the registries and keep multiple RUN_ID tags. We use
+    # common suffix ('gcr-v1') to be able to switch to different set of cache images if needed
+    # - for example when some images gets broken (might happen with Github Actions Registries) or when
+    # the storage capacity per image is reached (though it is apparently unlimited)
+    #
+    # Some examples:
+    #
+    # In case of Github Container Registry:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow-master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow-master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow-master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all Prod/Build/CI images)
+    # And python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow-python-gcr-v1:3.6-slim-buster-<RUN_ID>"
+    #
+    # In case of Github Packages image must be part of the repository:
+    #
+    # * Prod Image: "apache/airflow:master-python3.8" ->  "apache/airflow/master-python3.8-gcr-v1:<RUN_ID>"
+    # * Prod build image: "apache/airflow:master-python3.8-build" ->  "apache/airflow/master-python3.8-build-gcr-v1:<RUN_ID>"
+    # * CI build image: "apache/airflow:master-python3.8-ci" ->  "apache/airflow/master-python3.8-ci-gcr-v1:<RUN_ID>"
+    #
+    # The python base image/tag mapping is slightly different (the base images are shared by all
+    # Prod/Build/CI images) and python version is part of the tag.
+    #
+    # "apache/airflow:python-3.6 ->  "apache/airflow/python/gcr-v1:3.6-slim-buster-<RUN_ID>"
+
+
+    local image_name
+    image_name="${GITHUB_REGISTRY}/$(get_github_container_registry_image_prefix)"
+    local image_separator
+    if [[ ${GITHUB_REGISTRY} == "ghcr.io" ]]; then
+        image_separator="-"
+    elif [[ ${GITHUB_REGISTRY} == "docker.pkg.github.com" ]]; then
+        image_separator="/"
+    else
+        echo
+        echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+        echo
+        exit 1
+    fi
+
+    export GITHUB_REGISTRY_AIRFLOW_PROD_IMAGE="${image_name}${image_separator}${AIRFLOW_PROD_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_AIRFLOW_PROD_BUILD_IMAGE="${image_name}-${AIRFLOW_PROD_BASE_TAG}${image_separator}build${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
+
+    export GITHUB_REGISTRY_AIRFLOW_CI_IMAGE="${image_name}${image_separator}${AIRFLOW_CI_BASE_TAG}${GITHUB_REGISTRY_IMAGE_SUFFIX}"
+    export GITHUB_REGISTRY_PYTHON_BASE_IMAGE="${image_name}${image_separator}python${GITHUB_REGISTRY_IMAGE_SUFFIX}:${PYTHON_BASE_IMAGE_VERSION}-slim-buster"
 }
 
-# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and GITHUB_TOKEN
-function build_image::login_to_github_registry_if_needed() {
+# If GitHub Registry is used, login to the registry using GITHUB_USERNAME and
+# either GITHUB_TOKEN or CONTAINER_REGISTRY_TOKEN depending on the registry.
+# In case Personal Access token is not set, skip logging in
+function build_image::login_to_github_container_registry_if_github_registry_enabled() {
     if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
-        if [[ -n ${GITHUB_TOKEN=} ]]; then
-            start_end::group_start "Login to GitHub registry"
-            echo "${GITHUB_TOKEN}" | docker login \
+        start_end::group_start "Determine Github Registry token used"
+        local token=""
+        if [[ "${GITHUB_REGISTRY}" == "ghcr.io" ]]; then
+            # For now ghcr.io can only authenticate using Personal Access Token with package access scope.
+            # There are plans to implement GITHUB_TOKEN authentication but this is not implemented yet
+            token="${CONTAINER_REGISTRY_TOKEN=}"
+            echo
+            echo "Using CONTAINER_REGISTRY_TOKEN!"
+            echo
+        elif [[ "${GITHUB_REGISTRY}" == "docker.pkg.github.com" ]]; then
+            token="${GITHUB_TOKEN}"
+            echo
+            echo "Using GITHUB_TOKEN!"
+            echo
+        else
+            echo
+            echo  "${COLOR_RED}ERROR: Bad value of '${GITHUB_REGISTRY}'. Should be either 'ghcr.io' or 'docker.pkg.github.com'!${COLOR_RESET}"
+            echo
+            exit 1
+        fi
+        if [[ "${token}" == "" ]] ; then
+            echo
+            echo "Skip logging in to Github Registry. No Token available!"
+            echo
+        fi
+        start_end::group_end
+        if [[ "${token}" != "" ]]; then
+            start_end::group_start "Login to GitHub Registry ${GITHUB_REGISTRY}"
+            echo "${token}" | docker login \
                 --username "${GITHUB_USERNAME:-apache}" \
                 --password-stdin \
                 "${GITHUB_REGISTRY}"
             start_end::group_end
+        else
+            start_end::group_start "Skip Login to GitHub Registry ${GITHUB_REGISTRY} as token is missing"
+            start_end::group_end
         fi
     fi
 }
 
+function build_image::enable_docker_experimental_features_if_github_registry_enabled() {
+    if [[ ${USE_GITHUB_REGISTRY} == "true" ]]; then
+        start_end::group_start "Enable experimental features in Docker ('docker manifest' command)"
+        python <<EOF
+import json
+from os.path import expanduser, join
+
+DOCKER_CONFIG_FILE=join(expanduser("~"), ".docker", 'config.json')
+with open(DOCKER_CONFIG_FILE) as f:
+    config = json.load(f)
+print("\nBefore:\n")
+print(json.dumps(config, indent=4))
+print("\nAdding experimental flag to enable docker manifest command\n")
+config['experimental'] = "enabled"
+print("\nAfter:\n")
+print(json.dumps(config, indent=4))
+with open(DOCKER_CONFIG_FILE,"wt") as f:
+    json.dump(config, f)
+EOF
+        start_end::group_end

Review comment:
       This is possible with much less code
   
   ```suggestion
              local new_config
              new_config="$( jq '.experimental = "enabled"' ~/.docker/config.json)
              echo "new_config" > ~/.docker/config.json
   ```




----------------------------------------------------------------
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



[GitHub] [airflow] ashb commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
ashb commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559801532



##########
File path: IMAGES.rst
##########
@@ -247,29 +250,92 @@ significant changes are done in the Dockerfile.CI.
 The images are named differently (in Docker definition of image names - registry URL is part of the
 image name if DockerHub is not used as registry). Also GitHub has its own structure for registries
 each project has its own registry naming convention that should be followed. The name of
-images for GitHub registry are:
+images for GitHub registry are different as they must follow limitation of the registry used.
+
+We are still using Github Packages as registry, but we are in the process of testing and switching
+to GitHub Container Registry, and the naming conventions are slightly different (GitHub Packages
+required all packages to have "organization/repository/" URL prefix ("apache/airflow/",
+where in GitHub Container Registry, all images are in "organization" not in "repository" and they are all
+in organization wide "apache/" namespace rather than in "apache/airflow/" one).
+We are adding "airflow-" as prefix for image names of all Airflow images instead.
+The images are linked to the repository via ``org.opencontainers.image.source`` label in the image.
+
+Naming convention for GitHub Packages
+-------------------------------------
+
+Images built as "Run ID snapshot":
+
+.. code-block:: bash
+
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-ci-gcr-v1:<RUNID>    - for CI images
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-gcr-v1:<RUNID>       - for production images
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-build-gcr-v1:<RUNID> - for production build stage
+  ghcr.io/apache/airflow/pythonX.Y-<BRANCH>-gcr-v1:X.Y-slim-buster-<RUN_ID>  - for base python images
+
+Latest images (pushed when master merge succeeds):
 
 .. code-block:: bash
 
-  docker.pkg.github.com/apache/airflow/<BRANCH>-pythonX.Y       - for production images
-  docker.pkg.github.com/apache/airflow/<BRANCH>-pythonX.Y-ci    - for CI images
-  docker.pkg.github.com/apache/airflow/<BRANCH>-pythonX.Y-build - for production build state
-  docker.pkg.github.com/apache/airflow/pythonX.Y-<BRANCH>       - for base python images
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-ci-gcr-v1:latest    - for CI images

Review comment:
       I also don't see in the code where the `gcr` part of the image name comes from -- are the docs incorrect now?




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk commented on a change in pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk commented on a change in pull request #13726:
URL: https://github.com/apache/airflow/pull/13726#discussion_r559873369



##########
File path: IMAGES.rst
##########
@@ -247,29 +250,92 @@ significant changes are done in the Dockerfile.CI.
 The images are named differently (in Docker definition of image names - registry URL is part of the
 image name if DockerHub is not used as registry). Also GitHub has its own structure for registries
 each project has its own registry naming convention that should be followed. The name of
-images for GitHub registry are:
+images for GitHub registry are different as they must follow limitation of the registry used.
+
+We are still using Github Packages as registry, but we are in the process of testing and switching
+to GitHub Container Registry, and the naming conventions are slightly different (GitHub Packages
+required all packages to have "organization/repository/" URL prefix ("apache/airflow/",
+where in GitHub Container Registry, all images are in "organization" not in "repository" and they are all
+in organization wide "apache/" namespace rather than in "apache/airflow/" one).
+We are adding "airflow-" as prefix for image names of all Airflow images instead.
+The images are linked to the repository via ``org.opencontainers.image.source`` label in the image.
+
+Naming convention for GitHub Packages
+-------------------------------------
+
+Images built as "Run ID snapshot":
+
+.. code-block:: bash
+
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-ci-gcr-v1:<RUNID>    - for CI images
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-gcr-v1:<RUNID>       - for production images
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-build-gcr-v1:<RUNID> - for production build stage
+  ghcr.io/apache/airflow/pythonX.Y-<BRANCH>-gcr-v1:X.Y-slim-buster-<RUN_ID>  - for base python images
+
+Latest images (pushed when master merge succeeds):
 
 .. code-block:: bash
 
-  docker.pkg.github.com/apache/airflow/<BRANCH>-pythonX.Y       - for production images
-  docker.pkg.github.com/apache/airflow/<BRANCH>-pythonX.Y-ci    - for CI images
-  docker.pkg.github.com/apache/airflow/<BRANCH>-pythonX.Y-build - for production build state
-  docker.pkg.github.com/apache/airflow/pythonX.Y-<BRANCH>       - for base python images
+  ghcr.io/apache/airflow/<BRANCH>-pythonX.Y-ci-gcr-v1:latest    - for CI images

Review comment:
       Yeah. It was documenting "future" change coming from a follow-up PR I had about switching.
   But yeah "-gcr" can be remved. And since  we are at -v2 now, I iwll leave it with "-v2". 




----------------------------------------------------------------
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



[GitHub] [airflow] potiuk merged pull request #13726: Adds capability of switching to Github Container Registry

Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #13726:
URL: https://github.com/apache/airflow/pull/13726


   


----------------------------------------------------------------
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