sap_control: Add SAP System-level Function Support with Asynchronous Status Tracking

[nbttmbrg]

This PR adds support for SAP System-level functions (StartSystem, StopSystem, RestartSystem, and UpdateSystem) to the sap_control role. These functions operate across all instances of an SAP system rather than on individual components.

Should fix #36 and #30

Any comment and advices are very welcome and I can rework things if needed.
This has been tested on Linux.

Changes

• (defaults/main.yml) Added new functions :

restartsystem_all_nw
updatesystem_all_nw
startsystem_all_nw
stopsystem_all_nw

• (defaults/main.yml) Added new dictionary sap_control__waitforasync that provides the necessary elements for task file sapcontrol_async.yml to handle correctly the polling
• prepare.yml will now detect when the function name ends with system and include the according task file
• system function only run on the ASCS or SCS, ensuring that they'll run on each system in the limit but only once per system

[nbttmbrg]

I should add : I modified the Readme but not the picture. Don't know if that's mandatory.

[nbttmbrg]

These waitforstarted/waitforstopped aren't used anymore, I was using these instead of the current "waitforasync".
They technically work because they loop through every instance and wait for each of them to be started or stopped, but it's not very clean.

[nbttmbrg]

I changed this because it made little sense in my situation and couldn't figure out how to efficiently display only what was used. To be honnest I don't really get when this is useful at all

[rhmk]

I need to investigate, what the current issues, but we have some failed checks, that need to be resolved, and maybe an incompabilty with a prior patch, that has recently approved and merged

[sean-freeman]

With such a significant improvement to the looping logic and waiting, and given the other GH Issues raised about lack of functionality.... please see below context/history.....

---

The Ansible Collection sap_libs, contains the LOW-level Ansible Module sap_control_exec (community.sap_libs/plugins/modules/sap_control_exec.py).

This is a much more powerful handler for the sapcontrol binary, and you'll even find this directly inside of the pre-bundled Ansible Community Edition (ansible-core + various community Ansible Collections).

This Ansible Collection sap_operations, contains the UPPER-level with looping logic Ansible Role sap_control

---

The upper-level is based upon an extract from a legacy/rough approach by IBM Lab for SAP Solutions, it is very outdated.

It was supposed to be overhauled the Ansible Tasks using the Ansible Module ansible.builtin.shell that execute calls to the sapcontrol binary - were supposed to be replaced by Ansible Tasks using the Ansible Module sap_libs.sap_control_exec.

You can see this overhaul was partially completed in other areas, as there was another legacy bit of code that was replaced:

• Legacy Ansible Module written in Shell, included within the Ansible Collection >>> community.sap_operations/plugins/modules/sap_facts.sh
• Replaced by much more powerful Ansible Module, in the sap_libs Ansible Collection >>> community.sap_libs/plugins/modules/sap_system_facts.py

If you are updating the code (big applause here!!), then utilising the low-level sap_libs.sap_control_exec is probably the way to go to allow the upper-level sap_operations.sap_control to flourish.

[nbttmbrg]

Thank you very much for your input, I will go back and adapt my code to use sap_libs.sap_control_exec instead. I might update that sap_control role too if I still use it.

In the meantime, it might be a good idea to redirect people to the correct module and warn them about the depreciation of that role somewhere in the readme.

[marcelmamula]

@nbttmbrg @sean-freeman Please me mindful of requirements of sap_libs modules.
sap_libs.sap_control_exec requires Python library suds to be installed.

[nbttmbrg]

@nbttmbrg @sean-freeman Please me mindful of requirements of sap_libs modules. sap_libs.sap_control_exec requires Python library suds to be installed.

Also, sap_libs.sap_control_exec is using the sapcontrol web service, so it requires a username and password to work (I was previously calling the role using become_user: "{{ sapsid | lower }}adm" ; that can't work with that method).

That will break backwards compatibility : we will have to add a couple of variables for username/password, and users will need to correctly configure their system (secured web methods, service/admin_users, etc.) for it to work.

I still think that it's the way to go if you want cleaner and more secure implementation.

[marcelmamula]

I dont believe credentials are needed. I was testing it 2 days ago with this, without any credentials defined.

- name: Ansible Play to test sap_libs modules
  hosts: bw
  become: true
  any_errors_fatal: true
  max_fail_percentage: 0
  serial: 1
  vars:
    __sap_sysnr: '01'
  tasks:
    - name: Execute module sap_control_exec
      community.sap_libs.sap_control_exec:
        sysnr: "{{ __sap_sysnr }}" 
        function: GetProcessList

[nbttmbrg]

It depends on the webmethod used. GetProcessList is not protected, try it with Start for example, it will will fail :

fatal: [geuw1-lsapdb]: FAILED! => changed=false 
  error: 'Server raised fault: ''Invalid Credentials'''
  msg: Something went wrong connecting to the SAPCONTROL SOAP API.
  out: {}

Edit : We should probably be discussing this inside #6 instead

[marcelmamula]

We are discussing decreasing dependencies on sap_libs, so it would make sense to proceed as you originally planned without using dedicated module. It would also mean no need to introduce new variables as it would break backwards compatibility.

[marcelmamula]

Please make sure that you validate your PR with ansible-core 2.19. This does not mean you have to fix role if there are issues beforehand, but all newly added code has to be working.
ansible-community/ansible-build-data#538

Also approved #39 is now in merge conflict with this PR.

[marcelmamula]

@nbttmbrg Did you test these changes?
None of these functions have corresponding var_list so they will fail.

  - restartsystem_sap_nw
  - updatesystem_sap_nw
  - startsystem_sap_nw
  - stopsystem_sap_nw

fatal: [b01hana]: FAILED! =>
    changed: false
    msg: 'Task failed: object of type ''dict'' has no attribute ''startsystem_sap_nw_list'''

[marcelmamula]

regex_search is no longer truthy in ansible-core 2.19 so you must include check for type or length.
When value is found = value.
When value is not found = NoneType

task path: /home/mmamula/ansible_collections/community/sap_operations/roles/sap_control/tasks/sapcontrol_async.yml:6
fatal: [b01hana]: FAILED! =>
    changed: false
    msg: 'Task failed: Conditional result was ''GREEN'' of type ''str'', which evaluates
        to True. Conditionals must have a boolean result.'

This is example from my recent PR where I had to deal with this:

              failed_when:
                - __sap_ha_pacemaker_cluster_register_ers_ha_failover_config_restart.stdout | d('') | regex_search('^HAActive:\\s+FALSE$', multiline=True) is not none

[nbttmbrg]

This is now tested against Ansible 2.19

I also added a check before polling for async functions that don't change the final state of the system (namely RestartSystem and UpdateSystem) because I encountered false positives on my system when it took longer than expetcted before reacting to the command into account.