Reboot control host before provisioning #837
Conversation
Codecov Report❌ Patch coverage is ❌ Your patch check has failed because the patch coverage (7.35%) is below the target coverage (60.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## main #837 +/- ##
==========================================
- Coverage 69.60% 69.14% -0.46%
==========================================
Files 106 106
Lines 9142 9210 +68
Branches 841 847 +6
==========================================
+ Hits 6363 6368 +5
- Misses 2605 2668 +63
Partials 174 174
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
amalinowski75
left a comment
amalinowski75
left a comment
There was a problem hiding this comment.
Make sense. Just a few minor comments.
| cmd, | ||
| shell=True, | ||
| check=True, | ||
| timeout=300, |
| logger.error( | ||
| "Unexpected error running command %s: %s", cmd, str(e) | ||
| ) | ||
|
|
There was a problem hiding this comment.
TBH I don't know how reboot scripts are built but in't better to run the entire script? What if there are some conditionals, etc?
There was a problem hiding this comment.
It ususally looks like [$POWER_OFF_CMD, "sleep N", $POWER_ON_CMD ]
| self.__reboot_control_host() | ||
|
|
||
| # Wait for control host to be reachable via ping | ||
| self.__wait_back_alive(control_host, 60) |
There was a problem hiding this comment.
For some devices it takes a few minutes to become fully operational after they become pingable. I think it's good to wait here at least 3 minutes.
| logger.debug("No control host configured for this agent.") | ||
| return | ||
|
|
||
| with contextlib.suppress(FileNotFoundError): |
There was a problem hiding this comment.
FileNotFoundError exception seems a bit odd. ConnectionError looks better IMO.
| ) | ||
| time.sleep(int(timeout)) | ||
|
|
||
| def __ping(self, host: str) -> None: |
There was a problem hiding this comment.
We’ve seen cases where the control host is pingable but not reachable via SSH. I’d suggest using SSH to verify connectivity, as it provides better coverage.
p-gentili
changed the title
Description
This PR propose a programmatic way to reboot the control host just before provisioning in case it's not reachable. I think ping is a good enough way to do this evaluation, considering different type of control hosts might have different ways of checking if it's really ready.
This is still opt-in (you need to call the super() method), but I think it's safe to call it everywhere for now, since it doesn't raise any error. We could also wrap it in
try/except Exceptionif it makes sense.Resolved issues
N/A
Documentation
Pending...
Web service API changes
No!
Tests
Not yet...