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

Build for ARM (ARM64 + ARMv7) #109

Merged
merged 11 commits into from Aug 6, 2020
Merged

Build for ARM (ARM64 + ARMv7) #109

merged 11 commits into from Aug 6, 2020

Conversation

klausenbusk
Copy link
Contributor

With this commit we switch from "Automated Builds"[1] to using a GitHub
Actions for building the Docker images. This makes it possible to use
buildx[2] and build the images for other platforms besides amd64.

The workflow is only triggered when a tag is pushed and the latest tag
(latest or alpine) is only pushed when the tag match the following
regex: "^[0-9].[0-9].[0-9]*$".

[1] https://docs.docker.com/docker-hub/builds/
[2] https://github.com/docker/buildx

Fix #99


@xPaw automated builds needs to be disabled, and you need to create the secret DOCKER_USERNAME and DOCKER_PASSWORD (you should use a access token).

@xPaw
Copy link
Member

xPaw commented Jul 28, 2020

That docker login and echoing password is a bit sketchy, is there no better way?

Did you copy this workflow from somewhere (for reference)?

EDIT: It probably would be nice to test this workflow without actually pushing to hub, to see that it still builds.

@klausenbusk
Copy link
Contributor Author

That docker login and echoing password is a bit sketchy, is there no better way?

Not that I'm aware, but echo is a builtin so it shouldn't show up in the process list. I could change it to <<< "${DOCKER_PASSWORD} as I do the other places (if).

Did you copy this workflow from somewhere (for reference)?

I was inspired by: https://github.com/radicand/docker-mailserver/blob/63a9ea44e3435ddfc68e1aaa7f98b41478f03d10/.github/workflows/dockerimage.yml (ref: docker-mailserver/docker-mailserver#1092)

EDIT: It probably would be nice to test this workflow without actually pushing to hub, to see that it still builds.

Sure, will do.

With this commit we switch from "Automated Builds"[1] to using a GitHub
Actions for building the Docker images. This makes it possible to use
buildx[2] and build the images for other platforms besides amd64.

The workflow is only triggered when a tag is pushed and the latest tag
(`latest` or `alpine`) is only pushed when the tag match the following
regex: "^[0-9]*\.[0-9]*\.[0-9]*$".

[1] https://docs.docker.com/docker-hub/builds/
[2] https://github.com/docker/buildx

Fix #99
All the base image seems to use linux/arm64/v8
@klausenbusk
Copy link
Contributor Author

EDIT: It probably would be nice to test this workflow without actually pushing to hub, to see that it still builds.

Done. I don't think there is more for me to do.

@xPaw
Copy link
Member

xPaw commented Jul 28, 2020

Looks fine I think. @williamboman will need to check the rest.

@williamboman
Copy link
Member

williamboman commented Jul 28, 2020

Sweet! Before diving in to the diff, do I understand it correctly that the purpose of this PR is to enable multi arch distributions?

@klausenbusk
Copy link
Contributor Author

Sweet! Before diving in to the diff, do I understand it correctly that the purpose of this PR is to enable multi arch distributions?

Correct

Copy link
Member

@williamboman williamboman left a comment

Choose a reason for hiding this comment

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

Sweet! It seems like there's some kind of default behaviour where it'll push the commit SHA as the tag if it's unable to fetch the tag, correct? Could we make it so that it only attempts to push if it's able to gather a semver-tag on HEAD, so something like:

  • apply the existing if: clause on the entire action
  • only build & push the image if it passes the version regex
  • on top of this, if it's not a pre-release or a RC we want to push the :latest and :alpine tags as well. FWIW, the current regex in Dockerhub for this is: /^\d(?:\.\d){2}$

Or what do you say?

.github/workflows/docker-image-push.yml Show resolved Hide resolved
.github/workflows/docker-image-push.yml Show resolved Hide resolved
.github/workflows/docker-image-push.yml Show resolved Hide resolved
@klausenbusk
Copy link
Contributor Author

only build & push the image if it passes the version regex

That was the initial design (kind of, only build/push on tag push), but the whole workflow was changed per @xPaw request. I just need to know what we want :)

@xPaw
Copy link
Member

xPaw commented Jul 28, 2020

Well my idea was to build for all commits but only push for tags. That way we can see that buildx is working in pull requests for example.

@xPaw
Copy link
Member

xPaw commented Aug 5, 2020

William was worried about the security of actions. As far as I understand it, github secrets are not shared to forks, so a malicious user should not be able to push to docker hub by creating a pull request, correct?

@klausenbusk
Copy link
Contributor Author

klausenbusk commented Aug 5, 2020 via email

@williamboman williamboman merged commit cc6bc2c into thelounge:master Aug 6, 2020
@c-x-berger
Copy link

Now that there's support for ARM builds, will an ARM image be (possibly manually) pushed to Docker Hub? Looks like images are only pushed on new releases of The Lounge proper, which leaves ARM users stuck until then.

@xPaw
Copy link
Member

xPaw commented Aug 16, 2020

which leaves ARM users stuck until then.

Not like they had one before.

@c-x-berger
Copy link

... fair.

@williamboman
Copy link
Member

I could rebuild latest version, give this a go for real

@GustavoMotaDF
Copy link

Na espectativa para uma versão oficial ARM

@williamboman
Copy link
Member

Na espectativa para uma versão oficial ARM

Desde 4.2.0, já temos compilações ARM oficiais no DockerHub: https://hub.docker.com/r/thelounge/thelounge/tags?page=1&ordering=last_updated

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.

Prebuilt container appears to be x86 only, add instructions for ARM
5 participants