Skip to content

Commit 8dcab21

Browse files
authored
Fix daemon reload with backoff SIGINT handling (#4179)
* Fix daemon reload with backoff SIGINT handling * Add unit tests
1 parent ff0b7b5 commit 8dcab21

File tree

2 files changed

+60
-12
lines changed

2 files changed

+60
-12
lines changed

internal/pkg/agent/cmd/enroll_cmd.go

+22-12
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,9 @@ type enrollCmd struct {
7272
remoteConfig remote.Config
7373
agentProc *process.Info
7474
configPath string
75+
76+
// For testability
77+
daemonReloadFunc func(context.Context) error
7578
}
7679

7780
// enrollCmdFleetServerOption define all the supported enrollment options for bootstrapping with Fleet Server.
@@ -192,10 +195,11 @@ func newEnrollCmdWithStore(
192195
store saver,
193196
) (*enrollCmd, error) {
194197
return &enrollCmd{
195-
log: log,
196-
options: options,
197-
configStore: store,
198-
configPath: configPath,
198+
log: log,
199+
options: options,
200+
configStore: store,
201+
configPath: configPath,
202+
daemonReloadFunc: daemonReload,
199203
}, nil
200204
}
201205

@@ -462,17 +466,21 @@ func (c *enrollCmd) prepareFleetTLS() error {
462466
return nil
463467
}
464468

469+
const (
470+
daemonReloadInitBackoff = time.Second
471+
daemonReloadMaxBackoff = time.Minute
472+
daemonReloadRetries = 5
473+
)
474+
465475
func (c *enrollCmd) daemonReloadWithBackoff(ctx context.Context) error {
466-
signal := make(chan struct{})
467-
defer close(signal)
468-
backExp := backoff.NewExpBackoff(signal, 1*time.Second, 1*time.Minute)
476+
backExp := backoff.NewExpBackoff(ctx.Done(), daemonReloadInitBackoff, daemonReloadMaxBackoff)
469477

470478
var lastErr error
471-
for i := 0; i < 5; i++ {
479+
for i := 0; i < daemonReloadRetries; i++ {
472480
attempt := i
473481

474482
c.log.Infof("Restarting agent daemon, attempt %d", attempt)
475-
err := c.daemonReload(ctx)
483+
err := c.daemonReloadFunc(ctx)
476484
if err == nil {
477485
return nil
478486
}
@@ -486,14 +494,16 @@ func (c *enrollCmd) daemonReloadWithBackoff(ctx context.Context) error {
486494
lastErr = err
487495

488496
c.log.Errorf("Restart attempt %d failed: '%s'. Waiting for %s", attempt, err, backExp.NextWait().String())
489-
backExp.Wait()
490-
497+
// backoff Wait returns false if context.Done()
498+
if !backExp.Wait() {
499+
return ctx.Err()
500+
}
491501
}
492502

493503
return fmt.Errorf("could not reload agent's daemon, all retries failed. Last error: %w", lastErr)
494504
}
495505

496-
func (c *enrollCmd) daemonReload(ctx context.Context) error {
506+
func daemonReload(ctx context.Context) error {
497507
daemon := client.New()
498508
err := daemon.Connect(ctx)
499509
if err != nil {

internal/pkg/agent/cmd/enroll_cmd_test.go

+38
Original file line numberDiff line numberDiff line change
@@ -463,6 +463,44 @@ func TestValidateEnrollFlags(t *testing.T) {
463463
})
464464
}
465465

466+
func TestDaemonReloadWithBackoff(t *testing.T) {
467+
log, _ := logger.New("tst", false)
468+
469+
ctx, cn := context.WithCancel(context.Background())
470+
// Cancel context
471+
cn()
472+
473+
tests := []struct {
474+
name string
475+
daemonReloadFunc func(ctx context.Context) error
476+
wantErr error
477+
}{
478+
{
479+
name: "daemonReloadSucceeded",
480+
daemonReloadFunc: func(ctx context.Context) error { return nil },
481+
},
482+
{
483+
name: "retryWithContextCancelled",
484+
daemonReloadFunc: func(ctx context.Context) error {
485+
return errors.New("failed") // Return some (not context's) error so it retries
486+
},
487+
wantErr: context.Canceled,
488+
},
489+
}
490+
491+
for _, tc := range tests {
492+
t.Run(tc.name, func(t *testing.T) {
493+
cmd := enrollCmd{
494+
log: log,
495+
daemonReloadFunc: tc.daemonReloadFunc,
496+
}
497+
498+
err := cmd.daemonReloadWithBackoff(ctx)
499+
assert.ErrorIs(t, err, tc.wantErr)
500+
})
501+
}
502+
}
503+
466504
func withServer(
467505
m func(t *testing.T) *http.ServeMux,
468506
test func(t *testing.T, host string),

0 commit comments

Comments
 (0)