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

Arm64 support, DependaBot alerts, Docker Build Cache #57

Draft
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

just5ky
Copy link

@just5ky just5ky commented Aug 14, 2023

Do Not merge this yet. I overlooked that you were building from docker-compose.yml file instead of DOCKERFILE.

Multiple Updates


BREAKING CHANGE TO CI

  • By Default, CI will tag the image to branch name. i.e, main
  • Changed the secret name to GIT_TOKEN from GITHUB_TOKEN
    • You will need to rename the secret name in the repository secret.

P.S. Reiverr is amazing, still needs a lot of improvements, but I'm sure it will get there.

@aleksilassila aleksilassila self-assigned this Aug 14, 2023
@aleksilassila
Copy link
Owner

Hey, looks good! Let me know when this is ready to merge. Also If possible, squash commits like "test fix 1".

I'm not familiar with all the actions you used so if you could clarify a couple of things before I merge:

  • Not having worked with dependabot.yml file before, do you recon I should add an entry for package.json, too?
  • Are the QEMU & Buildx actions for the multi-platform functionality? What do they do exactly or what guide did you follow regarding the multi-platform setup
  • overmindtech/buildkit-cache-dance seems really small project, why is it necessary and is it maintained? I would rather use something more widely used to avoid running into issues regarding maintainability.
  • Does your setup still publish the latest tag and the version tag? Are actions still ran automatically whenever I create a tag

No rush, implement all your changes before answering

@just5ky
Copy link
Author

just5ky commented Aug 14, 2023

I based this commit thinking you were building the container from DOCKERFILE, but after I created the pull request I noticed that you are using docker-compose.yml to build it, by passing some more commands in the compose file.
Now i have to redo the CI file. xD

  • I will quash test fix 1
  • Dependabot.yml provides updates to dependencies, you can add npm, yarn, among many others to it.
  • QEMU is just an emulator to run ARM on top of an x86 system.
  • Buildx provides functionality to build multi-arch containers docker buildx
  • Both are needed to build multiarch containers.
  • i added overmindtech/buildkit-cache-dance because you are using ghcr.io and not docker hub to store the container.
    • it caches the container layers, so that in future builds, it can just use cached layers instead of re-building it again. (unless there is any changes)
    • I have experienced that building multiarch containers takes a lot of time with many of my previous containers.
    • I can find others build cache methods, or you can add dockerhub, which makes it really easy.
  • I will have to take a look at the tags again. But the workflow will publish the default tag as the branch name. So if the branch name is main then the container tag will be aleksilassila/reiverr:main

@aleksilassila
Copy link
Owner

Okay, gotcha

When it comes to the tags, I think it would be best to have the :latest tag as a convention, as well as the release tag (e.g. v0.3.0) name for now. It makes it more intuitive for people to choose between the latest release and a specific one. Lmk if you run into issues regarding that or if you have any suggestions.

@just5ky
Copy link
Author

just5ky commented Aug 15, 2023

@aleksilassila can you explain to me why you are building using docker compose instead of docker build or docker buildx build
I'm a confused why there are two different sections in Dockerfile and two compose file.
image
Why is the development section needed for production build?

@aleksilassila
Copy link
Owner

@aleksilassila can you explain to me why you are building using docker compose instead of docker build or docker buildx build

The multiple docker-compose files are there to make docker-compose up build the development environment by targetting the development stage of the dockerfile and docker-compose -f docker-compose.yml -f docker-compose.prod.yml up the production environment by targetting the production stage respectively. The different stages in Dockerfile allow for a) having only one dockerfile for multiple environments and b) smaller production imgage sizes since pre-production stage gets discarded except the build/ directory.

Using docker compose build is probably unnecessary, however. I just used it to easily reuse the values in docker-compose files (container_name, image, target build stage etc). I assume you can pass those with the docker build command.

@just5ky
Copy link
Author

just5ky commented Aug 15, 2023

It would be better to separate out the dockerfiles for dev and prod
Thats what was causing me to end up with development env. In the final container.
And its also not possible to use buildx with docker compose for multiarch builds

@aleksilassila
Copy link
Owner

It would be better to separate out the dockerfiles for dev and prod
Thats what was causing me to end up with development env. In the final container.
And its also not possible to use buildx with docker compose for multiarch builds

If you're not using the docker compose, you'll need to specify the build stage with flags, for example docker build --target production. Does that work with buildx? It seems like at least they have also the --target flag.

https://docs.docker.com/build/building/multi-stage/

https://docs.docker.com/engine/reference/commandline/buildx_build/

@just5ky
Copy link
Author

just5ky commented Aug 15, 2023

I think it would
I'll give it a try later today.

@just5ky
Copy link
Author

just5ky commented Aug 15, 2023

okay, so I got it to work with buildx and build a multiarch image.
https://github.com/just5ky/reiverr/pkgs/container/reiverr
but i am not able to tag it with version.
Did you have to create a release first manually, i.e, like v0.4.0 for it to take that as tag

@aleksilassila
Copy link
Owner

The way it works currently is I'll run npm version minor/major/patch. It will bump package.json version, create a release commit and add the version number as a tag to that commit. Then I'll push the the tag (and the commit). Every tag that is pushed and that starts with "v" triggers the build.yml. The tag name is extracted from ${{ github.ref_name }}.

name: Build & Deploy

on:
  push:
    tags:
      - v*

# ...

env:
  REGISTRY: ghcr.io
  IMAGE_NAME: ${{ github.repository }}
  TAG: ${{ github.ref_name }}

@just5ky
Copy link
Author

just5ky commented Aug 15, 2023

okay i see, then i think i got everything working

@just5ky
Copy link
Author

just5ky commented Aug 15, 2023

I will double-check the code and redo the pull request, from the master branch of my fork.

@just5ky just5ky marked this pull request as draft August 15, 2023 17:48
@aleksilassila aleksilassila marked this pull request as ready for review August 20, 2023 17:23
@aleksilassila aleksilassila marked this pull request as draft August 20, 2023 17:24
@aleksilassila
Copy link
Owner

Whoops, let me know when this is ready for merge

@just5ky
Copy link
Author

just5ky commented Aug 20, 2023

Out on vacation, will be back in a few days.

@rflrkn
Copy link

rflrkn commented May 20, 2024

Are there any updates on this..? Really looking forward to test Reiverr on my ARM server!

@just5ky
Copy link
Author

just5ky commented May 21, 2024

@rflrkn i stopped working on it after Alex posted about project state
https://github.com/aleksilassila/reiverr?tab=readme-ov-file#state-of-the-project-%EF%B8%8F
#98

@rflrkn
Copy link

rflrkn commented May 21, 2024

Ah, didn't see that. Thanks!

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

3 participants