Supported database versions and integration testing

[steveteahan]

SUMMARY

I'm creating this issue hoping to start a discussion about which database server/versions that this collection supports and how that affects integration testing. Based on the code, it seems that the intent is to support MySQL <5.7, MySQL 5.7, MySQL 8.0, and MariaDB. The integration tests only execute on MySQL 5.7 and 8.0, however, which limits the guarantees of compatibility for all other servers and versions.

I had some follow-up questions that the maintainers may or may not find are worth discussing:

• Would it be worth explicitly documenting the database servers and versions that the collection supports?
• If the above makes sense, should there be a standard for dropping support? For instance, support will be dropped for a particular version after the end of support date? A 3-6 month buffer could seem reasonable too? This could also provide opportunities to remove old code paths.
• Should all the supported versions be included in integration tests? Or are there specific limitations that are preventing doing so?

ISSUE TYPE

• Feature Idea

COMPONENT NAME

N/A

ADDITIONAL INFORMATION

It is worth mentioning that it looks like end of support for MySQL 5.6 appears to be quickly approaching, so I can see that it might not be worth including that version in integration tests now. This isn't the case for MariaDB, however.

Some examples where coverage is being missed:

• https://codecov.io/gh/ansible-collections/community.mysql/src/main/plugins/modules/mysql_user.py#L362
• https://codecov.io/gh/ansible-collections/community.mysql/src/main/plugins/modules/mysql_user.py#L504
• https://codecov.io/gh/ansible-collections/community.mysql/src/main/plugins/modules/mysql_user.py#L600
• https://codecov.io/gh/ansible-collections/community.mysql/src/main/plugins/modules/mysql_user.py#L719
• https://codecov.io/gh/ansible-collections/community.mysql/src/main/plugins/modules/mysql_user.py#L911

[Jorge-Rodriguez]

@steveteahan I very much she with you, and it's true that we're not testing against MariaDB and we should.
The reason for this is that, AFAIK we do not have a MariaDB container setup to test against. Currently the tests run against a Percona cluster.

If someone could put the work into setting up such a container (I believe there work would go into the setup.yml file of the integration tests) we could start testing against MariaDB.

[steveteahan]

Thank you for the information, @Jorge-Rodriguez. I don't have a vested interest in MariaDB support, but some of the changes that I'm making inevitably hit a MariaDB conditional which then affects the coverage of the patch. I do have an interest in making sure that my changes don't break any MariaDB users if the intent is to continue to support that database.

Any thoughts on publishing a list of supported database versions? MySQL >= 5.7 would seem to make sense given that 5.6 is end-of-life next month. It looks like MariaDB >= 10.2 are the versions of that server that aren't out of support.

I can spend a few hours seeing if I can get MariaDB integration tests working. I do see a variable related to testing MariaDB that at least is properly installing it from what I can see. The tests fail pretty early in the process, however.

[steveteahan]

Enabling integration tests for MariaDB looks like it will be a bit more of a time investment than I can make at the moment. There were some initial hurdles to being able to enable the tests:

• MariaDB 10.2 and 10.5 don't enable ssl by default, so there is some extra work to generate certificates
• openssl s_client doesn't appear to return the full certificate chain for the self-signed configuration on MariaDB (broke some tests that relied on this)
• MariaDB deviates from the MySQL plugin authentication in that you can provide IDENTIFIED WITH mysql_native_password, but the output from SHOW CREATE USER will be IDENTIFIED BY PASSWORD.

I'm making those fixes available here in case it'll save anyone else time who is able to run with this: steveteahan@0af1d2b. There are a couple of changes in there that are not intended to actually be carried over to the main branch, but are there for demonstration purposes.

After fixing those I also hit these issues:

• MariaDB is always detecting a change during the test_mysql_user : Update the user with the same hash (no change expected) test. As a whole, it seems like there are some notable differences in how MariaDB handles plugin auth compared to MySQL.
• CREATE USER statements are failing on MariaDB 10.5 with: 1449, \"The user specified as a definer ('mariadb.sys'@'localhost') does not exist\". It looks like MariaDB renamed the mysql.user table to mysql.global_priv.

My guess is that these aren't the only issues. That is simply as far as I was able to get on the mysql_user side of things.

The growing differences between MySQL and MariaDB look like it should make things interesting moving forward. I see that there is also now a MariaDB connector. I'd imagine that to provide good MariaDB support going forward the project would need:

• Integration tests running on MariaDB 10.2 and 10.5 (oldest + latest) at a minimum
• Add mariadb-connector-python to the matrix of connectors
• Refactoring to move more of the logic that handles generating the SQL statements into separate classes for MySQL and MariaDB. Without this things might get messy with even more MariaDB branching throughout the code.

I think it does lead to the question of at what point does it make sense for MariaDB to be its own Ansible collection? There are certainly some downsides to this such as potentially fragmenting the community and not benefiting from where they are still similar. The benefits of splitting them apart might include less churn trying to support both server flavors. I could see some future contributors avoiding making a change that is more complicated that it needs to be for this reason.

I'm not an expert on the differences between MySQL and MariaDB, so I'll defer to others for additional feedback on this. I figure that it is worth discussing in any case. From what I've seen there are at least a couple of bugs that exist today due to the lack of MariaDB integration tests.

[steveteahan]

For completeness, I quickly checked to see if enabling integration tests for MySQL 5.6 would be trivial. I quickly hit a number of issues just in the mysql_user module:

• MySQL 5.6 doesn't enable SSL by default (like MariaDB, so can reuse some of the changes there)
• SQL queries being tested in resource_limits.yml aren't supported
• Passwords >16 characters aren't supported

Given this initial list, and the assumption that there are likely to be many other issues, I'm thinking that it doesn't make sense to put any effort towards enabling integration tests for 5.6. The end of support for that version is two weeks away. Perhaps it would make the most sense to drop support for 5.6 at that point?

[Andersson007]

@Jorge-Rodriguez @bmalynovytch what do you think about dropping support for 5.6?
If we decide that, I'll announce this fact somehow.

[bmalynovytch]

Hello there !

Thank you for your time dudes.
This thread is the Pandora Box I never wanted to open while still knowing we should have way earlier.
I struggled with the same problems of compatibility years ago.

While I think we should restrict supported environments to limit complexity of code and tests, we can't, suddenly, drop features IMOH.
I checked about MySQL 5.6 EOL (which I wasn't aware of) and found that Percona is offering 3 more years of support on MySQL 5.6. If they do so, I guess it's because there is demand.
I also know there are hundreds (probably thousands) of servers around the world still using 5.6, as it was one of the most used in the past 10 years.

IMOH, the best would be to rework the entire set of modules to better handle all "supported versions", splitting the code and the tests depending on the target db server. When we started to split cases, a couple of years ago, there were only a few differences to handle. Nowadays, there seem to be around 50% of the whole codebase that would need to be adapted to the whole set of supported versions.
This means a lot of work, as well as a good coordination with the developers of the modules to handle all cases depending on their own knowledge.

[steveteahan]

While I think we should restrict supported environments to limit complexity of code and tests, we can't, suddenly, drop features IMOH.
I checked about MySQL 5.6 EOL (which I wasn't aware of) and found that Percona is offering 3 more years of support on MySQL 5.6. If they do so, I guess it's because there is demand.
I also know there are hundreds (probably thousands) of servers around the world still using 5.6, as it was one of the most used in the past 10 years.

Thank you for the additional background information, @bmalynovytch. I didn't know that Percona was extending the support for 5.6. I wasn't necessarily suggesting just dropping support 5.6 immediately (although it may have sounded that way), but there would need to be some documented procedure for doing so? I could see that documented procedure covering the fact that Percona provides extended support beyond standard MySQL and should continue to be supported.

IMOH, the best would be to rework the entire set of modules to better handle all "supported versions", splitting the code and the tests depending on the target db server. When we started to split cases, a couple of years ago, there were only a few differences to handle. Nowadays, there seem to be around 50% of the whole codebase that would need to be adapted to the whole set of supported versions.
This means a lot of work, as well as a good coordination with the developers of the modules to handle all cases depending on their own knowledge.

I guess that brings up my other question which was at what point does it make sense to have community.mariadb? That would be another way of achieving the splitting of the code and tests. Both of you have a much better idea of the implications of this than I do.

[bmalynovytch]

I wasn't necessarily suggesting just dropping support 5.6 immediately

Not a problem, we absolutely need to stop doing just like if everything was fine, it's not the case.
We're having a hard time maintaining those modules because of the legacy 🙂

I guess that brings up my other question which was at what point does it make sense to have community.mariadb ?

I'm not sure splitting the modules would be the best, because MySQL and MariaDB still share a lot of common bits and have a general interoperability. I guess users would expect that mysql and mariadb to be equivalent.

That's my own opinion, not what I expect our working group to do without prior discussion.

[Jorge-Rodriguez]

I'm not sure splitting the modules would be the best, because MySQL and MariaDB still share a lot of common bits and have a general interoperability. I guess users would expect that mysql and mariadb to be equivalent.

@bmalynovytch @Andersson007 do you think it would make sense to make the split suggested by @steveteahan transparent within the collection? We could refactor to have two separate codebases and determine early in the module execution what server we're targeting, thus selecting an early execution branch for MySQL or MariaDB.
This would keep the codebase free of endless conditionals while retaining the logic within the collection. Also we'd get rid of the conditionals on the tests because the check and decision would be made by the module's logic.

[bmalynovytch]

We could refactor to have two separate codebases and determine early in the module execution what server we're targeting, thus selecting an early execution branch for MySQL or MariaDB.

It’s probably a good idea. The main inconvenient I see is the risk of code duplication within the different parts servings equivalent functions.

[Jorge-Rodriguez]

We could refactor to have two separate codebases and determine early in the module execution what server we're targeting, thus selecting an early execution branch for MySQL or MariaDB.

It’s probably a good idea. The main inconvenient I see is the risk of code duplication within the different parts servings equivalent functions.

I suppose the risk is inherent when dealing with this kind of pattern. We could deal with that in CI by running a code examination tool that would detect such duplication and alert accordingly.

In the end there would be three different parts, MySQL, MariaDB and commons shared by both to reduce duplication.