sap-linuxlab / community.sap_install

Automation for SAP - Collection of Ansible Roles for various SAP software installation
Apache License 2.0
53 stars 57 forks source link

Fixes for testing with molecule #807

Closed berndfinger closed 4 months ago

berndfinger commented 4 months ago

These changes should ensure that the molecule idempotency tests no longer fail, see issue #752.

ja9fuchs commented 4 months ago

Why the overhead of executing "x entries + 1" commands per sysctl file and not just sysctl -p <file> without those checks first? The reload does not make changes as such, it only activates what has been defined.

Since the checks do not validate the content of the config file, there is no benefit to checking if all of the settings are already active or not. If one deviates, it will also reload the whole file, not just the deviating entry.

It's not 100% idempotent unless you add even more overhead to check and load each entry individually. But again, there is no benefit of doing this, unless there is an actual sanity verification of the settings to prevent a mis-configuration from being loaded.

berndfinger commented 4 months ago

Why the overhead of executing "x entries + 1" commands per sysctl file and not just sysctl -p <file> without those checks first? The reload does not make changes as such, it only activates what has been defined.

We have three different sysctl files to modify according to applicable SAP notes, and because we want to follow the SAP notes as closely as possible, we do the modification for each file separately. For each file, we perform the modification of the file and then reload the parameters from the file (using sysctl -p) so that it becomes effective immediately.

Since the checks do not validate the content of the config file, there is no benefit to checking if all of the settings are already active or not. If one deviates, it will also reload the whole file, not just the deviating entry.

I am not checking if all of the settings of a config file are active or not. I want to know if any, and which one(s) of the entries or which of the current settings have been modified after reloading all the settings.

So we need to compare the desired state with the current state. The solution I offered appears to be robust and easy to understand. Happy to implement a better solution if there is one!

It's not 100% idempotent unless you add even more overhead to check and load each entry individually. But again, there is no benefit of doing this, unless there is an actual sanity verification of the settings to prevent a mis-configuration from being loaded.

We want to detect if there was a change or not. A misconfiguration should result in a task error in the tasks Reload kernel parameters from file XXX, so I don't think we need to do a sanity verification.

But maybe I misunderstood some of your points. In this case, can you please provide examples?

glavoix commented 4 months ago

Dear @berndfinger After reviewing your commit, I believe the approach looks nice and we should now be able to enable "idem-potency" into our molecule testing.

Thanks a lot for working on it.

Guillaume