sap_ha_pacemaker_cluster: regression on clusters with FQDN

[rob0d]

@marcelmamula
This code that was added in #829 breaks cluster configurations which are using FQDN as node names.
I am not clear why was it added as it worked fine with pacemaker before and after commenting out the code it still works fine. I didn't want to commit a removal as I'm not sure if any code that I can't test relies on it.
Basically if the defaults are not set the ha_cluster role does a good job generating the correct config based on ansible facts and provided variables.
This change introduces a regression by setting defaults which don't match ansible facts when FQDNs are used in inventory and cluster node names.
It may be possible to fix this by changing node_name: "{{ ansible_hostname }}" to node_name: "{{ inventory_name }}", but as I mention it doesn't seem like this code is needed?

[marcelmamula]

@rob0d This change was done to remove need to provide mandatory input, as we can default into known values.

This is why it contains task above one you linked, which uses User input variable sap_ha_pacemaker_cluster_ha_cluster that you can use as before with FQDN, and only when it is not defined and filled, it will default into code you linked.

- name: "SAP HA Prepare Pacemaker - Register sap_ha_pacemaker_cluster_ha_cluster"
  when:
    - __sap_ha_pacemaker_cluster_ha_cluster is not defined
    - sap_ha_pacemaker_cluster_ha_cluster is defined
  ansible.builtin.set_fact:
    __sap_ha_pacemaker_cluster_ha_cluster: "{{ sap_ha_pacemaker_cluster_ha_cluster }}"

[ja9fuchs]

@rob0d Did you define sap_ha_pacemaker_cluster_ha_cluster?
Does it work for you when you define it the way you suggested, using {{ inventory_name }}?

Previously, when not defined, the defaults of the ha_cluster role would apply. Which is not visible to users of this SAP role. By defining a role default in our role, we keep control over this default and it is easier for a user to troubleshoot in one place.

If you define the role variable the way that your environment needs, it should work as expected.
Let us know if it doesn't work. If it does, can we consider this issue resolved? :)

[rob0d]

Hi @ja9fuchs,

I haven't tried that (I couldn't at that time due to time pressure). I have commented out that piece of code.

My view is that ha_cluster has been tested by way more people than sap_ha_pacemaker_cluster and ha_cluster does a good job at deciding the default values.
We are taking on a responsibility by predefining some of these defaults and it those may or may not (like in my case) work. This forces the user to guess what ha_cluster would have done and define it to override what we are doing.
I think that we should remove the code and let ha_cluster do what it needs to do. In cases where the users are not happy, they can use sap_ha_pacemaker_cluster_ha_cluster or just ha_cluster to change the default behaviour.

I may be able to try with "{{ inventory_name }}", but it will a few more weeks before I can do it.

[marcelmamula]

@ja9fuchs @rob0d
I have looked into current ha_cluster role design and we could swing this change by doing empty dictionary so it can be combined with stonith if stonith is defined.

- name: "SAP HA Prepare Pacemaker - Generate default sap_ha_pacemaker_cluster_ha_cluster"
  when:
    - not __sap_ha_pacemaker_cluster_ha_cluster is defined
    - not sap_ha_pacemaker_cluster_ha_cluster is defined
  ansible.builtin.set_fact:
    __sap_ha_pacemaker_cluster_ha_cluster: {}

ha_cluster is doing:

• RHEL: Uses __ha_cluster_node_name: "{{ ha_cluster.node_name | d(inventory_hostname) }}" which will be FQDN if defined in inventory.
• SLES: Uses host in ansible_play_batch which will be FQDN if defined in inventory.