znc / znc-docker

https://hub.docker.com/_/znc/
49 stars 29 forks source link

Reduce number of Docker layers #28

Closed ratijas closed 3 years ago

ratijas commented 3 years ago

I found it hilarious that there's a mile-long RUN statement in Dockerfile which combines all possible shell setup invocations, and there's multiple single-file COPY right after it.

This commit collapses 5 single-file layers into 2. As much as I'd like to reduce it further, Docker build engine dictates that

If is a directory, the entire contents of the directory are copied, including filesystem metadata.

DarthGandalf commented 3 years ago

mile-long RUN statement in Dockerfile which combines all possible shell setup invocations

Having that in separate layers would increase size a lot, because removal of the build dependencies would only mark them inaccessible, without actually removing them, because they still exist in the layer before that.

multiple single-file COPY right after it.

Technically, yes, they are separate layers now. But their separateness doesn't contribute much to the size. Well, how much space the metadata of one layer takes? Anyway, this was done like this because https://github.com/docker-library/official-images#cacheability says:

Avoid COPY/ADD whenever possible, but when necessary, be as specific as possible (ie, COPY one-file.sh /somewhere/ instead of COPY . /somewhere).

ratijas commented 3 years ago

Having that in separate layers would increase size a lot, because removal of the build dependencies would only mark them inaccessible, without actually removing them, because they still exist in the layer before that.

I know, right? Not my first time with Docker.

Anyway, this was done like this because https://github.com/docker-library/official-images#cacheability says:

Avoid COPY/ADD whenever possible, but when necessary, be as specific as possible (ie, COPY one-file.sh /somewhere/ instead of COPY . /somewhere).

Their reasoning has nothing to do with our current case. What they meant is, don't blindly copy the whole root directory, which might change often, thus causing the whole build cache to be invalidated. In this case, we are copying couple of related configuration files. I can't imagine, why would you wanna cache each of them separately, and in that specific order.

I'm just trying to keep things as clean as possible.

DarthGandalf commented 3 years ago

@tianon Hi, is this change fine for you?

I know, right? Not my first time with Docker.

I wasn't implying you're not. I tried to explain the difference why it's beneficial for RUN but not so for COPY

tianon commented 3 years ago

Yeah, given you're copying all four scripts (sequentially), it seems perfectly reasonable to do a single copy instead. :+1:

(Thanks for checking! :heart:)