From 2e23abc139c7ad2497c3950c1dceb7cefa6f1bd2 Mon Sep 17 00:00:00 2001 From: Utku Ozdemir Date: Thu, 10 Sep 2026 09:12:07 +0200 Subject: [PATCH] fix: reconnect the WireGuard over gRPC tunnel after a failure When the tunnel connection fails, the retry that should bring it back never runs: 1. Before starting the tunnel relay, the controller cancels the retry with `relayRetryTimer.Reset(0)`, which discards the timer so `relayRetryTimer.C()` becomes a nil channel. 2. The controller then waits on that nil `relayRetryTimer.C()`, which never fires. 3. When the relay fails, it reschedules with `relayRetryTimer.Reset(retryIn)`, which builds a new timer, but the select is still parked on the old nil channel and never sees it. 4. The relay is never restarted, so the tunnel stays down until the node reboots. Keep a single `time.Timer` inside `ResettableTimer` for the life of the controller and only stop or re-arm it, so `relayRetryTimer.C()` is always the same non-nil channel and the rescheduled retry reaches the select. Signed-off-by: Utku Ozdemir Signed-off-by: Andrey Smirnov --- .../pkg/controllers/siderolink/userspace.go | 64 +++++++------------ .../controllers/siderolink/userspace_test.go | 53 +++++++++++++++ 2 files changed, 75 insertions(+), 42 deletions(-) create mode 100644 internal/app/machined/pkg/controllers/siderolink/userspace_test.go diff --git a/internal/app/machined/pkg/controllers/siderolink/userspace.go b/internal/app/machined/pkg/controllers/siderolink/userspace.go index e517bcd9b9a..4c2a3042df7 100644 --- a/internal/app/machined/pkg/controllers/siderolink/userspace.go +++ b/internal/app/machined/pkg/controllers/siderolink/userspace.go @@ -61,10 +61,11 @@ func (ctrl *UserspaceWireguardController) Outputs() []controller.Output { func (ctrl *UserspaceWireguardController) Run(ctx context.Context, r controller.Runtime, logger *zap.Logger) error { eg, ctx := errgroup.WithContext(ctx) + relayRetryTimer := NewResettableTimer() + var ( - relayRetryTimer resettableTimer - tunnelDevice tunnelDeviceProps - tunnelRelay tunnelProps + tunnelDevice tunnelDeviceProps + tunnelRelay tunnelProps ) defer func() { @@ -209,59 +210,38 @@ type tunnelDeviceProps struct { mtu int } -// resettableTimer wraps time.Timer to allow resetting the timer to any duration. -type resettableTimer struct { +// ResettableTimer wraps time.Timer to allow arming, re-arming or stopping it. +type ResettableTimer struct { mx sync.Mutex timer *time.Timer } -// Reset resets the timer to the given duration. -// -// If the duration is zero, the timer is removed (and stopped as needed). -// If the duration is non-zero, the timer is created if it doesn't exist, or reset if it does. -func (rt *resettableTimer) Reset(delay time.Duration) { +// NewResettableTimer creates a stopped ResettableTimer. +func NewResettableTimer() *ResettableTimer { + timer := time.NewTimer(0) + timer.Stop() + + return &ResettableTimer{timer: timer} +} + +// Reset arms the timer for the given duration, or stops it when the duration is zero. +func (rt *ResettableTimer) Reset(delay time.Duration) { rt.mx.Lock() defer rt.mx.Unlock() - if delay == 0 { - if rt.timer != nil { - if !rt.timer.Stop() { - <-rt.timer.C - } + rt.timer.Stop() - rt.timer = nil - } - } else { - if rt.timer == nil { - rt.timer = time.NewTimer(delay) - } else { - if !rt.timer.Stop() { - <-rt.timer.C - } - - rt.timer.Reset(delay) - } + if delay > 0 { + rt.timer.Reset(delay) } } // Clear should be called after receiving from the timer channel. -func (rt *resettableTimer) Clear() { - rt.mx.Lock() - defer rt.mx.Unlock() - - rt.timer = nil +func (rt *ResettableTimer) Clear() { + rt.Reset(0) } // C returns the timer channel. -// -// If the timer was not reset to a non-zero duration, nil is returned. -func (rt *resettableTimer) C() <-chan time.Time { - rt.mx.Lock() - defer rt.mx.Unlock() - - if rt.timer == nil { - return nil - } - +func (rt *ResettableTimer) C() <-chan time.Time { return rt.timer.C } diff --git a/internal/app/machined/pkg/controllers/siderolink/userspace_test.go b/internal/app/machined/pkg/controllers/siderolink/userspace_test.go new file mode 100644 index 00000000000..d42de79e45d --- /dev/null +++ b/internal/app/machined/pkg/controllers/siderolink/userspace_test.go @@ -0,0 +1,53 @@ +// This Source Code Form is subject to the terms of the Mozilla Public +// License, v. 2.0. If a copy of the MPL was not distributed with this +// file, You can obtain one at http://mozilla.org/MPL/2.0/. + +package siderolink_test + +import ( + "testing" + "time" + + "github.com/stretchr/testify/require" + + "github.com/siderolabs/talos/internal/app/machined/pkg/controllers/siderolink" +) + +func TestResettableTimer(t *testing.T) { + t.Parallel() + + t.Run("a retry armed while the loop is parked wakes it", func(t *testing.T) { + t.Parallel() + + timer := siderolink.NewResettableTimer() + + timer.Reset(0) + + // Capture the channel the way a parked select does, before the retry is armed. + parked := timer.C() + require.NotNil(t, parked, "the timer channel must stay non-nil while the timer is stopped") + + timer.Reset(10 * time.Millisecond) + + select { + case <-parked: + case <-time.After(time.Second): + t.Fatal("the retry never fired, the relay would never be restarted") + } + }) + + t.Run("reset to zero stops a pending firing", func(t *testing.T) { + t.Parallel() + + timer := siderolink.NewResettableTimer() + + timer.Reset(10 * time.Millisecond) + timer.Reset(0) + + select { + case <-timer.C(): + t.Fatal("the timer fired after it was stopped with Reset(0)") + case <-time.After(100 * time.Millisecond): + } + }) +}