leafac / kill-the-newsletter

Convert email newsletters into Atom feeds
https://kill-the-newsletter.com
MIT License
2.31k stars 113 forks source link

Add basic Docker support #22

Closed ghost closed 3 years ago

ghost commented 3 years ago

See #20.

This is my first time creating a Dockerfile so I would appreciate if people could look this over!

I was trying to validate the container against my Traefik reverse-proxy setup, but no luck there. I haven't had tons of free time to debug and Traefik is temperamental at best. I do know that the container will build and the website seems to work if accessed behind the proxy, although I wasn't able to get email or RSS working.

My main points of concern before merging this are:

  1. I'm not sure if I've set up the volume correctly.
  2. I'm not sure if Node will properly pick up these environment variables.
  3. Once the Dockerfile has been validated, I need to update the README to be more descriptive.
leafac commented 3 years ago

Thanks for the pull request 😃

I’ll work on merging this soon, but first I have to merge https://github.com/leafac/www.kill-the-newsletter.com/pull/21, because it changes the infrastructure a bit: there will be a new volume to mount (/static/alternate/).

leafac commented 3 years ago

I merged #21 and will look at this soon.

leafac commented 3 years ago

I wrote some changes on top of your pull request. Please review and test them by following the instructions on the README. I don’t use Docker and don’t have it installed, so you should expect to find problems. I followed the instructions on https://nodejs.org/en/docs/guides/nodejs-docker-webapp/.

The goal here shouldn’t be to have a definitive Dockerfile for every use case, because people have different tastes and needs. The goal should be to have a good working example Dockerfile on which people can build, so we should make it as simple and standard as possible.

The changes are here: https://github.com/leafac/www.kill-the-newsletter.com/commit/33e84b345a421e7da9878259ddf7fb23295df3be

The highlights are:

leafac commented 3 years ago

@jtagcat, @nemosmithasf: Can you please test this as well?

nemosmithasf commented 3 years ago

@leafac I'd love to help out, but I'm not sure I know enough about Docker to be useful. I only know how to use it, the technical side is a bit beyond me.

But for what it's worth, I tried running docker build . on the Dockerfile and didn't get any output. I'm not sure if that's helpful or proves the above point.

leafac commented 3 years ago

Thank you all for your help. I tested the Docker support myself and it’s working. See https://github.com/leafac/www.kill-the-newsletter.com/tree/b9907a8021cebd6cd4a4b3131fd31ac5aa0a2e37