Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dnf clean after dnf install in Dockerfiles #948

Closed

Conversation

PeterDaveHello
Copy link
Contributor

This ensures that dnf doesn't leave unnecessary temporary files in the Docker images, thereby reducing their size.

Regarding the change in image size, it seems there isn't much difference for the rawhide and f39 builds. However, the f38 and el8 builds have shown significant improvement.

For the image size changes, see the result below.

Before:

REPOSITORY                                   TAG                  IMAGE ID       CREATED             SIZE
dockerfile.rawhide                           latest               2df081b6ec73   About an hour ago   519MB
dockerfile.f39                               latest               1d39a1d366ac   About an hour ago   519MB
dockerfile.f37                               latest               f86a5711c973   About an hour ago   832MB
dockerfile.el8                               latest               7ac668a725be   About an hour ago   571MB

After:

REPOSITORY                                   TAG                  IMAGE ID       CREATED             SIZE
dockerfile.rawhide-dnf-clean                 latest               e035b545c15b   About an hour ago   519MB
dockerfile.f39-dnf-clean                     latest               c35b262f3830   About an hour ago   519MB
dockerfile.f37-dnf-clean                     latest               15e78792159d   About an hour ago   441MB
dockerfile.el8-dnf-clean                     latest               59fa63bde3fe   About an hour ago   405MB

@codecov-commenter
Copy link

codecov-commenter commented Sep 10, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.52%. Comparing base (15e16ad) to head (e5413a3).
Report is 136 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #948   +/-   ##
=======================================
  Coverage   99.52%   99.52%           
=======================================
  Files         123      123           
  Lines       16353    16353           
  Branches     3327     3327           
=======================================
  Hits        16276    16276           
  Misses         68       68           
  Partials        9        9           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@caronc
Copy link
Owner

caronc commented Sep 10, 2023

Thanks for these changes. It's worth noting that there are test build environments only (for unit tests). So there isn't a huge gain from your great finds.

Shrink this docker container and we've got more of a rewarding outcome

@@ -48,13 +48,15 @@ RUN ( \
dnf install -y rpm-build rpmlint python3-pip python3-virtualenv rubygem-ronn \
dnf-plugins-core 'dnf-command(config-manager)' \
'dnf-command(builddep)' sudo rsync rpmdevtools; \
dnf config-manager --set-enabled powertools;
dnf config-manager --set-enabled powertools; \
dnf clean all;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This clean all is not needed, it slows down the second dnf install below

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be! Depends on the network environment and the upstream server being used.

The better way may be to merge those installs, so that the process will be fast, and the image will be slow?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just getting back to this, i think the containers referenced are gone. The fedora and el9 containers are only used to build an rpm and then exit. Your change can be applied to them if you want, but as the last entry after last install

caronc added a commit that referenced this pull request Sep 2, 2024
@caronc
Copy link
Owner

caronc commented Sep 2, 2024

Will close this issue off; was trying to point out that your clean all is a great idea, but it wasn't placed after the last dnf install in this current PR. Attached handles the situation, but only because you pointed out out. so Thank you for that! 🚀 . I will close this PR now in favor of 1dc22e1

@caronc caronc closed this Sep 2, 2024
caronc added a commit that referenced this pull request Sep 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants