-
Notifications
You must be signed in to change notification settings - Fork 476
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
Add alpine variants for all versions #49
Conversation
I updated the makefile so it also builds alpine variants, and the update script so it generates the alpine files and the env list for the travis build file. |
We tested this by replicating the DockerFile and initdb in our project (version 9.4) and everything seems to work so far 👍 Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. I'd just like to better understand why the edge
stuff is being done.
9.2-2.3/alpine/Dockerfile
Outdated
ENV POSTGIS_VERSION 2.3.1 | ||
ENV POSTGIS_SHA256 0f1e0fe08f55b6db791d8600bb343e4f77590725acbbec99094a30b8b7439cd8 | ||
|
||
RUN echo http://dl-cdn.alpinelinux.org/alpine/edge/main >> /etc/apk/repositories |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since the postgres:alpine
base images are not based on alpine:edge
, do you think you could explain this addition along with the call to apk upgrade
below?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have to add the edge main repository because that's where the packages for geos
and gdal
are, and the edge testing repository for proj4
.
I added the apk upgrade
after a build failure where there was a package version conflict, I don't remember which one exactly. The 1.15 version of this library was installed in the postgres base image, a new version (1.16) was available in the repository and when one of the packages needed for postgis that depends on that lib tried to install, it failed because it required the new version, but the old one was installed as a dependency of another package I wasn't upgrading.
apk add --upgrade
didn't work, but upgrading the packages first did.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tclarke @ncopa It looks like you guys are the maintainers of the gdal
and proj4
Alpine packages respectively, and I know that you @ncopa have been involved in the postgresql:alpine
Docker image.
Do either of you have a sense of whether geos
, gdal
, and proj4
will make it out of edge
into a numbered release any time soon? Also, I'm curious to get your thoughts @ncopa on the drawbacks of running an apk upgrade
. I'm hesitant to have that in the Dockerfile
since it seems like it could result in an inconsistent build artifact (not to mention bloat from upgrading packages found in the base alpine:3.5
or postgres:*-alpine
image).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From https://github.com/docker-library/official-images#repeatability:
For dependent packages installed by apt there is not usually a need to pin them to a version.
Since postgres is built from source in the base image, it's not going to be upgraded in any way by apk upgrade
. I could also build the dependencies for postgis from source to use specific versions, but I'm not sure it makes that much of a difference since the image provides the right versions of postgres and postgis anyway.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not worried about postgres
being upgraded, I'm worried about all the other unrelated upgrades that will happen when running apk upgrade
.
Here's a diff of running apk info -v
between the base postgres:9.6-alpine
image and mdillon/postgis:9.6-alpine
as built from your branch:
--- /dev/fd/63 2017-02-06 11:15:50.000000000 -0800
+++ /dev/fd/62 2017-02-06 11:15:50.000000000 -0800
@@ -1,29 +1,42 @@
+.postgis-rundeps-0
.postgresql-rundeps-0
+WARNING: Ignoring APKINDEX.066df28d.tar.gz: No such file or directory
+WARNING: Ignoring APKINDEX.30e6f5af.tar.gz: No such file or directory
WARNING: Ignoring APKINDEX.c51f8f92.tar.gz: No such file or directory
WARNING: Ignoring APKINDEX.d09172fd.tar.gz: No such file or directory
alpine-baselayout-3.0.4-r0
alpine-keys-1.3-r0
-apk-tools-2.6.8-r1
-bash-4.3.46-r5
-busybox-1.25.1-r0
-libc-utils-0.7-r1
+apk-tools-2.6.8-r2
+bash-4.3.48-r1
+busybox-1.26.1-r3
+gdal-2.1.0-r1
+geos-3.5.0-r0
+giflib-5.1.4-r1
+json-c-0.12.1-r0
+libc-utils-0.7.1-r0
libcrypto1.0-1.0.2k-r0
libedit-20150325.3.1-r3
-libgcrypt-1.7.3-r0
+libgcc-6.3.0-r1
+libgcrypt-1.7.5-r0
libgpg-error-1.24-r0
-libressl2.4-libcrypto-2.4.4-r0
-libressl2.4-libssl-2.4.4-r0
+libjpeg-turbo-1.5.1-r0
+libpng-1.6.27-r0
+libressl2.4-libcrypto-2.4.5-r0
+libressl2.4-libssl-2.4.5-r0
libssl1.0-1.0.2k-r0
+libstdc++-6.3.0-r1
libuuid-2.28.2-r1
libxml2-2.9.4-r0
libxslt-1.1.29-r0
-musl-1.1.15-r6
-musl-utils-1.1.15-r6
+musl-1.1.16-r3
+musl-utils-1.1.16-r3
ncurses-libs-6.0-r7
ncurses-terminfo-6.0-r7
ncurses-terminfo-base-6.0-r7
-readline-6.3.008-r4
+proj4-4.9.1-r0
+readline-6.3.008-r5
scanelf-1.1.6-r0
su-exec-0.2-r0
-tzdata-2016i-r0
-zlib-1.2.8-r2
+tiff-4.0.7-r1
+tzdata-2016j-r0
+zlib-1.2.11-r0
As you can see, there are a bunch of updates that happen that are unrelated to installing the dependencies needed for postgis
. I'm not comfortable with the idea that upgrading any arbitrary package from the numbered version of Alpine to edge
will always be safe to do.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition, the layer where you run apk upgrade
and build PostGIS is 105MB, bigger than any other layer in the image. I'd be curious to know how much of this is coming from additional packages that are needed for PostGIS and how much is coming from packages that are being upgraded.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I tried to reproduce the problem but it's working now without the apk upgrade
.
Just to answer your question, here's the difference in size with and without:
REPOSITORY TAG IMAGE ID CREATED SIZE
mdillon/postgis 9.2-alpine-upgrade 62412cc45521 4 minutes ago 140 MB
mdillon/postgis 9.2-alpine-no-upgrade 5580d6c1f7a1 7 minutes ago 133 MB
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mind trying to get it to work with repository pinning? If we can ensure that only the testing
packages are installed from the testing repo, I'd be happy with this change.
Thanks for your patience ❤️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW, this is the command I used to generate the diff above:
$ diff -u <(docker run --rm postgres:9.6-alpine apk info -v | sort) \
<(docker run --rm mdillon/postgis:9.6-alpine apk info -v | sort)
make \ | ||
perl \ | ||
\ | ||
&& apk add --no-cache --virtual .build-deps-testing \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to do it like this instead of just using tagged repositories because of this bug.
The new diff looks much better: --- /dev/fd/63 2017-02-08 09:42:27.000000000 -0800
+++ /dev/fd/62 2017-02-08 09:42:27.000000000 -0800
@@ -1,3 +1,5 @@
+.postgis-rundeps-0
+.postgis-rundeps-testing-0
.postgresql-rundeps-0
WARNING: Ignoring APKINDEX.c51f8f92.tar.gz: No such file or directory
WARNING: Ignoring APKINDEX.d09172fd.tar.gz: No such file or directory
@@ -6,14 +8,22 @@
apk-tools-2.6.8-r1
bash-4.3.46-r5
busybox-1.25.1-r0
+gdal-2.1.0-r1
+geos-3.5.0-r0
+giflib-5.1.4-r1
+json-c-0.12.1-r0
libc-utils-0.7-r1
libcrypto1.0-1.0.2k-r0
libedit-20150325.3.1-r3
+libgcc-6.2.1-r1
libgcrypt-1.7.3-r0
libgpg-error-1.24-r0
+libjpeg-turbo-1.5.1-r0
+libpng-1.6.25-r0
libressl2.4-libcrypto-2.4.4-r0
libressl2.4-libssl-2.4.4-r0
libssl1.0-1.0.2k-r0
+libstdc++-6.2.1-r1
libuuid-2.28.2-r1
libxml2-2.9.4-r0
libxslt-1.1.29-r0
@@ -22,8 +32,10 @@
ncurses-libs-6.0-r7
ncurses-terminfo-6.0-r7
ncurses-terminfo-base-6.0-r7
+proj4-4.9.1-r0
readline-6.3.008-r4
scanelf-1.1.6-r0
su-exec-0.2-r0
+tiff-4.0.7-r1
tzdata-2016i-r0
zlib-1.2.8-r2 |
The |
Closes #46.
The lib problem in the official postgres images is fixed (see docker-library/postgres#247) so this is quite easy now.