Prevents engine not stopping within 10 secs from being paired by another instance

We instead juts SKIP updating the policy, so
1. the MDM ticker will kick in 1 minute time,
2. find the policy misaligned,
3. enter the onMDMPolicyChange,
4. find the s.clientRunning == true
   (because it is set to false only in server cleanupConnection,
   and not by s.actCancel())
5. call s.actCancel() again if not nil
6. immediately return from <-s.clientGiveUpChan
7. finally call s.restartEngineForMDMLocked()
This commit is contained in:
riccardom
2026-06-11 12:50:04 +02:00
parent 650d2e3d58
commit 0206e318d1
3 changed files with 21 additions and 10 deletions

View File

@@ -27,7 +27,7 @@ var policyLoader = LoadPolicy
// goroutine; cancel the supplied context to stop.
type Ticker struct {
interval time.Duration
onChange func(prev, curr *Policy)
onChange func(prev, curr *Policy) error
prev *Policy
}
@@ -39,7 +39,7 @@ type Ticker struct {
// onChange when the policy actually changed since boot — without
// this baseline the first tick would report every currently-managed
// key as "added" and trigger a spurious engine restart.
func NewTicker(reloadInterval time.Duration, onChange func(prev, curr *Policy)) *Ticker {
func NewTicker(reloadInterval time.Duration, onChange func(prev, curr *Policy) error) *Ticker {
return &Ticker{
interval: reloadInterval,
onChange: onChange,
@@ -68,9 +68,13 @@ func (t *Ticker) Run(ctx context.Context) {
log.Infof("MDM policy changed: added=%v removed=%v changed=%v",
added, removed, changed)
prev := t.prev
t.prev = curr
if t.onChange != nil {
t.onChange(prev, curr)
if err := t.onChange(prev, curr); err != nil {
log.Errorf("MDM policy change handler failed (retrying in 1 minute): %v", err)
continue
}
t.prev = curr
}
}
}

View File

@@ -34,11 +34,12 @@ func TestTicker_FiresOnChangeWithDelta(t *testing.T) {
type change struct{ prev, curr *Policy }
changes := make(chan change, 1)
tk := NewTicker(testReloadInterval, func(prev, curr *Policy) {
tk := NewTicker(testReloadInterval, func(prev, curr *Policy) error {
select {
case changes <- change{prev, curr}:
default:
}
return nil
})
require.Equal(t, testReloadInterval, tk.interval)
@@ -70,11 +71,12 @@ func TestTicker_NoCallbackWhenPolicyUnchanged(t *testing.T) {
})
fired := make(chan struct{}, 1)
tk := NewTicker(testReloadInterval, func(_, _ *Policy) {
tk := NewTicker(testReloadInterval, func(_, _ *Policy) error {
select {
case fired <- struct{}{}:
default:
}
return nil
})
ctx, cancel := context.WithCancel(context.Background())

View File

@@ -33,7 +33,7 @@ var loadMDMPolicy = mdm.LoadPolicy
//
// The callback runs in the ticker's own goroutine. Ticker has already
// logged the per-key diff before invoking this hook.
func (s *Server) onMDMPolicyChange(_, curr *mdm.Policy) {
func (s *Server) onMDMPolicyChange(_, curr *mdm.Policy) error {
log.Warn("MDM policy changed; restarting engine to apply new configuration")
// Hold s.mutex for the entire restart sequence (cancel + quiescence
@@ -46,6 +46,10 @@ func (s *Server) onMDMPolicyChange(_, curr *mdm.Policy) {
s.mutex.Lock()
defer s.mutex.Unlock()
if !s.clientRunning {
// The client is not running, so there's no engine to restart.
return nil
}
if s.actCancel != nil {
s.actCancel()
}
@@ -60,14 +64,14 @@ func (s *Server) onMDMPolicyChange(_, curr *mdm.Policy) {
if s.clientGiveUpChan != nil {
select {
case <-s.clientGiveUpChan:
case <-time.After(5 * time.Second):
log.Warn("MDM restart: timeout waiting for previous engine goroutine; proceeding anyway")
case <-time.After(10 * time.Second):
return fmt.Errorf("failed to restart the engine due to timeout")
}
}
if err := s.restartEngineForMDMLocked(); err != nil {
log.Errorf("MDM restart failed: %v", err)
return
return err
}
// publishConfigChangedEvent has already fired inside
@@ -82,6 +86,7 @@ func (s *Server) onMDMPolicyChange(_, curr *mdm.Policy) {
"NetBird configuration was updated by your IT policy.",
map[string]string{"source": "mdm", "type": "policy_applied"},
)
return nil
}
// publishConfigChangedEvent broadcasts a SystemEvent informing any active