- Notifications
You must be signed in to change notification settings - Fork928
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
`coder vpn-daemon run` will instantiate a RPC connection with thespecified 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 thetunnel functionality isn't implemented, but it does make sure that thetunnel 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 atunnel.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 atunnel.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 atunnel.Wait() in a follow-up PR tomorrow since tunnel is mostly unimplemented anyways
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Also removes --log-file functionality for now.
d63bd21 intomainUh oh!
There was an error while loading.Please reload this page.
| { | ||
| 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 runwill 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