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

Updates to container images #530

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

mtneug
Copy link
Member

@mtneug mtneug commented May 17, 2024

This patch updates the build files for container images and adds a GitHub actions workflow. Further, the registry is changed to ghcr.io. Changes are separated by commit.

Comment on lines +54 to +55
-t "ghcr.io/opencast/pyca:latest" \
-t "ghcr.io/opencast/pyca:main" \
Copy link
Member

Choose a reason for hiding this comment

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

This will create latest and main tagged container images from pull request branches. They aren't pushed, but this might still be confusing.

I think that's what these docker Actions take care of:
https://github.com/opencast/opencast-admin-interface/blob/aec24429505cdd9d12f4587b027ed916a7090c11/.github/workflows/deploy-container-image.yaml#L32-L44

But to be fair, I just copied them from a college who ensured me that this is what I wanted :D

Copy link
Member Author

Choose a reason for hiding this comment

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

As you said, this only tags images within the build environment. In my CI pipelines, I usually tag images with any potential tag and push if necessary. For this reason, I don't use docker/build-push-action directly, as I want to control if and what tags are pushed.

Comment on lines +9 to +11
pull_request:
branches:
- main
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this will work since you are trying to use the GITHUB_TOKEN secret in an environment which is controlled by the pull request author, isn't it? This would need to be pull_request_target.

Copy link
Member Author

Choose a reason for hiding this comment

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

GITHUB_TOKEN is only used in steps with the condition github.event_name == 'push'. The idea of including pull_request is to check if the container image still builds with the PR.

org-name: opencast
image-names: pyca
untagged-only: true
cut-off: 1 day ago UTC
Copy link
Member

Choose a reason for hiding this comment

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

I sometimes reference commits hashes specifically if I need a newer version which is not yet released. Removing them after one day means that it could easily be that you couldn't re-deploy something. What do you think about increasing this to a year?

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

Dockerfile Outdated
&& npm i

RUN pip install --break-system-packages -r requirements.txt
RUN npm i
Copy link
Member

Choose a reason for hiding this comment

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

To make builds reproducible, I suggest

Suggested change
RUN npm i
RUN npm ci

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@mtneug
Copy link
Member Author

mtneug commented Jul 1, 2024

The latest commits also update Alpine and the container-retention-policy action.

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.

2 participants