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

Add PackSquash Docker image #111

Merged
merged 13 commits into from
Jun 8, 2022
Merged

Add PackSquash Docker image #111

merged 13 commits into from
Jun 8, 2022

Conversation

realkarmakun
Copy link
Contributor

That would build and push said Docker image to ghcr.io

Before merging it we need to adjust some URLs and repo owner (@AlexTMjugador I presume) need to add repository secret with name REGISTRY_TOKEN and put Github token with write:package permission.

We also can probably add more opencontainer lables to the image

That would build and push said Docker image to ghcr.io. This still needs to be adjusted to fit official PackSquash image (owner token, ghcr urls and etc.)
@CLAassistant
Copy link

CLAassistant commented May 14, 2022

CLA assistant check
All committers have signed the CLA.

@realkarmakun
Copy link
Contributor Author

I'll add a few examples with docker run command in few hours

@realkarmakun
Copy link
Contributor Author

realkarmakun commented May 14, 2022

@AlexTMjugador
Not sure where to put this.

Command to run PackSquash:

docker run -v path/to/host/dir/:/path/to/container/dir \
    ghcr.io/comunidadaylas/packsquash \
    packsquash --appimage-extract-and-run target/packsquash-config.toml

NOTE! Do not mount or write in /app directly avoid docker volume issues. Create sub-directory on host with needed data (resourcepack to squash, packsquash config file and etc.), and mount it in sub-directory in container.

docker run example:

Uses forked repo, use ghcr.io/comunidadaylas/packsquash:latest in production

docker run -v /home/user/packsquahs/:/app/target \
    ghcr.io/realkarmakun/packsquash \
    packsquash --appimage-extract-and-run target/packsquash-config.toml

Pack path on host: /home/user/packsquash/resource-pack
Config path on host: /home/user/packsquash/packsquash-config.toml

packsquash-config.toml:

pack_directory = 'target/resource-pack'
output_file_path = 'target/squashed-pack.zip'

Setup above would produce squashed-pack.zip in /home/user/packsquash directory

Gitlab CI/CD job example:

build-job:
  stage: build
  image: 
    name: ghcr.io/comunidadaylas/packsquash:latest
    entrypoint: [""]
  script:
    - packsquash --appimage-extract-and-run packsquash-config.toml
  artifacts:
    paths:
      - squashed-rp.zip

NOTE! Inside git repository resource pack files should be in sub-directory. Using root directory can lead to minimization of gitlab and git system files which is undesirable. In this example resource-pack directory is used

packsquash-config.toml

pack_directory = 'resource-pack'
output_file_path = 'squashed-rp.zip'

Example above produce squashed-pack.zip in root directory. Which can be passed to other jobs as artifact

@AlexTMjugador AlexTMjugador added the enhancement New feature or request label May 14, 2022
@AlexTMjugador AlexTMjugador added this to the PackSquash v0.4.0 milestone May 14, 2022
Copy link
Member

@AlexTMjugador AlexTMjugador left a comment

Choose a reason for hiding this comment

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

This PR is pretty useful and I look forward to merging it. The examples will come in handy for the wiki documentation. Thank you again! ❤️

I'm working on something else right now, but here it is a super-quick review.

.github/workflows/build.yml Outdated Show resolved Hide resolved
@AlexTMjugador
Copy link
Member

By the way, this PR would close #10! 🎉

Copy link
Contributor

@sya-ri sya-ri left a comment

Choose a reason for hiding this comment

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

Thank you for the PR ❤️ It’s some typos and job execution conditions.

.github/workflows/build.yml Show resolved Hide resolved
.github/workflows/build.yml Show resolved Hide resolved
.github/workflows/build.yml Show resolved Hide resolved
Make use of multi-stages to have conditional binary copy
Also fixing some typos
@realkarmakun
Copy link
Contributor Author

So I decided to go with single Dockerfile to make use of BuildX multiplatform building instead of separate ones. You can check the docker image manifest yourself for following tag https://github.com/users/realkarmakun/packages/container/packsquash/22142379?tag=edge

It makes use of multi-stages to perform conditional COPY depending on supplied TARGETARCH build ARG. (Source for conditional COPY: https://stackoverflow.com/a/54245466/14471910)

Usually when pulling image, docker will pull needed platform on it's own but to force pulling different platform we just need to supply --platform flag to docker as here (I still use my package repo for this):

docker pull --platform arm64 ghcr.io/realkarmakun/packsquash:edge

As of writing this I figured out I don't actually copy binaries just yet so need to push another commit. And fix some typos as well.

Add newline. Dockerfile should be in working state now.
Copy link
Contributor Author

@realkarmakun realkarmakun left a comment

Choose a reason for hiding this comment

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

Everything should be fine now
UPD: you can test the setup using ghcr.io/realkarmakun/packsquash:latest now. Two architectures should be available 🎉
UPD2: QEMU action docs suggest that there is no need to set it up for linux/arm64 I'll try removing it

@realkarmakun realkarmakun requested a review from sya-ri May 15, 2022 09:00
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
.github/workflows/build.yml Outdated Show resolved Hide resolved
Bump action versions, and remove unecasarry login
@realkarmakun realkarmakun requested a review from sya-ri May 15, 2022 10:30
Copy link
Contributor

@sya-ri sya-ri left a comment

Choose a reason for hiding this comment

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

Great work!! The workflow has been completed successfully.

docker/setup-qemu-action is not necessary in our case
Copy link
Contributor Author

@realkarmakun realkarmakun left a comment

Choose a reason for hiding this comment

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

Everything should be in working state now

@AlexTMjugador
Copy link
Member

AlexTMjugador commented May 16, 2022

I think this is almost ready to merge now. Thank you for your contribution!! ❤️

However, the build workflow is becoming pretty big. I think it'd be great to split the Docker image build job into a new workflow file, if possible.

I'm also wondering how the Docker image versioning reflects PackSquash releases and interacts with branching. After reading some documentation, I think it will create an image with the latest tag for each commit pushed to any branch, and then a new tagged image for each new semver tag. Is this understanding correct? Moreover, I wouldn't want commits in different branches to overwrite images generated for commits in the master branch.

After those small matters are taken care of, I'll need to double-check that the repo is set up correctly for the push to work fine, as you mentioned. Then we can merge this 🚀

@realkarmakun
Copy link
Contributor Author

realkarmakun commented May 16, 2022

@AlexTMjugador I'm not sure about new workflow. I believe fetching artifacts from other workflow (e.g. AppImages) can be a pain, but need to do more research before saying if this can be done (it most likely can with some kind of workarounds). Anyway I'll look into this

Right now, new image is pushed to ghcr.io on new semver complaint git tag. Latest git tag would get the latest docker tag but the same image would be available at let's say packsquash:0.3.1.

I also have set up edge tag that would push latest commit on the master branch to said docker tag (like a nightly build). If this undesirable we can disable this.
This behavior is defined right here

type=edge,branch=master

So I believe pushes on other branches won't trigger edge docker image generation.
And git tags are not bound to any branch anyway, so if you decide not to create tags on other branches it should be fine.

I have tested workflows in separate repository, you can check out tags setup there
https://github.com/realkarmakun/PackSquash/pkgs/container/packsquash
(Spawned a lot of git tags to test worlfow out so it's packsquash there is 0.4.6 right now 🤣 )

@AlexTMjugador
Copy link
Member

AlexTMjugador commented May 16, 2022

I believe fetching artifacts from other workflow (e.g. AppImages) can be a pain, but need to do more research before saying if this can be done (it most likely can with some kind of workarounds).

To do so you need you need to use the dawidd6/action-download-artifact action instead of the official GitHub one. Other than that, it should be pretty straightforward.

After reading your description and looking at your repository, I think the per-tag and per-commit image tag generation setup works as desired for the PackSquash project. Nothing to do there! 👍

@AlexTMjugador
Copy link
Member

Hey @realkarmakun, how are things? Do you have any plans to address the last review comments? I'm looking forward to see this PR merged at some point! 😄

@realkarmakun
Copy link
Contributor Author

Yeah sorry I'm figuring my midterms right now, don't have much time to test separate workflow. Will be able to get back to it if 2 or more days though!

@realkarmakun
Copy link
Contributor Author

realkarmakun commented Jun 6, 2022

@AlexTMjugador Here it is. docker.yml workflow will be ran on successful build.yml. Since I can't really show case that it works in PR repo (since static analysis and thus benchmark fail) - I've removed them in my test repo here https://github.com/realkarmakun/PackSquash/actions/runs/2448056138 and it works like a charm https://github.com/realkarmakun/PackSquash/actions/runs/2448129270

Actually no, wait for some reason only edge tag works, I need more testing

@realkarmakun
Copy link
Contributor Author

realkarmakun commented Jun 6, 2022

@AlexTMjugador ✋ Hey! So I figured it out. Running separate workflow looks very hacky too me. But I'll explain a few ways to run separate workflow and why we shouldn't do that. So there are two cases:

  1. When we use vanilla Github Action on: workflow_run: which runs child workflow once the parent one (mentioned in workflow_run) is ran. Our docker/metadata-action@v4 which automatically utilizes Action's trigger context from github.action variable and sets up appropriate tag (edge or semversion in our case). Since there is no way to pass parent's context, docker/metadata-action@v4 can't really figure out what tag to put there. If was triggered by workflow_run, the child workflow won't know that parent was triggered by semver tag, and would put edge docker tag on the image. In message above this is exactly what happened, I pushed test tag, but it was marked as edge thought everything worked.
  2. I found a way to utilize on: [workflow_dispatch] to trigger child workflow. We would need to make another job in parent workflow, which will be using benc-uk/workflow-dispatch@v1. This actions actually passes variable github.action to child workflow, as far as I can tell (as of writing this, I didn't test just yet because building takes a lot of time) but pretty sure it would work.
    In second case build.yml looks like this:

изображение

So using first case is not preferable because we can't build proper tags for docker image, but second case still requires changes to build.yml to add additional job (Though we could add "invoke" step at the end of benchmark probably, but IMHO this could be confusing)

I believe Pack Squash doesn't need a separate workflow for docker image. Separate workflow make sense in the project where separation of CI and CD in different workflows makes sense, for example when deployment process consists of multiple jobs, like deploying to multiple clusters, or tackling some legacy authorizations process to deploy the project. I doubt build.yml would grow larger in a way that new jobs would make sense in second workflow. Like you would make changes to CI part, maybe adjust building processes or add another platform but those jobs would still need to be in build.yml. Something like another deployment job, or submitting to some kind of service would need to be in other workflow. More to that docker build job is relatively small and won't grow any time soon.

I have experience with Gitlab CI/CD, and all of the jobs are defined in the same file. Gitlab do have another abstraction layer called stages, every job in repo workflow should be in a stage, thus you can order jobs, allowing some of them run in parallel, and for some to wait for stage to finish. But everything still is contained in one "workflow" file. (Syntax is much cleaner than Github Actions though but that's irrelevant)

I do see a way to implement this using first case as well, it is to add a job that would build bake file using metadata action, upload is as artifact, and then download it for build action but that would still result in additional step or job in build.yml. Might as well build and push from there :D and as a treat I need just to revert some changes and PR would be ready.

But if after everything above you still want a separate workflow, I'm would do it ofc, just need more time for tests, so I can be sure docker tags works as intended.

UPD: We also could run docker build push workflow on the same events as build.yml, but that would require a wait action, that would make docker build push workflow wait for artifacts, but that's just another dimension of workaround 👯

@AlexTMjugador
Copy link
Member

AlexTMjugador commented Jun 7, 2022

Thanks for the detailed explanation! ❤️ I've changed my mind, and now I agree that it is more "idiomatic" to keep the Docker image build job in the same workflow.

My main reason to separate things was to avoid making the build workflow even bigger, but while writing this I'm realizing that maybe that can be achieved while keeping conceptually related jobs in the same workflow by using matrix jobs. Anyway, that refactor would be out of scope for the PR, so don't worry about it for now.

Can you please put it back on the same workflow, so the context is correct and the images are tagged as expected?

Reverting back to single wokflow file.

This reverts follwoing commits:
ba73a4f
edc6d0e
5475750
@realkarmakun
Copy link
Contributor Author

@AlexTMjugador Everything works as intended now 🥳 Every commit on master would create edge docker tag and every git tag that compiles with semver would create version tag accordingly. Latest semver tag is marked as latest.
You can check test repo with test tags here: https://github.com/realkarmakun/PackSquash
Package page: https://github.com/realkarmakun/PackSquash/pkgs/container/packsquash
All examples in the messages above are still valid.

Looking forward for this to be merged so I can use PackSquash in my Gitlab resource pack pipeline! 👍

@AlexTMjugador
Copy link
Member

This Docker image is really useful. Congratulations for your first PR! 🎉

I've reviewed and tested it in depth and I think it's good to go. I've made some minor changes that I wanted to push to this PR, but I couldn't due to a 403 error, so I'll merge it as-is and then commit the changes to master.

@AlexTMjugador AlexTMjugador merged commit c82b983 into ComunidadAylas:master Jun 8, 2022
@AlexTMjugador
Copy link
Member

AlexTMjugador commented Jun 8, 2022

As a part of the tweaks I made, the generated Docker containers now run PackSquash as their entrypoint, the image working directory is the root directory (as it is most usual in Docker images), and the PackSquash AppImage is copied to a standard path.

Thus, the "command to run PackSquash" above becomes (notice how the extra arguments passed to docker run are now extra arguments passed to PackSquash, and how specifying the label is necessary):

$ docker run -v /path/to/host/dir/:/path/to/container/dir \
    ghcr.io/comunidadaylas/packsquash:edge \
    /path/to/container/dir/packsquash-config.toml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging this pull request may close these issues.

4 participants