no-wait option in tests.yaml#688
no-wait option in tests.yaml#688hemanthnakkina wants to merge 1 commit intoopenstack-charmers:masterfrom
Conversation
To run multiple bundles in different models, zaza
need to wait for first bundle applications to acheive
status as specified in target_deploy_status before
deploying second bundle using configure. (Deploying
second bundle using configure is actually a hacky
way). This increases the time of deployment as both
bundles are deployed sequentially. sunbeam-charms
project has total timeout of 2 hours for its
functional test.
Add option no-wait in tests.yaml so that zaza did not
wait for target_deploy_status for the applications and
proceed with configure.
tests_options:
no_wait_deploy:
smoke
The option --no-wait already exists for functest-deploy
cli command.
aca7203 to
869a40a
Compare
There was a problem hiding this comment.
Pull request overview
Adds a tests_options.no_wait_deploy setting in tests.yaml to allow selectively skipping deployment waiting so multi-bundle functional test runs can proceed to configuration sooner and reduce overall runtime.
Changes:
- Introduces
utils.no_wait_deploy()to readtests_options.no_wait_deploy(per bundle). - Wires the option into
func_test_runner.run_env_deployment()to passwait=intodeploy.deploy()and to conditionally skip the post-deploy settle wait. - Adds unit tests for the new config option and runner behavior; documents the new
tests.yamloption.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| zaza/charm_lifecycle/utils.py | Adds no_wait_deploy() config helper for reading tests_options.no_wait_deploy. |
| zaza/charm_lifecycle/func_test_runner.py | Applies no_wait_deploy to deployment waiting behavior in the lifecycle runner. |
| unit_tests/test_zaza_charm_lifecycle_utils.py | Adds unit coverage for no_wait_deploy() config parsing behavior. |
| unit_tests/test_zaza_charm_lifecycle_func_test_runner.py | Updates expected deploy calls to include wait=... and adds a no-wait runner test. |
| doc/source/addingcharmtests.rst | Documents the new tests_options.no_wait_deploy option. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| state. This is equivalent to passing ``--no-wait`` directly to | ||
| ``functest-deploy``. |
There was a problem hiding this comment.
The new docs state this option is equivalent to passing --no-wait to functest-deploy, but the implementation also skips the additional post-deploy block_until_all_units_idle settle step in func_test_runner.run_env_deployment. Either update the docs to describe the extra behavior (and drop the equivalence claim), or change the runner to still perform the model-settle wait even when skipping target_deploy_status waiting.
| state. This is equivalent to passing ``--no-wait`` directly to | |
| ``functest-deploy``. | |
| state or for the model to settle (the additional post-deploy | |
| ``block_until_all_units_idle`` wait is also skipped). |
| In the ``tests.yaml`` the option is added as a list item:: | ||
|
|
||
| charm_name: vault | ||
| gate_bundles: | ||
| - focal-ussuri | ||
|
|
||
| target_deploy_status: | ||
| vault: | ||
| workload-status: blocked | ||
| workload-status-message: Vault needs to be initialized | ||
|
|
||
| tests_options: | ||
| no_wait_deploy: | ||
| - focal-ussuri | ||
|
|
There was a problem hiding this comment.
The PR description example shows tests_options.no_wait_deploy: - smoke, but the implemented behavior (and this doc example) expects a list of bundle names (e.g. focal-ussuri). If smoke/dev/gate selectors were intended, the code needs to be adjusted; otherwise please update the PR description/example to avoid confusion.
| """Ask if the wait step should be skipped after deploying bundle. | ||
|
|
||
| Setting no_wait_deploy for a bundle means that zaza will not wait for | ||
| the applications to reach the states defined in target_deploy_status (or | ||
| active/idle) after deploying the bundle. |
There was a problem hiding this comment.
no_wait_deploy is described here as skipping only the target_deploy_status/active-idle wait, but in func_test_runner.run_env_deployment the same flag is also used to skip the post-deploy block_until_all_units_idle settle step. Please clarify the docstring to match the actual behavior (or adjust the runner if the settle wait should still occur).
| """Ask if the wait step should be skipped after deploying bundle. | |
| Setting no_wait_deploy for a bundle means that zaza will not wait for | |
| the applications to reach the states defined in target_deploy_status (or | |
| active/idle) after deploying the bundle. | |
| """Ask if the deploy wait/settle steps should be skipped for a bundle. | |
| Setting no_wait_deploy for a bundle means that zaza will not: | |
| * wait for the applications to reach the states defined in | |
| target_deploy_status (or active/idle) after deploying the bundle; or | |
| * perform the post-deploy block_until_all_units_idle "settle" wait. |
| # When deploying bundles with cross model relations, hooks may be | ||
| # triggered in already deployedi models so wait for all models to | ||
| # triggered in already deployed models so wait for all models to | ||
| # settle. | ||
| for deployment in env_deployment.model_deploys: | ||
| if utils.no_wait_deploy(deployment.bundle): | ||
| logging.info( | ||
| "Skipping post-deploy wait for {} " | ||
| "(no_wait_deploy is set)".format( | ||
| deployment.model_name)) | ||
| continue |
There was a problem hiding this comment.
The comment says we must “wait for all models to settle” for cross-model relations, but the new no_wait_deploy check skips the settle wait (block_until_all_units_idle) entirely for those deployments. Either update the explanatory comment to reflect the conditional behavior, or reconsider skipping this settle step since it’s specifically meant to avoid cross-model hook races.
|
|
||
| tests_options: | ||
| no_wait_deploy: | ||
| - focal-ussuri |
There was a problem hiding this comment.
zaza supports adding a bundle with a prefix , e.g. https://github.com/openstack-charmers/charmed-openstack-tester/blob/master/tests/distro-regression/tests/tests.yaml#L5 , will this option honor it?
gate_bundles:
- security:focal-ussuri
- focal-ussuri
test_options:
no_wait_deploy:
- focal-ussuri
in a tests.yaml like this ^, the no-wait should only match the second bundle and not security:focal-ussuri
To run multiple bundles in different models, zaza
need to wait for first bundle applications to acheive status as specified in target_deploy_status before deploying second bundle using configure. (Deploying second bundle using configure is actually a hacky
way). This increases the time of deployment as both bundles are deployed sequentially. sunbeam-charms
project has total timeout of 2 hours for its
functional test.
Add option no-wait in tests.yaml so that zaza did not wait for target_deploy_status for the applications and proceed with configure.
The option --no-wait already exists for functest-deploy cli command.