Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

Optimise docker build #85

Open
wants to merge 4 commits into
base: 2.20.4
Choose a base branch
from

Conversation

lilatomic
Copy link

This MR restructures the dockerfiles so that it rebuilds fewer things when changing the scripts, resulting in performance improvements:

current optimised
fresh build 138.60 140.64
change pip requirements 70.39 16.82
change script 71.21 2.20

I started with this one because it's simpler than the geonode one.

The dependencies change less than the files
and take longer to build.
Moving them earlier in the Dockerfile means that
file changes won't force us to reinstall all the dependencies.
@frafra
Copy link

frafra commented May 5, 2022

Note that COPY has a --chmod option to make the file executable, while if you copy the file and then change its attributes, the file is going to be stored twice.

Edit: this is valid only when BUILDKIT is enabled, which is not yet the default.

@afabiani afabiani requested a review from lpasquali May 5, 2022 12:34
@lilatomic
Copy link
Author

oh neat, I didn't know about the (upcoming) --chmod, that's cool.
We could also change the permission of the files outside the container. thoughts?

I can combine the COPY instructions to reduce the number of layers. It reduces the number of layers, which I think helps with size (although I'm not clear on how big an impact this has).

@frafra
Copy link

frafra commented May 6, 2022

We could also change the permission of the files outside the container. thoughts?

That would not have any effect on the resulting file inside the container actually, but it would make sense to mark executables as such, as a general rule.

I can combine the COPY instructions to reduce the number of layers. It reduces the number of layers, which I think helps with size (although I'm not clear on how big an impact this has).

It would not reduce the image size significantly. You spare some space if you avoid of saving intermediate files, that gets deleted or overridden/modified.

@lilatomic
Copy link
Author

We could also change the permission of the files outside the container. thoughts?

That would not have any effect on the resulting file inside the container actually, but it would make sense to mark executables as such, as a general rule.

Are you sure about that? I tried with a simple dockerfile:

FROM	bash
COPY	a.txt .
CMD	ls -la a.txt

Changing the permissions on a.txt seems to work fine

> sudo chmod 451 a.txt
> sudo docker run -it (docker build -q .)
-r--r-x--x    1 root     root             0 May  6 00:53 a.txt
> sudo chmod 770 a.txt
> sudo docker run -it (docker build -q .)
-rwxrwx---    1 root     root             0 May  6 00:53 a.txt

The docs have this to say, buried in the COPY reference, which isn't super clear to me:

If is any other kind of file, it is copied individually along with its metadata.

@frafra
Copy link

frafra commented May 8, 2022

I might be wrong or confusing with one of the various different build methods. If it works, great :)

@lilatomic
Copy link
Author

sorry folks, are we waiting on me for something on this?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants