Refactor: Dockerimage #5007

wolflu05 · 2023-06-08T22:59:42Z

This PR refactors the docker image to reduce its size dramatically. It now weights 524MB.

TODO

Give feedback

remove gnupg

remove gnupg
Options
Successfully updated the issue's project

There was an error updating the issue's project
rewrite init script with ash and remove bash

rewrite init script with ash and remove bash
Options
Successfully updated the issue's project

There was an error updating the issue's project
test with existing installation

test with existing installation
Options
Successfully updated the issue's project

There was an error updating the issue's project
test with all dbms

test with all dbms
Options
Successfully updated the issue's project

There was an error updating the issue's project
figure out how to get the other db drivers to work, there are some native dependencies missing

figure out how to get the other db drivers to work, there are some native dependencies missing
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

fixes #4924

wolflu05 · 2023-06-09T00:20:07Z

Is git actually required in the image now? I would say yes, as some plugins are installed via git.

SchrodingersGat · 2023-06-09T00:21:23Z

Is git actually required in the image now? I would say yes, as some plugins are installed via git.

To install via git we would need the git executable, yeah

Dockerfile

wolflu05 · 2023-06-09T09:21:19Z

Now the image weights about 629MB. I'd love if we could reduce the needed dependencies for the database drivers.

wolflu05 · 2023-06-10T10:12:33Z

What's really interesting and what could be used to consider using an alpine image would be this Dockerfile. It's an other open source project that's also based on Django and also has to deal with lots of images (from your cooked meals) and also has ldap support already built in and weights out 350mb (uncompressed).

SchrodingersGat · 2023-06-10T10:15:17Z

@wolflu05 two things immediately stick out in the linked image

Running purge step after install
Running all install steps on the one line

Might be worth applying that to our image

wolflu05 · 2023-06-10T16:38:47Z

Running purge step after install

I already do this as much as I can and know with multistage builds. The packages that are installed in the production stage are needed for inventree to work, because they provide native linked libraries that get imported from e.g. the pip psql lib.

Running all install steps on the one line

I tried squashing all image layers, but this also doesn't reduce the size of the image.

wolflu05 · 2023-06-10T18:17:09Z

I just tried how alpine turns out. Here is a little comparison with the current state (0080176) of pushed files:

	Size	Build time *
Old	`1.03GB`	`119.0s`
New debian slim based	`778MB`	`135.8s`
New alpine based	`556MB`	`141.8s`

* all builds on MacBook Pro 2019 (docker cache disabled)

SchrodingersGat · 2023-06-10T21:31:41Z

The alpine image is looking pretty appealing then!

wolflu05 · 2023-06-11T07:23:22Z

FYI, adding LDAP needed packages adds about 5MB to the alpine image, so it weights: 561MB. And about 5MB to the Debian image so it weights: 781MB.

Dockerfile changes

diff --git a/Dockerfile b/Dockerfile
index d60ab788b..46c0ba518 100644
--- a/Dockerfile
+++ b/Dockerfile
@@ -77,7 +77,7 @@ FROM inventree_base as build_base
 
 # Install build required system packages
 RUN apt-get update && apt-get install -y  --no-install-recommends \
-    gcc g++ libffi-dev libssl-dev && \
+    gcc g++ python3-dev libffi-dev libssl-dev libldap2-dev libsasl2-dev && \
     apt-get autoclean && apt-get autoremove
 
 # For ARMv7 architecture, add the pinwheels repo (for cryptography library)
diff --git a/Dockerfile.alpine b/Dockerfile.alpine
index 5990bd53a..0f261bae2 100644
--- a/Dockerfile.alpine
+++ b/Dockerfile.alpine
@@ -46,7 +46,7 @@ LABEL org.label-schema.schema-version="1.0" \
       org.label-schema.vcs-ref=${commit_tag}
 
 RUN apk add --no-cache \
-    git gnupg gettext py-cryptography bash \
+    git gnupg gettext py-cryptography bash openldap \
     # Image format support
     libjpeg libwebp zlib \
     # Weasyprint requirements : https://doc.courtbouillon.org/weasyprint/stable/first_steps.html#alpine-3-12
@@ -69,7 +69,7 @@ COPY ./docker/requirements.txt base_requirements.txt
 COPY ./requirements.txt ./
 
 RUN apk add --no-cache --virtual .build-deps \
-    gcc g++ musl-dev openssl-dev libffi-dev cargo python3-dev \
+    gcc g++ musl-dev openssl-dev libffi-dev cargo python3-dev openldap-dev \
     # Image format dev libs
     jpeg-dev openjpeg-dev libwebp-dev zlib-dev \
     # DB specific dev libs
diff --git a/requirements.txt b/requirements.txt
index 487dcfead..8bac49a90 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -305,5 +305,8 @@ zipp==3.15.0
 zopfli==0.2.2
     # via fonttools
 
+python-ldap==3.4.3
+django-auth-ldap==4.2.0
+
 # The following packages are considered to be unsafe in a requirements file:
 # setuptools

wolflu05 · 2023-06-11T09:28:29Z

Interested to hear what @matmair has to say to this alpine approach.

SchrodingersGat · 2023-06-11T09:39:03Z

It certainly saves a lot of bandwidth. And the LDAP addition seems minor, but I have no horse in that race :)

matmair · 2023-06-11T09:48:35Z

If it works why not. Have you verified that ldap actually resolved all objectTypes and both ldap and ldaps with the installed dependencies?

@wolflu05 The size reduction is very impressive. Is there something significantly different to the linked project? They seem to be even smaller so I wonder how much we need to change to achieve that size. With the amount the image gets pulled this PR certainly has positive carbon-footprint implications.

The change from Debian to Alpin should to be marked breaking and mentioned in the release notes, plugins will need to adapt their (native) install instructions to work on the docker as apt will not work.

wolflu05 · 2023-06-11T10:18:07Z

If it works why not. Have you verified that ldap actually resolved all objectTypes and both ldap and ldaps with the installed dependencies?

Not fully, need some further testing.

The size reduction is very impressive. Is there something significantly different to the linked project? They seem to be even smaller so I wonder how much we need to change to achieve that size. With the amount the image gets pulled this PR certainly has positive carbon-footprint implications.

They don't use lots of things we need:

weasyprint
mariadb
git
gnupg
bash (maybe we rewrite the init script to use ash)
our codebase weights about 90MB

The change from Debian to Alpin should to be marked breaking and mentioned in the release notes, plugins will need to adapt their (native) install instructions to work on the docker as apt will not work.

Yes definitely it will break some things.

Idea: provide both alpine and Debian for the next release for a smooth transition period until plugins are adapted.

Further analysis needed: Does switching out the images work easily with an already installed InvenTree. Maybe the venv that's mounted via volume contains some files that are not compatible with alpine.

SchrodingersGat · 2023-06-11T10:25:11Z

Further analysis needed: Does switching out the images work easily with an already installed InvenTree. Maybe the venv that's mounted via volume contains some files that are not compatible with alpine.

We definitely need to investigate this before we push this :)

wolflu05 · 2023-06-11T10:29:39Z

Oh and I forgot the biggest point why our alpine image is bigger than tandoors. Our codebase weights about 90MB with all the translated templates and po files.

matmair · 2023-06-11T11:17:38Z

Oh and I forgot the biggest point why our alpine image is bigger than tandoors. Our codebase weights about 90MB with all the translated templates and po files.

That should be solved by #2789 as we will only ship messages

matmair · 2023-06-11T13:28:21Z

@wolflu05 I think we can remove gnupg - we do not ship the code that used it anymore.

Would be interested in your thoughts on #5017 - background

SchrodingersGat · 2023-06-20T12:39:30Z

@wolflu05 just a friendly bump to see where you are up to with this :)

wolflu05 · 2023-06-20T16:08:51Z

Need to remove gnupg and do some more testing with other db drivers and also with existing installs.

matmair · 2023-07-07T06:02:43Z

@wolflu05 do you want/need any support on this one? Would like to use this for #5011

wolflu05 · 2023-07-09T14:30:57Z

Are we still want to support the dev target from our current image? I don't think it's necessary as we have devcontainer and native install.

SchrodingersGat · 2023-07-09T14:47:05Z

I would think so from my side. What do you mean @SchrodingersGat .

I support moving entirely to alpine for base image

Are we still want to support the dev target from our current image? I don't think it's necessary as we have devcontainer and native install.

I suppose this is a fair point? But is there a downside to leaving it in, at least for a while, for anyone who would still be using this?

wolflu05 · 2023-07-09T15:13:38Z

I suppose this is a fair point? But is there a downside to leaving it in, at least for a while, for anyone who would still be using this?

I plan to replace the current Dockerfile completely with the .alpine one. And in the alpine one is no dev target. There is just the production target. So it would be more effort to recreate a dev target with alpine smoothly.
EDIT: Added it, was not that much work as I thought it would be.

wolflu05 · 2023-07-09T23:30:16Z

@matmair @SchrodingersGat I'm done now with the changes I wanted to do. The image now weights 524MB. The biggest things are (estimated):

weasyprint, fonts, ... (~100MB)
mysql related stuff (~70MB)
translated js templates (~50MB)
git (~20MB)

🚧 Help needed

I already tested a lot of scenarios with existing installs, other dbms, already installed plugins, ... and everything worked so far. Before this is ready to merge, I'd like to ask if you could test this too so we do not overlook something.

SchrodingersGat · 2023-07-09T23:36:36Z

@wolflu05 awesome :) I'll take a look at this tonight

SchrodingersGat · 2023-07-10T04:26:59Z

The development image works as expected on my end. @matmair any final thoughts before we merge this in?

matmair · 2023-07-10T05:36:32Z

LGTM; I take it that we are fully removing support for the Raspberry Pi architecture with this as the package resources are removed?

matmair · 2023-07-10T05:38:15Z

Dockerfile

-    # Weasyprint requirements : https://doc.courtbouillon.org/weasyprint/stable/first_steps.html#debian-11
-    poppler-utils libpango-1.0-0 libpangoft2-1.0-0 \
+RUN apk add --no-cache \
+    git gettext py-cryptography \


Not sure we need git any more

What about to install a plugin from a remote git repo?

I do not think it is a good idea to push that way to install plugins; discovery is slower and the support for nested plugins is limited; pip as a distribution way is way cleaner

I would argue to keep git and add a big note somewhere to the plugin dev
section. Sometimes you just want to test something and then git is easier.

pip as a distribution way is way cleaner

Absolutely agreed, but there may be users who have their own plugins not available via pip?

matmair · 2023-07-10T05:40:28Z

Dockerfile

-
-# Server init entrypoint
-ENTRYPOINT ["/bin/bash", "./init.sh"]
+COPY InvenTree ./InvenTree


Should we clean up what we ship in a next step? Ie. remove contrib things, assets and anything else that is not needed to run the app.

I don't think there is much we can remove, we already only copy the InvenTree folder.

SchrodingersGat · 2023-07-10T06:36:06Z

LGTM; I take it that we are fully removing support for the Raspberry Pi architecture with this as the package resources are removed?

@wolflu05 does removing RPi support gain us any space? I would imagine not? Perhaps it is beneficial to leave it in for anyone who wants to target raspbian

wolflu05 · 2023-07-10T06:39:06Z

Not sure, haven't tested, because I have no access to one, should I add it and Someone of you test it?

SchrodingersGat · 2023-07-10T06:55:01Z

Not sure, haven't tested, because I have no access to one, should I add it and Someone of you test it?

I don't have a way to test it really, but I think maybe we should just leave those lines in. Unless you are building to target raspbian, they don't do anything

SchrodingersGat · 2023-07-10T09:52:27Z

@wolflu05 thanks for the fine work here :)

SchrodingersGat · 2023-07-10T10:25:51Z

@wolflu05 this PR truly was a breaking change: https://github.com/inventree/InvenTree/actions/runs/5506646753/jobs/10035587151

Looks like we did not run through the complete set of unit tests...

Update

ok, I missed that this line was deleted - these are pretty critical for report generation! Was this intentional, or an oversight? We'll need to add these back in.

Fix

See #5213

refactor docker image to reduce size

f99b844

wolflu05 requested a review from SchrodingersGat as a code owner June 8, 2023 22:59

wolflu05 added refactor docker Docker / docker-compose labels Jun 8, 2023

Added required deps for database drivers

3c9c40c

wolflu05 commented Jun 9, 2023

View reviewed changes

Dockerfile Outdated Show resolved Hide resolved

wolflu05 added 2 commits June 10, 2023 19:58

Added alpine test dockerfile

fd70a1b

Readded image libs to Dockerfile

0080176

Fix link in dockerfile

7b43c45

matmair mentioned this pull request Jun 26, 2023

Platform UI - React integration #5011

Merged

5 tasks

wolflu05 added 8 commits July 9, 2023 19:37

Replaced Dockerfile

8045c67

Remove bash from image

45abf90

Added base target to Dockerfile

94468f0

Merge remote-tracking branch 'upstream/master' into refactor/dockerfile

e27a810

Added missing mysql dev headers

cd0eb89

Add dev target back to Dockerfile

7f992e5

fix: Dockerfile style

f84f03e

Add old comments back to dockerfile

6ff801b

SchrodingersGat added this to the 0.13.0 milestone Jul 10, 2023

matmair approved these changes Jul 10, 2023

View reviewed changes

wolflu05 added 2 commits July 10, 2023 08:56

Removed wrong comment

7515daa

Added back piwheels repo

9c6d0f0

wolflu05 added the breaking Indicates a major update or change which breaks compatibility label Jul 10, 2023

SchrodingersGat merged commit 4a1f733 into inventree:master Jul 10, 2023
14 checks passed

wolflu05 deleted the refactor/dockerfile branch July 10, 2023 09:53

SchrodingersGat mentioned this pull request Jul 10, 2023

Docker: add missing reporting libs #5213

Merged

wolflu05 mentioned this pull request Jul 11, 2023

Native LDAP support #5226

Merged

Refactor: Dockerimage #5007

Refactor: Dockerimage #5007

Conversation

wolflu05 commented Jun 8, 2023 • edited

TODO

wolflu05 commented Jun 9, 2023

SchrodingersGat commented Jun 9, 2023

wolflu05 commented Jun 9, 2023

wolflu05 commented Jun 10, 2023

SchrodingersGat commented Jun 10, 2023

wolflu05 commented Jun 10, 2023 • edited

wolflu05 commented Jun 10, 2023 • edited

SchrodingersGat commented Jun 10, 2023

wolflu05 commented Jun 11, 2023 • edited

wolflu05 commented Jun 11, 2023

SchrodingersGat commented Jun 11, 2023

matmair commented Jun 11, 2023 • edited

wolflu05 commented Jun 11, 2023 • edited

SchrodingersGat commented Jun 11, 2023

wolflu05 commented Jun 11, 2023

matmair commented Jun 11, 2023

matmair commented Jun 11, 2023

SchrodingersGat commented Jun 20, 2023

wolflu05 commented Jun 20, 2023

matmair commented Jul 7, 2023

wolflu05 commented Jul 9, 2023

SchrodingersGat commented Jul 9, 2023

wolflu05 commented Jul 9, 2023 • edited

wolflu05 commented Jul 9, 2023 • edited

🚧 Help needed

SchrodingersGat commented Jul 9, 2023

SchrodingersGat commented Jul 10, 2023

matmair commented Jul 10, 2023

matmair Jul 10, 2023

Choose a reason for hiding this comment

SchrodingersGat Jul 10, 2023

Choose a reason for hiding this comment

matmair Jul 10, 2023

Choose a reason for hiding this comment

wolflu05 Jul 10, 2023

Choose a reason for hiding this comment

SchrodingersGat Jul 10, 2023

Choose a reason for hiding this comment

matmair Jul 10, 2023

Choose a reason for hiding this comment

wolflu05 Jul 10, 2023

Choose a reason for hiding this comment

SchrodingersGat commented Jul 10, 2023

wolflu05 commented Jul 10, 2023

SchrodingersGat commented Jul 10, 2023

SchrodingersGat commented Jul 10, 2023

SchrodingersGat commented Jul 10, 2023 • edited

Update

Fix

wolflu05 commented Jun 8, 2023 •

edited

wolflu05 commented Jun 10, 2023 •

edited

wolflu05 commented Jun 10, 2023 •

edited

wolflu05 commented Jun 11, 2023 •

edited

matmair commented Jun 11, 2023 •

edited

wolflu05 commented Jun 11, 2023 •

edited

wolflu05 commented Jul 9, 2023 •

edited

wolflu05 commented Jul 9, 2023 •

edited

SchrodingersGat commented Jul 10, 2023 •

edited