chore(docker): Clean up the docker image by williamdes · Pull Request #1954 · docker-mailserver/docker-mailserver

williamdes · 2021-05-09T19:16:15Z

Description

Using the tool dive I found out that some files are removed in other layers but this is not right because of the union file system design.

Removed the files in the layer they where created
Merged some layers
Removed some useless files

Before:

Image name: mailserver/docker-mailserver
Total Image size: 622 MB
Potential wasted space: 8.1 MB
Image efficiency score: 98 %

Image name: mailserver/docker-mailserver:ci
Total Image size: 626 MB
Potential wasted space: 16 MB
Image efficiency score: 98 %

After:

Image name: mailserver/docker-mailserver:ci
Total Image size: 612 MB
Potential wasted space: 0 B
Image efficiency score: 100 %

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Improvement (non-breaking change that does improve existing functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation (README.md or ENVIRONMENT.md or the documentation)
If necessary I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes

georglauterbach · 2021-05-09T19:53:29Z

The PR description was altered to not include the checks that PR creators should perform or indicate that such a check may be missing. What's more, CI is failing.

I like the image improvements, but as is, this is definitely WIP.

williamdes · 2021-05-09T21:41:07Z

I improved my PR description, and hope the tests will pass. I do not see anything wrong that could fail them

Here is the container diff

container-diff diff remote://mailserver/docker-mailserver:edge daemon://mailserver-testing:ci --type=file
ERRO[0075] Error diffing contents of /home/williamdes/.container-diff/cache/remote_mailserverdocker-mailserver_edge/etc/cron.d/dovecot-purge.disabled and /home/williamdes/.container-diff/cache/daemon_mailserver-testing_ci/etc/cron.d/dovecot-purge.disabled: open /home/williamdes/.container-diff/cache/remote_mailserverdocker-mailserver_edge/etc/cron.d/dovecot-purge.disabled: permission denied 

-----File-----

These entries have been added to mailserver/docker-mailserver:edge:
FILE                                              SIZE
/etc/ssl/certs/7c066291.0                         1K
/etc/ssl/certs/b33ae3c5                           1K
/etc/ssl/certs/b33ae3c5.0                         1K
/var/lib/amavis/.razor/identity-ruqvoMbDea        90B

These entries have been deleted from mailserver/docker-mailserver:edge:
FILE                                                             SIZE
/etc/ssl/certs/27542cee.0                                        1K
/etc/ssl/certs/ce275665                                          1K
/etc/ssl/certs/ce275665.0                                        1K
/home/syslog                                                     0
/usr/share/applications/python2.7.desktop                        220B
/usr/share/applications/python3.7.desktop                        220B
/usr/share/apps/konqueror                                        345B
/usr/share/apps/konqueror/servicemenus                           345B
/usr/share/apps/konqueror/servicemenus/cabextract.desktop        345B
/usr/share/apps/konsole                                          278B
/usr/share/apps/konsole/python.desktop                           139B
/usr/share/apps/konsole/python2.desktop                          139B
/usr/share/doc-base                                              3.7K
/usr/share/doc-base/altermime                                    396B
/usr/share/doc-base/dbapp-policy                                 805B
/usr/share/doc-base/dbconfig-common                              600B
/usr/share/doc-base/dbconfig-common-design                       327B
/usr/share/doc-base/fetchmail                                    347B
/usr/share/doc-base/findutils                                    323B
/usr/share/doc-base/p7zip-full                                   589B
/usr/share/doc-base/users-and-groups                             423B
/var/cache/apt                                                   0
/var/cache/apt/archives                                          0
/var/cache/apt/archives/lock                                     0
/var/cache/apt/archives/partial                                  0
/var/cache/dbconfig-common                                       0
/var/cache/dbconfig-common/backups                               0
/var/cache/debconf                                               3.5M
/var/cache/debconf/config.dat                                    39.4K
/var/cache/debconf/config.dat-old                                39.4K
/var/cache/debconf/passwords.dat                                 1.2K
/var/cache/debconf/templates.dat                                 1.7M
/var/cache/debconf/templates.dat-old                             1.7M
/var/cache/ldconfig                                              11.7K
/var/cache/ldconfig/aux-cache                                    11.7K
/var/cache/razor                                                 0
/var/lib/amavis/.razor/identity-ruDzXXxU96                       90B
/var/log/alternatives.log                                        6.7K
/var/log/apt/eipp.log.xz                                         16.3K
/var/log/apt/history.log                                         12K
/var/log/apt/term.log                                            72.4K
/var/log/auth.log                                                0
/var/log/dbconfig-common                                         179B
/var/log/dbconfig-common/dbc.log                                 179B
/var/log/dpkg.log                                                125.4K
/var/log/faillog                                                 156.3K
/var/log/lastlog                                                 1.4M

These entries have been changed between mailserver/docker-mailserver:edge and mailserver-testing:ci:
FILE                                                              SIZE1         SIZE2
/var/lib/clamav/daily.cvd                                         101.2M        101.3M
/usr/lib/python3.7/__pycache__/pydoc.cpython-37.pyc               82.7K         82.7K
/usr/lib/python3.7/__pycache__/_pyio.cpython-37.pyc               71.1K         71.1K
/usr/lib/python3.7/__pycache__/difflib.cpython-37.pyc             58K           58K
/usr/lib/python3.7/__pycache__/zipfile.cpython-37.pyc             48.7K         48.7K
/usr/lib/python3.7/__pycache__/ftplib.cpython-37.pyc              27.4K         27.4K
/etc/sasldb2                                                      12K           12K
/etc/aliases.db                                                   12K           12K
/usr/lib/python3.7/json/__pycache__/decoder.cpython-37.pyc        9.6K          9.6K
/usr/lib/python3.7/__pycache__/_markupbase.cpython-37.pyc         7.6K          7.6K
/usr/lib/python3.7/__pycache__/hashlib.cpython-37.pyc             6.4K          6.4K
/usr/lib/python3.7/__pycache__/rlcompleter.cpython-37.pyc         5.6K          5.6K
/usr/lib/python3.7/__pycache__/netrc.cpython-37.pyc               3.7K          3.7K
/var/lib/ucf/hashfile                                             2.9K          2.9K
/var/lib/spamassassin/sa-update-keys/pubring.kbx                  2.9K          2.9K
/etc/dbconfig-common/opendmarc.conf                               2.9K          2.9K
/var/lib/ucf/cache/:etc:dbconfig-common:opendmarc.conf            2.9K          2.9K
/var/lib/ucf/hashfile.0                                           2.9K          2.9K
/var/lib/ucf/hashfile.1                                           2.8K          2.8K
/var/lib/ucf/hashfile.2                                           2.8K          2.8K
/etc/ssl/private/ssl-cert-snakeoil.key                            1.7K          1.7K
/etc/dovecot/ssl/dovecot.key                                      1.7K          1.7K
/etc/dovecot/ssl/dovecot.pem                                      1.2K          1.2K
/var/lib/spamassassin/sa-update-keys/trustdb.gpg                  1.2K          1.2K
/etc/ssl/certs/ssl-cert-snakeoil.pem                              1K            1K
/etc/shadow                                                       953B          953B
/etc/shadow-                                                      892B          892B
/var/lib/amavis/.razor/server.n002.cloudmark.com.conf             845B          785B
/var/lib/amavis/.razor/server.n003.cloudmark.com.conf             785B          845B
/var/spool/cron/crontabs/root                                     212B          212B
/usr/lib/python2.7/dist-packages/supervisor/__init__.pyc          142B          142B
/var/lib/amavis/.razor/identity                                   90B           90B
/var/lib/amavis/.razor/servers.nomination.lst                     76B           76B
/var/lib/clamav/mirrors.dat                                       69B           69B
/var/lib/spamassassin/sa-update-keys/pubring.kbx~                 32B           32B

casperklein · 2021-05-09T22:19:13Z

Merged some layers

BTW: Why not merging all layers easily with something like:

FROM docker.io/debian:buster-slim as build
[..]
# build final image
FROM scratch
COPY --from=build / /

williamdes · 2021-05-09T22:47:53Z

Merged some layers

BTW: Why not merging all layers easily with something like:
FROM docker.io/debian:buster-slim as build
[..]
# build final image
FROM scratch
COPY --from=build / /

Indeed!
Good idea, will do that

Edit: the down side is base layer re-use and audit 0% possible

williamdes · 2021-05-09T23:20:47Z

If it is possible it would be better to have 3 layers, because with this current config the user will not be able to re-use a already existing base image from the file system.

Would be cool if possible to have:

base image
install software layer
update configs layer

Not sure how to do this

casperklein · 2021-05-09T23:43:38Z

Yes, that is the big disadvantage when squashing all layers into one. There is an open issue that would solve this.

wernerfred · 2021-05-10T08:03:07Z

[x] New and existing unit tests pass locally with my changes

I doubt it 😄

The checkmarks are there to remind contributors about what to cover and to reflect the current progress of the PR. Simply checking everything does not help anyone. Then it would be better to leave it unchecked as everyone can simply see that tests are not covered right now and might produce some errors in the current state.

wernerfred

Currently the tests are failing mainly to missing log entries. This might be the case that either the expected lines are not in the log (e.g. problem with startup) or the log file is missing completely (e.g. bc it gets removed). Could it be that your cleanup removed a bit too much of log files? 😄

All-in-all i appreciate a smaller image size (although not always useful; we had a discussion already between image size vs. startup time) but the current state is maximal broken:

bats warning: Executed 65 instead of expected 339 tests
(and those 65 are all failing, too)

wernerfred · 2021-05-10T08:04:47Z

+  rm /usr/share/applications/*.desktop && \
+  rm -rf /usr/share/apps/konqueror && \
+  rm -rf /usr/share/apps/konsole && \


Do these locations/files really get created or is this just a "most complete" list of dirs to remove after installation?

Yes, they get created
I would recommend you to use the tool dive to have a look ;)

williamdes · 2021-05-10T08:28:31Z

Currently the tests are failing mainly to missing log entries. This might be the case that either the expected lines are not in the log (e.g. problem with startup) or the log file is missing completely (e.g. bc it gets removed). Could it be that your cleanup removed a bit too much of log files?

All-in-all i appreciate a smaller image size (although not always useful; we had a discussion already between image size vs. startup time) but the current state is maximal broken:

bats warning: Executed 65 instead of expected 339 tests
(and those 65 are all failing, too)

I am not able to run the tests in local, not sure why but last time I also could not. (some get stuck)

Could it be that your cleanup removed a bit too much of log files?

Probably, I will try to find out 😄

And maybe revert the image squash command as it is not acceptable.
And maybe do another PR after this one is accepted to only squash layers

williamdes requested a review from georglauterbach May 9, 2021 19:16

williamdes marked this pull request as draft May 9, 2021 20:21

chore(docker): Clean up the docker image

2e02958

wernerfred reviewed May 10, 2021

View reviewed changes

georglauterbach added kind/improvement Improve an existing feature, configuration file or the documentation meta/wip priority/low area/ci labels May 16, 2021

georglauterbach added this to the v10.0.1 milestone May 19, 2021

georglauterbach added meta/feature freeze On hold due to upcoming release process meta/stale This issue / PR has become stale and will be closed if there is no further activity labels May 20, 2021

wernerfred removed the meta/feature freeze On hold due to upcoming release process label Jun 1, 2021

georglauterbach mentioned this pull request Jun 6, 2021

chore(cleanup): Cleaned up >/dev/null in Dockerfile and fixed em dash "problem" #2024

Merged

7 tasks

georglauterbach closed this in #2024 Jun 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

chore(docker): Clean up the docker image#1954

chore(docker): Clean up the docker image#1954
williamdes wants to merge 1 commit intodocker-mailserver:masterfrom
williamdes:master

williamdes commented May 9, 2021 •

edited

Loading

Uh oh!

georglauterbach commented May 9, 2021

Uh oh!

williamdes commented May 9, 2021

Uh oh!

casperklein commented May 9, 2021

Uh oh!

williamdes commented May 9, 2021 •

edited

Loading

Uh oh!

williamdes commented May 9, 2021

Uh oh!

casperklein commented May 9, 2021

Uh oh!

wernerfred commented May 10, 2021

Uh oh!

wernerfred left a comment

Uh oh!

wernerfred May 10, 2021

Uh oh!

williamdes May 10, 2021

Uh oh!

williamdes commented May 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Uh oh!

Conversation

williamdes commented May 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Type of change

Checklist:

Uh oh!

georglauterbach commented May 9, 2021

Uh oh!

williamdes commented May 9, 2021

Uh oh!

casperklein commented May 9, 2021

Uh oh!

williamdes commented May 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

williamdes commented May 9, 2021

Uh oh!

casperklein commented May 9, 2021

Uh oh!

wernerfred commented May 10, 2021

Uh oh!

wernerfred left a comment

Choose a reason for hiding this comment

Uh oh!

wernerfred May 10, 2021

Choose a reason for hiding this comment

Uh oh!

williamdes May 10, 2021

Choose a reason for hiding this comment

Uh oh!

williamdes commented May 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

williamdes commented May 9, 2021 •

edited

Loading

williamdes commented May 9, 2021 •

edited

Loading