Skip to content

sap_swpm: Option to enable SWPM observer mode#749

Merged
berndfinger merged 4 commits into
sap-linuxlab:devfrom
rob0d:swpm_observer_mode
Jun 26, 2024
Merged

sap_swpm: Option to enable SWPM observer mode#749
berndfinger merged 4 commits into
sap-linuxlab:devfrom
rob0d:swpm_observer_mode

Conversation

@rob0d
Copy link
Copy Markdown
Contributor

@rob0d rob0d commented Jun 5, 2024

Comment thread roles/sap_swpm/tasks/pre_install.yml Outdated
@sean-freeman sean-freeman changed the title Option to enable SWPM observer mode sap_swpm: Option to enable SWPM observer mode Jun 14, 2024
Copy link
Copy Markdown
Member

@sean-freeman sean-freeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rob0d I'm good to merge this PR, it's non-destructive and can work before the overhaul of the sap_swpm code structure in the coming weeks.

However, in defaults the sap_swpm_swpm_remote_access_user should be uncommented and I would recommend it is by default set to root. If an end user wants to enable this feature and observe using a different OS User, then it can be changed of course. All SWPM executions are otherwise root so I think this default is OK.

@berndfinger
Copy link
Copy Markdown
Member

Hi @rob0d, we are working on finalizing a new version, and your PR can be part of it. Can you please respond to the suggestion from @sean-freeman?

@rob0d
Copy link
Copy Markdown
Contributor Author

rob0d commented Jun 20, 2024

@sean-freeman @berndfinger apologies, a bit snowed under.
I would prefer not to do that as it will change the role's default behaviour.
Currently when SWPM is executed SAPINST_REMOTE_ACCESS_USER= and SAPINST_REMOTE_ACCESS_USER_IS_TRUSTED=true are not set. This makes SWPM use root (unix) or Administrator(windows) automatically.
By setting the default to root, these two parameters will be set even thought it's not required as they basically emulate the default behaviour. I have never tried to use SAPINST_REMOTE_ACCESS_USER=root and I don't know how will SWPM react. It should behave the same way, but I am not sure and I won't be able to test it for at least 4 weeks.

So setting the default to root, will change the current default behaviour of the role and I am not sure what impact it will have.

@berndfinger
Copy link
Copy Markdown
Member

So setting the default to root, will change the current default behaviour of the role and I am not sure what impact it will have.

@rob0d I think in this case, we better not merge this PR into 1.4.1 but continue this discussion afterwards, for including it into 1.4.2, together with more changes for role sap_swpm.

@sean-freeman
Copy link
Copy Markdown
Member

... in defaults the sap_swpm_swpm_remote_access_user should be uncommented and I would recommend it is by default set to root. If an end user wants to enable this feature and observe using a different OS User, then it can be changed of course. All SWPM executions are otherwise root so I think this default is OK.

Still awaiting this change to be made in defaults, otherwise PR is blocked

@rob0d
Copy link
Copy Markdown
Contributor Author

rob0d commented Jun 25, 2024

Updated the default to be an empty variable and added code to handle this in a way that it doesn't change SWPM parameters and behaviour.
Unless this is enabled by setting sap_swpm_swpm_observer_mode to true, the behaviour is the same as before, so this is a non breaking change.
If sap_swpm_swpm_remote_access_user is not explicitly set and sap_swpm_swpm_observer_mode = true, it will execute SWPM with standard parameters.
If sap_swpm_swpm_remote_access_user is set, it will add appropriate SWPM command line parameters to use the requested user.

Copy link
Copy Markdown
Member

@sean-freeman sean-freeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

Copy link
Copy Markdown
Member

@berndfinger berndfinger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Wabri Wabri self-requested a review June 26, 2024 09:27
@berndfinger berndfinger merged commit 4eb06b4 into sap-linuxlab:dev Jun 26, 2024
@rob0d rob0d deleted the swpm_observer_mode branch July 24, 2024 18:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants