Docker feedback

[jarshwah]

This will be a meta issue for collecting feedback. Otherwise we can just create an issue per item and close this one off, up to you.

Regarding the extra dependencies, can you add them from the dockerfile, install them, then volume mount over the top? Don't copy the entire django folder over, just the requirements.

Try combining as many instructions as you can to minimise the number of layers. For example, you have 5 RUN instructions in a row, they could all be combined. https://github.com/orf/django-docker-box/blob/master/Dockerfile#L23

I have no other specific feedback yet because I've just glanced at some of the files, I'm waiting for the weekend before I take it for a spin.

[orf]

Thanks for this! The meta issue is fine 👍

Regarding the dependencies: this brings some trouble. I would like to be able to have Django live in a directory outside of this project. Unfortunately the Django .git directory is rather large, so if you build the project with the django root as the context it has to copy the entire 400+mb tree (including .git) to the docker daemon before building which is pretty slow. The solution is to add .git to a .dockerignore but we don't control the django source directory and so cannot do this.

A fix might be to use a symlink within this project, i.e ./django/ -> ~/projects/django/, and have django/.git in a .dockerignore here but I am unsure how or if this would work on Windows.

I've worked around this by persisting the pip cache across runs, so installing the dependencies is pretty fast. If this does get accepted into the django project we should add a .dockerignore to speed things up and remove the need for that volume.

About the Dockerfile, good points, but I'm more concerned right now about getting a green build across the board just to prove that this is viable: https://travis-ci.com/orf/django-docker-box/builds/87770210

Right now there is:

1. a persistent gis failure on Sqlite (Add a gc.collect() to make GDALBandTests.test_band_data more deterministic django/django#10501, but this isn't the correct solution)
2. two failures in master on mysql 8 (https://code.djangoproject.com/ticket/29847#ticket)
3. persistent failures regarding the database images taking too long to start, resulting in the test runner failing to connect, so we need to add a healthcheck of some kind

[jarshwah]

Regarding the requirements, what I was thinking about was something like:

ARG DJANGO_PATH
RUN mkdir /requirements
COPY $DJANGO_PATH/tests/requirements/*.txt /requirements/
COPY extra-requirements.txt /requirements/
RUN pip install -r /requirements/py3.txt -r /requirements/..etc.txt

Then you can still volume mount the django directory, and all of the requirements have been built into the image.

Which of the databases is taking too long to start? I see pg has a healthcheck but the mysql varieties do not.

Also missing a headless test runner using the chrome or firefox drivers. There's an xvfb example in the vagrant image but I'm not sure how well that works. It might be worth looking into using chrome headless directly - but I have no experience with it.

If this project can get total coverage then it might even be attractive to depend on travis.ci directly and move the django project away from Jenkins. I don't know how well that would be received, but there's at least some burden of managing those jenkins instances and the ansible scripts behind them so I'd be keen to raise that possibility.

(Sorry, I was away for most of the weekend and didn't get much chance to dive in like I wanted to)

[apollo13]

I am very much interested in moving this forward; so if there is anything where you need help please ping me on IRC.

[orf]

Thanks for your help @apollo13, much appreciated!

So, a few things about the implementation. If we go with @jarshwah's suggestion (which is more clean, for sure) we run into some issues. To do this we need to specify the Django path as a the build context. That's fine. The problem is we have a .env file specifying the default environment variables (like PYTHON_VERSION), however it seems that this file has to be relative to the build context. So it will look in $DJANGO_PATH/.env. This is quite annoying, as every single compose run needs at least two variables.

So, a quickstart would become: PYTHON_VERSION=3.6 POSTGRES_VERSION=10 docker-compose run postgres ..., which is pretty verbose. We'd also loose all default values, and to be extra annoying docker-compose fails with a pretty unobvious error in the case above (because postgres:${POSTGRES_VERSION} resolves to postgres: if POSTGRES_VERSION it's not defined, which is in invalid reference). We could work around this by defining defaults in the Dockerfile and the compose definition I guess, I will try this tonight.

Also, while it's conceptually more 'dockery' it will actually be slower on the CI run, as you cannot mount volumes during the build. So the dependencies would always need to be downloaded, whereas now they are cached between runs in a volume. I'm not too fussed about this as the CI is not the primary use case here (yet!), however even a 40 second increase in each run across a full matrix equals ~40 minutes or so added in total.

As a side note, if we are going to consider a Jenkins replacement, Travis CI gives large open source projects special accounts with more concurrent runners. The exact specifics are done on a case-by-case basis, but Django qualifies for this.

[orf]

Also, as a side note, in the future with this we can potentially test on MacOS (while not a deployment platform, it's a huge development platform) and Windows. A full run across each of these would be out of the question but a minimal test suite may not be.

[jarshwah]

I wasn't suggesting to use the Django directory as the build context by the way. I was suggesting we should copy the requirements files from the django directory, but leave the build context as-is. Does that change things in any way?

[orf]

Oh right! Unfortunately that is not possible without the build context being Django (unless we duplicate the requirements files here). Docker COPY commands are relative to the build context, so you can't do COPY /any/directory/any/file.

Making Django the build context isn't a bad idea, I've implemented it in #11 and it all seems to work OK.

[carltongibson]

Hi @orf. I've been using this now, it's great. Thank you!

I was going to mark the Trac ticket as Patch needs improvement since I don't think it's quite ready to progress, but here seems the place to discuss currently so... (we can move the discussion elsewhere if you prefer).

So, first off, a question: what's the benefit to moving the files into django/django rather than say, django/django-docker-box and setting DJANGO_PATH as we do here? The PR adds quite a lot of files, and I suspect quite frequent edits, so lots of noise — could we not just keep that separate?

Second, for me at least, having done nothing to optimise in any way, performance is quite down on running without docker: 16mins vs 4mins.

(That's with the images already built etc.)

(That's running with --parallel=2 to keep it fair, because docker is setup with the default "2 CPUs", but none of that is even mentioned on the Quickstart...)

Any thoughts on this?

I have various other little thoughts, that tie-in to the tweaks suggested here and in the other issues here and on the PR, but they really come under "frequent edits, so lots of noise" above.

Current status: we should definitely pull this into the Django org, as an official project etc, point people towards it and so on, but not include the files directly in django/django or make it the top option in the testing Quickstart.

[adamchainz]

+1 to moving into django org and pointing people towards it without including in django/django.

[orf]

Thanks for your feedback Carlton! I'll try and address a few points below.

Moving under Django: yes please! I can transfer the repo directly or we could create a new one, up to you. Let me know how to proceed.

Speed: it really should not be that slow, unless perhaps you are on Windows and using Virtual box as your docker server? I'd love to diagnose why this is, so any info you can give me would be great.

On Linux the speed should be native, on Mac it should be nearly native. With hyper-v it should still be fast.

Regarding including this inside Django: the main reason is the simplicity of running this while it's inside Django. Just clone and go (in theory 😁).

The secondary reason is it unlocks a lot of cool improvements down the line. Using docker-compose as the primary test runner for Django has some real tangible benefits - we can adapt the way we run tests via pull requests, the CI is always in sync with the developers, less reliance on Jenkins, easier to adapt other services (like the VS CI solution that's recently been proposed). Plus there is a lot less maintenance of Jenkins in general, and it's easier to expand our coverage.

In short, this repo running a single full-matrix build a day has caught a few bugs. It would be good to run it on every PR, or at least improve our coverage, and I see docker-compose as a gateway to making faster improvements to this in the future.

And the third reason is that it's a bit annoying to keep it separate. It's not really a separate project, it's intimately tied to Django and needs to be kept in sync (specifically versions, but other stuff as well). It feels natural to me to move this into core.

But, baby steps I guess. If not everyone agrees with this then I'm happy to keep it separate.

Edit: I would definitely not like to maintain the PR if I do not have to, so it would be good to get a consensus on this. I started the PR after someone suggested bringing it into core on the mailing list, so perhaps it warrants some more discussion there?

[jarshwah]

I’d like to see inclusion directly within django/django also. I see one of the major benefits as helping new contributors get up and running quickly. Pointing at a separate repo and asking users to mess with env vars really harms that onboarding process. I see the future infrastructure goals as secondary but important too. As far as performance goes, docker for mac in particular is very fsync happy. Every single write to a database within a container will be fsynced to the host volume. You can use volume mount options cached/delegated to control how often fsync will be used. Unsure if this is the problem for Carlton, but it’s a common one with docker compose and databases.

…

On Tue, 19 Feb 2019 at 06:53, Tom Forbes ***@***.***> wrote: Thanks for your feedback Carlton! I'll try and address a few points below. Moving under Django: yes please! I can transfer the repo directly or we could create a new one, up to you. Let me know how to proceed. Speed: it really should not be that slow, unless perhaps you are on Windows and using Virtual box as your docker server? I'd love to diagnose why this is, so any info you can give me would be great. On Linux the speed should be native, on Mac it should be nearly native. With hyper-v it should still be fast. Regarding including this inside Django: the main reason is the simplicity of running this while it's inside Django. Just clone and go (in theory 😁 ). The secondary reason is it unlocks a lot of cool improvements down the line. Using docker-compose as the primary test runner for Django has some real tangible benefits - we can adapt the way we run tests via pull requests, the CI is always in sync with the developers, less reliance on Jenkins, easier to adapt other services (like the VS CI solution that's recently been proposed). Plus there is a lot less maintenance of Jenkins in general, and it's easier to expand our coverage. In short, this repo running a single full-matrix build a day has caught a few bugs. It would be good to run it on every PR, or at least improve our coverage, and I see docker-compose as a gateway to making faster improvements to this in the future. And the third reason is that it's a bit annoying to keep it separate. It's not really a separate project, it's intimately tied to Django and needs to be kept in sync (specifically versions, but other stuff as well). It feels natural to me to move this into core. But, baby steps I guess. If not everyone agrees with this then I'm happy to keep it separate. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAhBm2VXS6IeXDrqqC41o36dzhXhEIB9ks5vOwScgaJpZM4XWkPT> .

[carltongibson]

I'm using Docker on macOS. Just default settings.

I think we need to go back to the mailing list...

[carltongibson]

One thought: If we had an "official" image users could just docker pull to get up and running. (???)

[orf]

I'm using Docker on macOS. Just default settings.

Ok, let me dig into this a bit. As @jarshwah mentioned there are some volume options we can play with, so I will run some benchmarks locally and see any improvements.

One thought: If we had an "official" image users could just docker pull to get up and running. (???)

So that would ideally be part of this project. Right now the Travis builds pull and push an image under my account on every build and I would definitely like to keep that, which might require the Django project to create an account on Docker Hub. But yeah, this is the twofold advantage of not having to rebuild endlessly on every CI run, and also every developer can pull the latest image used to run the test suite for the latest versions of packages, configuration changes and obviously so they don't need to build it locally. That should be the preferred way of running this project.

It would be docker-compose pull mysql/postgres/sqlite/etc rather than docker-pull though, and you'd still need the docker-compose.yml to do anything interesting.

[jarshwah]

My last response was written on the go which is why it was so terse, apologies.

It seems like the major concern for you Carlton is that there are a lot of new files, and there'll be a lot of churn as a consequence. I don't believe this is something that'll actually happen. The docker-compose and .dockerignore would live in the root. The test settings could be moved to the tests directory. The Dockerfile/Entrypoint can be moved to a separate sub directory, though the convention is to leave them in the root.

The dockerfiles themselves should rarely change - and would only do so to provide optimisations. When a new Python/DB version is added or removed, the env var files will change as a consequence. Which brings me to one of the benefits. The configuration of the containers (and docker-compose) stays current with the version of django that is checked out.

@adamchainz keen to hear why you'd prefer the repo to remain separate?

If we're already using official channels to point towards a supported repo, then why not keep the middle step and bundle the extra files? The repo has little value without being connected to Django, so we can make that connection strong and easy for new contributors to use, or not as strong with supporting documentation that users are going to find somewhat more difficult to follow.