mhutter / ansible-docker-systemd-service

Ansible role for creating Systemd services for docker containers
https://galaxy.ansible.com/mhutter/docker-systemd-service
MIT License
48 stars 30 forks source link

Add support for a flexible `container_args` to avoid implementing every `docker run` command line argument #60

Open andygrunwald opened 1 year ago

andygrunwald commented 1 year ago

Context

This Ansible module uses its own parameters to support various docker run command line arguments. Out of those, multiple types are used:

Examples for single strings:

Examples for lists:

This is great and marks explicitly how to use this module. The big downside on this: You need to add additional support for each docker run command line argument into this module. And the docker run reference is looooong.

One proposal would be to make this Ansible more flexible and avoid the burden of implementing every single docker run command line argument by offering ways to add arbitrarily

How a possible Ansible usage can look like in the future / Usage of the module:

- name: Start image
  include_role:
    name: mhutter.docker-systemd-service
  vars:
    container_name: "hello-world"
    container_image: "hello:v1"
    container_docker_pull_force_source: false

    container_single_args:
      network: my-network
      user: nonroot
      hostname: world
      [...]

    container_multi_args:
      publish:
        - '127.0.0.1:3030:3000'
        - '127.0.0.1:1234:5678'
      volume:
        - '/opt/foo:/var/lib/bar:rw'
      [...]

    [...]

The top-level keys under container_single_args and container_multi_args are not validated They can be defined to whatever you want.

A few keys, like container_image might be good to keep separate, primarily when those are used in a different context (e.g., to craft the systems unit name).

The same can be done for boolean flags via a list like container_boolean_args for keys like --rm or --tty:

container_boolean_args:
  - 'rm'
  - 'tty'

Open decisions

Additional notes

andygrunwald commented 1 year ago

@mhutter, I would appreciate your thoughts on this.

Personally, I am just for a strict breaking change with changelog entry and migration guide + releasing a v1.X after this. Even with -alpha or -beta suffix at first.

mhutter commented 1 year ago

Hey, this is pretty much what I had in mind! Good catch with single string vs lists; I thought of just accepting both, but not all parameters can be specified once.

I would however argue that we should not try to validate this in this module.

It might even be possible to just have a container_args parameter, which accepts both key->value as well as key->list pairs (IIRC Jinja2 can do some magic with that). We don't even have to handle booleans, docker will also accept --rm=true etc.

A few keys, like container_image might be good to keep separate, primarily when those are used in a different context (e.g., to craft the systems unit name).

Yes I agree, for example mandatory things (name, image), or thing where the module offers some kind of abstraction (env, maybe ports and volumes - where to draw the line?).

andygrunwald commented 1 year ago

or thing where the module offers some kind of abstraction (env, maybe ports and volumes - where to draw the line?).

What abstraction do you have in mind?

mhutter commented 1 year ago

I think for a v3 I'd set up the following:

Explicit variables for mandatory & positional parameters

Examples: container_name, container_image, container_args (or container_cmd as it is currently called)

Explicit variables for "abstract" options

Things like container_docker_pull that don't map to a command line option or argument.

container_opts for everything else

Just a map of strings & lists that can be expanded into individual options, examples:

container_opts:
  memory: 1G
  privileged: true
  health-cmd: curl ....

I also thought about having explicit variables for commonly used options (like ports, volumes, env vars) but after thinking about it, this brings almost no advantages while at the same while making both usage and implementation more complicated.

@andygrunwald WDYT?

ednxzu commented 9 months ago

Hello,

I've cloned your role to make it work better for my usecase, and I am currently working on this feature. Here is the repo in case you want to have a look. I simplifies the template quite a bit in the process, tho I haven't tested it heavily yet to make sure it can handle all sorts of flags being passed.

I can make a PR for it if you're interested.