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

Fix failing builds to bump Gunicorn, Django, and gevent versions #53

Merged
merged 2 commits into from
Jun 6, 2018

Conversation

rbreslow
Copy link
Contributor

@rbreslow rbreslow commented Jun 5, 2018

Overview

Reviewing #52 revealed issues that I feel are out of the scope of that PR.

Builds for slim images were failing:

Step 3/9 : RUN apt-key adv --keyserver ha.pool.sks-keyservers.net --recv-keys B97B0AFCAA1A47F044F244A07FCC7D46ACCC4CF8
 ---> Running in 607bb7823ea0
E: gnupg, gnupg2 and gnupg1 do not seem to be installed, but one of them is required for this operation
The command '/bin/sh -c apt-key adv --keyserver ha.pool.sks-keyservers.net --recv-keys B97B0AFCAA1A47F044F244A07FCC7D46ACCC4CF8' returned a non-zero code: 255

This was unsettling because the previous build was successful and the only change I made was bumping the Django version in 2.0/requirements.txt. I didn't make any change that would have removed gpg.

tianon/docker-brew-debian#49 was opened by someone who appears to be a Debian maintainer:

Due to a change in APT (such that it no longer Depends: gnupg given that gnupg is only required for the optional apt-key functionality, and is thus now relegated to a Recommends relationship which deboostrap will not install), gnupg is no longer included in our sid/stretch images. Reading through #830696, this is firmly by-design (since otherwise, building a chroot without gpg would be a challenge).

The issue references an exchange from the Debian mailing list:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=830696

A commit from 2016 changes the apt package relationship for gnupg from Depends to Recommends:

This change was introduced in apt (1.3~exp1) experimental.

13 days ago, docker-library/official-images@f200d7f removed 3.5-slim and updated 2.7-slim and 3.6-slim to derive from debian:stretch-slim instead of debian:jessie-slim.

Jessie is built with apt 1.0.9.8.4, which was introduced before the gnupg package relationship was broken:

> rbreslow@maiden docker-django (feature/jrb/django-updates) $ docker run --entrypoint apt d6f -v

WARNING: apt does not have a stable CLI interface yet. Use with caution in scripts.

apt 1.0.9.8.4 for amd64 compiled on Dec 11 2016 09:48:19
...

While Stretch is built with apt 1.4.8:

> rbreslow@maiden docker-django (feature/jrb/django-updates) $ docker run --entrypoint apt f5d -v
apt 1.4.8 (amd64)

This explains why the slim-variant builds began to fail.

I borrowed a fix from docker-library/postgres, and refactored our Dockerfile templates to incorporate the new changes to Python images, as well as what I identified as best practice.

This was helpful reference:

Alpine images were also failing due to an update to gdal that introduced a missing dependency. I updated Dockerfile-alpine.template to install libressl2.7-libcrypto.

See:

Finally, I bumped the Gunicorn, Django, and gevent versions.

See:

Fixes #51

Notes

Alpine Linux removed all PostgresSQL 9.x packages from their 3.7 and edge apk repositories.

Instead of pulling postgresql-client on the slim variant, I pull postgresql-client-$PG_MAJOR. This is what I interpreted as best practice described by https://wiki.postgresql.org/wiki/Apt, and allowed me to source PostgreSQL 9.4 from the official apt.

I updated the README to reflect these changes, as well as to reflect changes to the Python images on Docker Hub (removal of 3.5-slim).

I am not sure if this is what you would consider best practice, @hectcastro, and I am aware that this changes core functionality. I am not sure if we have any services dependent on Python 3.5, etc. I would appreciate your feedback on my approach to solving the issues I describe in my PR.

Testing

See Travis CI build: https://travis-ci.org/azavea/docker-django/builds/388480749

Update Travis-CI tests to use the latest version of PostgreSQL:

 - Change Django 1.9 slim image to use `postgresql-client-9.4` from
 official PostgreSQL apt.
 - Remove unsupported Python interpreter versions.

Update Dockerfile-alpine.template:

 - Add libressl2.7-libcrypto dependency for gdal.

Update Dockerfile-slim.template:

 - Add `-stretch` to the end of parent image tag to avoid future
 compatibility issues.
 - Install gnupg and dependencies to compensate for updated version of
 apt in Debian Stretch.
 - Refactor PostgreSQL apt repo public key.

Refactor README.md to be more specific and include the change to
$PG_MAJOR behavior.

Bump Gunicorn to version 19.8.1, Django to version 2.0.6, and gevent to
version 1.3.2.post0.
@rbreslow rbreslow self-assigned this Jun 5, 2018
@rbreslow rbreslow requested a review from hectcastro June 5, 2018 22:52
@rbreslow
Copy link
Contributor Author

rbreslow commented Jun 6, 2018

Some of the Travis CI builds failed with an error similar to nodejs/docker-node#380. The jobs seem to have restarted themselves and recovered. Not sure what's going on?

fi

RUN set -ex; \
# pub 4096R/ACCC4CF8 2011-10-13 [expires: 2019-07-02]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this might be causing moby/moby#29005

Copy link
Contributor

@hectcastro hectcastro left a comment

Choose a reason for hiding this comment

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

What a great time to inherit the maintenance of this container image! Good work getting to the root of the jessie -> stretch swap and making the appropriate fixes.

Just to surface an alternate approach for discussion, would it have been possible to just pin things to jessie for now and deal with all of the changes required to support stretch later?

django==1.10.8
psycopg2==2.7.4 --no-binary psycopg2
gevent==1.2.2
gevent==1.3.2.post0
Copy link
Contributor

Choose a reason for hiding this comment

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

Just to confirm, you're using the 1.3.2.post0 package because the 1.3.2 one was busted?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah. There was a packaging error I didn't dive too deep into.

- `PG_MAJOR` - Major version number of the target PostgreSQL database instance
- `PG_VERSION` - Version number for `postgresql-client` package
- `VARIANT` - Base container image variant
- `PYTHON_VERSION` - Python interpreter version (for `python:*-slim-stretch` or `python:*-alpine` [images](https://hub.docker.com/_/python/))
Copy link
Contributor

Choose a reason for hiding this comment

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

Good README additions.

&& pip install --no-cache-dir -r /tmp/requirements.txt \
&& apt-get purge -y --auto-remove ${buildDeps} \
&& apt-get purge -y --auto-remove $buildDeps \
$(! command -v gpg > /dev/null || echo 'gnupg dirmngr') \
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the purpose of this?

Copy link
Contributor Author

@rbreslow rbreslow Jun 6, 2018

Choose a reason for hiding this comment

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

Removes gnupg and dirmngr if gpg exists (exits zero).

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I see now. Thanks for clarifying.

README.md Outdated

### Testing

An example of how to use `cibuild` to build and test an image:

```bash
$ CI=1 VERSION=1.9 PYTHON_VERSION=2.7 \
PG_MAJOR=9.6 PG_VERSION=9.5.4-r0 VARIANT=alpine \
$ CI=true VERSION=2.0 PYTHON_VERSION=3.6 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Although both work based on how we check, the use of integers as boolean flags seems more of a convention in Bash scripting than the strings true or false.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -1,14 +1,33 @@
FROM python:%%PYTHON_VERSION%%-slim
FROM python:%%PYTHON_VERSION%%-slim-stretch
Copy link
Contributor

Choose a reason for hiding this comment

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

Making this more explicit is a good idea.

postgresql-client=${PG_VERSION} \
postgresql-client=$PG_VERSION \
&& apk add --no-cache --repository http://dl-cdn.alpinelinux.org/alpine/edge/main \
libressl2.7-libcrypto \
Copy link
Contributor

Choose a reason for hiding this comment

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

This is confusing, but appears to be a required upgrade from 2.6 given that gdal is coming from edge. I wasn't able to make it work with another combination. 😞

Copy link
Contributor Author

Choose a reason for hiding this comment

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

.travis.yml Outdated
- VERSION=2.0 PYTHON_VERSION=3.6 PG_MAJOR=9.6 PG_VERSION=10+190.pgdg80+1 VARIANT=slim
- VERSION=2.0 PYTHON_VERSION=3.6 PG_MAJOR=9.6 PG_VERSION=9.5.12-r0 VARIANT=alpine
- VERSION=1.9 PYTHON_VERSION=2.7 PG_MAJOR=9.4 PG_VERSION=9.4.18-2.pgdg90+1 VARIANT=slim
- VERSION=1.9 PYTHON_VERSION=3.6 PG_MAJOR=9.4 PG_VERSION=9.4.18-2.pgdg90+1 VARIANT=slim
Copy link
Contributor

Choose a reason for hiding this comment

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

1.9 does not support Python 3.6, so I think that we should drop those items from the matrix (the image will still exist in Quay, it just won't get updated).

See: https://docs.djangoproject.com/en/2.0/faq/install/#what-python-version-can-i-use-with-django

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

.travis.yml Outdated
- VERSION=1.9 PYTHON_VERSION=2.7 PG_MAJOR=10 PG_VERSION=10.4-r0 VARIANT=alpine
- VERSION=1.9 PYTHON_VERSION=3.6 PG_MAJOR=10 PG_VERSION=10.4-r0 VARIANT=alpine
- VERSION=1.10 PYTHON_VERSION=2.7 PG_MAJOR=10 PG_VERSION=10.4-2.pgdg90+1 VARIANT=slim
- VERSION=1.10 PYTHON_VERSION=3.6 PG_MAJOR=10 PG_VERSION=10.4-2.pgdg90+1 VARIANT=slim
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here. 1.10 does not support Python 3.6.

@hectcastro
Copy link
Contributor

The jobs seem to have restarted themselves and recovered. Not sure what's going on?

I restarted the 2 builds that were failing manually via the Travis UI.

@rbreslow
Copy link
Contributor Author

rbreslow commented Jun 6, 2018

I restarted the 2 builds that were failing manually via the Travis UI.

Got it.

Just to surface an alternate approach for discussion, would it have been possible to just pin things to jessie for now and deal with all of the changes required to support stretch later?

I didn't connect the issues with slim builds to a change in Debian distribution until looking at the history of the apt repo, fixing gnupg, and then tying it together with debuerreotype/docker-debian-artifacts and docker-library/official-images@f200d7f.

In retrospect, knowing that *-slim had it's distribution swapped would have allowed me to complete #52 and open an issue to upgrade this repository to stretch.

@rbreslow
Copy link
Contributor Author

rbreslow commented Jun 6, 2018

@hectcastro Let me know if 2a2c54c resolves compatibility.

Copy link
Contributor

@hectcastro hectcastro left a comment

Choose a reason for hiding this comment

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

👍

@rbreslow rbreslow merged commit 0736d32 into master Jun 6, 2018
@rbreslow rbreslow deleted the feature/jrb/django-updates branch June 7, 2018 13:56
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.

Upgrade to Django 2.0.6
2 participants