Add compatibility with more distributions

[aruhier]

Use package and make a bit changes to add the compatibility with other distributions than the debian based ones. Variables in vars/ can be overridden.

I tested this role on Arch and Alpine, not RHEL, so you are free to remove the untested changes if you prefer. dhcp-probe is not available on Arch (at least not in the official repositories, only in AUR) and on RHEL, so it can be configured but will not be installed on these distributions.

[drybjed]

Important thing in DebOps roles is avoiding usage of variables in the vars/ directory of a role, since they cannot be overriden by the Ansible inventory. Therefore usage of include_vars + with_first_found is discouraged as well, instead you should expose the desired logic via Jinja statements in default variables.

On the other hand, I think that cramming DHCP server, DHCP relay and DHCP probe into one rile wasn't a really good decision. At the time it seemed useful, but now I would just create 3 separate roles and enable them selectively using Ansible inventory groups instead of conditional variable in the role itself. Since this is a major modification, I plan to do this after the roles are merged into one git repository.

I guess that to allow easier usage of this role for now, this PR could be merged, since the overhaul will be done in the future anyway, but I'm not sure if I should merge it in this state or wait for a redesigned role. Any other debops.dhcpd users that would want to sway me either way? @Anthony25, do you want to try and move the logic into default variables? That would certainly make the decision easier.

[aruhier]

Well that is the point, dist_vars.yml allows the variables to be overridden, exactly as default variables.

[drybjed]

I tested your PR in local development environment, and role seems to work correctly.

OK, I think I understand what's happening now - I wasn't aware that include_vars can load the variables from a file into a YAML dictionary. You then use set_fact to define the variables in main namespace with either an existing value, or the default value from the vars/ file. Interesting. And it seems that YAML dictionary keys can be templated if you use a strict YAML syntax with opening and closing { } characters?

I tried to override the list of packages trough inventory and it seems to work. I assume that other variables from the files can be handled like this as well. OK then, seems like it can be merged without issues. I'll do that in a bit.

[aruhier]

And it seems that YAML dictionary keys can be templated if you use a strict YAML syntax with opening and closing { } characters?

Exactly. That is a bit hackish, because Ansible does not allow to specify a precedence/priority when using include_vars.

Thanks for the review!

[ganto]

@Anthony25 Neat approach, I guess we could use this at many other places too. Thanks for this PR.

[drybjed]

@ganto I'm not really convinced, especially that @Anthony25 referred to this approach as "hackish", and it costs tasks. Here it seems the most sensible because role has three modes of operation, but I suspect that if it were just DHCP server without a realy/probe, going with normal list in default variables would be better. I decided to merge this PR since it will be some time before I will work on this role to split it, so it might as well work this way for now.

[aruhier]

@drybjed: It allows to include vars depending on the targeted system, and using default variables would be dirtier, in my opinion, and more difficult to override. I see 2 ways of doing it:

• Storing the list of packages of each distro in a single dict, for example dhcpd_server_packages: {'Archlinux': […], 'Alpine': […], 'Debian': […], …}. It would mean that to override the list of one distribution, (correct me if I am wrong) you would have to redefine the entire dict.
• Storing one list of packages by distro, for example dhcpd_server_packages_Archlinux: […], dhcpd_server_packages_Alpine: […], etc. But then, to use it, you would need to write more complex loops.

In both cases, default values in cases where the distribution has no defined variable should be more difficult to handle.

The best way, in my opinion, would be that Ansible implements an argument to specify the precedence level of include_vars.

[drybjed]

@Anthony25 You can also store list of packages conditionally, like this:

---
# defaults/main.yml
dhcpd__base_packages:
  - '{{ "isc-dhcp-server"
        if (ansible_distribution in [ "Debian", "Ubuntu" ]
        else [] }}'
  - '{{ "dhcp"
        if (ansible_distribution in [ "RedHat", "Archlinux", "Alpine" ]
        else [] }}'

---
# tasks/main.yml
- name: Install DHCP server packages
  package:
    name: '{{ item }}'
    state: 'present'
  with_flattened:
    - '{{ dhcpd__base_packages }}'

This should ensure that on known distributions the correct package name is used, with with_flattened lookup. If you need to, you can just write the dhcpd__base_packages list in the inventory as normal, no need to handle any YAML dictionary keys or complex loops. That's of course after the role is split into server, relay and probe, therefore it's less complicated.

[aruhier]

Humm, I see. Sure, it would work and it seems to be a good solution.