-
Notifications
You must be signed in to change notification settings - Fork 10
added s390x support and created github action workflow #8
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: master
Are you sure you want to change the base?
Conversation
|
|
||
| - name: Login to Quay | ||
| run: | | ||
| echo "${{ secrets.QUAY_TOKEN }}" | docker login quay.io -u "${{ secrets.QUAY_USER }}" --password-stdin |
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 is not the best way to go about the login. Please look into using docker/login-action, instead, please
|
|
||
| - name: Build and Push Multi-Arch Image | ||
| run: | | ||
| docker buildx build \ |
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.
There is a makefile with build and push instructions, which uses podman. Is there a reason you're doing the CI build using Docker, instead?
Using the Makefile for build and push would make sure whatever a user builds locally is consistent with what the CI would produce.
|
@hash-d made the changes as requested by you |
22162f6 to
456613d
Compare
hash-d
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.
Rajnish, I have added the user/token secrets, so when this is merged it should be good to push to Quay.
However, I think the on clause is too broad as it is. We do not want to push to quay on every git push. Please take a look at that, and we should be good to merge next.
| @@ -0,0 +1,22 @@ | |||
| name: Build and Push Multi-Arch Image | |||
| on: [push] | |||
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.
With this setting, I believe the workflow will run on any pushes, in any branches. That's probably not what we want. If we were only testing, that would be fine. But we're pushing to Quay, so we want it to run only on successful merges. I believe if you add branches: [main] here we should be good.
Added github workflow action to build multiarch images and added s390x architecture in the Makefile as well
cc: @hash-d