signalfx / maestro-ng

Orchestration of Docker-based, multi-host environments
https://signalfx.com
Apache License 2.0
682 stars 84 forks source link

Add support for externalised env files #203

Closed petrkalina closed 5 years ago

petrkalina commented 5 years ago

Allow specifying env mapping in ".env" file like in docker compose and allow including this file in container definition.

mpetazzoni commented 5 years ago

Can you clarify your request? Do you mean that you want the ability to include the contents of a file like Docker Compose's env_files (https://docs.docker.com/compose/compose-file/#env_file) as environment to a container?

petrkalina commented 5 years ago

exactly!

petrkalina commented 5 years ago

The .env files are also supported by docker run command i.e.

docker run --rm --env-file  ./my.env ...
$ cat ./my.env 
ENV_VAR1=value
ENV_VAR2=value

which can be useful in executing specific pre-configured commands via docker - which is for example use-case I would be after, if I could manage env variables in .env files. Another good reason is possibility to reuse the same .env file for multiple service instances (containers) preventing duplicities in maestro environment yaml file..

mpetazzoni commented 5 years ago

Maestro's environment file is templated which typically help with those situations. The fact that docker run itself supports env files means that Maestro doesn't actually have to do any parsing of those env files and just passes it along to Docker. That makes implementation easier.

petrkalina commented 5 years ago

hello .. just a question - when do you think you will have time to add this?

mpetazzoni commented 5 years ago

@petrkalina support for this just landed in master. Try it out and let me know what you think!

petrkalina commented 4 years ago

verified! But I found out it is not documented in the docs (https://maestro-ng.readthedocs.io/en/latest/environment.html?highlight=env)

    busybox:
        image: busybox
        instances:
            busybox1:
                ship: ship1
                envfile: /tmp/busybox1.env
                command: env
$ cat /tmp/busybox1.env
TESTVAR=test_value
$ maestro -f test.yaml start
  #  INSTANCE                                 SERVICE              SHIP                                     CONTAINER                  STATUS    
  1. busybox1                                 busybox              ship1                              latest:2e7a891             started
$ docker logs busybox1 | grep TEST
TESTVAR=test_value
petrkalina commented 4 years ago

According to https://docs.docker.com/compose/env-file/

Blank lines are ignored.

however, if they are present in the env file, an error is returned:

Traceback (most recent call last):
  File "/usr/local/lib/python3.8/dist-packages/maestro/__main__.py", line 186, in execute
    c = maestro.Conductor(config)
  File "/usr/local/lib/python3.8/dist-packages/maestro/maestro.py", line 71, in __init__
    entities.Container(
  File "/usr/local/lib/python3.8/dist-packages/maestro/entities.py", line 385, in __init__
    self.env = environment.build(
  File "/usr/local/lib/python3.8/dist-packages/maestro/environment.py", line 26, in build
    env.update(parse_env_file(envfile))
  File "/usr/local/lib/python3.8/dist-packages/docker/utils/utils.py", line 972, in parse_env_file
    raise errors.DockerException(
docker.errors.DockerException: Invalid line in environment file /opt/j4care/maestro/env/arc1-a.env:
mpetazzoni commented 4 years ago

@petrkalina Sorry about the docs, they didn't rebuild because the main branch was renamed (from master) and readthedocs was still trying to build from the now non-existent branch! Fixed now: https://maestro-ng.readthedocs.io/en/main/environment.html#service-instances

As for the empty lines, as you see I'm just using docker.utils.parse_env_file(), which should correctly skip empty lines: https://github.com/docker/docker-py/blame/master/docker/utils/utils.py#L454-L456

Can you confirm which version of docker-py you are using?