search

sap-linuxlab / community.sap_libs

Automation for SAP - Collection of Ansible Modules for SAP for low-level activities which are highly reusable
Apache License 2.0
16 stars 7 forks source link

add functions to check and start sapstartsrv if not running #36

Closed leocruz710 closed 8 months ago

leocruz710 commented 8 months ago

In the existing version of sap_system_facts.py assumes the sapstartsrv service is running in order to effectively pull details about the type of SAP system. My proposal is to add two additional functions.

  1. get_sap_srv_status(ins_sid, ins_nr) will check to see if sapstartsrv is running.
  2. start_sap_srv(ins_sid, ins_nr) will start the services if it is not running.
rainerleber commented 8 months ago

Hi @leocruz710,

thank you for open this PR. But I don't think it's really a good idea to start the system automatically by a facts module. Generally there are/should be reasons why a system is stopped. Do privileged escalation inside a module even facts or not is not the right way. This should be done with ansible_become instead. @sean-freeman what do you think.

sean-freeman commented 8 months ago

@rainerleber I agree, automatically starting the SAP System processes on that host could lead to a potential calamity. This Ansible Module is intended as "read only" to extract information without making changes to the host or application-level (the SAP System).

The start functionality proposed is better scoped under community.sap_libs.sap_control_exec Ansible Module and the upper-level community.sap_operations.sap_control Ansible Role (which really requires an overhaul of that outdated code).

Is community.sap_libs.sap_control_exec + community.sap_libs.sap_system_facts called sequentially by an Ansible Playbook, not suitable for your use case? Is there a use case we are not considering @leocruz710 ?

leocruz710 commented 8 months ago

@rainerleber - thank you for your feedback. You are correct about escalations inside the module, I did not think of it as I wrote the function.

@sean-freeman - The reason I coded it in the facts module, was because in my use case I don't always know what type of system is running on it. The services could be stopped and I can't pull the facts off the system. I can see the workflow as you describe as well, which I think would also suffice for those use cases.

I appreciate both of your feedback.

Wabri commented 8 months ago

At this point I would say to close the pr