protocol: add PortForwardProtocol - #1961
Conversation
Signed-off-by: Alex Tercete <alex.tercete@arm.com> Reviewed-by: Alex Tercete <alext.tercete@arm.com> # gatekeeper
Keep `forward_local_port()` and `forward_remote_port()` as deprecated compatibility wrappers around the new protocol methods. Signed-off-by: Alex Tercete <alex.tercete@arm.com> Reviewed-by: Alex Tercete <alext.tercete@arm.com> # gatekeeper
Capture the port reported for `-R0` forwards and retain the original request so the forwarding can be cancelled reliably. Signed-off-by: Alex Tercete <alex.tercete@arm.com> Reviewed-by: Alex Tercete <alext.tercete@arm.com> # gatekeeper
Add context managers for forwarding ports through the SSH endpoint of an exported resource, including automatic allocation and cleanup. Signed-off-by: Alex Tercete <alex.tercete@arm.com> Reviewed-by: Alex Tercete <alext.tercete@arm.com> # gatekeeper
Use `adb forward` and `adb reverse` for local and remote forwarding. For `RemoteUSBADBDevice`, chain the ADB socket through the exporter SSH connection. Signed-off-by: Alex Tercete <alex.tercete@arm.com> Reviewed-by: Alex Tercete <alext.tercete@arm.com> # gatekeeper
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1961 +/- ##
========================================
+ Coverage 60.8% 61.1% +0.3%
========================================
Files 184 185 +1
Lines 15099 15186 +87
========================================
+ Hits 9186 9290 +104
+ Misses 5913 5896 -17
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Select a configured port forwarding driver while retaining implicit SSH creation as a compatibility fallback. Add a client regression test using ADB. Signed-off-by: Alex Tercete <alex.tercete@arm.com> Reviewed-by: Alex Tercete <alext.tercete@arm.com> # gatekeeper
| self._run_socket_command("cancel", [forward]) | ||
| raise | ||
|
|
||
| self._r_forwards[remote_port, local_port, remote_bind] = forward |
There was a problem hiding this comment.
This needs a comment why we need to store the original forward string.
| def forward_remote_port(self, remoteport, localport): | ||
| def forward_local_port(self, remoteport, localport=None): | ||
| warnings.warn( | ||
| "SSHDriver.forward_local_port() is deprecated, use local_forward() instead", |
There was a problem hiding this comment.
Why do we need to deprecate the existing functions, instead of just making them explicit in the new Protocol?
There was a problem hiding this comment.
I wanted the protocol to support auto allocation when port 0 is requested, and make that the default. But the parameters in the existing SSHDriver methods didn't allow for that without breaking backwards compatibility.
I considered keeping the old method names and inspecting the parameters to make the method satisfy both the new protocol and the old signature, but I figured that was unnecessary complexity for what's probably niche functionality. So deprecation seemed the best route.
Happy to change the approach.
3 participants
Description
Introduce
PortForwardProtocoland implement it inSSHDriverandADBDriver.SSHDrivernow uses automatic allocation for remote ports and reads the allocated port fromssh -O forward. The existing methods are kept as deprecated compatibility wrappers.ADBDriverusesadb forwardandadb reverse. ForRemoteUSBADBDevice, it chains ADB and SSH port forwards through the exporter.labgrid-client forwardnow uses a configuredPortForwardProtocoldriver, with implicit SSH forwarding retained as a compatibility fallback.Checklist
doc/configuration.rsthas been updated