allow setting additional marathon configs using a generic list

[lhoss]

as proposed in #27

Example configs (for which it seems overkill to support settings these in a 'hardcoded way')

  marathon_additional_configs:
   - name: task_lost_expunge_interval
     value: 900000
   - name: task_lost_expunge_gc
     value: 3600000

Ref: Config example taken from: d2iq-archive/marathon#616 (comment)

[tsaridas]

Maybe you should delete the other options too and only have configuration done this way.

[ernestas-poskus]

Maybe you should delete the other options too and only have configuration done this way.

👍

@lhoss willing to do some refactoring ?

[lhoss]

@lhoss willing to do some refactoring ?

maybe, but things to be careful about:

• staying (downward)compatible (so we should keep the existing vars like marathon_port)
  • can be done using approach proposed for the mesos configs: Allow additional mesos master and slave configs ansible-mesos#56 (comment)
• the idea of the additional_options list was (more) for optional configs.. respectively more exotic configs, and maybe it's better to keep the more standard configs explicit (?)
  • so in case we really do this, we should rename the var (remove the additional_)
• Ensure proper handling of removed configs (where the corresponding /etc/marathon/$config-name file should rather be deleted, instead of set empty !)

After thinking+deciding on these points, I could work on that, but I'll prefer a sep PR

[lhoss]

@ernestas-poskus conflict solved

[ernestas-poskus]

sorry for late response but this looks a bit magic especially that custom_option.j2 has {{ item.value }} in it.

how about using copy module

      copy:
        content: "{{ item.value }}"
        dest: "/etc/marathon/conf/{{ item.name }}"

[lhoss]

That was my first idea, but I used the template to be consistent with the existing role tasks ;)

ok, I will change to use content then!

[lhoss]

ps2: I also agree we should more use the 100% json notation (like in your example above, with 1 line per attribute)
Should we do a big refactoring (after merging all acceptable PRs) !?

[lhoss]

OK I changed, tested and pushed the change!

[ernestas-poskus]

indeed this playbook requires some refactoring

[ernestas-poskus]

👏 👍 thank you for contribution

[tsaridas]

Thanks @lhoss