-
Notifications
You must be signed in to change notification settings - Fork 876
chore: add vpn-daemon run subcommand for windows #15526
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
`coder vpn-daemon run` will instantiate a RPC connection with the specified pipe handles and communicate with the (yet to be implemented) parent process. The tests don't ensure that the tunnel is actually usable yet as the tunnel functionality isn't implemented, but it does make sure that the tunnel tries to read from the RPC pipe.
} | ||
defer tunnel.Close() | ||
|
||
<-ctx.Done() |
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 don't think this will actually work in practice, should we add a tunnel.Wait()
or something instead to wait for the *vpn.Tunnel to encounter a permanent error?
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 way a clean shutdown should work is that the manager sends StopRequest, the tunnel replies StopResponse and closes the tunnel → manager pipe. We could also add a tunnel.Wait()
that returns a channel that closes when the tunnel is fully shut down like this, so that we can also have the daemon process exit.
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 will add a tunnel.Wait()
in a follow-up PR tomorrow since tunnel is mostly unimplemented anyways
Also removes --log-file functionality for now.
{ | ||
Flag: "rpc-read-handle", | ||
Env: "CODER_VPN_DAEMON_RPC_READ_HANDLE", | ||
Description: "The handle for the pipe to read from the RPC connection.", |
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.
Slightly ambiguous about which process's perspective. Is the daemon reading (and therefore the caller of this command is writing) or vice versa?
{ | ||
Flag: "rpc-write-handle", | ||
Env: "CODER_VPN_DAEMON_RPC_WRITE_HANDLE", | ||
Description: "The handle for the pipe to write to the RPC connection.", |
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.
Ditto on ambiguity on which process is reading and which is writing
coder vpn-daemon run
will instantiate a RPC connection with the specified pipe handles and communicate with the (yet to be implemented) parent process.The tests don't ensure that the tunnel is actually usable yet as the tunnel functionality isn't implemented, but it does make sure that the tunnel tries to read from the RPC pipe.
Closes #14735