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

Minimize Docker image #467

Closed
wants to merge 1 commit into from
Closed

Conversation

ArturKlauser
Copy link
Contributor

This is a proposal for a minimized Docker image that only contains the shfmt binary. It reduces the size a little by not packaging up busybox. Up to you if you want that.

$ docker images
REPOSITORY     TAG         IMAGE ID            CREATED             SIZE
shfmt          latest      fbf022530b4b        33 minutes ago      3.02MB
shfmt          3.0.0       4c876e317d99        38 minutes ago      4.48MB

Also added more usage instructions to the README.

Shfmt v3.0.0 no longer has an external dependency on diff, so we can package the
shfmt binary in the Docker image standalone, without busybox.
@mvdan
Copy link
Owner

mvdan commented Dec 30, 2019

We had briefly discussed this in #68; in particular, I laid out what I'd do in #68 (comment) a couple of weeks ago.

This PR as-is is probably not a good idea, because plenty of users need a Docker image with shfmt as well as a shell installed. For example, that's a requirement for Gitlab CI. A bit silly, I know, but that's how the world works.

If you want to have a try at the duplicate images (scratch and alpine), I'll be happy to review. If not, please open an issue, and I'll probably get to it sometime in January.

@ArturKlauser
Copy link
Contributor Author

I see that you're currently using Docker Hub autobuilds. Unfortunately, they are not very configurable, e.g. AFAIK you can't specify a --target for the build, which would be the handy way to build two images from a single docker file. Are you open to switch the docker image build and publish to a different CI? E.g. Github Actions, which you are already using, should be able to handle that.

@mvdan
Copy link
Owner

mvdan commented Dec 30, 2019

AFAIK you can't specify a --target for the build, which would be the handy way to build two images from a single docker file.

Ah, that's a neat trick. I was imagining that maintaining two Dockerfiles was the only way. I assume we'd still have scratch be the default target, or is there no way to do that?

E.g. Github Actions, which you are already using, should be able to handle that.

Yes, that's fine. Really, the only reason I did it via Docker Hub is because I liked how it shows you the Dockerfile directly, as well as how it built each of the images. Is there a way to show the Dockerfile from builds pushed from our CI here?

If you are happy to do most of the work, I can do the bits I assume you won't be able to, like turning off docker hub auto-builds and adding the right secret to CI.

@mvdan
Copy link
Owner

mvdan commented Dec 31, 2019

Do you want to focus on #468 and close this, since the changes here are included there? Or do you prefer keeping the changes as two commits?

@ArturKlauser
Copy link
Contributor Author

Doing everything in #468 SGTM.

@ArturKlauser ArturKlauser deleted the docker branch December 31, 2019 09:54
mvdan pushed a commit that referenced this pull request Jan 18, 2020
This is a proposal related to #467. It builds two docker images, the busybox 
image as is currently built as well as a minimal image containing only the shfmt
binary.

In addition, the docker images are built as multi-architecture images which is a 
recent feature supported by docker's buildx extension. See how my test images
report availability for multiple architectures. Each client pulling from Docker
Hub automatically pulls only the appropriate architecture version for itself, so
there is no additional overhead for the client. Clients don't need experimental
features enabled for this (only the builder need them).

The docker build and deployment is hooked into the existing Github Actions CI/CD 
pipeline, conditioned on all other builds and tests passing.
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.

2 participants