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

[TEAM-9968] Cluster deployment SAPHanaSR-angi package in Public Clouds #307

Merged
merged 17 commits into from
Jan 21, 2025

Conversation

jankohoutek
Copy link
Contributor

@jankohoutek jankohoutek commented Jan 17, 2025

TEAM-9968
SAPHanaSR-angi package cluster Public Cloud deployment

Verifiction runs:

@@ -40,6 +43,11 @@
- name: Load SAP HANA variables
ansible.builtin.include_vars: ./vars/hana_vars.yaml

- name: Re-set HANA Angi vars
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this refer to line 24, right? Var name is the same but value has to be different in case of Angi? Do I correctly understand it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes

Copy link
Collaborator

Choose a reason for hiding this comment

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

In this case, in a next PR, with the purpose of having one less task, we can try "conditional var definition".
Directly in var: section, on line 24

ms_saphanactl: "{% if use_hana_sr_angi | bool %}mst_SAPHanaCtl_{{ sap_hana_install_sid }}_HDB{{ sap_hana_install_instance_number }}{% else %}msl_SAPHanaCtl_{{ sap_hana_install_sid }}_HDB{{ sap_hana_install_instance_number }}{% endif %}"

@@ -36,7 +43,7 @@
state: present
replacefiles: true
when:
- ansible_facts['distribution_version'] == '15.1'
- ansible_facts['distribution_version'] == '15.1' and not use_hana_sr_angi | bool
Copy link
Collaborator

@mpagot mpagot Jan 17, 2025

Choose a reason for hiding this comment

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

Do we plan to have Angi ported to 15.1? If not we can remove this and and add a brand new assert task like (pseudocode):

died if use_hana_sr_angi and ansible_facts['distribution_version'] == '15.1'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just sanitation.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we plan to have Angi ported to 15.1?

No. Product is already out of support, so it will not get these packages.

Comment on lines +59 to +75
- name: Ensure SAPHanaSR hooks directory exists
ansible.builtin.file:
path: "{{ __hooks_dir }}"
owner: "{{ sap_hana_install_sid | lower }}adm"
group: sapsys
state: directory
mode: '0775'
when: not use_hana_sr_angi | bool

- name: Ensure SAPHanaSR-angi hooks directory exists
ansible.builtin.file:
path: "{{ __hooks_dir_angi }}"
owner: "{{ sap_hana_install_sid | lower }}adm"
group: sapsys
state: directory
mode: '0775'
when: use_hana_sr_angi | bool
Copy link
Collaborator

Choose a reason for hiding this comment

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

is path the only parameter changing between the two?

ansible.builtin.command:
cmd: crm configure property maintenance-mode=true
when: crm_maintainence_mode is false or crm_maintainence_mode == 'unknown'
cmd: crm maintenance on
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems an important change not related to Angi. Isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is correctly set of the maintenance mode, not forcing it to the configuratin file.

Copy link
Contributor Author

@jankohoutek jankohoutek Jan 17, 2025

Choose a reason for hiding this comment

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

And the correct value of maintenance-mode in configuration was changing along the versions of crm it's once 'on/off' then 'true/false' etc. This way is much safer.

Copy link
Collaborator

Choose a reason for hiding this comment

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

seems so important that can be in its own PR

cmd: crm maintenance off
when: refreshed_crm_maintenance_mode is true or refreshed_crm_maintenance_mode == 'unknown'

# For debug purpose only
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we like to keep it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

... and after changes are done, we shoud start cluster back to normal

Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it name: Get cluster status at the end a debug task?

@@ -23,21 +25,10 @@
migration_threshold: "{{ (crm_conf_show.stdout | regex_search('migration-threshold=([0-9]*)', '\\1'))[0] }}"
changed_when: false

- name: Create HANA topology resource
- name: Ensure maintenance mode is active
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one looks like important and not related to Angi.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Topology is still there, just diff is confused.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Seems like change is:

  • call one more crm maintenance on that was missing
  • postpone Create HANA topology resource task execution 4 tasks later

when: cluster_order | length == 0

- name: Ensure maintenance mode is off
Copy link
Collaborator

Choose a reason for hiding this comment

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

Important change not related to Angi

Copy link
Collaborator

@alvarocarvajald alvarocarvajald 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
Collaborator

@mpagot mpagot left a comment

Choose a reason for hiding this comment

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

LGTM

@mpagot mpagot merged commit f09aa15 into SUSE:main Jan 21, 2025
1 check passed
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.

3 participants