Skip to content
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

Fix validating SAP instance number #16

Merged
merged 14 commits into from
Jan 12, 2023
Merged

Fix validating SAP instance number #16

merged 14 commits into from
Jan 12, 2023

Conversation

varkoly
Copy link
Collaborator

@varkoly varkoly commented Sep 9, 2022

Problem

Valid SAP instance number is not recognized as valid.

  • https://bugzilla.suse.com/show_bug.cgi?id=1202979
    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.

yast2-sap-ha: S:M:24423:276258: csync2 configuration not enabled

Solution

Use a regex to validate the SAP instance number:
SAP_INST_NUM_REGEX = Regexp.new('^[0-9]{2}$')

During the installation csync2.socket will be started and enabled in %post.
It is not enough to enable/start csync2 in code. yast2-sap-ha will be only installed on the additional nodes, but will not be started.

@coveralls
Copy link

coveralls commented Sep 9, 2022

Pull Request Test Coverage Report for Build 3800133921

  • 40 of 144 (27.78%) changed or added relevant lines in 5 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.2%) to 29.477%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/lib/sap_ha/system/watchdog.rb 0 1 0.0%
src/lib/sap_ha/semantic_checks.rb 35 38 92.11%
src/lib/sap_ha/system/local.rb 1 16 6.25%
src/lib/sap_ha/system/hana.rb 4 27 14.81%
src/clients/sap_ha.rb 0 62 0.0%
Files with Coverage Reduction New Missed Lines %
src/clients/sap_ha.rb 1 0%
Totals Coverage Status
Change from base Build 3015065937: 0.2%
Covered Lines: 1228
Relevant Lines: 4166

💛 - Coveralls

Copy link
Member

@lslezak lslezak left a comment

Choose a reason for hiding this comment

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

LGTM

@varkoly varkoly merged commit 10664ff into yast:master Jan 12, 2023
@yast-bot
Copy link

❌ Internal Jenkins job #2 failed

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