Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 22 additions & 42 deletions internal/app/machined/pkg/controllers/siderolink/userspace.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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 {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

To keep the changes minimal, I kept this type but fixed its internals/usage. But considered removing it completely, because I'm not sure if t's a good abstraction. If you, the reviewer, would like me to do that, say the word :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this type tried to wrap old Go problems with resetting timers without consuming from the channel.

And it adds a mutex

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)
}
Comment thread
utkuozdemir marked this conversation as resolved.
}

// 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
}
53 changes: 53 additions & 0 deletions internal/app/machined/pkg/controllers/siderolink/userspace_test.go
Original file line number Diff line number Diff line change
@@ -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):
}
})
}
Loading