Add docker support #24
Conversation
| @@ -0,0 +1 @@ | |||
| DATABASE_URL=postgresql://postgres@db/test_track_development | |||
There was a problem hiding this comment.
what's our strategy for distributing env files? what i've seen elsewhere is committing an env.dist file to the repo to serve as a template and provide defaults, then the end user copies that into a .gitignored .env file and makes their changes as necessary.
There was a problem hiding this comment.
I like that idea. We could also just define this environment variable in the docker-compose.yml file since it's the only one
There was a problem hiding this comment.
We could, but then if other users wanted to change it for themselves they would have to fork the whole repo
There was a problem hiding this comment.
Good point. I'll change this file to env.dist for the release.
| @@ -0,0 +1,4 @@ | |||
| .dockerignore | |||
There was a problem hiding this comment.
ignore the dockerignore? what's this file for?
There was a problem hiding this comment.
An overview of the dockerignore file is here: https://docs.docker.com/engine/reference/builder/#/dockerignore-file, but in short it matches glob patterns and excludes them from any COPY and ADD instructions. Adding the dockerignore file to itself is basically saying "don't copy the dockerignore when you copy the rest of the directory over". The actual Dockerfile should probably be here too. There's not much gained by ignoring them, it just feels a bit neater to me.
edit: added the Dockerfile to the ignore
|
|
||
| RUN bundle install --binstubs | ||
|
|
||
| CMD puma -C config/puma.rb |
There was a problem hiding this comment.
can you walk me through why we have different commands from the docker-compose docs site?
RUN mkdir /myapp
WORKDIR /myapp
ADD Gemfile /myapp/Gemfile
ADD Gemfile.lock /myapp/Gemfile.lock
RUN bundle install
ADD . /myapp
There was a problem hiding this comment.
Mostly because I wasn't looking at that site when I wrote the Dockerfile :-). I think this should still get us to where we want to go though. After installing the deps (lines 1 - 5), the RUN and WORKDIR commands are nearly identical and just use an install path defined in line 7. I'm preferring the COPY instruction to the ADD instruction since we don't need to use ADD's URL or decompression capabilities, and COPY is a bit more transparent as outlined here: https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#/add-or-copy. The ordering of copying the gem and lock files, running bundle install, and copying the app directory shouldn't matter I don't think. Lastly, the CMD instruction was just to provide an entrypoint in case we wanted to run this standalone using the Docker CLI instead of docker-compose (ie, if we wanted to do something like use a local instance of postgres instead of a containerized version).
…on't be copied in COPY and ADD instructions
|
bump |
|
Can we add some docs to the readme(including the .env dance)? otherwise lgtm! thanks for doing that! |
|
Added some minimal instructions for deploying using docker-compose to the README. bump |
|
<< domainlgtm |
|
Needs somebody from @jmileham to claim platform review. Use the shovel operator to claim, e.g.:
|
|
nanda bump |
|
Needs somebody from @jmileham to claim platform review. Use the shovel operator to claim, e.g.:
|
Setting environment variables in a .env file is a rails pattern, and adding "infrastructure" variables there might be more confusion than its worth.
|
@rynonl @jmileham After speaking with @samandmoore offline, I ended up reworking the docker setup to make life a little easier for local development. Can I get another glance at it and maybe re-LG? |
| - bundler_cache | ||
| command: ./script/serve | ||
| environment: | ||
| - DATABASE_URL=postgresql://postgres@db/test_track_development |
There was a problem hiding this comment.
I won't block you merging, but I'm curious why we went this direction? It's now not configurable without forking the repo.
There was a problem hiding this comment.
@samandmoore can give his perspective, but basically the thinking is that if we're looking at this for local development then the user will have the repo cloned anyway, so it's not a problem to change this value. Also, this keeps all of the docker configuration in one place instead of spreading it to a .env file, which would potentially be used by a Rails project using dotenv.
There was a problem hiding this comment.
Ah, so this is strictly not for actually running the containerized server in a in a non-local environment?
There was a problem hiding this comment.
Yeah, at the moment. I figure that if someone wants to run TT in production then they'll probably either fork the repo or just run the TT container outside of compose.
|
this looks good to me. i'm curious what the story is for running tests. i am not super familiar with the docker approach yet, so it's hard for me to understand how i would go about running the test suite without having to install pg and ruby on my local machine too. |
|
You can run arbitrary programs in the docker container with |
|
nanda? |
|
Needs somebody from @jmileham to claim platform review. Use the shovel operator to claim, e.g.:
|
|
nanda? |
|
<< domainlgtm |
|
Needs somebody from @jmileham to claim platform review. Use the shovel operator to claim, e.g.:
|
/domain @rynonl
/platform @jmileham
cc @nlopez
This is a first shot at addressing #11. Tested with Docker For Mac, but it should work on native Linux Docker as well.