Update Docker workflow for permissions and manifests #1080
Conversation
Added permissions for Docker actions and updated login steps for GHCR. Enhanced manifest creation for multi-architecture images.
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe GitHub Actions Docker workflow is enhanced to support multi-registry publishing, adding authentication and build logic for both Docker Hub and GHCR. The workflow upgrades build-push-action to v6, extends image tagging strategy, and implements separate manifest creation and push operations across both registries. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
codeant-ai
bot
added
the
size:M
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
There was a problem hiding this comment.
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
|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
Pull request overview
This PR adds GitHub Container Registry (GHCR) as a secondary container registry alongside the existing Docker Hub deployment. The changes enable the workflow to push container images to both registries simultaneously while maintaining proper permissions for GitHub Packages.
Key changes:
- Added workflow-level permissions for
contents: readandpackages: writeto enable GHCR publishing - Introduced dual registry authentication (Docker Hub + GHCR) in both Container and manifests jobs
- Updated image build steps to tag and push to both Docker Hub and GHCR with OCI labels
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| tags: | | ||
| ${{ secrets.DOCKERHUB_USERNAME }}/mfc:${{ env.TAG }}-${{ matrix.config.name }} | ||
| ghcr.io/${{ github.repository_owner }}/mfc:${{ env.TAG }}-${{ matrix.config.name }} |
There was a problem hiding this comment.
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 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/docker.yml (2)
12-14: Consider removing unnecessarycontents: readpermission.The workflow clones the repository via URL and doesn't read repository content via Git operations, so this permission may be unnecessary. However, it's harmless and could be kept defensively if future changes might require it.
151-184: Manifest creation approach works but has consistency gaps.The workflow uses two different manifest creation methods:
- CPU:
docker buildx imagetools create(modern, buildx-native)- GPU:
docker manifest create+docker manifest push(lower-level API)Both approaches work because CPU builds are multi-arch via buildx, while GPU builds are single-arch per runner. However, this inconsistency could be confusing for maintenance.
Consider:
- Adding explicit image availability checks before manifest operations to catch missing images early:
docker pull $DH_REGISTRY:$TAG-gpu-ubuntu-22.04
Documenting why two different methods are used in the workflow, especially since the GPU multi-runner manifest relies on the images being simultaneously available in the registry.
Optionally unifying the approach by using
docker buildx imagetools createfor GPU images as well (once images are pushed), which would be more consistent.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/docker.yml(5 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Detect File Changes
- GitHub Check: Build & Publish
🔇 Additional comments (2)
.github/workflows/docker.yml (2)
51-56: GHCR login configuration is correct.The GHCR login properly uses
github.actorandGITHUB_TOKEN, which is the standard approach for GitHub Container Registry authentication.
105-109: Multi-registry tag strategy is well-structured.Publishing to both Docker Hub and GHCR in a single build step via extended tags is efficient. The OCI image source label is a good addition for discoverability.
| 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}} |
There was a problem hiding this comment.
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.
|
@sbryngelson Why, not needed at all? MFC containers are just massive to be hosted by GitHub Registry. |
|
It would be better honestly to re-orient and push MFC (OpenMP) Images, as they should have existed since ~v5.0.6. They can go as a separate image with its own manifest. The only was to use OMP containers is by grabbing existing ACC ones for any release and re-building with OpenMP config which is not the best way for the long run. Will manually push OMP Images next week as I need them anyways. |
|
thank you for your suggestions |
2 participants
User description
User description
Add github container deployment
PR Type
Enhancement
Description
Add GitHub Container Registry (GHCR) deployment alongside Docker Hub
Implement proper workflow permissions for container operations
Enhance manifest creation with multi-registry support
Add OCI image labels for source repository tracking
Diagram Walkthrough
File Walkthrough
docker.yml
Add GHCR support and enhance multi-registry deployment.github/workflows/docker.yml
permissionsblock withcontents: readandpackages: writeforsecure GitHub Actions execution
github.actorandGITHUB_TOKENcredentialsOCI labels
registries with CPU and GPU variants
CodeAnt-AI Description
Publish multi-arch CPU/GPU images and manifests to Docker Hub and GHCR
What Changed
Impact
✅ Dual registry image publishing (Docker Hub + GHCR)✅ Multi-arch CPU and GPU images available in both registries✅ Images traceable to source repository via OCI label💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.