-
Notifications
You must be signed in to change notification settings - Fork 4
feat: add GHCR support for container registry #48
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.
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
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Gabor Boros <gabor@opencraft.com>
magajh
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for this contribution!
Regarding the “Show image name for service” part, we’re planning to apply a fix in the upcoming sprint so that the image tag push doesn’t fail (in the dynamic tag case) when multiple builds are running or when there’s a push to the repository in the middle of a build. That said, the change you’re proposing here still feels very useful as a safeguard for edge cases where updating the tag in the repo fails for some reason and the user still needs to see which tag was generated
| if: ${{ contains(env.DOCKER_REGISTRY, 'aws') }} | ||
| uses: aws-actions/amazon-ecr-login@v2 | ||
|
|
||
| - name: Login to GitHub Container Registry if Docker registry is GitHub Container Registry |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good, I think we’re only missing a short doc update
It would be great if we could mention somewhere that the workflow now supports using GitHub Container Registry as a Docker registry and that, by default, it relies on the GITHUB_TOKEN generated by GitHub. My understanding is that the only requirement on the caller side is to invoke the Picasso workflow with the appropriate permissions (like packages: write), right?
Maybe we could add this under the Key features section in the Workflow Overview, so users can easily see that GHCR is supported and what is needed to use it
| # Determine the target key for the service | ||
| if [ -z "$TARGET_KEY" ]; then | ||
| # If TARGET_KEY is not set (static image tag scenario), determine it from service | ||
| TARGET_KEY=$(python picasso/.github/workflows/scripts/get_service_target_key.py --service ${{ inputs.SERVICE }}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gabor-boros I tested this in the static-tag scenario (USE_DYNAMIC_IMAGE_TAG = false) and the Set job outputs for image tag and name step is failing with:
Run # Determine the target key for the service
python: can't open file '/home/runner/work/ednx-strains/ednx-strains/strains/teak/base/picasso/.github/workflows/scripts/get_service_target_key.py': [Errno 2] No such file or directory
This seems to be related to the job defaults
defaults:
run:
working-directory: strains/${{ inputs.STRAIN_PATH }}This command python picasso/.github/workflows/scripts/get_service_target_key.py --service ${{ inputs.SERVICE }} ends up resolving to strains/<path>/picasso/..., but the picasso repo is actually checked out at ${{ github.workspace }}/picasso
I think to fix this we could either set working-directory: ${{ github.workspace }} for this step, or use an absolute path to the script
This PR adds GHCR support for the container registry, so GitHub can be used over DockerHub or AWS ECR. Also, the PR sets the generated images as job outputs, so the images can be referenced in later steps if needed.
The later one is particularly useful when someone wants to build a pipeline where service build jobs are running parallel and the
config.ymlfile should be updated at once.