[PSkaggs]

… starting docker.

Description of changes

Adds an entrypoint script for docker-compose to run database migrations when starting the container.

Issue Resolved

Fixes Issue Number: 61

[ohaiwalt]

So I believe this would result in lib/docker-entrypoint.sh bundle exec puma -C config/puma.rb being the command run in the container, from here.

That doesn't seem to be desirable. A possible solution for this would be to include the bundle exec puma command into the entrypoint script, and removing the command line.

[PSkaggs]

Well, you certainly could do it that way. The ticket said "Run migrations on container start". This project has several ways to build and run the docker container. So it isn't clear how you are doing it in production. This solution assumes you are using docker-compose to build, run and update the container.

[hpjaj]

@ohaiwalt - what do we want to do about this issue and PR? Looks like there is still some open discussion around its implementation, and failing tests.

[ohaiwalt]

@PSkaggs Perhaps I'm missing something here?

I've made a new clone of the backend repository, and pulled your changes in, and this is the result of the docker-compose up --build:

web_1                 | rake aborted!
web_1                 | ActiveRecord::NoDatabaseError: FATAL:  database "operationcode_development" does not exist
[...abbreviated...]
web_1                 | Tasks: TOP => db:migrate
web_1                 | (See full trace by running task with --trace)
web_1                 | I, [2017-10-29T00:28:33.262287 #1]  INFO -- : Raven 2.5.3 configured not to capture errors: DSN not set
web_1                 | W, [2017-10-29T00:28:34.440169 #1]  WARN -- : Creating scope :open. Overwriting existing method GitHubStatistic.open.
web_1                 | D, [2017-10-29T00:28:35.169725 #1] DEBUG -- : FATAL:  database "operationcode_development" does not exist
web_1                 |  excluded from capture: DSN not set
operationcodebackend_web_1 exited with code 1

The database doesn't exist on initial startup. Even after the database exists, based on the combination of ENTRYPOINT and CMD arguments, I don't think it will work with reference to the link I posted above.

Can you expand a bit on your thought process here? It's worth mentioning that database migrations and docker-compose are not my strong suits, but I'm reasonably familiar with Docker.

[PSkaggs]

@ohaiwalt It does work if the database is present. I used make to build and run the container. However, I didn't really pay attention to the output of make test when I ran it before. Apparently it was just failing in the middle of the test and not even finishing the test report. No final output of passing or failed tests.

I guess still don't have much clarity on what happens with the production environment. The README instructs users to use the Makefile to run the backend. Or if running the entire project use yarn from the front-end, which uses the Makefile. In any case, I didn't understand why the CMD was still in the Dockerfile if Compose was being used. All the examples I've seen of Docker with Compose lose the CMD argument from the Dockerfile and put it in the Compose .yml.

I've removed the CMD from the Dockerfile and changed the syntax of the command in the docker-compose.yml so the entrypoint and command can be used in conjunction. I also added a check to the entrypoint script to create the database if it isn't there. Running make test passes everything on my end even after dropping the database.

[ohaiwalt]

@PSkaggs Leaving the CMD in the Dockerfile or not is fine either way, it's usually overridden at runtime in Docker Compose or any container orchestration framework.

Production is running in Kubernetes using manifests in the https://github.com/OperationCode/operationcode_infra repository, which are slightly in flux as we do some moving around of cluster-up tools, and then change the manifests over to Helm charts, or similar.

I appreciate you taking the time to address this. Once we can get a set of passing tests, it LGTM.

[PSkaggs]

@ohaiwalt

Production is running in Kubernetes using manifests in the https://github.com/OperationCode/operationcode_infra repository, which are slightly in flux as we do some moving around of cluster-up tools, and then change the manifests over to Helm charts, or similar.

Ah, that makes more sense.

I appreciate you taking the time to address this. Once we can get a set of passing tests, it LGTM.

No problem. If this one doesn't do it, I'll tinker with a fresh install.

[PSkaggs]

Am I wrong or is the TravisCI error due to a configuration issue?

[PSkaggs]

Down the rabbit hole we go...

I managed to get a successful build by removing the postgres timeout in the travis.yml.

- |
    while ! psql --host=localhost --username=postgres -c 'SELECT 1'> /dev/null 2>&1; do
      echo 'Waiting for postgres...'
      sleep 1;
    done;

There is already a timeout in the make test command.

However, there was still an issue that I think might be a problem and maybe my successful build was a fluke of timing.
From my build:

docker-compose run web bash -c 'export RAILS_ENV=test && rake db:test:prepare && rake test'
Starting operationcodebackend_redis_1 ... 
Starting operationcodebackend_operationcode-psql_1 ... 
Starting operationcodebackend_bundle_1
D, [2017-10-31T16:55:57.604416 #5] DEBUG -- : FATAL:  database "operationcode_development" does not exist
 excluded from capture: DSN not set
Database 'operationcode_development' already exists
Database 'operationcode_test' already exists
I, [2017-10-31T16:56:01.961131 #17]  INFO -- : Raven 2.5.3 configured not to capture errors: DSN not set
D, [2017-10-31T16:56:02.634791 #17] DEBUG -- :    (106.1ms)  DROP DATABASE IF EXISTS "operationcode_test"

Compared to the last passing build on master:

docker-compose run web bash -c 'export RAILS_ENV=test && rake db:test:prepare && rake test'
Starting operationcodebackend_redis_1 ... 
Starting operationcodebackend_operationcode-psql_1 ... 
W, [2017-10-30T18:31:03.744918 #7]  WARN -- : Creating scope :open. Overwriting existing method GitHubStatistic.open.
D, [2017-10-30T18:31:03.842046 #7] DEBUG -- :    (0.1ms)  DROP DATABASE IF EXISTS "operationcode_test"

So the .env file was setting the RAILS_ENV to develop and I found in the docker documentation that the order of precedence is shell->env_file->Dockerfile. So I commented out the RAILS_ENV setting in the .env file. I managed to get passing builds on a tester branch that I ran this on. However, it seems to be failing here. Not exactly sure what is different at this point.

[PSkaggs]

@ohaiwalt Well, this one took a little digging but I think I finally figured it out. It turns out Rails 5.0 had a little bug that defaulted to the 'development' database when running tests unless you explicitly set it before hand. Which you can see I've done in the Makefile test. The bug is reported here and fixed here. I believe they backported the fix to Rails 5.0.6.

So if and when Rails is upgraded you can remove those explicit environment settings in the Makefile test.

Also, I added a test to check the database is correct. test/lib/verify_db_test.rb

[hpjaj]

@ohaiwalt - How are we feeling about this PR?

[ohaiwalt]

Took me a bit to get back around to this, but I gave it a run through and everything looks good!

[hpjaj]

@ohaiwalt has approved. I'm approving to merge.