Templating: Only use wsrep_provider_options when galera_extra_wsrep_p…

[andanotheruser]

…rovider_options is defined

Description

Only use wsrep_provider_options when galera_extra_wsrep_provider_options is defined :)

Related Issue

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Chore (non-breaking change that does not add functionality or fix an issue)

Checklist:

• I have read the CONTRIBUTING document.
• I have run the pre-merge tests locally and they pass.
• I have updated the documentation accordingly. (No change required)
• I have added tests to cover my changes.
• All new and existing tests passed.

[eRadical]

LGTM - @elcomtik thoughts?

[elcomtik]

I think it brings no improvement, this variable is defined in defaults, so this condition will be always evaluated to true.

[andanotheruser]

defined in defaults, so this condition will be always evaluated to true.

Except if you don't use the defaults and define something else.

[elcomtik]

How is it possible to undefine defaults?

[andanotheruser]

How is it possible to undefine defaults?

You'd just set a different value for the play. For example as part of host vars or groups vars or for the playbook itself.

Those have higher precedence: https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html#variable-precedence-where-should-i-put-a-variable

Defaults are just that, after all: Defaults. Sometimes there's a need to differ from the defaults. :D

[elcomtik]

Ofc I didn't mean this, however that you added condition which checks if variable is defined. But I'm not aware you can undefine variable by hostvars, grouvars or extra vars

[eRadical]

I now see some things in here:

• galera_extra_wsrep_provider_options is anyway populated in setup_cluster.yml
• this is already in templates/etc/mysql/conf.d/galera.cnf.temp.j2 - @andanotheruser is just bringing this is the non-temp file but
• this is present in another 2 files and if we decide on this we should add to all

Note (learned the hard way): wsrep_provider_options is a special case of setting. You cannot set it multiple times and it ads up... we need to set this in one line only.

[eRadical]

• galera_extra_wsrep_provider_options

Yeah... I think the check should be on it having at least 1 element.

[andanotheruser]

• @andanotheruser is just bringing this is the non-temp file but

This line is a bit broken. What do you want to say?

[eRadical]

@andanotheruser - read the last 2 bullet points in one sentence

[eRadical]

I was just saying that if we change, we should change it in everywhere.

[eRadical]

But to go back to @elcomtik points... in defaults we define an empty galera_extra_wsrep_provider_options then in setup_cluster.yml we add things in there anyway.

If we want to add your patch we need to make sure every set_fact from https://github.com/mrlesmithjr/ansible-mariadb-galera-cluster/blob/master/tasks/setup_cluster.yml#L44-L78 have a when: clause. Currently the 1st does not. And if we add stuff in the future we need to make sure we add a when clause.

[andanotheruser]

If we want to add your patch we need to make sure every set_fact from https://github.com/mrlesmithjr/ansible-mariadb-galera-cluster/blob/master/tasks/setup_cluster.yml#L44-L78 have a when: clause. Currently the 1st does not. And if we add stuff in the future we need to make sure we add a when clause.

What sort of condition would you like to see in there?