[JUJU-4231] wait_for_idle to consider app status #905
Conversation
cderici
added
bug fix
hint/2.9
|
@cderici why not to go with a second function and unload a bit existing wait for idle? |
Cause I don't have time. This is just an unplanned patch on wait_for_idle that I prioritized and added to my pulse to help you guys out when @DnPlas asked on MM. The new waiting functions should be planned out separately, and they'll be put up on separate PRs. |
Removing the workaround as the proposed fix in [1] should take care of it. Links: [1] juju/python-libjuju#905
That's awesome, @DnPlas, thanks for opening a whole PR to test this! Even if it the test fails, that should prove very useful ironing this out! Soon we'll introduce integration tests on libjuju that basically does what that PR is doing, so we'll know if something breaks even before users do, which should make things much smoother for you guys 👍 Watching the bundle test here: https://github.com/canonical/kfp-operators/actions/runs/5542344265/jobs/10120007914?pr=249 |
|
@cderici I have run the exact same tests of canonical/kfp-operators#249 (failing due to unrelated issues) locally, and it seems like this issue is not present anymore with the change proposed in this PR. At least on that side we are covered. |
Sounds great, @DnPlas! Thanks for your help confirming this, hopefully in the near future we'll make waiting for entities much cleaner 👍 |
|
/merge |
cderici
removed
the
area/forward-port
#910 #### Description This brings forward from `2.9` the following PRs: #901, #905, and #896 into `3.x` (the main branch). #### QA Steps There were a couple of conflicts that needed to be fixed, so QA will be necessary, make sure the following tests are passing: ``` tox -e integration -- tests/integration/test_application.py::test_app_charm_name ``` ``` tox -e py3 -- -m wait_for_idle ``` ``` tox -e integration -- -m wait_for_idle ``` ```sh tox -e integration -- tests/integration/test_model.py::test_deploy_bundle_local_charm_series_manifest ```
#919 ## What's Changed * [JUJU-4110] `charm_name` on Application object by @cderici in #901 * [JUJU-4204] Model.name on 2.9 by @cderici in #902 * [JUJU-4231] wait_for_idle to consider app status by @cderici in #905 * Stabilize sphinx doc builds for `2.9` track by @cderici in #911 * Add pytest.ini to register custom pytest marks by @cderici in #912 * [JUJU-4291] Removing Upper Constraint by @markbeierl in #916 [JUJU-4110]: https://warthogs.atlassian.net/browse/JUJU-4110?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [JUJU-4204]: https://warthogs.atlassian.net/browse/JUJU-4204?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [JUJU-4231]: https://warthogs.atlassian.net/browse/JUJU-4231?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [JUJU-4291]: https://warthogs.atlassian.net/browse/JUJU-4291?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ
Description
This adds logic to
wait_for_idleto also consider application status when computing status via the unit statuses.Should fix #831 and #900
QA Steps
This adds some scenarios to the existing unit tests for
wait_for_idle. Also adds a pytest mark "wait_for_idle", so only the tests related towait_for_idlecan be run as a subset:Notes & Discussion
wait_for_idleis becoming more and more convoluted. Basically we need two versions of this function. 1)wait_for_applications()for users who want to wait on an application to be in a certain state and doesn't particularly care about units, and 2)wait_for_units()for users who want finer detailed control and want to wait on certain number of units in certain applications to be in certain states etc.wait_for_idle, however, I'd greatly appreciate if you guys could also give this PR a good stress test. I'm basically asking you to run all the integration test you have that useswait_for_idleagainst this patch and see if it can take a punch. Thanks!