Skip to content

Update Docker workflow for permissions and manifests #1080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
sbryngelson wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Update Docker workflow for permissions and manifests #1080

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 64 additions & 12 deletions .github/workflows/docker.yml
View file
Open in desktop
Copy link
Contributor

@qodo-code-review qodo-code-review bot Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

High-level Suggestion

The manifest creation script in the manifests job contains duplicated logic for Docker Hub and GHCR. This should be refactored into a loop over the registries to improve readability and maintainability. [High-level, importance: 7]

Solution Walkthrough:

Before:

# Create and push for Docker Hub
docker buildx imagetools create -t $DH_REGISTRY:latest-cpu ...
docker manifest create $DH_REGISTRY:$TAG-gpu ...
docker manifest create $DH_REGISTRY:latest-gpu ...
docker manifest push $DH_REGISTRY:$TAG-gpu
docker manifest push $DH_REGISTRY:latest-gpu

# Create and push for GHCR (duplication)
docker buildx imagetools create -t $GH_REGISTRY:latest-cpu ...
docker manifest create $GH_REGISTRY:$TAG-gpu ...
docker manifest create $GH_REGISTRY:latest-gpu ...
docker manifest push $GH_REGISTRY:$TAG-gpu
docker manifest push $GH_REGISTRY:latest-gpu

After:

for REGISTRY in $DH_REGISTRY $GH_REGISTRY; do
  docker buildx imagetools create -t $REGISTRY:latest-cpu $REGISTRY:$TAG-cpu

  docker manifest create $REGISTRY:$TAG-gpu \
    $REGISTRY:$TAG-gpu-ubuntu-22.04 \
    $REGISTRY:$TAG-gpu-ubuntu-22.04-arm

  docker manifest create $REGISTRY:latest-gpu \
    $REGISTRY:$TAG-gpu-ubuntu-22.04 \
    $REGISTRY:$TAG-gpu-ubuntu-22.04-arm

  docker manifest push $REGISTRY:$TAG-gpu
  docker manifest push $REGISTRY:latest-gpu
done

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ on:
description: 'tag to containerize'
required: true

permissions:
contents: read
packages: write

concurrency:
group: Containerization
cancel-in-progress: false
Expand All @@ -24,6 +28,7 @@ jobs:
runs-on: ${{ matrix.config.runner }}
outputs:
tag: ${{ steps.clone.outputs.tag }}

steps:
- name: Free Disk Space
uses: jlumbroso/free-disk-space@main
Expand All @@ -36,12 +41,20 @@ jobs:
docker-images: true
swap-storage: true

- name: Login
# ----- Logins -----
- name: Login to Docker Hub
uses: docker/login-action@v3
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_PASSWORD }}

- name: Login to GHCR
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Setup Buildx
uses: docker/setup-buildx-action@v3

Expand Down Expand Up @@ -89,12 +102,16 @@ jobs:
FC_COMPILER=${{ 'gfortran' }}
COMPILER_PATH=${{ '/usr/bin' }}
COMPILER_LD_LIBRARY_PATH=${{ '/usr/lib' }}
tags: ${{ secrets.DOCKERHUB_USERNAME }}/mfc:${{ env.TAG }}-${{ matrix.config.name }}
labels: |
org.opencontainers.image.source=https://github.com/${{ github.repository }}
tags: |
${{ secrets.DOCKERHUB_USERNAME }}/mfc:${{ env.TAG }}-${{ matrix.config.name }}
ghcr.io/${{ github.repository_owner }}/mfc:${{ env.TAG }}-${{ matrix.config.name }}
Comment on lines +107 to +109
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Normalize GHCR image names to lowercase

The GHCR tags are built from ${{ github.repository_owner }} as-is (e.g., ghcr.io/${{ github.repository_owner }}/mfc:${{ env.TAG }}-${{ matrix.config.name }}), but the owner here is MFlowCode with capital letters. Docker/OCI image references must be lowercase, so this produces an invalid reference and the GHCR pushes/manifests will fail with “repository name must be lowercase” whenever the workflow runs.

Useful? React with 👍 / 👎.

push: true

- name: Build and push image (gpu)
if: ${{ matrix.config.name == 'gpu' }}
uses: docker/build-push-action@v5
uses: docker/build-push-action@v6
with:
builder: default
context: /mnt/share
Expand All @@ -107,26 +124,61 @@ jobs:
FC_COMPILER=${{ 'nvfortran' }}
COMPILER_PATH=${{ '/opt/nvidia/hpc_sdk/Linux_x86_64/compilers/bin' }}
COMPILER_LD_LIBRARY_PATH=${{ '/opt/nvidia/hpc_sdk/Linux_x86_64/compilers/lib' }}
tags: ${{ secrets.DOCKERHUB_USERNAME }}/mfc:${{ env.TAG }}-${{ matrix.config.name }}-${{ matrix.config.runner}}
labels: |
org.opencontainers.image.source=https://github.com/${{ github.repository }}
tags: |
${{ secrets.DOCKERHUB_USERNAME }}/mfc:${{ env.TAG }}-${{ matrix.config.name }}-${{ matrix.config.runner}}
ghcr.io/${{ github.repository_owner }}/mfc:${{ env.TAG }}-${{ matrix.config.name }}-${{ matrix.config.runner}}
Comment on lines +129 to +131
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Reconsider GPU image tag naming strategy.

Using the full runner name (ubuntu-22.04-arm) as part of the image tag creates implementation-specific, verbose identifiers. Image tags should reflect semantic versioning or architecture, not the CI runner environment.

Current tags like mfc:v1.0-gpu-ubuntu-22.04-arm are opaque to end users.

Consider one of these alternatives:

  • Use architecture-neutral semantic tags (e.g., mfc:v1.0-gpu) and rely on manifest multi-arch resolution
  • Use explicit architecture suffixes if per-architecture access is needed (e.g., mfc:v1.0-gpu-amd64, mfc:v1.0-gpu-arm64)
  • Include only the Ubuntu version, not the runner architecture (e.g., mfc:v1.0-gpu-ubuntu22.04)

If you want to maintain per-runner image artifacts for debugging or pinning specific builds, consider using a different approach such as:

  • Storing runner-specific metadata in image labels instead of tags
  • Using separate tag namespaces (e.g., mfc:debug-ubuntu-22.04-arm-v1.0)
  • Documenting the expected tag format for users
🤖 Prompt for AI Agents 
.github/workflows/docker.yml around lines 129-131: the current tag uses the full
runner name (e.g., ubuntu-22.04-arm) which is verbose and CI-implementation
specific; change the tagging strategy to emit semantic/architecture-friendly
tags instead (recommended: ${env.TAG}-gpu for multi-arch manifests or
${env.TAG}-gpu-amd64 / ${env.TAG}-gpu-arm64 when per-arch images are required),
derive architecture from a matrix variable like matrix.config.arch or map the
runner to amd64/arm64, and keep runner-specific info out of the tag by adding it
to image labels or a separate debug tag/namespace (e.g., debug-ubuntu-22.04-arm)
if needed; update the workflow tag lines to use the new tag variable and add
labels/metadata steps to record runner details.

push: true

manifests:
runs-on: ubuntu-latest
needs: Container
steps:
- name: Login
- name: Login to Docker Hub
uses: docker/login-action@v3
with:
username: ${{ secrets.DOCKERHUB_USERNAME }}
password: ${{ secrets.DOCKERHUB_PASSWORD }}

- name: Create and Push Manifest Lists
- name: Login to GHCR
uses: docker/login-action@v3
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Create and Push Manifest Lists (Docker Hub + GHCR)
env:
TAG: ${{ needs.Container.outputs.tag }}
REGISTRY: ${{ secrets.DOCKERHUB_USERNAME }}/mfc
DH_REGISTRY: ${{ secrets.DOCKERHUB_USERNAME }}/mfc
GH_REGISTRY: ghcr.io/${{ github.repository_owner }}/mfc
run: |
docker buildx imagetools create -t $REGISTRY:latest-cpu $REGISTRY:$TAG-cpu
docker manifest create $REGISTRY:$TAG-gpu $REGISTRY:$TAG-gpu-ubuntu-22.04 $REGISTRY:$TAG-gpu-ubuntu-22.04-arm
docker manifest create $REGISTRY:latest-gpu $REGISTRY:$TAG-gpu-ubuntu-22.04 $REGISTRY:$TAG-gpu-ubuntu-22.04-arm
docker manifest push $REGISTRY:$TAG-gpu
docker manifest push $REGISTRY:latest-gpu
# ---- CPU multi-arch "latest-cpu" from the already pushed $TAG-cpu ----
docker buildx imagetools create -t $DH_REGISTRY:latest-cpu $DH_REGISTRY:$TAG-cpu
docker buildx imagetools create -t $GH_REGISTRY:latest-cpu $GH_REGISTRY:$TAG-cpu

# ---- GPU manifests across the two runners ----
# Tag these for versioned + latest GPU
docker manifest create $DH_REGISTRY:$TAG-gpu \
$DH_REGISTRY:$TAG-gpu-ubuntu-22.04 \
$DH_REGISTRY:$TAG-gpu-ubuntu-22.04-arm

docker manifest create $DH_REGISTRY:latest-gpu \
$DH_REGISTRY:$TAG-gpu-ubuntu-22.04 \
$DH_REGISTRY:$TAG-gpu-ubuntu-22.04-arm

docker manifest push $DH_REGISTRY:$TAG-gpu
docker manifest push $DH_REGISTRY:latest-gpu

# GHCR equivalent
docker manifest create $GH_REGISTRY:$TAG-gpu \
$GH_REGISTRY:$TAG-gpu-ubuntu-22.04 \
$GH_REGISTRY:$TAG-gpu-ubuntu-22.04-arm

docker manifest create $GH_REGISTRY:latest-gpu \
$GH_REGISTRY:$TAG-gpu-ubuntu-22.04 \
$GH_REGISTRY:$TAG-gpu-ubuntu-22.04-arm

docker manifest push $GH_REGISTRY:$TAG-gpu
docker manifest push $GH_REGISTRY:latest-gpu