Skip to content
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

AG-1472: push/pull data images from GHCR #1313

Merged
merged 6 commits into from
Jun 14, 2024

Conversation

hallieswan
Copy link
Contributor

Will create a GHCR package called "agora-data" in the user's account (for forked repos) or in the Sage-Bionetworks organization (for the base repo). See example GHCR package in user account here.

Comment on lines +20 to +24
outputs:
DATA_MANIFEST_ID: ${{ steps.get-ids-from-package-json.outputs.DATA_MANIFEST_ID }}
DATA_VERSION: ${{ steps.get-ids-from-package-json.outputs.DATA_VERSION }}
DATA_IMAGE_PATH: ${{ steps.get-ids-from-package-json.outputs.DATA_IMAGE_PATH }}
IMAGE_EXISTS: ${{ steps.check-container-repo.outputs.IMAGE_EXISTS }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Outputs will be used by subsequent jobs

run: |
echo "==> data image tag: ${{ env.DATA_IMAGE_TAG }}"
echo "IMAGE_EXISTS=false" >> "$GITHUB_ENV"
GH_PATH=$( [[ "${{ env.NAMESPACE }}" == "${{ env.ORG_NAME }}" ]] && echo "${{ env.ORG_PATH }}" || echo "${{ env.USER_PATH }}" )
Copy link
Contributor Author

Choose a reason for hiding this comment

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

If the job is running in the Sage-Bionetworks repo, then check for the image in the organization namespace using the ORG_PATH. Otherwise, check for the image in the user's namespace using the USER_PATH.

Comment on lines +78 to +80
build-image:
needs: check-for-image
if: ${{ needs.check-for-image.result == 'success' && needs.check-for-image.outputs.IMAGE_EXISTS == 'false' }}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The workflow will build the image if it does not already exist in the GHCR package, as in this run.

Comment on lines +164 to +166
# always() prevents skipping this job when build-image was skipped
# see https://github.com/actions/runner/issues/491
if: ${{ always() && needs.check-for-image.result == 'success' && (needs.build-image.result == 'success' || needs.build-image.result == 'skipped')}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The workflow will run the run-e2e-test job even if the image did not need to be built, as in this run.

@hallieswan hallieswan marked this pull request as ready for review June 14, 2024 00:15
@hallieswan hallieswan requested a review from sagely1 June 14, 2024 00:15
@@ -1,5 +1,9 @@
FROM mongo:latest

# GHCR labels
ARG SOURCE_REPO="https://github.com/Sage-Bionetworks/agora"
Copy link
Contributor

Choose a reason for hiding this comment

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

to make it more reusable for other repos consider using:
ARG SOURCE_REPO="https://github.com/${{ github.repository }}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed on reusability! I've tried to do that by making SOURCE_REPO a build-time ARG, so it can be overwritten when the Docker image is built. The current value is a default that primarily demonstrates what SOURCE_REPO should look like.

In the GHA workflow, we override the value at build-time here, but I agree that using ${{ github.repository }} would be clearer, so I'll update to use that instead!

# TODO - push image to remote repository

# Note: if a new package is created using this workflow, then the package
# visibility will default to the source repository visbility. For example,
Copy link
Contributor

Choose a reason for hiding this comment

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

typo:visibility

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the catch! I'll update in the next commit.

Copy link
Contributor

@sagely1 sagely1 left a comment

Choose a reason for hiding this comment

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

Looks great! I left some optional feedback.

@hallieswan hallieswan requested a review from sagely1 June 14, 2024 18:44
Copy link
Contributor

@sagely1 sagely1 left a comment

Choose a reason for hiding this comment

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

Looks better than it already did!

@hallieswan hallieswan merged commit ea0d1f4 into Sage-Bionetworks:develop Jun 14, 2024
3 checks passed
@hallieswan hallieswan deleted the AG-1472 branch June 14, 2024 19:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants