Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve docker examples #57

Merged
merged 6 commits into from
May 6, 2021
Merged

Improve docker examples #57

merged 6 commits into from
May 6, 2021

Conversation

tmattio
Copy link
Contributor

@tmattio tmattio commented May 6, 2021

The Docker examples are amazing and really useful. It's rare to see libraries documenting the deployment part, so your efforts on these are really appreciated 🙂

I thought I'd propose some improvements to the example. I've been using Docker for developing and deploying Opium apps with Docker quite a lot, so hopefully, the setup here should be relatively real-world-proof.

  • Building the binaries can be done in Docker instead of copying the files over to a machine compatible with the Docker image. This unlocks one of the main benefits of Docker: the images can be built by any developer, independently of their operating system. To do this, the Dockerfile now contains 2 stages: a stage for the build context where we install the build dependencies and copy the source code in the Docker context. The second stage is a very minimal alpine image where we only install the runtime dependencies and copy the binary produced in the first stage.
  • The logging driver in docker-compose.yml defaults to json-file to be compatible to OSX (journald is not available there), and we read the env var LOGGING_DRIVER to make sure we use journald in the deploy.sh script.
  • I added a variant of the z-docker example for Opam: the Dockerfile is a bit different, and I figured having deployement example is something Opam users will need as well

Copy link
Collaborator

@ulrikstrid ulrikstrid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of minor comments

example/z-docker/Dockerfile Outdated Show resolved Hide resolved
example/z-docker-opam/Dockerfile Show resolved Hide resolved
example/z-docker/Dockerfile Outdated Show resolved Hide resolved
Copy link
Owner

@aantron aantron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice. However, I am not sure if this PR is targeted at the right example!

Building the binaries can be done in Docker instead of copying the files over to a machine compatible with the Docker image.

The Docker example isn't doing that — it has a build completely contained on the target machine, and sends only sources. So, it should be "portable" in the sense that a macOS developer can develop locally using opam or esy, push sources, and a separate build on the remote Ubuntu machine will build the Linux container using whatever setup is there.

The Heroku example does involve copying binaries, because that seems like the only sensible way to deploy to Heroku at the moment. So, that example would benefit greatly from a containerized build.

An argument could be made for an example that builds Linux containers locally for the sake of reproducing a complex app setup with Docker Compose in local testing, but I think that's a slightly different kind of example.

example/z-docker-opam/.dockerignore Outdated Show resolved Hide resolved
example/z-docker-opam/Dockerfile Outdated Show resolved Hide resolved
example/z-docker-opam/Dockerfile Outdated Show resolved Hide resolved
example/z-docker/esy.json Outdated Show resolved Hide resolved
example/z-docker/esy.json Outdated Show resolved Hide resolved
example/z-docker/Dockerfile Outdated Show resolved Hide resolved
example/z-docker/Dockerfile Outdated Show resolved Hide resolved
example/z-docker/.dockerignore Outdated Show resolved Hide resolved
example/z-docker-opam/dune-project Show resolved Hide resolved
example/z-docker-opam/deploy.sh Outdated Show resolved Hide resolved
@aantron
Copy link
Owner

aantron commented May 6, 2021

The Docker example is now deployed by CI (https://github.com/aantron/dream/runs/2517717269). As you can see, it consists of rsyncing the sources and then running the deploy script remotely, so even GitHub Actions isn't doing a build inside the CI runners. Also, the remote build is extremely fast because we get caching "for free" since there is a filesystem. That run deployed the new server in less than a minute, from push to live.

By contrast, the Heroku workflow (https://github.com/aantron/dream/runs/2511934235) actually builds the binary in CI, and sends that to Heroku. That's the part that is definitely not portable to Mac (although a local build will still work, you just can't deploy the output to Heroku, of course).

@tmattio
Copy link
Contributor Author

tmattio commented May 6, 2021

@aantron I addressed the remaining reviews I think, apart from the alpine vs debian-slim discussion that @ulrikstrid started, but as mentionned, ocaml/opam does not have a debian-slim variant, so it would complexify the example. Not sure what's the best course of action there.

@tmattio
Copy link
Contributor Author

tmattio commented May 6, 2021

I also renamed z-docker to z-docker-esy to make it explicit that the setup uses Esy only. I can revert this part if you prefer though

Copy link
Owner

@aantron aantron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, it looks good, I just left some final trivial nits :)

example/README.md Outdated Show resolved Hide resolved
example/z-docker-opam/Dockerfile Outdated Show resolved Hide resolved
example/z-docker/Dockerfile Outdated Show resolved Hide resolved
example/z-docker/README.md Outdated Show resolved Hide resolved
@aantron
Copy link
Owner

aantron commented May 6, 2021

@aantron I addressed the remaining reviews I think, apart from the alpine vs debian-slim discussion that @ulrikstrid started, but as mentionned, ocaml/opam does not have a debian-slim variant, so it would complexify the example. Not sure what's the best course of action there.

I think we can merge the PR and think about this later. One option is to use Ubuntu, in hopes that it will make future debian-slim support from the ocaml images easier to transition to. However, there is not so much code here, that it will be difficult to change from Alpine to debian-slim.

@aantron aantron merged commit 528b261 into aantron:master May 6, 2021
@aantron
Copy link
Owner

aantron commented May 6, 2021

Thank you! I'll set up some GitHub Actions deployment for this, and rename z-docker.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants