8000 chore: switch to new wgtunnel via tunnelsdk by deansheather · Pull Request #6489 · coder/coder · GitHub
[go: up one dir, main page]
Skip to content
8000

chore: switch to new wgtunnel via tunnelsdk #6489

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 13 commits into from
Mar 22, 2023
Merged

chore: switch to new wgtunnel via tunnelsdk #6489

merged 13 commits into from
Mar 22, 2023

Conversation

deansheather
Copy link
Member
@deansheather deansheather commented Mar 7, 2023 •
edited
Loading

Switch the devtunnel wrapper to use our new open source wgtunnel package.

This removes all of the tunnel code in favor of github.com/coder/wgtunnel/tunnelsdk and updates the tests to use a real github.com/coder/wgtunnel/tunneld server.

TODO:

  • User customizable tunnel URL via deployment config
  • Manual testing with older versions against a newer tunnel
  • Manual testing with this version against a newer tunnel
  • Deploy the newer tunnel to the same URL as the old tunnel

Closes #6360

@hh hh mentioned this pull request Mar 8, 2023
Closed
@hh
Copy link
hh commented Mar 15, 2023

User customizable tunnel URL via deployment config
I like this, do you have an approach / design for this yet?

@mtojek
Copy link
Member
mtojek commented Mar 16, 2023

@deansheather @coadler Did you have a chance to move this forward?

@deansheather
Copy link
Member Author

Tried to get wgtunnel working again on the box we currently use in prod. Didn't work, so ordered a VPS from teraswitch to try deploying on instead. It works locally and in tests but doesn't on that server so I want to try another server.

@deansheather deansheather requested a review from coadler March 17, 2023 04:42
@deansheather deansheather marked this pull request as ready for review March 17, 2023 04:42
mtojek
mtojek requested changes Mar 20, 2023
View reviewed changes
coderd/devtunnel/tunnel.go
Comment on lines +158 to +159
// If we fail to find the closest node, default to a random node from
// the first region.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are nodes sorted, so that we need to use random numbers? Could it be always the first node or the healthiest/biggest one?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't have any mechanism to judge which node is healthiest currently. We only have one node so this actually does nothing right now. When/if we add more nodes in the future we can optimize this or perhaps force it to fail on error.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if a map, wouldn't be better than array then, but it's a nit-pick.

codersdk/deployment.go Outdated
{
Name: "Custom Tunnel Host",
Description: `Hostname of HTTPS server that runs https://github.com/coder/wgtunnel. By default, this will pick the best available wgtunnel server hosted by Coder. e.g. "tunnel.example.com".`,
Flag: "tunnel-host",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wg-tunnel-host? Tunnel host might be a too generic name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fact that the tunnel is wireguard is transparent to the user, so adding wg seems to me like it would confuse people. This is a hidden var since we don't intend on many people using it though so maybe wgtunnel-host is fine unless we want to unhide it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like wgtunnel-host.

@deansheather deansheather requested a review from mtojek March 20, 2023 14:00
coadler
coadler approved these changes Mar 20, 2023
View reviewed changes
codersdk/deployment.go Outdated
{
Name: "Custom Tunnel Host",
Description: `Hostname of HTTPS server that runs https://github.com/coder/wgtunnel. By default, this will pick the best available wgtunnel server hosted by Coder. e.g. "tunnel.example.com".`,
Flag: "tunnel-host",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like wgtunnel-host.

@hh
Copy link
hh commented Mar 20, 2023

Works for me!

hh@Max coder % rm -rf ~/Library/Application\ Support/coderv2 
hh@Max coder % coder server --tunnel-host try.ii.nz        
Coder v0.19.2-devel+2ba29479 - Your Self-Hosted Remote Development Platform
Using built-in PostgreSQL (/Users/hh/Library/Application Support/coderv2/postgres)
Started HTTP listener at http://127.0.0.1:3000
Opening tunnel so workspaces can connect to your deployment. For production scenarios, specify an external access URL
Using tunnel in Unknown with latency  11.21ms .

View the Web UI: https://q1bneu1bas94rnrm.try.ii.nz
2023-03-20 18:26:15.789 [INFO]	(coderd.update_checker)	<./coderd/updatecheck/updatecheck.go:179>	(*Checker).update	checking for update

==> Logs will stream in below (press ctrl+c to gracefully exit):
2023-03-20 18:26:16.211 [INFO]	(coderd.update_checker)	<./coderd/updatecheck/updatecheck.go:206>	(*Checker).update	update check result	{"latest_version": "v0.19.2"}
2023-03-20 18:26:16.212 [INFO]	(coderd)	<./cli/server.go:721>	Server.func1.3	new version of coder available	{"new_version": "v0.19.2", "url": "https://github.com/coder/coder/releases/tag/v0.19.2", "upgrade_instructions": "https://coder.com/docs/coder-oss/latest/admin/upgrade"}
2023-03-20 18:26:16.213 [INFO]	(coderd.update_checker)	<./coderd/updatecheck/updatecheck.go:148>	(*Checker).start	time until next update check	{"duration": "23h59m59.998119584s"}

@deansheather deansheather enabled auto-merge (squash) March 21, 2023 03:01
@deansheather deansheather merged commit 5460ab4 into main Mar 22, 2023
@deansheather deansheather deleted the dean/tunnelsdk branch March 22, 2023 13:13
@github-actions github-actions bot locked and limited conversation to collaborators Mar 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@mtojek mtojek mtojek approved these changes

@coadler coadler coadler approved these changes

Assignees

@deansheather deansheather

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

chore: switch tunnel to use new wgtunnel
4 participants
@deansheather @hh @mtojek @coadler
0