[client] Add WireGuard interface lifecycle monitoring #4370
Conversation
Implement a background watcher that restarts the engine when the WireGuard interface is created or deleted. This addition enhances the engine's resilience to external changes in the interface state.
Refine the background watcher to handle interface lifecycle changes more accurately. The monitor now skips execution on mobile platforms and tracks interface recreation by comparing indices, improving the engine's response to external interface modifications.
There was a problem hiding this comment.
Pull Request Overview
Implements a WireGuard interface lifecycle monitoring system that automatically restarts the engine when the WireGuard interface is externally deleted or recreated, improving the client's resilience to interface state changes.
- Adds background monitoring of WireGuard interface using polling approach
- Tracks interface index changes to detect recreation scenarios
- Implements cross-platform solution with mobile platform exclusions
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
client/internal/engine.go
Outdated
| ifi, err := net.InterfaceByName(name) | ||
| if err != nil { | ||
| // Check if it's specifically a "not found" error | ||
| if errors.Is(err, &net.OpError{}) { |
There was a problem hiding this comment.
The error check errors.Is(err, &net.OpError{}) is incorrect. errors.Is compares error values, not types. Use var opErr *net.OpError; errors.As(err, &opErr) to check if the error is of type *net.OpError.
| if errors.Is(err, &net.OpError{}) { | |
| var opErr *net.OpError | |
| if errors.As(err, &opErr) { |
Copilot uses AI. Check for mistakes.
client/internal/engine.go
Outdated
| go func(ctx context.Context, ifaceName string, expectedIndex int) { | ||
| log.Infof("Interface monitor: watching %s (index: %d)", ifaceName, expectedIndex) | ||
|
|
||
| ticker := time.NewTicker(2 * time.Second) |
There was a problem hiding this comment.
[nitpick] The polling interval of 2 seconds is a magic number that should be made configurable or defined as a named constant for better maintainability and potential tuning.
| ticker := time.NewTicker(2 * time.Second) | |
| ticker := time.NewTicker(wgIfaceMonitorPollInterval) |
Copilot uses AI. Check for mistakes.
…nagement Add a new WGIfaceMonitor to manage the WireGuard interface lifecycle, allowing the engine to respond to interface deletions and recreations. This implementation replaces the previous monitoring method, improving reliability and maintaining compatibility with mobile platforms.
| // It relies on the provided context cancellation to stop. | ||
| func (m *WGIfaceMonitor) Start(ifaceName string) { | ||
| // Skip on mobile platforms as they handle interface lifecycle differently | ||
| if runtime.GOOS == "android" || runtime.GOOS == "ios" { |
There was a problem hiding this comment.
I'd probably put this check in the code calling NewWGIfaceMonitor() instead of here, it's not the Monitor's concern whether it should run or not, especially when you're already passing an unspecified callback function.
client/internal/wg_iface_monitor.go
Outdated
| } | ||
|
|
||
| // Stop stops the monitor and waits for the goroutine to exit. | ||
| func (m *WGIfaceMonitor) Stop() { |
There was a problem hiding this comment.
If you run Stop and m.retartEngine() at the same time, then it can cause a deadlock for 5 sec because both of them use the same mutex in the engine.
|
Implement a background watcher that restarts the engine when the WireGuard interface is created or deleted. This addition enhances the engines resilience to external changes in the interface state.
Describe your changes
Issue ticket number and link
Stack
Checklist
Documentation
Select exactly one:
Internal change
Docs PR URL (required if "docs added" is checked)
Paste the PR link from https://github.com/netbirdio/docs here:
https://github.com/netbirdio/docs/pull/__