Skip to content

Conversation

@fopina
Copy link
Contributor

@fopina fopina commented Sep 12, 2025

Description

Buildkit allows mounting during RUN so we don't need to use COPY before (creating a layer that will persist the wheels).

I was going to open PR with just that change but ended up adding a few more suggestions, each in its own commit so it's easier to review/remove (to be squashed later/on merge).

Note that trying to remove things from the production container is not only to reduce build time (irrelevant in these tests) and size (not huge) but also reducing attack surface.

Looking forward any feedback - I think at least the wheel dropping is a solid and harmless win :)

Test results

Command used for test: time docker build -t x9x -f Dockerfile.django-??? . --progress plain --no-cache

  • This builds final stage, django-unittests, not the django one
  • Builds done in an M4 Macbook, for arm64 only
Change Debian Time Debian Size Alpine Time Alpine Size
None 1m42 960MB 1m55 732MB
Drop wheels 1m42 866MB 1m46 641MB
Move out tests and other minor 1m40 842MB 1m46 637MB
Split requirements 1m40 836MB 1m45 632MB

Checklist

This checklist is for your information.

  • Make sure to rebase your PR against the very latest dev.
  • Features/Changes should be submitted against the dev.
  • Bugfixes should be submitted against the bugfix branch.
  • Give a meaningful name to your PR, as it may end up being used in the release notes.
  • Your code is flake8 compliant.
  • Your code is python 3.11 compliant.
  • If this is a new feature and not a bug fix, you've included the proper documentation in the docs at https://github.com/DefectDojo/django-DefectDojo/tree/dev/docs as part of this PR.
  • Model changes must include the necessary migrations in the dojo/db_migrations folder.
  • Add applicable tests to the unit tests.
  • Add the proper label to categorize your PR.

@github-actions
Copy link
Contributor

This pull request has conflicts, please resolve those before we can evaluate the pull request.

@Maffooch
Copy link
Contributor

Hi @fopina thank you for your contribution! We are planning to restructure these docker files in the near future to accommodate the points you've raised in addition to some other ones. For now, we would prefer to keep the docker files as is to avoid a moving target. We will absolutely keep your suggestions in mind for when the broader restructuring lands!

@fopina
Copy link
Contributor Author

fopina commented Sep 15, 2025

Hi @Maffooch thanks for feedback, makes sense and also @valentijnscholten made a change that is similar to one of these commits already

What about if I strip this down just to the first commit, to remove wheels from the image (1/6 of the size)?
It should be pretty harmless and good benefit even if we dismiss bandwidth/storage

@valentijnscholten
Copy link
Member

I think it will also make builds faster?

@fopina
Copy link
Contributor Author

fopina commented Sep 15, 2025

When I started going through it, I hoped I'd find something to speed up the build, but just keeping the wheels out does not speed it up (apart from pushing it to registry).

I think the biggest culprits are uwsgi and psycopg as they're always built. Not a big issue on a decent machine for its own arch but quite noticeable when doing multiplatform builds and using qemu

In my case, non-native arch takes about 6min to build with <2min on native one.
And <2min on non-native, if switching to binary versions of those 2 dependencies (not recommending that, just stating 😃 )

@fopina fopina mentioned this pull request Sep 17, 2025
10 tasks
@fopina
Copy link
Contributor Author

fopina commented Sep 17, 2025

re-created a clean PR in #13209 for comparison/consideration

@fopina
Copy link
Contributor Author

fopina commented Sep 17, 2025

replaced by #13209

@fopina fopina closed this Sep 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants