Some test_upgrade fixes#726
Conversation
| # Get the paths to write test logs | ||
| output_dir = Path(session_cloud.settings.LOCAL_LOG_PATH) | ||
| output_dir.mkdir(parents=True, exist_ok=True) | ||
| base_filename = 'test_upgrade_{os}_{{stage}}_{time}.log'.format( |
There was a problem hiding this comment.
Is there an issue/etc for this change? Was the filename always wrong?
There was a problem hiding this comment.
It's the output log of the running test. It outputs some of the same commands before and after the upgrade and writes the output to before and after log files. When running tests in batch, getting a number of log files like:
test_upgrade_gce_focal_before_1.log
test_upgrade_ec2_focal_before_2.log
test_upgrade_lxd_focal_before_3.log
is much preferable to
test_upgrade_focal_before_1.log
test_upgrade_focal_before_2.log
test_upgrade_focal_before_3.log
OddBloke
left a comment
OddBloke
left a comment
There was a problem hiding this comment.
One nit inline, but otherwise this looks good, thanks!
|
|
||
|
|
||
| def _restart(instance): | ||
| # work around pad.lv/1890528 |
There was a problem hiding this comment.
Could you explain in a little more detail how this is doing that? My reading of that bug is that it only triggers if cloud-init status --wait exits non-zero: if that's exiting non-zero (and therefore raising the OSError, I assume?) then result.ok in the for loop below is never going to be True, so we'll never return early. What am I missing?
(Even if I'm not missing something and this is a bug, this doesn't need to block landing; we should fix it soon hereafter though.)
There was a problem hiding this comment.
Hmmm, maybe this is a different bug then. On ec2, cloud-init returns early with non-zero status (I'm inferring this from pycloudlib behavior). Connecting again, it'll complete and return 0. Nothing in the logs or result.json or status.json indicate any sort of failure or problem anywhere.
There was a problem hiding this comment.
I'll file a separate bug with logs and update the reference here
- workaround pad.lv/1890528 for restarting instances - move wait param from launch_kwargs to launch call - remove name param as it's not universally supported - add platfrom to log names
TheRealFalcon
force-pushed
the
fix-upgrade
branch
from
873356f to
8c311e4
Compare
3 participants
Proposed Commit Message
Some test_upgrade fixes
Additional Context
n/a
Test Steps
pytest tests/integration_tests/test_upgrade.py
Checklist: