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_ha_pacemaker_cluster: Allow to append manual cluster config to the generated config #922

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

rob0d
Copy link
Contributor

@rob0d rob0d commented Jan 3, 2025

@marcelmamula @ja9fuchs
This is based on the discussion we had several months ago with @ja9fuchs.
This role generally respects pre-defined ha_cluster* variables and tries to merge the configuration it generates with the content of the pre-defined variables. It works really well, however, in some cases the order of resources is important.
The new variables allow to append a specific cluster configuration AFTER the configuration generated by this role.
I've added information to the README file which explains the behaviour. It's a non-breaking change as it just enhances the functionality without changing anything else.

@marcelmamula
Copy link
Contributor

@rob0d
I think you need to provide example of how current code affects output, because appending post fact results in advanced mode, because you 100% trust that whoever uses it knows everything: dict format of ha_cluster variable.


- _Type:_ `list`

This role generally respects pre-defined ha_cluster* variables and tries to merge the configuration it generates with the content of the pre-defined variables. <br>
Copy link
Contributor

Choose a reason for hiding this comment

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

@rob0d These sentences are bit on the nose, because it contains too many negatives as it tells me: "Hey, this role barely works"

It has to be reworked if PR is to be accepted.

@ja9fuchs
Copy link
Contributor

ja9fuchs commented Jan 7, 2025

This specific role has certain limits and will not be able to cover all custom cases.
The use case in question is of such specific nature, that it should rather be covered by making further advanced use of the ha_cluster role native parameters. If the order of resources is critical, then the source definition can be pre-defined including the resources which the role would add - in the desired order. The SAP HA role checks for duplicate resource IDs (names) and in the cases of presumably unique resources it also skips if the resource agent is already present in the resource primitives parameter. The user defined primitive content should always take precedence.

As @marcelmamula already mentioned, just appending the variables in this role is not good enough due to the vars having to follow the correct syntax. It is not safe to do this without quite some effort of syntax validation and such logic, to make sure that what will be fed into ha_cluster is sane input.

Please share more details of your particular use case, so we could assess if it would justify putting effort into this enhancement.

Note: The variables section in the README is generated from meta/argument_specs.yml and vars content manually added to the README file will be lost later if not added there accordingly. I will follow up on highlighting this. You can't know if we don't tell. :)

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