Opened 5 years ago

Closed 6 months ago

#5574 closed enhancement (duplicate)

Improve docker builds for deployment and/or development

Reported by: anongoblin Owned by:
Priority: minor Milestone: 0.13.0
Component: programming Keywords:
Cc: michael@…, Olivier Mehani Parent Tickets:

Description

[This is Michael Lynch (mtlynch.io). Boris suggested I post here under the anon account as new registrations on the issue tracker seem to be down and the mailing list is not accepting new messages]

I'm interested in contributing to MediaGoblin.

Its test suite is very impressive, so I tried to run the tests and ran into some issues with finding the correct dependencies. One good way to solve this would be to have a continuous integration (CI) service that establishes a "known-good" configuration for running tests.

There was discussion a few years ago about setting up CI, but it doesn't look like it ever went through:

https://wiki.mediagoblin.org/20160206_Log

I spent a few hours setting up MediaGoblin on Travis and got it to successfully execute the tests, but it skips 12 out of 183:

https://travis-ci.org/mtlynch/mediagoblin/builds/449333107

If I force the tests to run, the skipped tests fail:

https://travis-ci.org/mtlynch/mediagoblin/builds/449339819

A few questions:

  • Are all tests currently in a good state? (i.e. should I expect to see all 183 tests pass?)
  • If so, what dependencies am I missing? I tried installing every apt package in the HackingHowTo but the tests still fail.
  • Is there interest in setting up official CI for MediaGoblin? There are many free options available, but it would require the project to move (or at least officially mirror) to more modern git hosting (Github, BitBucket, Gitlab, etc).

Here are all my changes:

https://github.com/mtlynch/mediagoblin/compare/mainline-head...master?expand=1

The most relevant files are my Dockerfile to set up the test environment:

https://github.com/mtlynch/mediagoblin/blob/master/Dockerfile.test

and my Travis configuration:

https://github.com/mtlynch/mediagoblin/blob/master/.travis.yml

Change History (20)

comment:1 by Ben Sturmfels, 5 years ago

Hi Michael,

I agree that CI is essential and would put the extensive test suite to good use. CI would also help us ensure we have reliable installation processes on a range of operating systems (eg. Debian, CentOS, Ubuntu, Fedora). Thanks for your work on this front.

I haven't investigated the skipped tests, but the will be skipped for good reason - likely because they are known to be broken and there isn't an easy fix. So that's good news - the tests that are expected to pass are passing!

When running the tests on Python 3 under Guix (as documented in guix-env.scm on master), I'm seeing 1 failed 174 passed and 8 skipped, with the failure in "TestSubmissionBasics.test_collection_selection".

As to how to fix the 8 skipped tests, these will just need to be investigated one at a time. The first two seem be failing due to a missing "numpy" python package, but the others look harder.

Regarding adopting Travis CI, I haven't used it myself. I gather that the underlying software is technically free software but it appears that it's really only feasible to use it as a hosted service provided by travis-ci.com. This may be an issue for the project, I'm not sure. Let's keep talking about this.

Regards,
Ben

comment:2 by Ben Sturmfels, 4 years ago

Status: newaccepted

comment:3 by Ben Sturmfels, 4 years ago

Cc: michael@… added

Hi Michael,

I've now fixed a bunch of those dependency issues you raised a while back and ironed out the kinks in our Python 2 and Python 3 dockerfiles. I'd be interested in your thoughts:

$ docker build -t mediagoblin-python2 - < Dockerfile-debian-python2-sqlite --
$ docker build -t mediagoblin-python3 - < Dockerfile-debian-python3-sqlite --

These now also run the test suite, and we should now be 100% green on both.

Ben

comment:4 by Michael Lynch, 4 years ago

[posting my original comment from a few days ago, but Trac was blocking it]

Awesome, that looks good. I hope we can eventually unify my Dockerfile and yours because it’s silly for us to duplicate our efforts.

The biggest issue in the Dockerfiles right now is that it’s git cloning the current master head instead of the local source. That means that the developer can’t tell if the code is breaking until after it’s already merged into master. I read the notes about the reasoning, but I’m having trouble understanding what prevents us from just doing this swap:

#RUN git clone --depth=1 git://git.savannah.gnu.org/mediagoblin.git --branch master .
COPY --chown=www-data:www-data . .

I just tried it in my local repo and I got one failing test:

https://gist.github.com/mtlynch/74d7905bd7a6075e749cd4a09c736d6b

I’m not able to debug this tonight, but maybe it’ll make more sense to you.

Just a note on Dockerfile readability, I find it helpful to assign names so I can refer to them by constant instead of magic string, so like:

ARG MEDIAGOBLIN_USER="www-data"
ARG MEDIAGOBLIN_GROUP="www-data"
ARG MEDIAGOBLIN_DIR="/opt/mediagoblin"
RUN mkdir "$MEDIAGOBLIN_DIR"
RUN chown -R "${MEDIAGOBLIN_USER}:${MEDIAGOBLIN_GROUP}" "$MEDIAGOBLIN_DIR"
WORKDIR "$MEDIAGOBLIN_DIR"

The working builds become like 50x more valuable if we can combine it with CI that ensures a clean build on every check in. I know you weren’t so enthusiastic about Travis. I’ve stopped using them for other reasons and switched to CircleCI, who also offers free builds to open source projects, but they're not open source themselves either.

There’s also CI through Gitlab, which is open source and offers self-hosting, but they’re a whole integrated Git hosting, bug tracking platform, so I’m not sure how easy it is to use only the CI piece.

comment:5 by Michael Lynch, 4 years ago

I took a crack at refactoring the Dockerfile to use the local version of the MediaGoblin source:

https://github.com/mtlynch/mediagoblin/commit/6a9f23a554893c55d8ec185972451cfc94aceb04

You can run it as follows:

docker build -f Dockerfile-debian-python3-sqlite --tag mediagoblin .
docker run --tty --detach --publish 6543:6543 --name mediagoblin mediagoblin

I took the tests out of the build image because I think they make more sense as a runtime step. To verify tests passing (e.g., in CI), run the following command after building:

docker run --entrypoint ./bin/python mediagoblin -m pytest ./mediagoblin/tests --boxed

I strongly encourage you to always use the local version of MediaGoblin's source rather than on a remote repo. You mentioned that previous builds can cause it to break, but in that case, we should be adding intermediate files to the .dockerignore so that they don't contaminate the clean build in the Docker container.

For CI, it's also better if the Docker build uses the local version of source so that we can ensure that any changes in the incoming merge request don't break the build.

Anyone who needs to guarantee a clean build should be making a clean checkout of the master branch and then building that. I think for all other scenarios, we want Docker to build the local copy of the code.

comment:6 by Ben Sturmfels, 4 years ago

Thanks Michael, I like this approach!

On my first run of this Docker image, I got ModuleNotFoundError: No module named 'psycopg2' because I have a mediagoblin.ini that's configured for PostgreSQL. This is the kind of hard-to-reproduce weirdness I was talking about. If this is for CI, then I think we either need to white-list the files COPY'd (rather than blacklist with .dockerignore) in or do some kind of git bundle tricks so that this works the same for everyone. Do you have any thoughts on how we could do this?

Thanks again, I'm excited about having a way to test the build/install process reliably and automatically.

comment:7 by Michael Lynch, 4 years ago

On my first run of this Docker image, I got ModuleNotFoundError: No module named 'psycopg2' because I have a mediagoblin.ini that's configured for PostgreSQL. This is the kind of hard-to-reproduce weirdness I was talking about.

Oh, I think we should just add mediagoblin.ini to the .dockerignore. We never want to hardcode a static config file into the image itself anyway.

If the user wants to run the Docker image with a custom mediagoblin.ini they can just mount it as a file like this (mounting individual files as volumes is a neat Docker trick):

docker run --tty --detach --publish 6543:6543 --name mediagoblin \
  -v "${PWD}/mymediagoblinconfig.ini:/opt/mediagoblin/mediagoblin.ini" \
  mediagoblin

But if the user doesn't mount a mediagoblin.ini, the docker-entrypoint.sh creates a default one at runtime.

If this is for CI, then I think we either need to white-list the files COPY'd (rather than blacklist with .dockerignore) in or do some kind of git bundle tricks so that this works the same for everyone.


Whitelist is a good long-term strategy, but I don't think it's strictly necessary, and it's totally fine for us to blacklist in the short-term.

Long term, if we organize the directory structure so that source files always appear in their own directory and nothing adds non-source files to those directories, it should be easy for us to just COPY the source directory without a lot of exceptions.

But on CI especially, it shouldn't be hard because CI is grabbing only the files under source control. In your mediagoblin.ini example, it wouldn't have caused a problem on CI unless you also checked your mediagoblin.ini into source control.

I'm happy to push forward on this if you agree with this strategy. If I make the same changes to the other two Dockerfiles that I made to Dockerfile-debian-python3-sqlite, would that work for you?

comment:8 by Ben Sturmfels, 4 years ago

Sounds good. Let's push forward with the blacklist approach for now and get something up and running. Ignore the other two Dockerfiles for now and let's get Python 3/Debian up and running. We're likely dropping Python 2 anyhow.

My main concern with the blacklist approach is not wasting anyone's time troubleshooting why their local build isn't running exactly the same as in CI. I've been using Guix a lot and it's become clear to me how much of a productivity boost reproducibility is. And we don't have any productivity to waste. :)

comment:9 by Michael Lynch, 4 years ago

Okay, I've added mediagoblin.ini to the .dockerignore:

https://github.com/mtlynch/mediagoblin/commit/d9f8b40036809335ab16c37c36ef9a0ee37c2bb2

Both changes are now in my dockerfile-refactor branch:

https://github.com/mtlynch/mediagoblin/commits/dockerfile-refactor

Anything else I can do to help get these changes merged in?

comment:10 by Michael Lynch, 4 years ago

Actually, hang on. I think the setup process actually creates mediagoblin.ini, so it's a little more complicated than I first thought. Let me play with it a bit more to ensure I'm creating the right file. It might make more sense to create the config at mediagoblin_local.ini, as that seems to be the one MediaGoblin expects as a user-defined config.

comment:11 by Ben Sturmfels, 4 years ago

Hi Michael, I don't think it's necessarily an issue to use the stock mediagoblin.ini which is automatically copied from mediagoblin.example.ini on first run. Explicitly creating a config would be ok too. Don't call it mediagoblin_local.ini though, as the fallback functionality you mention is for backwards-compatibility only.

So I just tried to build this latest version. This builds fine, but errors out on run because it has sucked my local mediagoblin.db into the build context, which is expecting some plugins enabled that aren't in the default mediagoblin.ini:

$ docker run --tty --publish 6543:6543 mediagoblin
...
alembic.util.exc.CommandError: Can't locate revision identified by '38feb829c545'

For what it's worth, I've also set up a basic .gitlab-ci.yml build script:

image: docker:stable

services:
  - docker:dind

docker build:
  stage: build
  script:
    - docker build -f Dockerfile-debian-python3-sqlite --tag mediagoblin .

test:
  stage: test
  script:
    - docker run --entrypoint ./bin/python mediagoblin -m pytest ./mediagoblin/tests --boxed

When I push this to a branch on gitlab.com, the build stage completes successfully, which is great. The test stage fails with Unable to find image 'mediagoblin:latest' locally. Sounds as though we may need to explicitly store the build somewhere and reference it in the test stage - not sure how we do that.

comment:13 by Ben Sturmfels, 4 years ago

One quick fix but not ideal fix is to run both the docker build and docker run in the same build stage:

image: docker:stable

services:
  - docker:dind

docker build:
  stage: build
  script:
    - docker build -f Dockerfile-debian-python3-sqlite --tag mediagoblin .
    - docker run --entrypoint ./bin/python mediagoblin -m pytest ./mediagoblin/tests --boxed

That ran and gave only one test failure.

comment:14 by Michael Lynch, 4 years ago

One quick fix but not ideal fix is to run both the docker build and docker run in the same build stage

Running build and test in the same Gitlab stage doesn't sound like an issue to me. But the alternative is to push to Gitlab container registry to persist the image across stages, which also sounds doable.

There's a working example here in Gitlab's docs:

https://docs.gitlab.com/ee/user/packages/container_registry/#container-registry-examples-with-gitlab-cicd

I don't think it's necessarily an issue to use the stock mediagoblin.ini which is automatically copied from mediagoblin.example.ini on first run. Explicitly creating a config would be ok too. Don't call it mediagoblin_local.ini though, as the fallback functionality you mention is for backwards-compatibility only.

Okay, that makes sense. I'd still like to support a host-specified config file under Docker, but I'll handle that in a separate bug because getting a working CI build in place will make everything else 100x easier.

That ran and gave only one test failure.

Can we ignore the failure for now and create a bug to track the work item to fix it? Everything will much easier if we get CI up and running, so I'd rather not block on this test.

I pushed an MR into your branch to ignore the failure:

https://gitlab.com/BenSturmfels/mediagoblin/-/merge_requests/3

comment:15 by Ben Sturmfels, 4 years ago

Summary: Travis CI for MediaGoblinContinuous integration (CI) testing for MediaGoblin

comment:16 by Ben Sturmfels, 4 years ago

Michael, I think you've probably already come across Loic Dachary's work in the MediaGoblin/Docker area, but just noting it here for completeness. I hadn't seen any of this when I quickly put together the Dockerfiles currently in the MediaGoblin source tree:

https://hub.docker.com/r/dachary/mediagoblin
https://notabug.org/dachary/mediagoblin-docker

comment:17 by Ben Sturmfels, 3 years ago

Milestone: 0.13.0
Summary: Continuous integration (CI) testing for MediaGoblinImprove docker builds

Renaming to reflect that we now have CI builds running on SourceHut and that the ticket now relates to improving our Dockerfiles.

Sorry for the inaction on this Michael. Life has been pretty demanding for the last couple of years!

CI builds:
https://builds.sr.ht/~mediagoblin/mediagoblin

comment:18 by Ben Sturmfels, 3 years ago

Cc: Olivier Mehani added
Summary: Improve docker buildsImprove docker builds for deployment

Looping in Olivier on this because he's in the thick of it with Docker at the moment.

Olivier, as discussed the challenge has been that the Dockerfiles I originally wrote were purely a way for me to automatically run through the installation instructions and test suite on Debian and Fedora in a completely clean environment prior to releasing. They've worked pretty well for that. Many of the issues people have had with MediaGoblin have been related to newer PyPI packages that weren't compatible with some of our older pinned version, including dependencies of dependiencies. The Dockerfiles are also good for that, even though it's less of an issue now we've dropped Python 2 and upgrade a few key ones. Basically saves me time when someone in IRC reports an installation issue on Debian or Fedora - I can run a docker build and verify whether it's an issue with MediaGoblin itself, an issue with a updated PyPI dependency, or an issue with their setup. I may have mixed up my words at times and called them development dockerfiles, but that wasn't originally the purpose at all. So from that perspective, these Dockerfiles have been a big success.

Michael has been rightly advocating for CI for some time now, which has been delayed while I got the existing infrastructure running smoothly again and some bugs fixed and releases out, but also due to us needing a new forge and trying our best to be good leaders in the free software community and avoid proprietary options. We now also have the SourceHut Builds for CI, which is working pretty well, yay! See .build/*.yml:

https://builds.sr.ht/~mediagoblin/mediagoblin

These .build files are in some ways better than my Dockerfiles because they can pull in an arbitrary source tree and build in a clean environment. Michael and I discussed doing something similar with the Dockerfiles, but my initial work on this had hit problems where local configuration would mean that my docker builds were affected by local dev state and config, so were no longer serving my original clean-room local testing purpose.

I have no doubt that's solvable - I just haven't put further time into it. Michael may well have the right answers mentioned earlier in this thread.

Olivier is deploying with Docker already, but wants to significantly improve his setup, possibly with multiple separate containers for different services. This docker deployment would be really useful for people who already like Docker and want to get a MediaGoblin going fast. Michael, are you deploying with Docker too? I forget.

So that's three use-cases for Docker:

  • CI/clean-room test suite run/install instruction testing
  • local development
  • production deployment

Can these all be served by the same Dockerfiles? Is that even a good idea?

And FYI, the other work that's going on in this "making MediaGoblin easy to deploy" direction is that I'm working on an OS package for the Guix operating system and Eli is working on an OpenSuse package. I think we'll be asking similar questions like "can we separate the code and the data?" and "can we move to a more standard Python package (ditch configure/make)?"

comment:19 by Ben Sturmfels, 3 years ago

Summary: Improve docker builds for deploymentImprove docker builds for deployment and/or development

comment:20 by Ben Sturmfels, 6 months ago

Resolution: duplicate
Status: acceptedclosed

I'm going to close this ticket and continue the Docker discussion on the SourceHut issue tracker here:
https://todo.sr.ht/~mediagoblin/mediagoblin/36

Note: See TracTickets for help on using tickets.