-
Notifications
You must be signed in to change notification settings - Fork 873
feat(cli): use coder connect in coder ssh --stdio
, if available
#17572
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
@@ -203,14 +206,14 @@ func (r *RootCmd) ssh() *serpent.Command { | |||
parsedEnv = append(parsedEnv, [2]string{k, v}) | |||
} | |||
|
|||
deploymentSSHConfig := codersdk.SSHConfigResponse{ | |||
cliConfig := codersdk.SSHConfigResponse{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Despite the name, this was always populated from CLI arguments, which in half of the cases are not the deployment SSH config (i.e. for the VS Code extension it's something like vscode-coder
)
// SSHClient calls SSH to create a client that uses a weak cipher | ||
// to improve throughput. | ||
// SSHClient calls SSH to create a client | ||
func (c *AgentConn) SSHClient(ctx context.Context) (*ssh.Client, error) { | ||
return c.SSHClientOnPort(ctx, AgentSSHPort) | ||
} | ||
|
||
// SSHClientOnPort calls SSH to create a client on a specific port | ||
// that uses a weak cipher to improve throughput. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The weak cipher part hasn't been true for years, it used to use arcfour
. Updating the comment since it confused me for a moment.
// command via the ProxyCommand SSH option. | ||
networkInfoFilePath := filepath.Join(networkInfoDir, fmt.Sprintf("%d.json", os.Getppid())) | ||
stats := &sshNetworkStats{ | ||
UsingCoderConnect: true, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll put up a PR for the vscode extension to read this.
The PTY test is failing on Windows, but the PTY functionality works fine in an actual powershell session on Windows, so I'll need to investigate. |
This is great @ethanndickson 🎉
Thanks. Given the PR description I believe that's automatically solved too but would appreciate it if you can test that. |
I've tested by swapping out the binary path in the SSH config, and specifying a Of note is that because there's currently no way for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Currently this PR checks if it can use coder connect, and if it does runs a different function that doesn't handle special features like forwarding, containers, usage, etc.
You should instead just write some code that determines which mode to use, returns a net.Conn
for it (or some similar interface with methods for getting mode, stats, etc.), then keep all of the rest of the code the exact same without any further connection-mode-specific branches.
c90e471
to
be118e6
Compare
I made the mistake of increasing the scope of this PR beyond what was necessary for the IDE integrations. Supporting Coder Connect throughout all I spoke with Dean about it, and for now, we'll only support using the Coder Connect tunnel if |
coder ssh
, if availablecoder ssh --stdio
, if available
I think this is the reason we can not do coder/vscode-coder#113 |
Closes coder/vscode-coder#447
Closes coder/jetbrains-coder#543
Closes coder/coder-jetbrains-toolbox#21
This PR adds Coder Connect support to
coder ssh --stdio
.When connecting to a workspace, if
--force-new-tunnel
is not passed, the CLI will first do a DNS lookup for<agent>.<workspace>.<owner>.<hostname-suffix>
. If an IP address is returned, and it's within the Coder service prefix, the CLI will not create a new tailnet connection to the workspace, and instead dial the SSH server running on port 22 on the workspace directly over TCP.This allows IDE extensions to use the Coder Connect tunnel, without requiring any modifications to the extensions themselves.
Additionally,
using_coder_connect
is added to thesshNetworkStats
file, which the VS Code extension (and maybe Jetbrains?) will be able to read, and indicate to the user that they are using Coder Connect.One advantage of this approach is that running
coder ssh --stdio
on an offline workspace with Coder Connect enabled will have the CLI wait for the workspace to build, the agent to connect (and optionally, for the startup scripts to finish), before finally connecting using the Coder Connect tunnel.As a result,
coder ssh --stdio
has the overhead of looking up the workspace and agent, and checking if they are running. On my device, this meantcoder ssh --stdio <workspace>
was approximately a second slower than just connecting to the workspace directly usingssh <workspace>.coder
(I would assume anyone serious about their Coder Connect usage would know to just do the latter anyway).To ensure this doesn't come at a significant performance cost, I've also benchmarked this PR.
Benchmark
Methodology
All tests were completed on
dev.coder.com
, where a Linux workspace running in AWSus-west1
was created.The machine running Coder Desktop (the 'client') was a Windows VM running in the same AWS region and VPC as the workspace.
To test the performance of specifically the SSH connection, a port was forwarded between the client and workspace using:
where
host
was either an alias for an SSH ProxyCommand that calledcoder ssh
, or a Coder Connect hostname.For latency,
tcping
was used against the forwarded port:For throughput,
iperf3
was used:where an
iperf3
server was running on the workspace on port 7001.Test Cases
Testcase 1:
coder ssh
ProxyCommand
that bicopies from Coder ConnectThis case tests the implementation in this PR, such that we can write a config like:
With Coder Connect enabled,
ssh -p 22 -L7001:localhost:7001 codercliconnect
will use the Coder Connect tunnel. The results were as follows:Throughput, 10 tests, back to back:
Latency, 100 RTTs:
Testcase 2:
ssh
dialing Coder Connect directly without aProxyCommand
This is what we assume to be the 'best' way to use Coder Connect
Throughput, 10 tests, back to back:
Latency, 100 RTTs:
Testcase 3:
coder ssh
ProxyCommand
that creates its own Tailnet connection in-processThis is what normally happens when you run
coder ssh
:Throughput, 10 tests, back to back:
Latency, 100 RTTs:
Analysis
Performing a two-tailed, unpaired t-test against the throughput of testcases 1 and 2, we find a P value of
0.9450
. This suggests the difference between the data sets is not statistically significant. In other words, there is a 94.5% chance that the difference between the data sets is due to chance.Conclusion
From the t-test, and by comparison to the status quo (regular
coder ssh
, which uses gvisor, and is noticeably slower), I think it's safe to say any impact on throughput or latency by theProxyCommand
performing a bicopy against Coder Connect is negligible. Users are very much unlikely to run into performance issues as a result of using Coder Connect viacoder ssh
, as implemented in this PR.Less scientifically, I ran these same tests on my home network with my Sydney workspace, and both throughput and latency were consistent across testcases 1 and 2.