Add actor for mysql migration RHEL9->RHEL10

[SlouchyButton]

Basic actor that just checks that the mysql-server package is installed and inform user if so.

There are incompatibilities, so there is an article attached describing the process (the article is WIP)

[github-actions]

Thank you for contributing to the Leapp project!

Please note that every PR needs to comply with the Leapp Guidelines and must pass all tests in order to be mergeable.
If you want to request a review or rebuild a package in copr, you can use following commands as a comment:

• review please @oamg/developers to notify leapp developers of the review request
• /packit copr-build to submit a public copr build using packit

Packit will automatically schedule regression tests for this PR's build and latest upstream leapp build.
However, here are additional useful commands for packit:

• /packit test to re-run manually the default tests
• /packit retest-failed to re-run failed tests manually
• /packit test oamg/leapp#42 to run tests with leapp builds for the leapp PR#42 (default is latest upstream - main - build)

Note that first time contributors cannot run tests automatically - they need to be started by a reviewer.

It is possible to schedule specific on-demand tests as well. Currently 2 test sets are supported, beaker-minimal and kernel-rt, both can be used to be run on all upgrade paths or just a couple of specific ones.
To launch on-demand tests with packit:

• /packit test --labels kernel-rt to schedule kernel-rt tests set for all upgrade paths
• /packit test --labels beaker-minimal-8.10to9.4,kernel-rt-8.10to9.4 to schedule kernel-rt and beaker-minimal test sets for 8.10->9.4 upgrade path

See other labels for particular jobs defined in the .packit.yaml file.

Please open ticket in case you experience technical problem with the CI. (RH internal only)

Note: In case there are problems with tests not being triggered automatically on new PR/commit or pending for a long time, please contact leapp-infra.

[SlouchyButton]

/packit copr-build

[SlouchyButton]

Added configuration checks, so the user is informed that current MySQL configuration won't be compatible with newer MySQL present in RHEL 10.

When incompatible config is detected, severity is also raised to HIGH, as proceeding will result in mysqld failing to launch until issues with config are resolved.

[pirat89]

@SlouchyButton hi \o thanks for the contribution! regarding the severity, if I understand it right, the worst scenario here is that mysqldb will not be launched on the new system - will not be working - until they fix the configuration. fro mthis point I do not think we should use high severity.

• High - Very likely to result in a deteriorated system state.
• Medium - Can impact both the system and applications.
• Low - Should not impact the system but can have an impact on applications.
• Info - Informational with no expected impact to the system or applications.

[SlouchyButton]

Hi @pirat89
Thank you, taking a look at this. I was originally planning on requesting a review today anyway.
Regarding the severity, what u describe makes sense to me. I'll lower the severity - keep the severity same for all cases detected by the actor.

I also have another question regarding tests. Are they required? There is a small sanity test similar to other our database components, which only tests the detection of the package on the system.

Testing other parts of the actor feels a bit problematic to me, as it is starting new commands with subprocess, or reading files (for configuration). Is it mandatory to somehow test even this functionality, and if so what would be your proposed approach for this?

Thank you!

[pirat89]

@SlouchyButton I made just a short overview the last time, we will try to do proper review the next week. Regarding the subprocess, use the run function instead. See my comment above. Regarding unit tests. Mocking run is usually simple (just set the function to return an output you would expect to obtain - e.g. from a file). Similar for the reading of files. Similar can be done for the open builtin function. We can help you with that if you want.

Of course, when some unit-tests are pretty tricky to write and their value would not be high, it is ok to mention it in a comment (in the test script file) for others and keep the testing on integration tests, etc. The full coverage is not required. Also, in such a case, you could still add files with example outputs under tests/files/ directory so people can see some examples of outputs at least. And it could help them if they would like to create some tests based on these IO data.

I will cover such tips into the guidelines/tutorials we are writting in these days.

Update: but I would suggest anyway to postpone writing of another unit-tests until someone do a better review - so they will not need to be rewritten in case of bigger changes.

[SlouchyButton]

Regarding the subprocess, use the run function instead. See my comment above.

Oh, I must have missed it in the docs, will change this.

I will cover such tips into the guidelines/tutorials we are writing in these days.

That would be great, the documentation/guidelines are great, but they sometimes do feel like they miss some edge cases. Originally I was wanting to do multiple reports but eventually figured you can't do that from one actor and can't have more actors in one folder. I think this isn't explicitly mentioned so maybe some guidelines (best practices) how to report multiple things relating to one task (package upgrade) might help others.

Update: but I would suggest anyway to postpone writing of another unit-tests until someone do a better review - so they will not need to be rewritten in case of bigger changes.

Right that's a good point.

[pirat89]

Regarding the subprocess, use the run function instead. See my comment above.

Oh, I must have missed it in the docs, will change this.

I will cover such tips into the guidelines/tutorials we are writing in these days.

That would be great, the documentation/guidelines are great, but they sometimes do feel like they miss some edge cases. Originally I was wanting to do multiple reports but eventually figured you can't do that from one actor and can't have more actors in one folder. I think this isn't explicitly mentioned so maybe some guidelines (best practices) how to report multiple things relating to one task (package upgrade) might help others.

It's possible we haven't mentioned that in guidelines earlier. I will make a note to ensure we will put it into the new guidelines we are working on. The reason for the requirement to use run instead of subprocess is that it allows us to investigate what happened etc as we have data about each execution (stdout, stderr, exit code, command, ....) stored for the investigation and it's literally life saver. Also we can simply check all commands executed by any actors.

Update: but I would suggest anyway to postpone writing of another unit-tests until someone do a better review - so they will not need to be rewritten in case of bigger changes.

Right that's a good point.

well, you can create e.g. mysql directory and create several actors inside (each one in a separate dir). let's maybe rather sync the next week to tell us what everything you would like to do, so we can guide you what is the best way to do it :)

Also note, that inside the ChecksPhase actors must not interact with the system in any way - so you cannot call any subprocess, read files, etc. You can work only with information inside leapp messages created by other actors. going through the code, I see there multiple things that will need to be changed. so I suggest to sync during the next week to discuss it. I will ensure that this is also part of guidelines if it's not there already.

[pirat89]

This will need several changes. Also, checking the current solution, I think that it should be pretty simple to cover the complete functionality by unit-tests. let's sync about it yet. I skipped review of current unit-tests, due to number of changes that will be done in the code anyway.

[pirat89]

Using \n in the text is not acturally so good as the formatting could look weird in various webUIs. Also the Warning line is not good I would omit it. Basically the hole report is kind of the warning. Now you need to just provide information:

• What is the problem in genera?
• What does it mean for readers? What do you want them to understand?
• What specifically we discovered that should concern the user? (the list of problematic entities)

In the remediation instructions:

• What should I do?
• Can resolve it before the upgrade?
• Do I have to do something after the upgrade?
• .....

Note that current summary could be confusing for the user as whatever they do, they will see this msg but remediation instructions says they have to deal with it after the upgrade (and the preupgrade part suggest only the backup of data - btw, do you mean actually dump the database?).

[pirat89]

btw, update -> upgrade base on RHEL docs:

• update RPMs using dnf update - staying on same RHEL X major version
• upgrade -> upgrading to the next system major version

[SlouchyButton]

\n resolved (removed where not necessary).

Wording might still be improved on.

[SlouchyButton]

Most of the changes should be hopefully done. Missing more thorough description of actors and tests.

[SlouchyButton]

All tests should be present. run is being mocked as it requires mysqld present and when you run run it will want to log even during tests, which is not possible and crashes.

open is being mocked via passing a different path to check with a fake service override file present in the test folder. Idk if the approach I chose is valid or not, so please lmk if change is needed.

[SlouchyButton]

With this, all points from review should be hopefully resolved 🤞, so I think this is ready for a second round of review.