fix(monitoring): synchronise snapshots with mock state reset

Mock-mode reset replaces the state pointer while API and mock-alert readers can still be active. Capture the initialized pointer under the monitor lock and preserve its fallback alerts rather than reading the replaceable pointer again.

Reproduce the race without Start workers, verify reset snapshot isolation, and retain live alert-manager precedence. This repairs the beta.3 preparation failure without masking its test cleanup schedule or changing monitoring schemas.

Contract-Neutral: Synchronization-only snapshot repair; resource identity, API fields, alert policy and canonical ownership are unchanged.
Change-source: pulse-maintainer
This commit is contained in:
pulse-triage[bot] 2026-09-10 21:04:37 +01:00
parent 1b420b506f
commit 03dabbcdab
3 changed files with 98 additions and 5 deletions

View file

@ -462,7 +462,8 @@ func TestGetStateRefreshesLiveAlertSnapshots(t *testing.T) {
source := string(data)
for _, snippet := range []string{
"state := m.state.GetSnapshot()",
"currentState := m.state",
"state := currentState.GetSnapshot()",
"state.ActiveAlerts = m.activeAlertsSnapshot()",
"state.RecentlyResolved = m.recentlyResolvedAlertsSnapshot()",
} {

View file

@ -3163,16 +3163,26 @@ func (m *Monitor) GetState() models.StateSnapshot {
}
return state
}
if m.state == nil {
// Mock-mode transitions replace the state under m.mu. Capture the fully
// initialized pointer under the same lock, then let State protect its own
// snapshot without holding the monitor lock across downstream readers.
m.mu.RLock()
currentState := m.state
m.mu.RUnlock()
if currentState == nil {
return models.StateSnapshot{}
}
state := m.state.GetSnapshot()
state := currentState.GetSnapshot()
// Keep externally served alert arrays aligned with the live alert manager
// even between explicit sync points, so APIs do not expose stale alert
// counts or recently resolved incidents from cached state.
state.ActiveAlerts = m.activeAlertsSnapshot()
state.RecentlyResolved = m.recentlyResolvedAlertsSnapshot()
if m.alertManager != nil {
state.ActiveAlerts = m.activeAlertsSnapshot()
state.RecentlyResolved = m.recentlyResolvedAlertsSnapshot()
}
// Without an alert manager, retain alerts from this same captured state;
// the fallback helpers would read the replaceable pointer again.
// Surface filesystems reported by a unified pulse-agent inside a guest
// (for example ZFS mounts that qemu-guest-agent's get-fsinfo cannot see
// on PBS, #1438) in the guest overview disk listing.

View file

@ -0,0 +1,82 @@
package monitoring
import (
"sync"
"testing"
"time"
"github.com/rcourtman/pulse-go-rewrite/internal/mock"
"github.com/rcourtman/pulse-go-rewrite/internal/models"
)
// Exercise the production reset boundary without Start or its background
// goroutines: shutting down a test differently cannot repair this race.
func TestGetStateConcurrentReset(t *testing.T) {
previous := mock.IsMockEnabled()
mustSetMockEnabled(t, false)
defer mustSetMockEnabled(t, previous)
startTime := time.Unix(1700000000, 0)
m := &Monitor{startTime: startTime}
m.mu.Lock()
m.resetStateLocked()
m.mu.Unlock()
start := make(chan struct{})
var workers sync.WaitGroup
workers.Add(2)
go func() {
defer workers.Done()
<-start
for i := 0; i < 1000; i++ {
m.mu.Lock()
m.resetStateLocked()
m.mu.Unlock()
}
}()
go func() {
defer workers.Done()
<-start
for i := 0; i < 1000; i++ {
snapshot := m.GetState()
if snapshot.Stats.StartTime != startTime || snapshot.Stats.Version != "2.0.0-go" {
t.Errorf("reader observed partially initialized reset stats: %+v", snapshot.Stats)
return
}
}
}()
close(start)
workers.Wait()
}
func TestGetStateResetSnapshotIsolation(t *testing.T) {
previous := mock.IsMockEnabled()
mustSetMockEnabled(t, false)
defer mustSetMockEnabled(t, previous)
m := &Monitor{state: models.NewState(), startTime: time.Unix(1700000000, 0)}
m.state.UpdateNodes([]models.Node{{ID: "old-node", Name: "old-node"}})
m.state.UpdateActiveAlerts([]models.Alert{{ID: "old-alert"}})
m.state.UpdateRecentlyResolved([]models.ResolvedAlert{{Alert: models.Alert{ID: "old-resolved"}}})
before := m.GetState()
m.mu.Lock()
m.resetStateLocked()
m.mu.Unlock()
after := m.GetState()
if len(before.Nodes) != 1 || before.Nodes[0].ID != "old-node" {
t.Fatal("reset changed the previously returned snapshot")
}
if len(before.ActiveAlerts) != 1 || before.ActiveAlerts[0].ID != "old-alert" ||
len(before.RecentlyResolved) != 1 || before.RecentlyResolved[0].ID != "old-resolved" {
t.Fatal("snapshot lost state-backed alerts without an alert manager")
}
if len(after.ActiveAlerts) != 0 || len(after.RecentlyResolved) != 0 {
t.Fatal("reset retained stale state-backed alerts")
}
if len(after.Nodes) != 0 || after.Stats.StartTime != m.startTime || after.Stats.Version != "2.0.0-go" {
t.Fatalf("reset did not expose an initialized empty state: %+v", after)
}
if snapshot := (&Monitor{}).GetState(); len(snapshot.Nodes) != 0 {
t.Fatal("nil state must return an empty snapshot")
}
}