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 CI to automate building and pushing estargz-kind-node and pre-converted estargz images #715

Closed
ktock opened this issue Mar 28, 2022 · 15 comments
Labels
enhancement New feature or request

Comments

@ktock
Copy link
Member

ktock commented Mar 28, 2022

We should have CI to automate building & pushing of estargz-kind-node and pre-converted estargz images.
We publish these images on stargz-containers org so we should have that CI repository under that org.

@ktock
Copy link
Member Author

ktock commented Mar 28, 2022

cc @AkihiroSuda

@ktock ktock added the enhancement New feature or request label Mar 28, 2022
@gabrieldemarmiesse
Copy link

gabrieldemarmiesse commented Sep 4, 2022

I'm interested in having more pre-converted images and allow users to add more, by making a pull request, if they need it. I believe this will help adoption a lot at a time when Docker is making the stargz snapshotter the default in their Docker desktop with containerd beta.

I've read through what the CI does right now and it seems it's a python script executed on a remote VM that does the job of converting, pushing and benchmarkinh the images if I'm not mistaken.

If I do a python3 script running in github action calling ctr-remote and nerdctl to convert and push a list of docker images et every commit on master, would that work for you? You would likely need to add the gcr.io credentials in github secrets too.

Also I'm not sure if I understand this part, does it need to be in this repository or another?

@gabrieldemarmiesse
Copy link

gabrieldemarmiesse commented Sep 4, 2022

I made a repository that does the conversion and push: https://github.com/gabrieldemarmiesse/estargz-images

It does not include the estargz-kind-node image.

If that works for you we can move it to the stargz-containers org.

@ktock
Copy link
Member Author

ktock commented Sep 5, 2022

@gabrieldemarmiesse Thank you for your interest. Yes, I agree that we should have a CI in github.com/stargz-containers that has a list of pre-converted images and automatically pushes them to stargz-containers.

If I do a python3 script running in github action calling ctr-remote and nerdctl to convert and push a list of docker images et every commit on master, would that work for you?

SGTM. The CI should based on script/benchmark. This already contains a list of images and image conversion scripts.

Same as the current script/benchmark, the CI should support specifying the optimization target workload (e.g. entrypoint command, custom envvar, etc). And it should create images having -org, -esgz, esgz-noopt and -zstdchunked suffixes. And on each commit, the CI should skip conversion and pushing of images already existing on ghcr.io/stargz-containers.

@gabrieldemarmiesse
Copy link

gabrieldemarmiesse commented Sep 5, 2022

Thank you, I believe it's clearer now. I'll continue my work on my custom repo and ping you when the conversion and push is implemented (for the moment, I only support entrypoint, -org and -esgz). I'll then move the benchmark in a second time

Update: I'm nearly done, I had to skip some images because they were too old and used an unsupported media type. See https://github.com/gabrieldemarmiesse/estargz-images/blob/master/list_of_images_to_optimize.py

@gabrieldemarmiesse
Copy link

gabrieldemarmiesse commented Sep 10, 2022

@ktock could I have feedback on https://github.com/gabrieldemarmiesse/estargz-images ? I did all the code to convert and push images automatically if they don't exist in the registry yet, I've added all the images of the benchmark.

The only pain point so far is some kind of flakyness I have a hard time to debug. When checking if the manifest exists in ghcr.io with crane (that happened with dockerhub too) I sometimes had 403. It was very random and couldn't make sense of it.

The code to benchmark the Docker images does not exist yet. I think that we may want to have a repo in charge of converting existing images for stargz users AND benchmarks. Something like github.com/stargz-containers/pre-converted-images. Then we can have a second repository dedicated to bencharking. The idea is that while we wait for upstream to provide estargz images out of the box, we provide users a way to use many of the existing image with our list of pre-converted images. If a user needs an image that is missing, the user might want to do a pull request to add this image to the list.

It's just an idea, I'm trying to think of ways people can use estargz without waiting for all upstream repository to switch. This would be very useful in CI/CD as well as just using docker run normally.

Also related I opened an issue to discuss a possible additional tag in docker.io/library: docker-library/official-images#13123

@ktock
Copy link
Member Author

ktock commented Sep 14, 2022

@gabrieldemarmiesse Thank you.

could I have feedback on https://github.com/gabrieldemarmiesse/estargz-images ? I did all the code to convert and push images automatically if they don't exist in the registry yet, I've added all the images of the benchmark.

  • Please add set -eu to all shellscripts.
  • Please check sha256sum of the downloaded nerdctl.

Remaining issue is that estargz-images doesn't seems to support passing stdin to ctr-remote (i.e. conversion of CMD_STDIN in hello.py). But maybe we can do them in the following up patches.

The only pain point so far is some kind of flakyness I have a hard time to debug. When checking if the manifest exists in ghcr.io with crane (that happened with dockerhub too) I sometimes had 403. It was very random and couldn't make sense of it.

Does using the command other than crane digest fix it (e.g. crane manifest)?

Also related I opened an issue to discuss a possible additional tag in docker.io/library: docker-library/official-images#13123

Thank you! 👍

@gabrieldemarmiesse
Copy link

Thanks, it's quite clear.

I could implement the stdin but I'm not sure it's necessary. It's hard to find docker images that can be optimized only by using stdin to run them. Even the ones already in the list you gave me could use --entrypoint instead.

Maybe, to avoid having dead code in the repo, we could implement the stdin only when we encounter a docker image where stdin is the only way to optimize?

@ktock
Copy link
Member Author

ktock commented Sep 16, 2022

@gabrieldemarmiesse

Maybe, to avoid having dead code in the repo, we could implement the stdin only when we encounter a docker image where stdin is the only way to optimize?

SGTM

If you're interested in moving that repo under github.com/stargz-containers, that code will be maintained by stargz-snapshotter project so please keep the following in mind:

  • That repo needs to be licensed under Apache License 2.0 (https://github.com/containerd/stargz-snapshotter/blob/v0.12.0/LICENSE).
  • Because the transferred repo will be maintained under stargz-snapshotter, you will not have a write (including commit) access to that repo until you become a maintainer (i.e. you'll need to create a PR to commit patches to that repo).

BTW, I'm attending at Open Source Summit at Dublin. Are you there as well? It would be nice if we talk there.

cc @AkihiroSuda

@gabrieldemarmiesse
Copy link

gabrieldemarmiesse commented Sep 18, 2022

Here are some updates on the repo https://github.com/gabrieldemarmiesse/estargz-images :

  • ✅ Please add set -eu to all shellscripts
  • ✅ Please check sha256sum of the downloaded nerdctl.
  • ✅ That repo needs to be licensed under Apache License 2.0
  • ✅ No more 403 errors with crane, it has been fixed by creating a custom token and adding it to the repository secrets. It seemed the default token didn't have sufficient permissions.
  • 🟠 Implementation of the stdin option for optimization will be delayed until we absolutely need it.
  • 🟠 The commits are messy. When transfering the repository between my userspace and github.com/stargz-containers, I believe we should squash them all
  • 🟠 When opening a pull request, the CI will fail because it will try to push the images. In pull requests, we should only try to convert the new images and not push them.
  • 🟠 When pushing new Docker images, I don't know how to force them to be available publicly without manual intervention

Let's not transfer the repo until I remove the push step when the CI is running in a pull request

If you would like to talk, I'm available on the docker slack or we could do a google meet. You can find my email address in my github user profile page

@ktock
Copy link
Member Author

ktock commented Sep 21, 2022

Let's not transfer the repo until I remove the push step when the CI is running in a pull request

Thank you. Please let us know when it's ready.

@gabrieldemarmiesse
Copy link

@ktock it's done. When in a pull request, the CI will only try to convert the images, but will not try to push them

We can transfer the repo. Since I made a lot of commits just for testing purposes, you might want to squash the whole history into one commit :)

@ktock
Copy link
Member Author

ktock commented Oct 5, 2022

@gabrieldemarmiesse

FYI: https://github.com/stargz-containers/image-ci
Thank you for the contribution!

@gabrieldemarmiesse
Copy link

Very cool! :)

@ktock
Copy link
Member Author

ktock commented Oct 17, 2022

Now images are created from our CI. Closing this issue.

https://github.com/containerd/stargz-snapshotter/blob/4aaaf58ab09e7e77722b476c6e09673296425edb/docs/pre-converted-images.md

You can request new pre-converted images from our CI repository (github.com/stargz-containers/image-ci).

You can enable lazy pulling of eStargz on KinD using our prebuilt node image ghcr.io/containerd/stargz-snapshotter:${VERSION}-kind namespace.

@ktock ktock closed this as completed Oct 17, 2022
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
None yet
Development

No branches or pull requests

2 participants