fix: machine scp/ssh operations#1016
fix: machine scp/ssh operations#1016yanksyoon wants to merge 17 commits intojuju:masterfrom yanksyoon:fix/machine_ssh
Conversation
1 similar comment
|
@cderici mind taking a look? Or happy to do a dual review if you want to shadow me through this to make sure my assumptions are correct? |
cderici
left a comment
cderici
left a comment
There was a problem hiding this comment.
This is nice, but instead of failing in machine.ssh and machine.scp, I'd add an optional wait_until_active=False argument and inline the body of wait() in where you're currently raising an error. Note that block_until is defined in the model, so you're gonna need to call it like self.model.block_until() in Machine methods.
Also this in it's current implementation requires heavy testing, as the status is a bit finicky in pylibjuju, as the status we get through the api is not exactly the same status you'd see via juju-cli (i.e. juju status). For machines it shouldn't be too much of a problem, however, I would refrain from checking the status directly (like self.agent_status == "started"), as there might be a race where in between the block_until check intervals the status might change and you'd lose the time to catch it, so I wouldn't want my code to rely on the status itself. Instead I'd check just the addresses, as that's what you essentially need.
However, safe_data shouldn't be used outside the class, as it's internal, so let's add a machine.addresses() method and use that (also in the machine.dns_name() as well). WDYT?
@yanksyoon if it's not too much trouble it would be nice to isolate this change to just fixing #997, thanks again for working on this! (If it'll take time feel free to wait until we decide how to proceed with #1015) |
#1020 #### Description This is a follow-up PR to #1016 , which contains cherry-picked commits without type imports. Fİxes #997 #### QA Steps *<Commands / tests / steps to run to verify that the change works:>* ``` tox -e py3 -- tests/unit/... ``` ``` tox -e integration -- tests/integration/test_machine.py ``` All CI tests need to pass. *<Please note that most likely an additional test will be required by the reviewers for any change that's not a one liner to land.>* #### Notes & Discussion *<Additional notes for the reviewers if needed. Please delete section if not applicable.>*
4 participants
Description
<Please add why this change is needed and what it does.>
This is a follow up PR to #1015
This addresses the issue #997
The current machine.ssh or machine.scp will fail due to machine address not being ready.
This makes the machine ssh command become
ssh ubuntu@None.<Fixes: >
Added a function
machine.wait()which waits for machine agent to become ready and the machine address to exist.The scp/ssh functions now will raise if
await machine.wait()hasn't been called.QA Steps
All CI tests need to pass.
<Please note that most likely an additional test will be required by the reviewers for any change that's not a one liner to land.>
Notes & Discussion
<Additional notes for the reviewers if needed. Please delete section if not applicable.>