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

sap_general_preconfigure - Checking for english locales, issue 907 #914

Open
wants to merge 13 commits into
base: dev
Choose a base branch
from

Conversation

rhmk
Copy link
Member

@rhmk rhmk commented Dec 16, 2024

This pull request checks that English locales are installed and set as the default. Additionally, a new variable allows for the enforcement of a specific English locale. This pull request is compatible with RHEL and SLES.

Copy link
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!

success_msg: "PASS: An English locale is installed."

- name: Get the current default locale
ansible.builtin.command: awk '/^LANG=/&&/en_/{print}' /etc/locale.conf
Copy link

Choose a reason for hiding this comment

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

this does not take into account that C.UTF-8 is also english

Copy link
Member Author

@rhmk rhmk Dec 16, 2024

Choose a reason for hiding this comment

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

You are right. Although C is English it is not sufficient for HANA2 SPS08. The en_* locales have to be installed.
A test run showed that having the system locale C while having en_* installed was sufficient.
I am not sure how we can deal with it. For now I suggest accepting C and en_* locales, as long as en_* is installed. IIRC SAP recommends en_US.UTF-8, so if some body has a proof for that and put here or to the issue would be great. Then we could at least print a warning.

Copy link
Member

Choose a reason for hiding this comment

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

@rhmk I just installed HANA SPS08 (rev 81) on RHEL 9.4 using:

  • control node:

    • LANG set to en_US.UTF-8 in the shell which called the ansible-playbook command
  • managed node:

    • LANG="C.UTF-8" in /etc/locale.conf
    • package glibc-langpack-en being installed but not package langpacks-en. An attempt to remove the package glibc-langpack-en failed due to a dependency on the package grub2-tools-minimal.

Copy link
Member

Choose a reason for hiding this comment

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

suggested new awk statement:
awk '/^LANG=/&&(/C.UTF-8/||/en_/){print}' /etc/locale.conf

@rhmk rhmk linked an issue Dec 16, 2024 that may be closed by this pull request
@berndfinger berndfinger changed the title Issue 907 - Checking for english locales sap_swpm - Checking for english locales, issue 907 Dec 19, 2024
success_msg: "PASS: An English locale is installed."

- name: Get the current default locale
ansible.builtin.command: awk '/^LANG=/&&/en_/{print}' /etc/locale.conf
Copy link
Member

Choose a reason for hiding this comment

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

suggested new awk statement:
awk '/^LANG=/&&(/C.UTF-8/||/en_/){print}' /etc/locale.conf

Copy link
Member Author

Choose a reason for hiding this comment

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

Change made and also fixed an issue with the variable check to set the locale

@berndfinger berndfinger changed the title sap_swpm - Checking for english locales, issue 907 sap_general_preconfigure - Checking for english locales, issue 907 Dec 20, 2024
@rhmk rhmk requested review from berndfinger and Klaas- December 20, 2024 10:38
Copy link

@Klaas- Klaas- left a comment

Choose a reason for hiding this comment

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

I did not run it (on holidays already) -- but from reading the code this now looks good to me (except for the small c&p problem in the example definition)

description:
- Use this variable to specify the default system locale.
example:
sap_general_preconfigure_db_group_name: 'en_US.UTF-8'
Copy link

Choose a reason for hiding this comment

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

I am guessing you mean:
sap_general_preconfigure_default_locale: 'en_US.UTF-8' -- or C.UTF-8 depending what you think should be the "default" recommendation :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks @Klaas- fixed this one, in addition @berndfinger and I found a bug in the algorithm so that this was variable was never used in certain circumstances (non-English locale set as default would always stop although variable was set)
This variable above is just an example, per default nothing is changed (variable empty in defaults/main.yml)

Copy link
Member Author

Choose a reason for hiding this comment

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

I also removed the code from being executed on SLES because the reviewers are not available for the next weeks -- We may need to open another PR to enable this on SLES

Copy link
Member Author

@rhmk rhmk left a comment

Choose a reason for hiding this comment

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

Line 31 needs improvement UTF-8 and utf8 can be used synonymously but locale -a always lists utf8

fail_msg: "FAIL: No English locale is installed. Please install an English locale!"
success_msg: "PASS: An English locale is installed."

- name: Configure English locale
Copy link
Member

Choose a reason for hiding this comment

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

I tested and now suggest to use the following code (including the comments to explain the logic a bit):

# We are setting a new locale from sap_general_preconfigure_default_locale if:
# - the new locale starts with "en_" or if it is "C.UTF-8" AND
# - the new locale (replacing "UTF-8" by "utf8" if necessary) is part of the installed locales
    - name: Configure English locale
      when:
        - sap_general_preconfigure_default_locale is defined
        - sap_general_preconfigure_default_locale | length > 0
        - sap_general_preconfigure_default_locale.startswith('en_') or sap_general_preconfigure_default_locale == 'C.UTF-8'
        - __sap_general_preconfigure_locales_installed.stdout_lines | select('match', sap_general_preconfigure_default_locale | regex_replace('UTF-8', 'utf8')) | join('') | trim | length > 0

It allows for setting an English locale using either the UTF-8 or utf8 notation (the output of locale -a contains only utf8 on RHEL 8 and RHEL 9) and also avoids setting an unsupported locale.

If there is an SAP supported OS which has UTF-8 in the output of locale -a, we could of course add the regex_replace filter also after __sap_general_preconfigure_locales_installed.stdout_lines.

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.

sap_general_preconfigure: SAP Note 2369910 not correctly implemented
3 participants