endlessm / azafea

Service to track device activations and usage metrics
Mozilla Public License 2.0
10 stars 2 forks source link

Docker image does not use config generated by entrypoint #188

Closed dbnicholson closed 2 months ago

dbnicholson commented 2 months ago

In the current code, the entrypoint script templates config.toml.j2 to /tmp/config.toml from environment variables. However, the full ENTRYPOINT command does not instruct azafea to use it and instead it tries to read the configuration from /etc/azafea/config.toml unless you specify the command at runtime as -c /tmp/config.toml ....

The fix would be simple - add -c /tmp/config.toml to the ENTRYPOINT array. However, that's at odds with the running documentation. That says you should bind mount /etc/azafea from the host with the expectation that azafea will use /etc/azafea/config.toml. If the ENTRYPOINT command is changed to use -c /tmp/config.toml, that usage would break.

I think the options are:

  1. Make the change to have the image use /tmp/config.toml and update the documentation to pass the configuration as environment variables. This is how the postgres image (and many others in the Docker world) are handled.
  2. Drop the custom entrypoint script and assume the configuration has been been added to the container at /etc/azafea/config.toml like the documentation says.

I prefer 1 as it fits better with container orchestration software. The reason this came up is that someone is trying to use azafea on AWS ECS and it can't really work out of the box without some customization. In either case you can always build an image with a custom entrypoint to handle the configuration in the way you prefer. That's what we do at Endless and why we don't run into this problem.

wjt commented 2 months ago

What happens if you pass -c more than once? Does the last one win? If so I don't see a downside to option 1.

dbnicholson commented 2 months ago

Yes it does. The CLI handling makes this a little awkward since -c has to be passed before the subcommand. I.e., pipenv run azafea -c /foo.toml run works, but pipenv run azafea run -c /foo.toml doesn't. So, when running the container you have to pass it first like docker run -v $PWD/foo.toml:/foo.toml azafea:latest -c /foo.toml run. Not to mention that the wrappers (entrypoint and pipenv run) make the ENTRYPOINT and CMD handling awkward (azafea-metrics-proxy has them arranged slightly differently), but that's a different story.

adarnimrod commented 2 months ago

IIRC, I was more in favor of environment variables and the developer (I don't remember who it was at the time) preferred the config file, so we ended up with this neither this nor that status. That is why the CI workflow overrides the container entrypoint as that is how he ran the tests locally. Something else to take in to account is that the config template expects a Redis password. I don't know what would happen if you don't pass the REDIS_PASSWORD environment variable.

dbnicholson commented 2 months ago

Hey Nimrod!

Yeah, it's a bit at odds. I think the config file at a standard location makes perfect sense if you're running straight from the host, but it makes less sense in a container. Another thing I considered was templating to the expected location in /etc so that you don't have to change the arguments to the command. Maybe another day. I think this irons out some of the kinks and should make it more predictable.

As it turns out, I was trying to help someone set this up using ElasticCache. It seems they don't let you set a password unless you also enable encryption. Since SSL didn't work out of the box and I didn't want to walk through the TLS proxy container, we just left it unencrypted with no password to test. To have it run correctly right then, setting REDIS_PASSWORD to the empty string worked. If you don't set it at all, you'll just get the default CHANGE ME! set in the Dockerfile.