Merge pull request #1084 from marcelmamula/ascsers

sap_ha_pacemaker_cluster: Enhance handling of cluster properties and improve ASCS ERS post steps

Author: marcelmamula

roles/sap_ha_pacemaker_cluster/README.md

@@ -243,17 +243,17 @@ sap_ha_pacemaker_cluster_cluster_nodes:
 ```
 ### sap_ha_pacemaker_cluster_cluster_properties
 - _Type:_ `dict`<br>
-- _Default:_ `{'concurrent-fencing': True, 'stonith-enabled': True, 'stonith-timeout': 900}`<br>
 
 Standard pacemaker cluster properties are configured with recommended settings for cluster node fencing.<br>
 When no STONITH resource is defined, STONITH will be disabled and a warning displayed.<br>
+Default values are predefined by OS and Platform if the variable is not defined.<br>
+Some mandatory properties are appended, if not defined.<br>
 
 Example:
 ```yaml
 sap_ha_pacemaker_cluster_cluster_properties:
-  concurrent-fencing: true
   stonith-enabled: true
-  stonith-timeout: 900
+  stonith-timeout: 150
 ```
 ### sap_ha_pacemaker_cluster_corosync_totem
 - _Type:_ `dict`<br>

roles/sap_ha_pacemaker_cluster/defaults/main.yml

@@ -53,21 +53,23 @@ sap_ha_pacemaker_cluster_host_type: "{{ sap_host_type | d(['hana_scaleup_perf'])
 ### VIP resource default patterns
 sap_ha_pacemaker_cluster_vip_client_interface: ''
 
-## A custom stonith definition that takes precedence over platform defaults.
+### STONITH
+## Recommended fencing methods are defined only for platforms, where applicable.
+## It is not recommended to configure cluster without fencing resources (e.g. SBD).
+## You can define custom stonith resource using variable 'sap_ha_pacemaker_cluster_stonith_custom'.
+## NOTE: Not providing stonith resource will result in cluster property 'stonith-enabled: false'.
 # sap_ha_pacemaker_cluster_stonith_custom:
 #   - name: ""
 #     agent: "stonith:"
 #     options:
 #       pcmk_host_list: ""
 
-# sap_ha_pacemaker_cluster_stonith_custom: []
-
-# Simpler definition format here which gets transformed into the 'ha_cluster' LSR native
-# 'ha_cluster_cluster_properties' parameter.
-sap_ha_pacemaker_cluster_cluster_properties:
-  stonith-enabled: true
-  stonith-timeout: 900
-  concurrent-fencing: true
+## The variable 'sap_ha_pacemaker_cluster_cluster_properties' is used to configure cluster properties (e.g. cib-bootstrap-options).
+# NOTE: Default values are defined by Operating System and Platforms, if the variable is not defined.
+# Simpler definition format here which gets transformed into the 'ha_cluster' LSR native 'ha_cluster_cluster_properties' parameter.
+# sap_ha_pacemaker_cluster_cluster_properties:
+#   stonith-enabled: true
+#   stonith-timeout: 150  # Default value 150 is for SBD method.
 
 ### Constraints:
 # score is dynamic and automatically increased for groups

roles/sap_ha_pacemaker_cluster/meta/argument_specs.yml

@@ -297,20 +297,17 @@ argument_specs:
 
       sap_ha_pacemaker_cluster_cluster_properties:
         type: dict
-        default:
-          stonith-enabled: true
-          stonith-timeout: 900
-          concurrent-fencing: true
         description:
           - Standard pacemaker cluster properties are configured with recommended settings for
             cluster node fencing.
           - When no STONITH resource is defined, STONITH will be disabled and a warning displayed.
+          - Default values are predefined by OS and Platform if the variable is not defined.
+          - Some mandatory properties are appended, if not defined.
 
         example:
           sap_ha_pacemaker_cluster_cluster_properties:
             stonith-enabled: true
-            stonith-timeout: 900
-            concurrent-fencing: true
+            stonith-timeout: 150
 
       sap_ha_pacemaker_cluster_corosync_totem:
         type: dict

roles/sap_ha_pacemaker_cluster/tasks/configure_nwas_abap_ascs_ers_post_install.yml

@@ -88,8 +88,23 @@
         else __sap_ha_pacemaker_cluster_fence_agent_packages_minimal }}"
 
 
+# Prepare cluster properties variable with either:
+# - User provided 'sap_ha_pacemaker_cluster_cluster_properties' if defined and not empty.
+# - Platform specific defaults, if defined.
+# - OS specific defaults.
+# If the variable 'ha_cluster_properties' is defined, it will append only missing keys.
+- name: "SAP HA Prepare Pacemaker - Prepare cluster properties variable"
+  ansible.builtin.set_fact:
+    __sap_ha_pacemaker_cluster_cluster_properties:
+      "{{ sap_ha_pacemaker_cluster_cluster_properties
+        if sap_ha_pacemaker_cluster_cluster_properties is defined and sap_ha_pacemaker_cluster_cluster_properties | length > 0
+        else (__sap_ha_pacemaker_cluster_cluster_properties_platform
+          if __sap_ha_pacemaker_cluster_cluster_properties_platform is defined
+          else __sap_ha_pacemaker_cluster_cluster_properties_default) }}"
+
+
 # Prepare corosync totem variable with either:
-# - User provided sap_ha_pacemaker_cluster_corosync_totem if present
+# - User provided 'sap_ha_pacemaker_cluster_corosync_totem' if present
 # - Combine corosync totem from OS variables and Platform variables if present
 # - Use default corosync totem from OS variables if Platform variable is not present
 - name: "SAP HA Prepare Pacemaker - Prepare corosync totem settings"

roles/sap_ha_pacemaker_cluster/tasks/construct_vars_common.yml

@@ -74,6 +74,8 @@
         - attrs:
             - name: resource-stickiness
               value: "{{ __sap_ha_pacemaker_cluster_nwas_cs_sapinstance_resource_stickiness }}"
+            - name: priority
+              value: 100
       operations:
         # TODO: Add values for start and stop when they are published.
         - action: monitor

roles/sap_ha_pacemaker_cluster/tasks/construct_vars_nwas_abap_ascs_ers_simple_mount.yml

@@ -35,10 +35,13 @@
       )
   block:
 
+    # Ensure that property 'stonith-enabled' is set to 'false' if no Stonith resources are defined.
+    # NOTE: We cannot allow user set 'stonith-enabled' 'true' if there are no fencing resources.
     - name: "SAP HA Prepare Pacemaker - (STONITH) Set to disabled when no fencing resource is defined"
+      when: ha_cluster_cluster_properties is not defined  # We cannot append to defined variable due to precedence.
       ansible.builtin.set_fact:
-        sap_ha_pacemaker_cluster_cluster_properties:
-          "{{ sap_ha_pacemaker_cluster_cluster_properties | combine({'stonith-enabled': false}) }}"
+        __sap_ha_pacemaker_cluster_cluster_properties:
+          "{{ __sap_ha_pacemaker_cluster_cluster_properties | combine({'stonith-enabled': false}) }}"
 
     - name: "SAP HA Prepare Pacemaker - Warn that there is no STONITH configured"
       ansible.builtin.pause:
@@ -53,38 +56,64 @@
 
 # END of block for disabling stonith
 
-# Add additional stonith properties to sap_ha_pacemaker_cluster_cluster_properties
-# Checks if pcmk_delay_max is defined and non zero, then multiples it by 2.
+# Stonith cluster properties
+# Property 'concurrent-fencing' is 'false' by default, but Scale-Out requires 'true'.
+# NOTE: Reversed combine ensures that user defined values are retained.
+# TODO: Enable during implementation of Scale-Out.
+# - name: "SAP HA Prepare Pacemaker - Disable concurrent-fencing in properties"
+#   when: ha_cluster_cluster_properties is not defined  # We cannot append to defined variable due to precedence.
+#   ansible.builtin.set_fact:
+#     __sap_ha_pacemaker_cluster_cluster_properties:
+#       "{{ {'concurrent-fencing': true} | combine(__sap_ha_pacemaker_cluster_cluster_properties) }}"
+#   when:
+#     - sap_ha_pacemaker_cluster_host_type | select('search', 'hana_scaleout') | length > 0
+
+
+# Property 'priority-fencing-delay' is required to ensure proper fencing order.
+# The value is based on 'pcmk_delay_max', if it is defined in '__sap_ha_pacemaker_cluster_stonith_default'.
+# This task will not change property if it is already defined.
+# NOTE: Reversed combine ensures that user defined values are retained.
 - name: "SAP HA Prepare Pacemaker - (STONITH) Add priority-fencing-delay property"
+  when: ha_cluster_cluster_properties is not defined  # We cannot append to defined variable due to precedence.
   ansible.builtin.set_fact:
-    sap_ha_pacemaker_cluster_cluster_properties:
-      "{{ sap_ha_pacemaker_cluster_cluster_properties | combine({'priority-fencing-delay':
-          __sap_ha_pacemaker_cluster_stonith_default.options.pcmk_delay_max | int * 2})
-        if __sap_ha_pacemaker_cluster_stonith_default.options.pcmk_delay_max is defined
-        and __sap_ha_pacemaker_cluster_stonith_default.options.pcmk_delay_max | int != 0
-        else sap_ha_pacemaker_cluster_cluster_properties }}"
+    __sap_ha_pacemaker_cluster_cluster_properties:
+      "{{ {'priority-fencing-delay': (__pcmk_delay_max | int * 2
+          if __pcmk_delay_max is defined and __pcmk_delay_max | int != 0
+          else 30)} | combine(__sap_ha_pacemaker_cluster_cluster_properties)
+        if __sap_ha_pacemaker_cluster_cluster_properties['priority-fencing-delay'] is not defined
+        else __sap_ha_pacemaker_cluster_cluster_properties }}"
+  vars:
+    __pcmk_delay_max:
+      "{{ (__sap_ha_pacemaker_cluster_stonith_default['instance_attrs'][0]['attrs']
+        | selectattr('name', 'equalto', 'pcmk_delay_max') | first).value }}"
 
-- name: "SAP HA Prepare Pacemaker - (STONITH) Define cluster properties"
+# Ensure that property 'concurrent-fencing' is set to 'true' for Azure.
+# Source: https://learn.microsoft.com/en-us/azure/sap/workloads/high-availability-guide-suse-pacemaker?tabs=msi#create-a-fencing-device-on-the-pacemaker-cluster
+# NOTE: Reversed combine ensures that user defined values are retained.
+- name: "SAP HA Prepare Pacemaker - (STONITH) - MSAZURE VM - Add cluster property 'concurrent-fencing'"
   when:
-    - sap_ha_pacemaker_cluster_cluster_properties is defined
-    - sap_ha_pacemaker_cluster_cluster_properties is iterable
-    - sap_ha_pacemaker_cluster_cluster_properties | length > 0
+    - ha_cluster_cluster_properties is not defined  # We cannot append to defined variable due to precedence.
+    - __sap_ha_pacemaker_cluster_platform == "cloud_msazure_vm"
   ansible.builtin.set_fact:
-    __sap_ha_pacemaker_cluster_cluster_properties: "{{ __sap_ha_pacemaker_cluster_cluster_properties | d([]) + __stonith_properties }}"
-  vars:
-    __stonith_properties:
+    __sap_ha_pacemaker_cluster_cluster_properties:
+      "{{ {'concurrent-fencing': true} | combine(__sap_ha_pacemaker_cluster_cluster_properties) }}"
+
+
+# Prepare structure compatible with the variable 'ha_cluster_cluster_properties'.
+- name: "SAP HA Prepare Pacemaker - (STONITH) Define cluster properties"
+  # This task is skipped if 'ha_cluster_cluster_properties' is defined, as both structures are different.
+  when: ha_cluster_cluster_properties is not defined  # We cannot append to defined variable due to precedence.
+  ansible.builtin.set_fact:
+    __sap_ha_pacemaker_cluster_cluster_properties:
       - attrs: |-
-          {% set attrs = __sap_ha_pacemaker_cluster_cluster_properties | map(attribute='attrs') | flatten -%}
-          {%- for default_cluster_properties in (sap_ha_pacemaker_cluster_cluster_properties | dict2items) -%}
-            {% if default_cluster_properties.key not in
-            (__sap_ha_pacemaker_cluster_cluster_properties | map(attribute='attrs') | flatten | map(attribute='name')) -%}
-              {% set role_attrs = attrs.extend([
-                {
-                  'name': default_cluster_properties.key,
-                  'value': default_cluster_properties.value
-                }
-              ]) -%}
-            {%- endif %}
+          {% set attrs = [] -%}
+          {%- for default_cluster_properties in (__sap_ha_pacemaker_cluster_cluster_properties | dict2items) -%}
+            {% set role_attrs = attrs.extend([
+              {
+                'name': default_cluster_properties.key,
+                'value': default_cluster_properties.value
+              }
+            ]) -%}
           {%- endfor %}
           {{ attrs }}
 

roles/sap_ha_pacemaker_cluster/tasks/construct_vars_stonith.yml

@@ -23,18 +23,6 @@
     - "(role_path + '/vars/' + include_item + '.yml') is file"
 
 
-# Disable concurrent-fencing for Scale-up scenario.
-# This assignment cannot be in scaleup var file, because it results in nested error.
-- name: "SAP HA Prepare Pacemaker - Disable concurrent-fencing in properties"
-  ansible.builtin.set_fact:
-    sap_ha_pacemaker_cluster_cluster_properties:
-      "{{ sap_ha_pacemaker_cluster_cluster_properties | combine({'concurrent-fencing': false})
-      if sap_ha_pacemaker_cluster_cluster_properties['concurrent-fencing'] is defined
-      else sap_ha_pacemaker_cluster_cluster_properties }}"
-  when:
-    - sap_ha_pacemaker_cluster_host_type | select('search', 'hana_scaleup') | length > 0
-
-
 # Private variables are assigned following logic:
 # 1. Use backwards compatible var if new var is empty
 # 2. Use user input if new var is not empty

roles/sap_ha_pacemaker_cluster/tasks/include_vars_hana.yml

@@ -55,6 +55,11 @@ __sap_ha_pacemaker_cluster_command:
   resource_restart: "pcs resource restart"
   resource_cleanup: "pcs resource cleanup"
 
+# Dictionary with default cluster properties
+__sap_ha_pacemaker_cluster_cluster_properties_default:
+  stonith-enabled: true
+  stonith-timeout: 150  # Default value 150 is for SBD method.
+
 # Default corosync options - OS specific
 __sap_ha_pacemaker_cluster_corosync_totem_default:
   options: {}

roles/sap_ha_pacemaker_cluster/vars/RedHat.yml

@@ -12,12 +12,18 @@ __sap_ha_pacemaker_cluster_connector_config_lines:
   - "service/halib_cluster_connector = /usr/bin/sap_suse_cluster_connector"
 
 # Cluster commands to manage resources - crmsh commands in SUSE OS family.
+# --force is required only for resource restart and maintenance on/off.
 __sap_ha_pacemaker_cluster_command:
   resource_stop: "crm resource stop"
   resource_start: "crm resource start"
-  resource_restart: "crm resource restart"
+  resource_restart: "crm --force resource restart"
   resource_cleanup: "crm resource cleanup"
 
+# Dictionary with default cluster properties
+__sap_ha_pacemaker_cluster_cluster_properties_default:
+  stonith-enabled: true
+  stonith-timeout: 150  # Default value 150 is for SBD method.
+
 # Default corosync options - OS specific
 __sap_ha_pacemaker_cluster_corosync_totem_default:
   options: