cmd/sshpiperd: allow starting without plugins running #653
Conversation
|
The three PRs I just sent may have conflicts with each other. I'm happy to rebase, sequence them, etc, just say the word! I sent them all independently to start with in case you had opinions about wanting some but not others. :) |
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the sshpiperd daemon to allow starting without plugins running, implementing a lazy plugin initialization strategy. Instead of failing startup when plugins can't be initialized, the daemon now defers plugin initialization until the first connection attempt.
- Deferred plugin initialization from startup to first connection
- Added lazy plugin loading with atomic state tracking and single-flight pattern
- Enhanced logging with structured fields for better observability
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| go.mod | Updated Go version and various dependency versions, added tailscale.com dependency |
| cmd/sshpiperd/main.go | Modified to store plugin configs instead of initializing plugins immediately |
| cmd/sshpiperd/daemon.go | Added lazy plugin initialization with atomic state and wrapper listener |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| daemon *daemon | ||
| } | ||
|
|
||
| func (l *lazyPluginListener) Accept() (net.Conn, error) { |
There was a problem hiding this comment.
may i know the reason using a listener to active
why not put this to
for {
accept
}
| return nil // previously initialized successfully | ||
| } | ||
|
|
||
| _, err, _ := d.pluginIniSingleFlight.Do(unused{}, func() (unused, error) { |
There was a problem hiding this comment.
may i know if a simple lock works?
seems introducing tailscale is too heavy
| } | ||
|
|
||
| func (d *daemon) initializePlugins() error { | ||
| var plugins []*plugin.GrpcPlugin |
There was a problem hiding this comment.
a bit worry about partial plugin failure and might create too many zombies
do not retry on inited plugin?
This is an initial implementation. It doesn't have many miles on it. I'm sharing it for early feedback and thoughts.
Fixes #640