Add support for Mariadb 11.x

[Marc-DRI]

Summary

Add support for Mariadb 11.x and use its mariadb and mariadbd binaries.

Additional Context

From Mariadb 11.0, mysql* names are deprecated in favor of mariadb* (cf. https://jira.mariadb.org/browse/MDEV-29582).
This pull request is:

• including commits from @OxCom, determining which binaries to use, depending on the mysqld_version facter
• fixing facters for mysql_version and mysqld_version whithin the MariaDB>11 context
• updating/adding tests for this new context

The initial PR #1626 causes an infinite loop in our case of fresh deployment, because facter is not properly initialized.

Related Issues

This pull request manages to fix #1580 .

[bastelfreak]

now we've two blocks for the same fact. Doesn't it make sense to unify it? What if mysql and mariadb is installed on the same server?

[neomilium]

Before MariaDB 11.0, using both mysql and mariadb was not supported by this module.
With this MariaDB 11.x support, it still dont work with both used at the same time but yes the facts collect can produce unexpected behavior.

As the two blocks contains de confine, I though that was the way to proceed.
https://www.puppet.com/docs/puppet/7/fact_overview.html#writing_facts_simple_resolutions-examples

Do you have some recommendation if you want to merge them?

[smortex]

This documentation feels weird. Inspecting various facts in the puppetlabs organization repositories, I don't see usage of confine with different prerequisites for the same facts, but a single fact with confinement that checks multiple conditions, e.g. https://github.com/puppetlabs/puppetlabs-java/blob/6319799bb310f824b18d9077905c46409535c977/lib/facter/java_default_home.rb#L16

It feels more natural for me to have a single Facter.add('mysql_version'), where the setcode logic attempts to get the expected value and return nil if none was found. A confinement does not even provide a short-circuit in this case, so we can probably go without it, i.e.

Facter.add('mysql_version') do
  setcode do
    mysql_ver = if Facter::Core::Execution.which('mysql')
                  Facter::Core::Execution.execute('mysql --version')
                elsif Facter::Core::Execution.which('mariadb')
                  Facter::Core::Execution.execute('mariadb --version')
                end
    mysql_ver.match(%r{\d+\.\d+\.\d+})[0] if mysql_ver
  end
end

So merging may make sense IMHO.

But beyond this consideration (and speaking as someone who does not use mysql nor mariadb), if mariadb and mysql are diverging one from the other and could be installed side by side on a single system, maybe it make sense to "fork" the puppetlabs-mysql module (puppet(labs)?-mariadb) just like mariadb forked mysql so that each module can track what the software they configure implement and provide a consistent interface and not some weak abstraction that may or may not work depending on the package you manage with the module?

[Marc-DRI]

Facters reworked and correctly working.
Thank you @smortex , code is much nicer !

[smortex]

Rubocop report some style issues, check the "Files changed" tab for details.

[Marc-DRI]

Should be better now...

[neomilium]

But beyond this consideration (and speaking as someone who does not use mysql nor mariadb), if mariadb and mysql are diverging one from the other and could be installed side by side on a single system, maybe it make sense to "fork" the puppetlabs-mysql module (puppet(labs)?-mariadb) just like mariadb forked mysql so that each module can track what the software they configure implement and provide a consistent interface and not some weak abstraction that may or may not work depending on the package you manage with the module?

@smortex I agree. It could be the right moment to split modules : mariadb and mysql.

@bastelfreak : What is your through about this split?

[neomilium]

Thinking it twice, there is no much interest to fork and no sufficient divergence to need a fork, so IMHO this PR could be accepted as is.

[smortex]

Looks good to me. I am not a member of this project so cannot trigger a CI run, @bastelfreak @alexjfisher can you please trigger it? Thanks!

[flokoe]

Any chance we can make progress on this PR. Is there anything missing?

It seems that just someone needs to accept and merge. The failing check for "mend" failed due to missing API key so nothing related to this PR

@bastelfreak maybe?

[bastelfreak]

Hey,
I'm currently waiting with most work on the Perforce owned modules until https://www.puppet.com/blog/open-source-puppet-updates-2025 is resolved.

[ekohl]

Is this really correct? With mysqld I'd have expected some server command

[ekohl]

You should probably use optional_commands to be explicit that they're not expected to be present (or the has_command definition together with is_optional).

[ekohl]

Also, I think we can assume Ruby 2.7 and use argument forwarding.

Once you've defined them as optional, I wonder if this works:

Suggested change

	def self.mysql_raw(*args)
	if newer_than('mariadb' => '11.0.0') && mysqld_version_string.scan(%r{mariadb}i)
	return mariadb_client(*args)
	end
	mysql_client(*args)
	def self.mysql_raw(...)
	command(:mariadb_client) ? mariadb_client(...) : mysql_client(...)

[ekohl]

I'm looking at this horrible hack above and wonder if it should use the proper API. Something like this probably works:

has_command(:mysql_client, 'mysql') do
  environment PATH: path, LD_LIBRARY_PATH: ld_library_path
end

You can also use is_optional according to https://github.com/puppetlabs/puppet/blob/e227c27540975c25aa22d533a52424a9d2fc886a/lib/puppet/provider.rb#L177-L214.