-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
YaST2 HA Setup for SAP Products - cannot input several instance numbe… #14
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
csync2
dependency and %post
?
@@ -47,6 +48,7 @@ Requires: rubygem(%{rb_default_ruby_abi}:cfa) | |||
Requires: sysvinit-tools | |||
|
|||
BuildRequires: augeas-lenses | |||
BuildRequires: csync2 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it really needed even for building the package? If it is need because of some unit test then you can mock the call...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You have right it is only needed in installed system
%post | ||
/usr/bin/systemctl enable csync2.socket | ||
/usr/bin/systemctl start csync2.socket | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if it makes sense to enable and start the service always. The usual approach is to enable the needed service when saving the configuration by the YaST module.
And IMHO this is also bad from the security POV, it runs the service whenever you install this YaST package, even if you possibly do not need that. IIRC only very few services are allowed to run by default, the security team might complain about this...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The situation is more complicated:
yast-sap-ha requires that csync2.socket is enabled on all nodes.
yast-sap-ha must be installed on 2 nodes but will be executed only on one node but it is required that the csync2.socket is enabled on both nodes.
In former SLES versions after installing csync2 csync2.socket was enabled. In SLES SP4 is no longer the case.
I don't know how else to solve this problem.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I see...
What about some small additional YaST module which would be run on the secondary node? It would just enable and start that service. Or just document this requirement?
Anyway, if there is no better solution then I think it needs to be consulted with the security team as they want to keep the list of default running services as small as possible. This might be a security problem...
This PR does not contain this change: https://github.com/yast/yast-sap-ha/pull/16/files#diff-18894a304ee05f252b1c8816fd8af7d95196ca7ecbfb86cb38fe8fcecdf62abdL244 Is that OK? |
You have right. I've removed it here also. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
✔️ Internal Jenkins job #2 successfully finished |
Problem
Valid SAP instance number is not recognized as valid.
The SAP instance number must contains 2 digits and it is in the range from 00 to 99.
Integer numbers starting with 0 are interpreted by Ruby as octal numbers. Therefore 08 and 09 are not recognized as valid.
csync2.socket is not enabled
Solution
Use a regex to validate the SAP instance number:
SAP_INST_NUM_REGEX = Regexp.new('^[0-9]{2}$')
Enable and start csync2.socket during installation of the package.