sap-linuxlab / community.sles-for-sap Goto Github PK

The regex test for hana instance numbers should match two numerical characters only, but the current regex test will match any string made up two or more numerical characters.

To fix this the current test ansible.builtin.regex('[0-9]{2}') can be replaced with ansible.builtin.regex('^[0-9]{2}$')

If password-free root login is not configured the role will pass its pre-checks but fail when trying to copy the KEY and DATA files from primary to secondary.

Adding a pre-check for the ssh will prevent this from happening.

saptune and cluster packages are not available in SLES for SAP making some roles incompatible. If standard SLES is used, a clear message should let the user know this!

Currently, no roles check the underling OS, it is assumed that SLES for SAP is being used.

However, some roles will work on any Linux (sap_storage) whilst other will work on any SLES (sles_register).

The roles that are only expected to work on SLES for SAP (cluster, hana-system-replication), should check that the OS is SLES for SAP in the pre-checks.

The hana_install role will alway unpack the HANA sar file if an install is needed. It does not check if the HANA file is already unpacked in the required location.

This causes the sar to be needlessly unpacked.

Currently, some task expose secrets.

All tasks that use secrets should use the no_log feature to ensure secrets are not exposed.

roles/sles_register/tasks/tasks.yml is configured with no_log: true. It is perfectly ok. When a failure happens it could be useful to see exit code of the executed command, what usually is showed in the Ansible log under rc. Possible stderr could be useful too but I'm not sure if it is going to leak the registration code.

The cluster role should allow the cluster configuration to be updated.

At present, cluster configuration is a one-shot affair.

Roles should use saptune 3.1, enabling JSON support.

The saptune role should validate available solution using the JSON output of saptune.

Some tasks, such as checking the validity of a HANA SIDs are repeated across various roles.

For efficiency common tasks should be created as dedicated task lists within a shared role.

Ensure that System Replication Hooks are correctly loaded using the HANA System View M_HA_DR_PROVIDERS rather than searching log files.

The hana install role has a number of post tasks that check various ports to ensure that HANA is up as expected. When HANA is configured as a HANA System Replication target, these ports are closed and the task fails.

The post tasks should check if the installed HANA system is configured as a HANA system replication target and skip the port test targets if it is.

The documentation needs to include:

Ansible and Ansible core versions.
jmespath required
Inventory expectations

Some role names have words separated by underbars '_' whilst others separate words with hyphens '-'.

A consistent naming approach should be used.

I'm creating a deployment with playbook using cluster role from this repo: SUSE/qe-sap-deployment#202

I noticed that, if I forgot to provide aws_region, the role fails as the variable is needed by task Ensure config file has cluster block.

That's fine but:

• it is not properly documented in the readme, in the Required variables for AWS section of

  community.sles-for-sap/roles/cluster/README.md

  Line 144 in 00c5182

  ### Required Variables for AWS

• it is not validated by task Ensure that AWS specific variables are supplied

HAWK is not started by default but should be.

Roles, such as backup and system replication need HANA to be up in order to work.

The pre-checks should check that HANA is up before moving on to tasks.

If HANA is not up, the user should be informed with a useful message.

It may be useful to have an option to bring HANA up if necessary but this should not be the default.

HANA System Replication role should support removing an old configuration and replacing it with a new one when HANA system replication is configured but it does not match the desired configuration.