docker: implement multi-stage-builds for ubuntu #3170

ninsbl · 2023-09-21T09:47:10Z

This PR implements multi-stage-builds for ubuntu, comparable to how they are set up for alpine. It also adds a test script for docker files, that should be usable across different distributions...

Image size is reduced to ~1.6 GB (from > 2GB), while some new features are added and datum grids are removed.

A major change is also that GIS packages now come from ubuntugis-unstable, so the image ships more recent versions of GDAL, GEOS. In addition, PDAL does not have to be compiled...

Some dev-packages (e.g. gdal-dev, geos-dev) are kept for addon installation, but they still take quite some space so, I am happy to remove some more, depending on what you consider a good size/functionality trade-off.

Adresses: #776

ninsbl · 2023-09-21T09:47:56Z

P.S.: Backporting may make sense (e.g. after 8.3.1 release)...

mmacata

Cool, thank you! I just added some tiny remarks.

docker/ubuntu/Dockerfile

mmacata · 2023-09-21T11:50:55Z

docker/ubuntu/README.md

+```bash
+# Test basic functionality
+$ docker run -ti \
+         -v /opt/src/grass:/grassdb \


In my understanding /grassdb is the (mounted) destination in the container where the grass data is stored, not the source code. I would distinguish between data and software and use a different target directory. In the end it is not really that important but it might prevent confusion.

Co-authored-by: Carmen Tawalika <mmacata@users.noreply.github.com>

wenzeslaus · 2023-09-21T12:45:14Z

...Image size is reduced to ~1.6 GB (from > 2GB), while some new features are added and datum grids are removed.
...
Some dev-packages (e.g. gdal-dev, geos-dev) are kept for addon installation, but they still take quite some space...

Is the (download?) size a big issue with Docker images these days? I think the original idea was that it's okay to take extra space. It would be good to avoid the majority of people ending up building their own images because they need datum grids or C addons.

echoix · 2023-09-22T13:28:45Z

docker/ubuntu/Dockerfile

 COPY . /src/grass_build/
+
 WORKDIR /src/grass_build

 # Cleanup potentially leftover GISRC file with wrong path to "demolocation"


On the Line after (168, I can't comment directly on it), there is a RUN rm line. This will create a new layer, to remove files in the build stage image, but the files should be cleaned in the same step as they are created to not waste space. This is correct if the files to remove were added from the COPY step. In that case, maybe consider using a .dockerignore file (https://docs.docker.com/engine/reference/builder/#dockerignore-file) to not have them transferred in the first place.

Since this rm is not in the final stage, it would not impact the image produced at the end.

echoix · 2023-09-22T13:31:11Z

docker/ubuntu/Dockerfile

 # Unset environmental variables to avoid later compilation issues
 ENV INTEL ""
 ENV MYCFLAGS ""


With the removed lines, there is no other compilation after, so these ENV are now useless, since these ENV don't affect the env when a new "FROM" is used.

echoix · 2023-09-22T13:44:29Z

docker/ubuntu/Dockerfile

+# Copy GRASS GIS from build image
+COPY --from=build /usr/local/bin/grass* /usr/local/bin/
+COPY --from=build /usr/local/grass84 /usr/local/grass84/
+COPY --from=build /src/site-packages /usr/lib/python3.10/
+COPY --from=build /usr/lib/gdalplugins /usr/lib/gdalplugins


Consider using COPY --link --from=build in order to not invalidate the cache below if the files changed. https://docs.docker.com/engine/reference/builder/#copy---link When using COPY --link, the files are copied as an independent scratch layer, and it's content can be changed without affecting the cache of others. It is useful in multi-stage builds when combining files built in independent stages at the end, since the files copied are really independent and no other RUN commands should be unconditionally run below.

Here, there is only the RUN ln that we might need to check that the behaviour is still correct.

Note: this needs to have

# syntax=docker/dockerfile:1

Or

# syntax=docker/dockerfile:1.4

as the first line, and use the buildx builder. Though usually, multi-stage builds are used with buildx, since it can optimismes what stages to build and built in parallel. The ancien builder would build all stages, needed or not, and wouldn't be faster.

Thanks. Included in 4d4da07

The symbolic link seems to work.

docker/ubuntu/Dockerfile

ninsbl · 2023-09-22T20:10:13Z

Is the (download?) size a big issue with Docker images these days? I think the original idea was that it's okay to take extra space. It would be good to avoid the majority of people ending up building their own images because they need datum grids or C addons.

Personally, I would be OK with adding more dev-packages and datum grids. Basic build tools for addons are kept already and central dev-packages (gdal-dev, geos-dev) are available in the resulting image too. The question is where do we want to draw the line?

Do we btw. have an overview over the dependencies for all official addons, so we could make an informed choice of which packages to include in the image(s)?

Maybe we want to provide (in another PR) different sets of docker images like other projects do: "slim", "full", "with_addons", "python3.9", ...

For now, changes to the Ubuntu Dockerfile in this PR are mostly oriented on the Dockerfile for Alpine. So it is more similar to the Alpine image...

Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>

wenzeslaus · 2023-09-22T20:37:26Z

Do we btw. have an overview over the dependencies for all official addons, so we could make an informed choice of which packages to include in the image(s)?

Not an overview, but we have some additional dependencies installed in the CI. Some info could be extracted from the Makefiles and imports in Python.

For now, changes to the Ubuntu Dockerfile in this PR are mostly oriented on the Dockerfile for Alpine. So it is more similar to the Alpine image...

It makes sense for Alpine to create a slim image since that's the idea behind Alpine. That does not apply to Ubuntu. Ubuntu may suggest the opposite than Alpine. The invented meaning of the word ubuntu the comes to my mind. Urban Dictionary:

Ubuntu is an ancient african word, meaning "I can't configure Debian"

ninsbl · 2023-09-25T09:34:54Z

Here is a list of imports of non-standard python packages in the AddOns repo:

cf_units
cvxopt
h5py
hdfs
ipdb
jinja2
keras
landsatxplore
lxml
matplotlib
mlpy
netCDF4
nnbathy
osgeo (GDAL)
owslib
pandas
PIL
ply
pycsw
pyearth
pyexcel_ods3
pygbif
pygments
pymodis
pyproj
pyspatialite
pysptools
requests
richdem
rpy2
scipy
seaborn
sentinelsat
skimage
sklearn
sqlalchemy
thredds_crawler
tqdm
urllib

So the list is quite long and some packages are relatively large. Python packages on the other hand can be installed as normal user. So maybe those are actually not that important to include?

C-libraries (that require root/admin access) are unfortunately not as straight forward to identify from the code. And even if people with C-knowledge can guess the name of the library from the include, the package of the distro can have different names / versions again.... Stuff I assume I identified are:

flex
fftw
OpenMP
CGAL

But I may have overlooked quite abit.

And then there is R and R packages that are used by some addons... Should R be added as well? Packages can be installed by normal users...

So, the questions remains what is a good level of "completeness" with regards to addon dependencies in Docker images (dependency list should be maintained at some point)...

I have no objections against adding stuff, but help from people with overview esp. of the C- (C++?)-addons would be very much appreciated.

wenzeslaus · 2023-09-25T14:00:40Z

C:

OpenMP is an optional core dependency. You likely want this in the image.
flex, fftw: Technically optional, but crucial. Do others agree?
CGAL: Addons-only

One feature of the addons repo is that the tools there can have an arbitrary dependency (R, CGAL, ... or Java, Hadoop, ...). With the current build and install system, it is hard to support dependencies in addons in a cross-platform way (this might be better with CMake). So leaving this for a custom image would be quite justifiable here. On the other hand, an Ubuntu image or Linux in general is the one place where a hardcoded path in Makefile may just work.

Anyway, when I was suggesting not to make the image too slim, I was more thinking about being able to install addons (in C, C++) which require the same *-dev packages the core requires, not necessary about being able to install and run an arbitrary addon tool.

ninsbl · 2023-09-26T08:57:42Z

Anyway, when I was suggesting not to make the image too slim, I was more thinking about being able to install addons (in C, C++) which require the same *-dev packages the core requires, not necessary about being able to install and run an arbitrary addon tool.

Addressed in:
4d4da07
Now central dev-packages (OpenMP, fftw, and others) are included in the final image, together with datum grids.

echoix

Downloading proj vdatum could be a good candidate for a separate stage, see full comment :)

echoix · 2023-09-26T11:09:00Z

docker/ubuntu/Dockerfile

+RUN mkdir vdatum && \
+    cd vdatum && \
+    wget -q http://download.osgeo.org/proj/vdatum/usa_geoid2012.zip && unzip -j -u usa_geoid2012.zip -d /usr/share/proj; \
+    wget -q http://download.osgeo.org/proj/vdatum/usa_geoid2009.zip && unzip -j -u usa_geoid2009.zip -d /usr/share/proj; \
+    wget -q http://download.osgeo.org/proj/vdatum/usa_geoid2003.zip && unzip -j -u usa_geoid2003.zip -d /usr/share/proj; \
+    wget -q http://download.osgeo.org/proj/vdatum/usa_geoid1999.zip && unzip -j -u usa_geoid1999.zip -d /usr/share/proj; \
+    wget -q http://download.osgeo.org/proj/vdatum/vertcon/vertconc.gtx && mv vertconc.gtx /usr/share/proj; \
+    wget -q http://download.osgeo.org/proj/vdatum/vertcon/vertcone.gtx && mv vertcone.gtx /usr/share/proj; \
+    wget -q http://download.osgeo.org/proj/vdatum/vertcon/vertconw.gtx && mv vertconw.gtx /usr/share/proj; \
+    wget -q http://download.osgeo.org/proj/vdatum/egm96_15/egm96_15.gtx && mv egm96_15.gtx /usr/share/proj; \
+    wget -q http://download.osgeo.org/proj/vdatum/egm08_25/egm08_25.gtx && mv egm08_25.gtx /usr/share/proj; \
+    cd .. && \
+    rm -rf vdatum


If these vdatum downloads aren't needed for the compilation of grass, these can be a really good candidate to be in a separate stage, and copied over with a COPY --link. Using a fast base image, that is, either a small image or an image already downloaded, will allow to download in parallel to the build. Copying them over in a COPY --link layer will make that layer not change when the rest of the build changes, and could be cached locally (and could even be the same if used in other Docker images that would result in the same layer hash).

I couldn't add the comment to these lines to the file, so I added them to the specific commit that reverted a change.

ninsbl · 2023-09-27T09:17:53Z

Thanks @echoix I am learning a lot here (still relatively new to docker).

Did you mean something like: 1822d00 ?

Additional follow-up on datum grids: datum grids fetched before this PR are meant for proj < 7, right? So I changed the source here as the docker image now uses proj > 9.0. Pleasea anyone, correct me if I am wrong. The mirroring of the entire https://cdn.proj.org/ should be also less prone to accidental ommissions... It is more data though, (703 MB in datum grids)...

neteler · 2023-09-27T09:58:16Z

The mirroring of the entire https://cdn.proj.org/ should be also less prone to accidental ommissions... It is more data though, (703 MB in datum grids)...

What about this alternative:

PROJ_NETWORK (New in version 7.0.0):

If set to ON, enable the capability to use remote grids stored on CDN (Content Delivery Network) storage, when grids are not available locally.

hence, using
export PROJ_NETWORK=ON
?

ninsbl · 2023-09-29T06:35:15Z

A simplification and de-duplication is most welcome.

Please have a look at the build_arg GUI.

echoix

I’m good for the Docker part! When the deduplication was mentioned by neteller the other day, that was the advanced multi-stage « conditional » way I was thinking, but didn’t think it was appropriate for me to suggest it this early, but it was really well done. For the other files, I can’t really comment more than looks good to me.

I get a little experience and practice on advanced Docker and CI when collaborating/maintaining Megalinter, as the bulk of the repo is building lots of Docker images, and pushing the CI workflows.

ninsbl · 2023-09-29T17:08:48Z

We may adjust the way docker images are built and pushed in a follow up PR, and remove the ubuntu_wxgui directory in such a PR...

ninsbl · 2023-10-13T12:14:45Z

Taking the liberty to merge. Since this is mainly packaging / shipping related, I would like to suggest to backport it after 8.3.1 is released, so "current" and "main" images become as comparable as possible!?!

Please feel free to remove the backport label if anyone disagrees...

wenzeslaus · 2023-10-13T16:11:35Z

This was merged with broken Additional checks check which is actually catching an issue relevant to this PR namely enforcing intentional code duplicity.

- Copies over the Ubuntu Dockerfile edited in #3170 to the main Dockerfile in the root of the repo. - Adds a note at the top for future edits to the file (in both files). Additionally, this includes a temporary fix for pytest-pylint where new version finds more issues than the old one, so we need to use the old one before we fix the issues.

* test grass.script.setup * test script for docker images * black * add test instructions * multi-stage-build similar to alpine * Apply suggestions from code review Co-authored-by: Carmen Tawalika <mmacata@users.noreply.github.com> * add test for GDAL-plugin * copy GDAL-plugin and set path * Update docker/ubuntu/Dockerfile Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com> * ignore dist.* in docker builds * address review comments * separate datum grid (proj >= 7) stage * use network grids * GUI build argument * GUI build argument --------- Co-authored-by: ninsbl <stbl@nve.no> Co-authored-by: Carmen Tawalika <mmacata@users.noreply.github.com> Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>

- Copies over the Ubuntu Dockerfile edited in OSGeo#3170 to the main Dockerfile in the root of the repo. - Adds a note at the top for future edits to the file (in both files). Additionally, this includes a temporary fix for pytest-pylint where new version finds more issues than the old one, so we need to use the old one before we fix the issues.

* test grass.script.setup * test script for docker images * black * add test instructions * multi-stage-build similar to alpine * Apply suggestions from code review Co-authored-by: Carmen Tawalika <mmacata@users.noreply.github.com> * add test for GDAL-plugin * copy GDAL-plugin and set path * Update docker/ubuntu/Dockerfile Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com> * ignore dist.* in docker builds * address review comments * separate datum grid (proj >= 7) stage * use network grids * GUI build argument * GUI build argument --------- Co-authored-by: ninsbl <stbl@nve.no> Co-authored-by: Carmen Tawalika <mmacata@users.noreply.github.com> Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>

- Copies over the Ubuntu Dockerfile edited in OSGeo#3170 to the main Dockerfile in the root of the repo. - Adds a note at the top for future edits to the file (in both files). Additionally, this includes a temporary fix for pytest-pylint where new version finds more issues than the old one, so we need to use the old one before we fix the issues.

* test grass.script.setup * test script for docker images * black * add test instructions * multi-stage-build similar to alpine * Apply suggestions from code review Co-authored-by: Carmen Tawalika <mmacata@users.noreply.github.com> * add test for GDAL-plugin * copy GDAL-plugin and set path * Update docker/ubuntu/Dockerfile Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com> * ignore dist.* in docker builds * address review comments * separate datum grid (proj >= 7) stage * use network grids * GUI build argument * GUI build argument --------- Co-authored-by: ninsbl <stbl@nve.no> Co-authored-by: Carmen Tawalika <mmacata@users.noreply.github.com> Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>

- Copies over the Ubuntu Dockerfile edited in OSGeo#3170 to the main Dockerfile in the root of the repo. - Adds a note at the top for future edits to the file (in both files). Additionally, this includes a temporary fix for pytest-pylint where new version finds more issues than the old one, so we need to use the old one before we fix the issues.

neteler · 2024-01-10T09:21:51Z

P.S.: Backporting may make sense (e.g. after 8.3.1 release)...

Do you plan to backport this PR, @ninsbl ?

landam · 2024-02-09T16:29:26Z

P.S.: Backporting may make sense (e.g. after 8.3.1 release)...

Do you plan to backport this PR, @ninsbl ?

There are unfortunately conflicts. @ninsbl Is the backport to 8.3 still relevant (8.4.0 is quite close)?

nvesbl added 5 commits September 21, 2023 09:10

test grass.script.setup

bacaaf2

test script for docker images

91baf3f

black

265d757

add test instructions

4464096

multi-stage-build similar to alpine

3fb56ef

ninsbl added the docker Docker related label Sep 21, 2023

ninsbl added this to the 8.4.0 milestone Sep 21, 2023

ninsbl requested review from neteler and wenzeslaus September 21, 2023 09:47

mmacata reviewed Sep 21, 2023

View reviewed changes

Apply suggestions from code review

991ec3c

Co-authored-by: Carmen Tawalika <mmacata@users.noreply.github.com>

nvesbl added 2 commits September 22, 2023 10:18

add test for GDAL-plugin

68a8a4d

copy GDAL-plugin and set path

cb8363b

echoix reviewed Sep 22, 2023

View reviewed changes

echoix requested changes Sep 22, 2023

View reviewed changes

docker/ubuntu/Dockerfile Outdated Show resolved Hide resolved

ninsbl and others added 2 commits September 22, 2023 22:11

Update docker/ubuntu/Dockerfile

8d9630f

Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>

ignore dist.* in docker builds

4203822

ninsbl mentioned this pull request Sep 25, 2023

actinia-ubuntu: multi-stage build oriented on Alpine actinia-org/actinia-docker#61

Open

address review comments

4d4da07

echoix reviewed Sep 26, 2023

View reviewed changes

separate datum grid (proj >= 7) stage

1822d00

nvesbl added 2 commits September 28, 2023 22:22

GUI build argument

60e930c

GUI build argument

58e4042

ninsbl requested a review from echoix September 29, 2023 06:31

echoix approved these changes Sep 29, 2023

View reviewed changes

ninsbl merged commit 0bc5039 into OSGeo:main Oct 13, 2023
17 of 18 checks passed

ninsbl modified the milestones: 8.4.0, 8.3.2 Oct 13, 2023

ninsbl added the backport to 8.3 PR needs to be backported to release branch 8.3 label Oct 13, 2023

wenzeslaus mentioned this pull request Oct 13, 2023

r.stats: Use long total_count to avoid int overflow #3203

Merged

This was referenced Oct 14, 2023

[Bug] Failing Additional checks since Dockerfile and docker/ubuntu/Dockerfile are different #3204

Closed

Docker: Sync main Dockerfile to docker/ubuntu/Dockerfile #3205

Merged

landam modified the milestones: 8.3.2, 8.4.0 Feb 9, 2024

landam assigned landam and ninsbl and unassigned landam Feb 9, 2024

landam removed the backport to 8.3 PR needs to be backported to release branch 8.3 label Feb 11, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docker: implement multi-stage-builds for ubuntu #3170

docker: implement multi-stage-builds for ubuntu #3170

ninsbl commented Sep 21, 2023

ninsbl commented Sep 21, 2023

mmacata left a comment

mmacata Sep 21, 2023

wenzeslaus commented Sep 21, 2023

echoix Sep 22, 2023

echoix Sep 22, 2023

echoix Sep 22, 2023 •

edited

Loading

ninsbl Sep 26, 2023

ninsbl commented Sep 22, 2023

wenzeslaus commented Sep 22, 2023

ninsbl commented Sep 25, 2023

wenzeslaus commented Sep 25, 2023

ninsbl commented Sep 26, 2023

echoix left a comment

echoix Sep 26, 2023

ninsbl commented Sep 27, 2023

neteler commented Sep 27, 2023

ninsbl commented Sep 29, 2023

echoix left a comment

ninsbl commented Sep 29, 2023

ninsbl commented Oct 13, 2023

wenzeslaus commented Oct 13, 2023

neteler commented Jan 10, 2024

landam commented Feb 9, 2024

docker: implement multi-stage-builds for ubuntu #3170

docker: implement multi-stage-builds for ubuntu #3170

Conversation

ninsbl commented Sep 21, 2023

ninsbl commented Sep 21, 2023

mmacata left a comment

Choose a reason for hiding this comment

mmacata Sep 21, 2023

Choose a reason for hiding this comment

wenzeslaus commented Sep 21, 2023

echoix Sep 22, 2023

Choose a reason for hiding this comment

echoix Sep 22, 2023

Choose a reason for hiding this comment

echoix Sep 22, 2023 • edited Loading

Choose a reason for hiding this comment

ninsbl Sep 26, 2023

Choose a reason for hiding this comment

ninsbl commented Sep 22, 2023

wenzeslaus commented Sep 22, 2023

ninsbl commented Sep 25, 2023

wenzeslaus commented Sep 25, 2023

ninsbl commented Sep 26, 2023

echoix left a comment

Choose a reason for hiding this comment

echoix Sep 26, 2023

Choose a reason for hiding this comment

ninsbl commented Sep 27, 2023

neteler commented Sep 27, 2023

ninsbl commented Sep 29, 2023

echoix left a comment

Choose a reason for hiding this comment

ninsbl commented Sep 29, 2023

ninsbl commented Oct 13, 2023

wenzeslaus commented Oct 13, 2023

neteler commented Jan 10, 2024

landam commented Feb 9, 2024

echoix Sep 22, 2023 •

edited

Loading