You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ma...@apache.org on 2024/02/09 04:58:42 UTC

(superset) branch master updated: feat(docker): allow for docker release builds to be multi-platform (#27055)

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 13915bbb54 feat(docker): allow for docker release builds to be multi-platform (#27055)
13915bbb54 is described below

commit 13915bbb54e017456f4a68bc432a796ad9bcfe5a
Author: Maxime Beauchemin <ma...@gmail.com>
AuthorDate: Thu Feb 8 20:58:36 2024 -0800

    feat(docker): allow for docker release builds to be multi-platform (#27055)
---
 .github/workflows/docker-release.yml     | 16 +++------
 .github/workflows/ephemeral-env.yml      |  7 ++--
 scripts/build_docker.py                  | 42 +++++++++++++++-------
 tests/unit_tests/scripts/docker_build.py | 60 +++++++++++++++++++-------------
 4 files changed, 72 insertions(+), 53 deletions(-)

diff --git a/.github/workflows/docker-release.yml b/.github/workflows/docker-release.yml
index 3ba7b6bf42..73f4133848 100644
--- a/.github/workflows/docker-release.yml
+++ b/.github/workflows/docker-release.yml
@@ -2,7 +2,7 @@ name: Docker Publish Release
 
 on:
   release:
-    types: [published]
+    types: [published, edited]
 
   # Can be triggered manually
   workflow_dispatch:
@@ -43,14 +43,6 @@ jobs:
     strategy:
       matrix:
         build_preset: ["dev", "lean", "py310", "websocket", "dockerize"]
-        platform: ["linux/amd64", "linux/arm64"]
-        exclude:
-          # disabling because slow! no python wheels for arm/py39 and
-          # QEMU is slow!
-          - build_preset: "dev"
-            platform: "linux/arm64"
-          - build_preset: "lean"
-            platform: "linux/arm64"
       fail-fast: false
     steps:
       - name: "Checkout ${{ github.ref }} ( ${{ github.sha }} )"
@@ -88,8 +80,10 @@ jobs:
             EVENT="release"
           fi
           pip install click
+          # Make a multi-platform image
           ./scripts/build_docker.py \
             ${{ matrix.build_preset }} \
             "$EVENT" \
-            --build_context_ref "$RELEASE" \
-            --platform ${{ matrix.platform }} $FORCE_LATEST
+            --build_context_ref "$RELEASE" $FORCE_LATEST \
+            --platform "linux/arm64" \
+            --platform "linux/amd64"
diff --git a/.github/workflows/ephemeral-env.yml b/.github/workflows/ephemeral-env.yml
index 8cc7966e00..8f804f741c 100644
--- a/.github/workflows/ephemeral-env.yml
+++ b/.github/workflows/ephemeral-env.yml
@@ -143,10 +143,9 @@ jobs:
         env:
           ECR_REGISTRY: ${{ steps.login-ecr.outputs.registry }}
           ECR_REPOSITORY: superset-ci
-          SHA: ${{ steps.get-sha.outputs.sha }}
           IMAGE_TAG: apache/superset:${{ steps.get-sha.outputs.sha }}-ci
         run: |
-          docker tag $IMAGE_TAG $ECR_REGISTRY/$ECR_REPOSITORY:$SHA
+          docker tag $IMAGE_TAG $ECR_REGISTRY/$ECR_REPOSITORY:pr-${{ github.event.issue.number }}-ci
           docker push -a $ECR_REGISTRY/$ECR_REPOSITORY
 
   ephemeral-env-up:
@@ -181,7 +180,7 @@ jobs:
         aws ecr describe-images \
         --registry-id $(echo "${{ steps.login-ecr.outputs.registry }}" | grep -Eo "^[0-9]+") \
         --repository-name superset-ci \
-        --image-ids imageTag=${{ steps.get-sha.outputs.sha }}
+        --image-ids imageTag=pr-${{ github.event.issue.number }}-ci
 
     - name: Fail on missing container image
       if: steps.check-image.outcome == 'failure'
@@ -204,7 +203,7 @@ jobs:
       with:
         task-definition: .github/workflows/ecs-task-definition.json
         container-name: superset-ci
-        image: ${{ steps.login-ecr.outputs.registry }}/superset-ci:pr-${{ github.event.issue.number }}
+        image: ${{ steps.login-ecr.outputs.registry }}/superset-ci:pr-${{ github.event.issue.number }}-ci
 
     - name: Update env vars in the Amazon ECS task definition
       run: |
diff --git a/scripts/build_docker.py b/scripts/build_docker.py
index de6b8444f8..2a9e676d92 100755
--- a/scripts/build_docker.py
+++ b/scripts/build_docker.py
@@ -40,6 +40,10 @@ def run_cmd(command: str) -> str:
             output += line
 
     process.wait()  # Wait for the subprocess to finish
+
+    if process.returncode != 0:
+        raise subprocess.CalledProcessError(process.returncode, command, output)
+
     return output
 
 
@@ -79,7 +83,7 @@ def make_docker_tag(l: list[str]) -> str:
 
 def get_docker_tags(
     build_preset: str,
-    build_platform: str,
+    build_platforms: list[str],
     sha: str,
     build_context: str,
     build_context_ref: str,
@@ -91,17 +95,18 @@ def get_docker_tags(
     tags: set[str] = set()
     tag_chunks: list[str] = []
 
-    short_build_platform = build_platform.replace("linux/", "").replace("64", "")
-
     is_latest = is_latest_release(build_context_ref)
 
     if build_preset != "lean":
         # Always add the preset_build name if different from default (lean)
         tag_chunks += [build_preset]
 
-    if short_build_platform != "amd":
-        # Always a platform indicator if different from default (amd)
-        tag_chunks += [short_build_platform]
+    if len(build_platforms) == 1:
+        build_platform = build_platforms[0]
+        short_build_platform = build_platform.replace("linux/", "").replace("64", "")
+        if short_build_platform != "amd":
+            # Always a platform indicator if different from default (amd)
+            tag_chunks += [short_build_platform]
 
     # Always craft a tag for the SHA
     tags.add(make_docker_tag([sha] + tag_chunks))
@@ -123,7 +128,7 @@ def get_docker_tags(
 
 def get_docker_command(
     build_preset: str,
-    build_platform: str,
+    build_platforms: list[str],
     is_authenticated: bool,
     sha: str,
     build_context: str,
@@ -160,7 +165,7 @@ def get_docker_command(
 
     tags = get_docker_tags(
         build_preset,
-        build_platform,
+        build_platforms,
         sha,
         build_context,
         build_context_ref,
@@ -170,8 +175,14 @@ def get_docker_command(
 
     docker_args = "--load" if not is_authenticated else "--push"
     target_argument = f"--target {build_target}" if build_target else ""
-    short_build_platform = build_platform.replace("linux/", "").replace("64", "")
-    cache_ref = f"{CACHE_REPO}:{py_ver}-{short_build_platform}"
+
+    cache_ref = f"{CACHE_REPO}:{py_ver}"
+    if len(build_platforms) == 1:
+        build_platform = build_platforms[0]
+        short_build_platform = build_platform.replace("linux/", "").replace("64", "")
+        cache_ref = f"{CACHE_REPO}:{py_ver}-{short_build_platform}"
+    platform_arg = "--platform " + ",".join(build_platforms)
+
     cache_from_arg = f"--cache-from=type=registry,ref={cache_ref}"
     cache_to_arg = (
         f"--cache-to=type=registry,mode=max,ref={cache_ref}" if is_authenticated else ""
@@ -187,7 +198,7 @@ def get_docker_command(
         {cache_from_arg} \\
         {cache_to_arg} \\
         {build_arg} \\
-        --platform {build_platform} \\
+        {platform_arg} \\
         --label sha={sha} \\
         --label target={build_target} \\
         --label build_trigger={build_context} \\
@@ -206,10 +217,12 @@ def get_docker_command(
 @click.option(
     "--platform",
     type=click.Choice(["linux/arm64", "linux/amd64"]),
-    default="linux/amd64",
+    default=["linux/amd64"],
+    multiple=True,
 )
 @click.option("--build_context_ref", help="a reference to the pr, release or branch")
 @click.option("--dry-run", is_flag=True, help="Run the command in dry-run mode.")
+@click.option("--verbose", is_flag=True, help="Print more info")
 @click.option(
     "--force-latest", is_flag=True, help="Force the 'latest' tag on the release"
 )
@@ -217,9 +230,10 @@ def main(
     build_preset: str,
     build_context: str,
     build_context_ref: str,
-    platform: str,
+    platform: list[str],
     dry_run: bool,
     force_latest: bool,
+    verbose: bool,
 ) -> None:
     """
     This script executes docker build and push commands based on given arguments.
@@ -262,6 +276,8 @@ def main(
                 """
             )
         script = script + docker_build_command
+        if verbose:
+            run_cmd("cat Dockerfile")
         stdout = run_cmd(script)
     else:
         print("Dry Run - Docker Build Command:")
diff --git a/tests/unit_tests/scripts/docker_build.py b/tests/unit_tests/scripts/docker_build.py
index ee9ad66ead..1f3fd708f3 100644
--- a/tests/unit_tests/scripts/docker_build.py
+++ b/tests/unit_tests/scripts/docker_build.py
@@ -56,12 +56,12 @@ def test_is_latest_release(release, expected_bool):
 
 
 @pytest.mark.parametrize(
-    "build_preset, build_platform, sha, build_context, build_context_ref, expected_tags",
+    "build_preset, build_platforms, sha, build_context, build_context_ref, expected_tags",
     [
         # PRs
         (
             "lean",
-            "linux/arm64",
+            ["linux/arm64"],
             SHA,
             "pull_request",
             PR_ID,
@@ -69,7 +69,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "ci",
-            "linux/amd64",
+            ["linux/amd64"],
             SHA,
             "pull_request",
             PR_ID,
@@ -77,7 +77,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "lean",
-            "linux/amd64",
+            ["linux/amd64"],
             SHA,
             "pull_request",
             PR_ID,
@@ -85,7 +85,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "dev",
-            "linux/arm64",
+            ["linux/arm64"],
             SHA,
             "pull_request",
             PR_ID,
@@ -97,7 +97,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "dev",
-            "linux/amd64",
+            ["linux/amd64"],
             SHA,
             "pull_request",
             PR_ID,
@@ -106,7 +106,7 @@ def test_is_latest_release(release, expected_bool):
         # old releases
         (
             "lean",
-            "linux/arm64",
+            ["linux/arm64"],
             SHA,
             "release",
             OLD_REL,
@@ -114,7 +114,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "lean",
-            "linux/amd64",
+            ["linux/amd64"],
             SHA,
             "release",
             OLD_REL,
@@ -122,7 +122,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "dev",
-            "linux/arm64",
+            ["linux/arm64"],
             SHA,
             "release",
             OLD_REL,
@@ -134,7 +134,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "dev",
-            "linux/amd64",
+            ["linux/amd64"],
             SHA,
             "release",
             OLD_REL,
@@ -143,7 +143,7 @@ def test_is_latest_release(release, expected_bool):
         # new releases
         (
             "lean",
-            "linux/arm64",
+            ["linux/arm64"],
             SHA,
             "release",
             NEW_REL,
@@ -156,7 +156,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "lean",
-            "linux/amd64",
+            ["linux/amd64"],
             SHA,
             "release",
             NEW_REL,
@@ -164,7 +164,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "dev",
-            "linux/arm64",
+            ["linux/arm64"],
             SHA,
             "release",
             NEW_REL,
@@ -177,7 +177,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "dev",
-            "linux/amd64",
+            ["linux/amd64"],
             SHA,
             "release",
             NEW_REL,
@@ -191,7 +191,7 @@ def test_is_latest_release(release, expected_bool):
         # merge on master
         (
             "lean",
-            "linux/arm64",
+            ["linux/arm64"],
             SHA,
             "push",
             "master",
@@ -199,7 +199,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "lean",
-            "linux/amd64",
+            ["linux/amd64"],
             SHA,
             "push",
             "master",
@@ -207,7 +207,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "dev",
-            "linux/arm64",
+            ["linux/arm64"],
             SHA,
             "push",
             "master",
@@ -219,7 +219,7 @@ def test_is_latest_release(release, expected_bool):
         ),
         (
             "dev",
-            "linux/amd64",
+            ["linux/amd64"],
             SHA,
             "push",
             "master",
@@ -228,21 +228,21 @@ def test_is_latest_release(release, expected_bool):
     ],
 )
 def test_get_docker_tags(
-    build_preset, build_platform, sha, build_context, build_context_ref, expected_tags
+    build_preset, build_platforms, sha, build_context, build_context_ref, expected_tags
 ):
     tags = docker_utils.get_docker_tags(
-        build_preset, build_platform, sha, build_context, build_context_ref
+        build_preset, build_platforms, sha, build_context, build_context_ref
     )
     for tag in expected_tags:
         assert tag in tags
 
 
 @pytest.mark.parametrize(
-    "build_preset, build_platform, is_authenticated, sha, build_context, build_context_ref, contains",
+    "build_preset, build_platforms, is_authenticated, sha, build_context, build_context_ref, contains",
     [
         (
             "lean",
-            "linux/amd64",
+            ["linux/amd64"],
             True,
             SHA,
             "push",
@@ -251,18 +251,28 @@ def test_get_docker_tags(
         ),
         (
             "dev",
-            "linux/amd64",
+            ["linux/amd64"],
             False,
             SHA,
             "push",
             "master",
             ["--load", f"-t {REPO}:master-dev "],
         ),
+        # multi-platform
+        (
+            "lean",
+            ["linux/arm64", "linux/amd64"],
+            True,
+            SHA,
+            "push",
+            "master",
+            [f"--platform linux/arm64,linux/amd64"],
+        ),
     ],
 )
 def test_get_docker_command(
     build_preset,
-    build_platform,
+    build_platforms,
     is_authenticated,
     sha,
     build_context,
@@ -271,7 +281,7 @@ def test_get_docker_command(
 ):
     cmd = docker_utils.get_docker_command(
         build_preset,
-        build_platform,
+        build_platforms,
         is_authenticated,
         sha,
         build_context,