eggheads / eggdrop-docker

25 stars 24 forks source link

refactored dockerfiles for readability and modularity #16

Closed zmunro closed 2 years ago

vanosg commented 3 years ago

Hi @zmunro thanks so much for your interest in helping with the project! At this time, we'd like to leave the &&'s in the Dockerfile, as that prevents us from adding extra layers (and thus, extra filesize) to the final image. Unfortunately no link I can find provides a clear explanation of what I'm trying to describe, but basically each of those RUN commands will add a new layer, instead of it all being done in a single layer. So now, when you do the RUN rm eggdrop1.6.21.tar.gz it doesn't actually remove that file from the image, it just hides it. The filespace is still present in the image (because that earlier layer can now still be used by a different instantiation of the Dockerfile). If you build an image from this PR, I suspect you'll see the file size is somewhat larger than the current implementation.

I wish I was able to speak more clearly on the topic, sorry if that didn't clear much up for you. I do hope you'll continue to contribute on this or the main Eggdrop repository! Thank you for spending some time on this effort!

zmunro commented 3 years ago

Why is it so important that the image remain so small? It is bad practice to have long commands strung together like this in a Dockerfile, especially things like cd being done in a RUN command, or setting environment variables in a RUN command.

vanosg commented 3 years ago

I don't know many people that don't value small filesize :) I don't know that I would agree it is bad practice to combine commands in a Dockerfile, this is generally an accepted way of minimizing layers, we've worked extensively with the official docker library maintainers on this. If you take a quick scan of the official images for Docker, you can see the Dockerfiles for haskell (https://github.com/haskell/docker-haskell/blob/master/8.8/buster/Dockerfile) Perl (https://github.com/Perl/docker-perl/blob/master/5.032.000-slim-buster/Dockerfile) MySQL (https://github.com/docker-library/mysql/blob/master/8.0/Dockerfile) or Redis (https://github.com/docker-library/redis/blob/master/6.0/Dockerfile) for examples of combining a large number of commands into a single RUN (granted, they sometimes use 'set -ex' instead of &&s, but the result is the same- a single layer with multiple commands in it). I hope that can provide some context for the discussion. Thanks!

zmunro commented 3 years ago

@vanosg I used multi-stage builds to rewrite the dockerfiles so that all the artifacts from the initial building don't get added to the final image, keeping the image size down (even lower than before and can likely still be decreased) while still having split up commands in the dockerfiles.

zmunro commented 3 years ago

Now the image size is 17MB down from 26MB

TimWolla commented 3 years ago

Not a maintainer of the eggdrop Docker image:

I don't feel that having a RUN in front of every line is really more readable than using &&. The former stands out much more, because it's an actual word and because it's highlighted on GitHub and in my editor. Compare to && which my eyes simply can scan over.

vanosg commented 3 years ago

Thanks! I need to get up to speed on multi-stage, I know it's recent as of this year. One challenge we have is making sure the image can continue to be accepted to the official image repository for Docker- I noted they have this guidance out: https://github.com/docker-library/faq#multi-stage-builds I haven't had a chance to really review this yet, but in the meantime do you know if this adheres to the intent they have laid out there? Any thoughts or changes from that would be great.

And thanks for helping push us into the future, whatever the result I think we've both learned something here!

zmunro commented 3 years ago

@vanosg Looking at that document, the multi-stage builds that I made abide by the rules for the official image repository for Docker. And I agree! Hacktoberfest is all about learning and helping others and this definitely helped me learn much more about multi-stage builds and how to minimize the size of docker images :)