Opened 23 months ago

Last modified 4 months ago

#5574 accepted enhancement

Continuous integration (CI) testing for MediaGoblin

Reported by: anongoblin Owned by:
Priority: minor Milestone:
Component: programming Keywords:
Cc: michael@… 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

Subtickets

Change History (16)

comment:1 Changed 12 months ago by Ben Sturmfels

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 Changed 6 months ago by Ben Sturmfels

Status: newaccepted

comment:3 Changed 5 months ago by Ben Sturmfels

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 Changed 5 months ago by Michael Lynch

[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 Changed 5 months ago by Michael Lynch

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 Changed 5 months ago by Ben Sturmfels

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 Changed 5 months ago by Michael Lynch

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 Changed 5 months ago by Ben Sturmfels

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 Changed 5 months ago by Michael Lynch

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 Changed 5 months ago by Michael Lynch

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 Changed 4 months ago by Ben Sturmfels

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:12 Changed 4 months ago by Ben Sturmfels

comment:13 Changed 4 months ago by Ben Sturmfels

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 Changed 4 months ago by Michael Lynch

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 Changed 4 months ago by Ben Sturmfels

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

comment:16 Changed 4 months ago by Ben Sturmfels

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

Note: See TracTickets for help on using tickets.