-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Using distroless base image for Docker #214
Conversation
I understand the point of moving to a distoless container image. One huge drawback with distroless in my opinion is debugging. So we probably would also need a debug image then. I will try it out these days. |
That sounds right, I should have had removed that. Thanks for this work, I have a couple of reasons for why its actually in the state it is. This are my opinions, the main reason we have a docker image is for the E2E/integration tests. We only publish it as a bonus to users whom prefer docker instead of running binaries themselves. Neither me or @juanfont use docker for our headscale setups and the documentation for the docker part is community driven. There are two main reasons (to me) that Ubuntu is used:
We could probably be clearer that while we publish the docker image, the main purpose is for the tests. That said, I am up for changes for the better, but it needs to ensure our requirements for the use case is met:
|
Thanks for sharing your thoughts! To start, note that I've fixed the integration tests and the CI should now be green. The issue is that the tests were expecting a shell so were invoking the I understand that you don't use Docker personally and I get why that is going to be the biggest hurdle. If that is the deal-breaker, I get it. Personally, I run everything inside Docker for both ease of management (easier to recreate everything) and for security (isolation, dependency management). That said, distroless images are different from You're right that with a distroless image you can't create a shell in the image. I'd argue this is a feature and not a bug, and it shouldn't make debugging much harder: anything that happens in the container should be replicable in any Debian/Ubuntu environment (Google's distroless images are based on Debian 11) |
Don't get me wrong, I see the advantages for a production container. I can see the tests are working and that is good, its just that the lack of shell is a relatively large problem. Whenever I develop I get that it is easy to setup something that looks like the container environment, but it is not the actually environment. We could of course have two Dockerfiles, one for production headscale and one for integration testing/debugging, but I would argue that we will have another "keep in sync" issue. We could run the integration tests against both the prod and debug dockerfiles to ensure there are no diff. Edit: |
@ItalyPaleAle perhaps the distro less "debug" image is a good base image? It still has a shell while otherwise being very limited. However I can't check right now if those are also available for the non-language specific base images. |
If The debug one has a shell and is viable, we could make it an argument in the Dockerfile:
And then we can override it in the integration tests with CONTAINER_NAME = distroless-debug. Very sudo code, but that could be a solution. |
I'm still not in favor for this change as it will not be backwards compatible with how you have to execute the binary in the container. This will probably break a lot of peoples setups. So it will be a breaking change. Is there someway to make headscale executable without the prefixed /? |
Yes those are available and there could be two images published: a production image (based on Want me to update the PR to create another Dockerfile?
Looking into that |
@ohdearaugustin 🎉 Turns out you can, just by setting The latest commits revert the |
Is that equivalent/a subset of saying About the :debug images for the non production image: if people want to easily run commands in the container I think using this for some of the images makes sense. It makes introspection easier. |
No, that's a different thing. Seems that On the debug images, I will create one later today. |
@ItalyPaleAle That are great news. yeah totally makes sense to set / as search path. So we would also have more backwards compatibility. I like it more and more. (= |
So I tested the suggested distroless build and so far it looks good for the production image. The hack with I would definitely like to get it running with distroless. My suggestion would be we build and publish two images like: headscale:x.x.x --> production image so you can easily interchange the images for debugging purposes. Therefore the issuse with the debug image needs to be resolved first. For now I'm out of ideas. Tried several things like adding the binary to /bin which works in production but not in the debug image. Or added the binary to the path like Furthermore I also would have already prepared the new release git workflow to publish those two images, still need to test it when the debug image is working. So @ItalyPaleAle Thank you already for your effort. (= |
@ohdearaugustin I actually run into the very same issue and I started looking into that but I couldn't figure it out and then I got busy :) My guess is that it's because of the ENTRYPOINT which essentially is telling busybox to run the headscale binary as a shell script - hence the wrong character error. I'm going to try doing some more debugging and see if I can make it work |
...and looking at this with a fresh mind helped! Hooray, it works now :) The trick was that I had to remove the Now the debug image works:
|
Awesome. Yeah I also came to the conclusion now that busybox executes the binary as shellscript and somewhere in the binary blob there is some So just found out if we just copy the binary to /bin/headscale in both images we won't need the hack with the set PATH. We definitely need to docu that^^ So for now I have to go to bed haha Thanks (= |
This way, we don't need to alter the PATH
@ohdearaugustin thanks for sharing that finding :) I've now updated the containers to put the binary in Also, I've written some docs for how to use the debug container. |
@ItalyPaleAle Added a PR with the promised changed for the release workflow to your fork. Tested already everything. |
Docker workflows
Merged, thanks for flagging that! |
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.
Looks good and I also tested it.
I'm willing to give this a go, but there are some conflicts that needs to be resolved. |
Uhm the merge conflicts in the docs were really weird. Anyways, fixed them |
We merge a pr with stricter formatting for the docs (and everything else) in between. |
I missed that you added prettier. I just ran it to fix the linting issues. |
Thanks for making Headscale!
I have a proposed improvement to the Docker image, switching the base image of the final container from Ubuntu to a "distroless" image, using images from https://github.com/GoogleContainerTools/distroless
If you've never heard of "distroless" images, that page contains a bunch of details on the benefits, but the TL;DR is that these images do not contain a full OS but rather just enough to run the application (ca-certificates, glibc, tzdata, and just a couple more things). By using a "distroless" image, the container is much smaller (they claim they're 2% of the size of a Debian base image) and it's safer, as there's a significantly smaller attack surface. Plus, "distroless" images are updated much less frequently so keeping the base image up-to-date (and include security fixes) is much simpler.
I've confirmed this builds and the
headscale
binary runs. I haven't performed full E2E tests however just yet.PS: I've also removed the stage
bufbuild/buf:1.0.0-rc6
which seemed unused