feat: ha_cluster_node_options allows per-node addresses and SBD options to be set

[tomjelinek]

Enhancement:
Allow users to specify node addresses and SBD options in a playbook. Previously, these were only read from an inventory.

Reason:
All cluster configuration is localized to a single place - a playbook. On top of being easier to use, this will also allow us to export cluster configuration in form of a playbook.

Result:
ha_cluster_node_options variable contains settings for node addresses and SBD options. Variables from inventory are still processed for backward compatibility.

Issue Tracker Tickets (Jira or BZ if any):

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.54%. Comparing base (620f0dd) to head (cab3e72).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #196   +/-   ##
=======================================
  Coverage   50.54%   50.54%           
=======================================
  Files           1        1           
  Lines          91       91           
  Branches       12       12           
=======================================
  Hits           46       46           
  Misses         45       45           

Flag	Coverage Δ
sanity	50.54% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[tomjelinek]

[citest]

[richm]

I'm not sure I understand the motivation for "moving" variables from inventory to playbooks. The general idea with Ansible is to separate the "code" from the "data", the "code" being the roles and the playbooks which are generally static, and the "data" being the inventory and other sources of variables outside of the "code", which are generally dynamic.

[tomjelinek]

[citest]

[richm]

I don't think we need to document where to define variables (inventory, playbook), only how to define them.
https://docs.ansible.com/ansible/latest/inventory_guide/intro_inventory.html#adding-variables-to-inventory

Typically, this is done in the inventory, by defining groups of hosts and variables for those groups, or defining host specific variables. Of course, for our test framework, we cannot provide an inventory for the tests/tests_*.yml files, but we should document best practices.

All we need to do in the README is to describe which parameters, but if you feel we should provide HA cluster users with more Ansible guidance, then we can do something like
"
If you need to define ha_cluster role variables on a per host or group of host basis in your inventory, see https://docs.ansible.com/ansible/latest/inventory_guide/intro_inventory.html#adding-variables-to-inventory. If you want to know how Ansible variable precedence works with respect to inventory and playbooks, see https://docs.ansible.com/ansible/latest/reference_appendices/general_precedence.html#variables
"

[tomjelinek]

I updated the readme and removed mentions of which variable should be defined where.

[tomjelinek]

The title "move variables from inventory to playbook" is actually a kind of a shortcut. Let me elaborate on what is actually being done.

To configure a cluster using the role, the user runs the role with a bunch of variables. They are prefixed with the name of the role, so there is ha_cluster_cluster_name, ha_cluster_primitive_resources and so on. These need to have the same value for all nodes in the cluster. So when I started writing the role, I assumed these would be specified for the role in a playbook.

Soon after that, I run into a case when I needed specific variables to have different values for each node. These are node addresses and SBD settings. So I created an ha_cluster variable with keys holding values for a particular node. This is nicely usable in an inventory - to easily define node addresses and SBD settings for each node.

Later, I found out that it doesn't actually matter where a variable is defined. But the naming kind of stuck to differentiate between ha_cluster structure and ha_cluster_* variables.

The point of this PR is to make the role read options, which were previously configurable only by ha_cluster structure, from ha_cluster_node_options variable. So instead of writing this (separate variables for each node, in an inventory):

all:
  hosts:
    node1:
      ha_cluster:
        pcs_address: node1-address
        corosync_addresses:
          - 192.168.1.11
          - 192.168.2.11
    node2:
      ha_cluster:
        pcs_address: node2-address:2224
        corosync_addresses:
          - 192.168.1.12
          - 192.168.2.12

one can write this:

ha_cluster_node_options:
  - node_name: node1
    pcs_address: node1-address
    corosync_addresses:
      - 192.168.1.11
      - 192.168.2.11
  - node_name: node2
    pcs_address: node2-address:2224
    corosync_addresses:
      - 192.168.1.12
      - 192.168.2.12

This allows to define all cluster settings using a single approach. It will also allow the role to export all cluster settings in a single structure.

[richm]

This allows to define all cluster settings using a single approach. It will also allow the role to export all cluster settings in a single structure.

I see - thanks for the explanation.

[richm]

The PR title - I think it should be something like "feat: ha_cluster_node_options allows per-node options to be set" ? Something more descriptive to what is actually being done