community.sap_operations icon indicating copy to clipboard operation
community.sap_operations copied to clipboard

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

Open nbttmbrg opened this issue 4 months ago • 10 comments

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 avatar Jul 18 '25 15:07 nbttmbrg

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

nbttmbrg avatar Jul 18 '25 15:07 nbttmbrg

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

rhmk avatar Jul 22 '25 13:07 rhmk

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:

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.

sean-freeman avatar Aug 04 '25 17:08 sean-freeman

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.

nbttmbrg avatar Aug 05 '25 09:08 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.

marcelmamula avatar Aug 05 '25 09:08 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.

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.

nbttmbrg avatar Aug 06 '25 11:08 nbttmbrg

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

marcelmamula avatar Aug 06 '25 11:08 marcelmamula

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

nbttmbrg avatar Aug 06 '25 12:08 nbttmbrg

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 avatar Aug 06 '25 12:08 marcelmamula

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.

nbttmbrg avatar Sep 11 '25 13:09 nbttmbrg