Skip to content

sap_ha_pacemaker_cluster: several bug fixes#965

Merged
ja9fuchs merged 5 commits into
sap-linuxlab:devfrom
ja9fuchs:ha-fixes
Feb 13, 2025
Merged

sap_ha_pacemaker_cluster: several bug fixes#965
ja9fuchs merged 5 commits into
sap-linuxlab:devfrom
ja9fuchs:ha-fixes

Conversation

@ja9fuchs
Copy link
Copy Markdown
Contributor

@ja9fuchs ja9fuchs commented Feb 12, 2025

A few bugs started showing during new tests and need fixing.

The combination of fixes was tested on AWS using

  • RHEL 9.4
  • HANA scale-up combined with NWAS ASCS/ERS on 2 nodes

1. Regression fix: Stonith resource duplicate definition

  • This is a regression introduced in my previous stonith change. :/

During the previous change of the stonith code a conditional was removed that did not apply to the GCP default, but is required for other platform default stonith resources that use host maps.

2. Fix: Check-mode

Fixed a conditional that failed in check-mode. The .stout var is always defined, but can be empty and that fails the version comparison in the last conditional.

3. Fix: HANA VIP colocation score calculation

  • This issue comes up due to the combination of HANA resources with NW groups.

This fixes the HANA VIP colocation constraint conditional to always apply the base score and recalculate only when HANA-VIP resource groups are present in the definition.
The previous code enhanced with an else in the loop for a default score ended up appending the loop results as strings.

Due to the combination of HANA and NW resource definitions the group loop was processed put the if-conditional does not match, so the issue started showing up in the form of an empty score returned due a missing fallback value inside the for-loop. This causes the ha_cluster role to fail when trying to apply the constraint.
The change to variable assignment was done to overcome the issue with the loop appending results instead of overriding one value.

Assigning new values inside blocks/loops does not work (anymore) and a namespace object has to be used instead.

Reference: https://jinja.palletsprojects.com/en/latest/templates/#assignments

4. Mini enhancement: tag ha_cluster role import task

Added a tag to allow running without executing the ha_cluster role by skipping the tag run_ha_cluster.
This speeds up the construction of the input config file for debugging purpose and could also be used to run everything else without recreating the cluster config again.

Example usage that saves ~7 minutes of execution time just for producing the input parameters for review:

ansible-playbook ... -e 'sap_ha_pacemaker_cluster_create_config_varfile: true' -C --skip-tags run_ha_cluster

5. Enhancement: flexibility for combined setups

  • fixing the hook dictionary lookup in combined setups
  • adds a check to the host type definition to prevent invalid combinations

  - fixed a conditional that failed in check-mode
  - added tag to allow running in check-mode without executing ha_cluster
During the previous change of the stonith code a conditional
was removed that did not apply to the GCP default, but is
required for other platform default stonith resources that
use host maps.
This fixes the HANA VIP colocation constraint conditional to always apply
the base score and recalculate only when HANA-VIP resource groups are
present in the definition.

Assigning new values inside blocks/loops does not work (anymore) and
and a namespace object has to be used instead.

Reference: https://jinja.palletsprojects.com/en/latest/templates/#assignments
@ja9fuchs ja9fuchs added the bug Something isn't working label Feb 12, 2025
@ja9fuchs ja9fuchs self-assigned this Feb 12, 2025
Same as in commit c6935e4, but for the task about the read-only VIP.
Copy link
Copy Markdown
Contributor

@marcelmamula marcelmamula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, tested ASCS/ERS and HANA HA clusters on GCP.

- fixes the hook dict definition for mixed setups
- adds a check to prevent invalid host type combinations
@ja9fuchs
Copy link
Copy Markdown
Contributor Author

@marcelmamula Sorry, I added one more small fix/enhancement for the topic. :)

Copy link
Copy Markdown
Contributor

@marcelmamula marcelmamula left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ja9fuchs ja9fuchs merged commit 9ecc238 into sap-linuxlab:dev Feb 13, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants