dogecoin / docker

Dogecoin Core on Docker
MIT License
28 stars 15 forks source link

Optimize the build order #22

Open patricklodder opened 3 years ago

patricklodder commented 3 years ago

_Originally posted by @AbcSxyZ in https://github.com/dogecoin/docker/pull/6#discussion_r759717146_

Separation of declaration and execution is not the rule in Dockerfile.

For exemple, php & golang are mixing declaration and execution. And probably all other Docker official images.

It's even a recommendation to use the cache with more efficiency, from cacheability recommendation:

Ensure that lines which are less likely to change come before lines that are more likely to change (with the caveat that each line should generate an image that still runs successfully without assumptions of later lines).

For example, the line that contains the software version number (ENV MYSOFTWARE_VERSION 4.2) should come after a line that sets up the APT repository .list file (RUN echo 'deb http://example.com/mysoftware/debian some-suite main' > /etc/apt/sources.list.d/mysoftware.list).

Doing all declarations at the top of a Dockerfile is defeating the utility of caching mechanism.

patricklodder commented 3 years ago

I'll do an analysis after #20

patricklodder commented 2 years ago

Below my initial analysis, I kept your suggestion of doing architecture detection together with verification. That will not improve readability of the latter, but can be done like that as long as we do not need architecture for any other decisions in the process.

Verify stage

Step Description Depends on Reposition Rationale
1 Workdir (static) 1
2 RLS_VERSION (static) 5 caught in dir structure
3 RLS_OS (static) 6
4 RLS_LIB 3 7 tied to OS
5 RLS_ARCH - 8
6 Architecture 4,5 - combine with 16
7 SIG_PATH 2,3 10
8 DESCRIPTOR_PATH 3 11
9 RLS_LOCATION 2 12
10 REPO_GITIAN_BUI (static) 2 can be on top
11 REPO_GITIAN_SIG (static) 3 can be on top
12 REPO_DOGECOIN_C (static) 4 can be on top
13 SHASUM pins (static), (2) 9 changes with version
14 apt install - 13
15 fetch repos 2,10,11,12,13,14 14
16 fetch & verify 2,6,7,8,13,14,15 15

Final stage

Step Description Depends on Reposition Rationale
1 USER (static) 1
2 DATADIR (static) 2
3 HOME (static) 3
4 useradd (static) 4
5 apt install - 12 as late as possible
6 workdir (static) 5
7 copy tarball 6 6 keep this here - should happen less often than py update
8 install 1,7 7
9 copy entrypoint - 13 changes sometimes
10 chmod 9 14
11 workdir 3,8 8
12 expose (static) 9
13 expose (static) 10
14 volume (static) 11
15 entrypoint 9,10 15
16 dogecoind 15 16
patricklodder commented 2 years ago

After thinking about this more, I think it's better when an entrypoint change invalidates the cache of the apt install of python, as a cached version may have a python that does not work with the entrypoint code the same that the latest python from the package maintainers repository. This means that if entrypoint changes, it MUST be built (and tested) with the latest python, because otherwise cached build steps can yield different results than non-cached builds ran at the same point in time.

patricklodder commented 2 years ago

Changed back to do apt before copy entrypoint. Make it more friendly now, but leaves this open and not fully solved with #31.

AbcSxyZ commented 2 years ago

Yeah, an update strategy would be needed for all dependencies and not only python. Some work is needed to figure out how we deal with it, I have it in mind, but it will be probably a tricky question. We may ask some advices from maintainers of docker official images ? Maybe by raising an issue to ask an update of the documentation, I didn't see information about it :)

It may also depend on the vulnerability scanner you were speaking about. We need to work specifically about image updates.