[DPE-9669] Test full refresh workflow including router

[astrojuanlu]

The tests themselves aren't difficult, but I've hesitated a lot on what to consolidate and refactor from other tests. There's already a lot of duplicity. I've attempted different things and walked them back because they ended up making the tests more obscure or more complex... Happy to take suggestions.

In particular I've decided to have 2 test files, even if they are almost identical. Having 1 test file to test 2 full refresh workflows involves resetting the model mid-test, which we don't do in other places. Parametrizing the test doesn't help, I think (we still need the test_deploy_.... And I decided against adding these to the existing test_upgrade.py as well, since the paths are different.

I've also decided to add a helper to refresh MySQL server - but not to make it more generic than that. The reason is that machines has some extra try...except that is not there in Kubernetes, and also that refreshing the single router unit seems to be simpler.

Finally, this also makes a minor update to the machines/poetry.lock, which was out of date.

Checklist

• I have added or updated any relevant documentation.
• I have cleaned any remaining cloud resources from my accounts.

[paulomach]

The tests themselves aren't difficult, but I've hesitated a lot on what to consolidate and refactor from other tests. There's already a lot of duplicity. I've attempted different things and walked them back because they ended up making the tests more obscure or more complex... Happy to take suggestions.

In particular I've decided to have 2 test files, even if they are almost identical. Having 1 test file to test 2 full refresh workflows involves resetting the model mid-test, which we don't do in other places. Parametrizing the test doesn't help, I think (we still need the test_deploy_.... And I decided against adding these to the existing test_upgrade.py as well, since the paths are different.

I've also decided to add a helper to refresh MySQL server - but not to make it more generic than that. The reason is that machines has some extra try...except that is not there in Kubernetes, and also that refreshing the single router unit seems to be simpler.

Finally, this also makes a minor update to the machines/poetry.lock, which was out of date.

Checklist

* [ ]  I have added or updated any relevant documentation.

* [ ]  I have cleaned any remaining cloud resources from my accounts.

Thanks for the explainer, it helps a lot.
The rational seems reasonable.

[paulomach]

Looking good - failures are unrelated and I like the naming for new files/tests.

[Soundarya03]

Looks good!

[sinclert-canonical]

Looks good overall! One question though:

We already deploy MySQL Router and MySQL Test app on the vanilla upgrade test. AFAIK, deploying MySQL Router to just relate it to MySQL Server and do nothing to it does not serve any purpose. Furthermore, it is a very naive action compared to the two scenarios been introduced here.

Am I to understand that we can now simplify the vanilla upgrade tests?

[sinclert-canonical]

Either we point at 8.4/edge like the rest of the tests, or 8.4/stable now that we promoted it.

[astrojuanlu]

I now forgot why I had to use candidate... Will bump to stable.

[sinclert-canonical]

You probably did that as 2 weeks ago we did not even had MySQL Router 8.4 in the 8.4/stable channel.

Not sure why you did not stick with 8.4/edge though.

[astrojuanlu]

Yeah I remember now. The whole point of the test is to refresh the router from stable to edge. There was no stable, so I chose candidate.

[astrojuanlu]

We already deploy MySQL Router and MySQL Test app on the vanilla upgrade test.

Yes, this was done separately on #383.

I agree these tests now go beyond what's covered in the recently added test_relation_through_router in test_upgrade.py. @paulomach should I go ahead and remove that test? @sinclert-canonical Any other simplification you had in mind?

[paulomach]

We already deploy MySQL Router and MySQL Test app on the vanilla upgrade test.

Yes, this was done separately on #383.

I agree these tests now go beyond what's covered in the recently added test_relation_through_router in test_upgrade.py. @paulomach should I go ahead and remove that test? @sinclert-canonical Any other simplification you had in mind?

This test purpose is exactly to ensure a new relation still works after the refresh. In the past, we could had catch issues if such a test, as simple as it looks, was in place.
E.g. we could bork a predefined role - this will show only when creating a new relation, not for the already established one.

[sinclert-canonical]

This test purpose is exactly to ensure a new relation still works after the refresh. In the past, we could had catch issues if such a test, as simple as it looks, was in place.

Alright with me then.