Skip to content
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

Feature/tailscale network tools #1589

Open
wants to merge 10 commits into
base: feature/tailscale-network-rewrite
Choose a base branch
from

Conversation

bkneis
Copy link
Contributor

@bkneis bkneis commented Jan 23, 2025

This PR contains the addition of the following CLI commands

devpod ts ping, devpod ts netcheck, devpod ts status and devpod ts metrics. Each are the same as running tailscale instead of devpod ts in the CLI. They were taken from https://github.com/tailscale/tailscale/tree/v1.78.3/cmd/tailscale/cli where the CLI tool was ported to cobra and the tailscale LocalClient was injected by starting a tailscale server using pkg/tailscale and returning it's userspace local client.

One open question in this PR are my changes to tsnet.go. I changed the call to Start with Up, looking at the docs Start and Up are the same except Up waits for the client to be connected. I thought this would be preferably than having to poll a local tailscale endpoint to check it. I then passed a channel to signal when the call to Start had completed calling Up (therefore was connected to the network) and before blocking on the context. Let me know your thoughts

Copy link
Member

@pascalbreuninger pascalbreuninger left a comment

Choose a reason for hiding this comment

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

LGTM overall, a couple of smaller comments here and there.

Bigger topic: What do you think of moving all logging to our logger implementation?
Right now it always prints to Stdout/err which will make it harder for us in the future to expose these commands from within a workspace.

cmd/ts/tailscale.go Show resolved Hide resolved
pkg/tailscale/tsnet.go Show resolved Hide resolved
cmd/ts/metrics.go Show resolved Hide resolved
cmd/ts/netcheck.go Show resolved Hide resolved
Copy link
Contributor

@janekbaraniewski janekbaraniewski left a comment

Choose a reason for hiding this comment

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

LGTM! Couple comments about formatting etc

cmd/ts/netcheck.go Outdated Show resolved Hide resolved
cmd/ts/netcheck.go Show resolved Hide resolved
cmd/ts/metrics.go Show resolved Hide resolved
cmd/ts/ping.go Show resolved Hide resolved
cmd/ts/ping.go Show resolved Hide resolved
cmd/ts/status.go Outdated Show resolved Hide resolved
cmd/ts/status.go Show resolved Hide resolved
pkg/tailscale/tsnet.go Show resolved Hide resolved
@bkneis
Copy link
Contributor Author

bkneis commented Jan 24, 2025

@pascalbreuninger good point, I guess this depends on how we intend to use the tool. So far I have been running in an SSH session on my workspace. I suppose the logger is only needed if we execute the commands in the SSH tunnel pragmatically? Would we do this? If not logging to STDOUT should suffice. Let me know if this needs to be implemented but I thought this would be run manually by the user

@bkneis bkneis force-pushed the feature/tailscale-network-tools branch from b05bb28 to 1ffbb01 Compare January 29, 2025 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants