sap_ha_pacemaker_cluster: fix(check-mode)#986
Merged
Conversation
- Suse tasks fixes - removed check-mode conditionals from shell/command tasks, they will be skipped anyway by default - added more check-mode logic to NW post tasks
`command` tasks are skipped in check-mode. However, the retry logic is executed nonetheless and needs to be covered too.
- ignore errors in tasks with become_user that may not exist
marcelmamula
approved these changes
Feb 28, 2025
Contributor
marcelmamula
left a comment
marcelmamula
left a comment
There was a problem hiding this comment.
LGTM
Only problem I can see is this part failing on empty server
- name: "SAP HA Pacemaker srHook - Get contents of global.ini"
ansible.builtin.command:
cmd: cat "{{ __sap_ha_pacemaker_cluster_hana_global_ini_path }}"
register: __sap_ha_pacemaker_cluster_global_ini_contents
changed_when: false
check_mode: false
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue
The check-mode was inconsistent and some of it only surfaced on fresh nodes with no previous cluster config yet.
Fix
Check-mode handling has been added to various tasks in different ways:
shell/commandtasks with no changes are executed, e.g. for discovering factsThe last point especially concerns the
ha_clusterrole include. The entire include will be executed, but since the role is not check-mode safe, it will display the ignored errors. This way it is possible to see the workflow and review it as much as possible before the actual execution.Tested
@marcelmamula
I was not sure about the desired check-mode behavior in the affected Suse tasks, that's why I did not make changes here:
tasks/Suse/post_steps_nwas_abap_ascs_ers.ymltasks/Suse/post_steps_nwas_java_scs_ers.ymlHowever, they do need some improvements as well in my opinion, since there are a lot of commands executed in check-mode, which would either not work on fresh systems, or make actual changes in existing systems explicitly. Which is incorrect behavior in check-mode. Not sure if this is intended, so I kept my hands off for now. :)