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

Feature: matchbox_server docker image GitHub action #420

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

AgustinRamiroDiaz
Copy link
Contributor

@AgustinRamiroDiaz AgustinRamiroDiaz commented Feb 18, 2024

Part of #372

What

Adds this github action configured for this repo

Testing done

You can see the triggers of my tests

The pushed images are here

Configuration from your side

Requires creating an access token in Docker Hub, and configuring the secret DOCKERHUB_TOKEN and the variable DOCKERHUB_USER in this repo

image

image

Signed-off-by: Agustín Ramiro Díaz <[email protected]>
Signed-off-by: Agustín Ramiro Díaz <[email protected]>
Signed-off-by: Agustín Ramiro Díaz <[email protected]>
Signed-off-by: Agustín Ramiro Díaz <[email protected]>
@simbleau
Copy link
Collaborator

Wow, thanks for this!

@johanhelsing
Copy link
Owner

We are already building and pushing images to the github container registry and then manually pushing to dockerhub.

With this PR, we'd build the server image twice, though. Would be good if there was just one image built per version. Also not sure if continuing to push to ghcr.io is valuable or not.

Maybe we should just choose one container registry, and either drop docker or ghcr? Either way, getting rid of the manual steps would be great.

@johanhelsing johanhelsing added the ci Only relevant for CI label Mar 9, 2024
@AgustinRamiroDiaz
Copy link
Contributor Author

@johanhelsing oh I haven't seen that before, it's under the code workflow.

I can do whatever you think it's best. We could merge the 2 workflows into the code one and keep building + pushing to both registries there, or if you plan on dropping support we can also move to the current implementation. Let me know so I update the PR :D

@johanhelsing
Copy link
Owner

Thanks, I think it's best to push to both registries, if not too complicated. I think the image should build on all pr runs, but only push on tags.

I think the push on tag path might be broken, perhaps the workflow is not triggering on tags anymore... at least I had to do it manually last release.

Thanks for working on this!

@AgustinRamiroDiaz AgustinRamiroDiaz marked this pull request as draft March 28, 2024 15:37
@AgustinRamiroDiaz
Copy link
Contributor Author

AgustinRamiroDiaz commented Mar 28, 2024

I think the push on tag path might be broken, perhaps the workflow is not triggering on tags anymore... at least I had to do it manually last release.

@johanhelsing I think that was because tags were not taken into account in the workflow triggers. I'll fix that too in this PR

Thanks for working on this!

You're welcome :)

@@ -145,11 +146,19 @@ jobs:
username: ${{ github.repository_owner }}
password: ${{ secrets.GITHUB_TOKEN }}

- name: Login to Docker Hub
uses: docker/login-action@v3
Copy link
Owner

Choose a reason for hiding this comment

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

I think at least some of these steps should have:

if: startsWith(github.ref, 'refs/tags/')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Only relevant for CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants