Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 20 additions & 4 deletions docker/Dockerfile-dev
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,26 @@ RUN apk --update --no-cache add \
sqlite
RUN mkdir /db

ADD requirements.txt /requirements.txt
RUN pip install --no-cache -r /requirements.txt
ADD dev-requirements.txt /dev-requirements.txt
RUN pip install --no-cache -r /dev-requirements.txt
COPY requirements.txt dev-requirements.txt /

RUN set -ex \
&& apk add --no-cache --virtual .build-deps \
gcc \
libc-dev \
musl-dev \
linux-headers \
pcre-dev \
&& pip install --no-cache -r /requirements.txt \
&& pip install --no-cache -r /dev-requirements.txt \
&& runDeps="$( \
scanelf --needed --nobanner --recursive /usr/local \
| awk '{ gsub(/,/, "\nso:", $2); print "so:" $2 }' \
| sort -u \
| xargs -r apk info --installed \
| sort -u \
)" \
&& apk add --virtual .python-rundeps $runDeps \
&& apk del .build-deps

# Create a Dockerflow version file at the root so
# we don't map over it below.
Expand Down
51 changes: 46 additions & 5 deletions docker/Dockerfile-prod
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,30 @@

FROM python:3.5-alpine

RUN addgroup -g 1001 app && \
adduser -D -u 1001 -G app -s /usr/sbin/nologin app
MAINTAINER mars@mozilla.com
# These are unlikely to change from version to version of the container
EXPOSE 9000
CMD ["/usr/local/bin/uwsgi"]

RUN addgroup -g 10001 app && adduser -D -u 10001 -G app -h /app app

# uWSGI configuration
ENV UWSGI_MODULE=landoapi.wsgi:app \
UWSGI_SOCKET=:9000 \
UWSGI_MASTER=1 \
UWSGI_WORKERS=2 \
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a way to let @ckolos and the ops team configure these values? I'm not sure what the best values are for the hardware they will deploy this on.

Copy link
Author

Choose a reason for hiding this comment

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

They can set these values by editing the Dockerfile, setting them in the Docker environment, or by passing in a uWSGI config file. I'll let @ckolos decide which is best. We can iterate on the Dockerfile to find the approach that works best for them.

Copy link

Choose a reason for hiding this comment

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

Env vars are fine; we can override those

UWSGI_THREADS=8 \
# Disable worker memory sharing optimizations. They can cause memory leaks
# and issues with packages like Sentry.
# See https://discuss.newrelic.com/t/newrelic-agent-produces-system-error/43446
UWSGI_LAZY_APPS=1 \
UWSGI_WSGI_ENV_BEHAVIOR=holy \
# Make uWSGI die instead of reload when it gets SIGTERM (fixed in uWSGI 2.1)
UWSGI_DIE_ON_TERM=1 \
# Check that the options we gave uWSGI are sane
UWSGI_STRICT=1 \
# Die if the application threw an exception on startup
UWSGI_NEED_APP=1

RUN apk --update --no-cache add \
sqlite
Expand All @@ -15,13 +37,32 @@ RUN chown app:app /db
COPY migrations /migrations

COPY requirements.txt /requirements.txt
RUN pip install --no-cache -r /requirements.txt

# Install pure-Python, compiled, and OS package dependencies. Use scanelf to
# uninstall any compile-time OS package dependencies and keep only the run-time
# OS package dependencies.
RUN set -ex \
&& apk add --no-cache --virtual .build-deps \
gcc \
libc-dev \
musl-dev \
linux-headers \
pcre-dev \
&& pip install --no-cache -r /requirements.txt \
&& runDeps="$( \
scanelf --needed --nobanner --recursive /usr/local \
| awk '{ gsub(/,/, "\nso:", $2); print "so:" $2 }' \
| sort -u \
| xargs -r apk info --installed \
| sort -u \
)" \
&& apk add --virtual .python-rundeps $runDeps \
Copy link

Choose a reason for hiding this comment

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

*nit: Do you anticipate this list changing often? if not, why not just add these packages as part of the container build?

Copy link

Choose a reason for hiding this comment

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

+1 for originality!
;)

Copy link
Author

Choose a reason for hiding this comment

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

The list of packages changes every time we add a new compiled dependency. So far that's been for database adapters, uWSGI, and crypto/SSL handling.

This is the second time a reviewer has said they would be less surprised by an explicit list of runtime packages, and I'm sensing a trend, so I'll look into changing it. :) But we might not want to block on the rewrite and make that a follow-up commit.

&& apk del .build-deps

COPY . /app
RUN pip install --no-cache /app

# run as non priviledged user
# Run as a non-privileged user
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of our invoke commands (specifically the ones that create the database) stopped working with the addition of these changes. I tracked this down to the Dockerfile-prod (since when editing the docker-compose.overrides.yml` file that it what it uses as the context for all invoke commands). The removal of

WORKDIR /app
RUN cd / && pip install --no-cache /app

is what causes these changes.

We either need to re-add them, or we need to 1) confirm that the production environment doesn't need them and 2) confirm that the our circle.yml (which runs our invoke commands for production, doesn't use the production image as the context).

I tested the app after re-adding those two lines and confirmed that it worked. I recommend that we re-add those two lines.

Copy link
Author

Choose a reason for hiding this comment

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

Reproduced. I added the WORKDIR /app line back in, invoke upgrade appears to work again.

USER app

# TODO allow ops to use this as a wsgi app
WORKDIR /app
11 changes: 11 additions & 0 deletions landoapi/wsgi.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# This Source Code Form is subject to the terms of the Mozilla Public
# License, v. 2.0. If a copy of the MPL was not distributed with this
# file, You can obtain one at http://mozilla.org/MPL/2.0/.
"""
Construct an application instance that can be referenced by a WSGI server.
"""
import os

from .app import create_app

app = create_app(os.environ.get('VERSION_PATH', '/app/version.json'))
2 changes: 2 additions & 0 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -70,3 +70,5 @@ python-dateutil==2.6.0 \
Flask-Alembic==2.0.1 \
--hash=sha256:05a1e6f4148dbfcc9280a393373bfbd250af6f9f4f0ca9f744ef8f7376a3deec \
--hash=sha256:7e67740b0b08d58dcae0c701d56b56e60f5fa4af907bb82b4cb0469229ba94ff
uWSGI==2.0.15 \
--hash=sha256:572ef9696b97595b4f44f6198fe8c06e6f4e6351d930d22e5330b071391272ff
1 change: 1 addition & 0 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
],
keywords='mozilla commits development conduit',
packages=find_packages(exclude=['docs', 'tests']),
package_data={'landoapi': ['spec/swagger.yml']},
install_requires=[],
extras_require={},
entry_points={
Expand Down