change: when possible (REST allows it), always power cycle the control host#1002
Merged
change: when possible (REST allows it), always power cycle the control host#1002
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #1002 +/- ##
==========================================
+ Coverage 75.00% 75.24% +0.23%
==========================================
Files 109 110 +1
Lines 10746 10756 +10
Branches 899 898 -1
==========================================
+ Hits 8060 8093 +33
+ Misses 2500 2467 -33
- Partials 186 196 +10
*This pull request uses carry forward flags. Click here to find out more.
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR centralizes “control host” handling by introducing a DefaultControlHost helper and updating device connectors/tests to use it, enabling an “always power cycle when REST supports it” pre-provision behavior.
Changes:
- Added
DefaultControlHost(REST health check/poweroff + SSH fallback + reboot script execution) and rewiredDefaultDevice.pre_provision_hook()to delegate to it. - Removed Zapper-specific REST readiness helpers (
wait_ready, REST health check) and updated tests accordingly. - Simplified MuxPi/Zapper code paths that previously waited for the Zapper REST API before proceeding.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| device-connectors/src/testflinger_device_connectors/devices/init.py | Adds DefaultControlHost and updates DefaultDevice.pre_provision_hook() to power-cycle via REST when available. |
| device-connectors/tests/test_devices.py | Updates unit tests to target DefaultControlHost + new power-cycle / SSH fallback behavior. |
| device-connectors/src/testflinger_device_connectors/devices/tests/test_devices.py | Updates DefaultDevice-related tests to use DefaultControlHost.wait_online/offline. |
| device-connectors/src/testflinger_device_connectors/devices/zapper/init.py | Removes Zapper REST readiness helpers and removes the “cannot reach REST API” guard from provision. |
| device-connectors/src/testflinger_device_connectors/devices/zapper/tests/test_zapper.py | Updates tests to validate DefaultControlHost REST health check and removes wait_ready expectations from Zapper flows. |
| device-connectors/src/testflinger_device_connectors/devices/zapper_kvm/init.py | Removes connector-specific pre_provision_hook override so it relies on the default centralized behavior. |
| device-connectors/src/testflinger_device_connectors/devices/zapper_kvm/tests/test_zapper_kvm.py | Removes tests for the deleted pre_provision_hook override. |
| device-connectors/src/testflinger_device_connectors/devices/muxpi/muxpi.py | Removes Zapper REST readiness wait from the zapper-based provisioning path. |
| device-connectors/src/testflinger_device_connectors/devices/muxpi/tests/test_muxpi.py | Updates tests to stop asserting a Zapper REST readiness wait. |
Comments suppressed due to low confidence (1)
device-connectors/src/testflinger_device_connectors/devices/zapper/init.py:156
ZapperConnector.provisionno longer checks that the control host REST API is reachable before starting provisioning. IfDefaultDevice.pre_provision_hook()falls back to SSH (or if the power-cycle is skipped), the subsequent_api_post/_api_getcalls can fail with rawrequestsexceptions instead of a clearProvisioningError. Consider reintroducing a readiness check usingDefaultControlHost(control_host).wait_ready(...)and convertingTimeoutError/ConnectionErrorinto aProvisioningErrorwith a helpful message.
def provision(self, args):
"""Provision device when the command is invoked."""
super().provision(args)
with open(args.job_data, encoding="utf-8") as job_json:
self.job_data = json.load(job_json)
logger.info("BEGIN provision")
logger.info("Provisioning device")
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
amalinowski75
previously approved these changes
Apr 7, 2026
amalinowski75
left a comment
amalinowski75
left a comment
There was a problem hiding this comment.
Looks good. I left only one comment and it'w probably not valid :)
gntzio
approved these changes
Apr 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
We currently power cycle the control host only if not reachable, because it might be a disruptive operation with a live system. After introducing a
poweroffAPI though, this action becomes safe to do every time, making sure we always test with a clean system.Resolved issues
Resolves https://warthogs.atlassian.net/browse/ZAP-1446
Documentation
N/A
Web service API changes
N/A
Tests
Fallback (online):
Fallback (offline):
REST poweroff + power cycle: