From 03dabbcdabff03aec4167a1dbca5ddbf2dbf3bb7 Mon Sep 17 00:00:00 2001 From: "pulse-triage[bot]" <249995291+pulse-triage[bot]@users.noreply.github.com> Date: Thu, 10 Sep 2026 21:04:37 +0100 Subject: [PATCH] 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 --- .../monitoring/canonical_guardrails_test.go | 3 +- internal/monitoring/monitor.go | 18 +++- .../monitoring/monitor_state_reset_test.go | 82 +++++++++++++++++++ 3 files changed, 98 insertions(+), 5 deletions(-) create mode 100644 internal/monitoring/monitor_state_reset_test.go diff --git a/internal/monitoring/canonical_guardrails_test.go b/internal/monitoring/canonical_guardrails_test.go index b5c380d7c..b30084293 100644 --- a/internal/monitoring/canonical_guardrails_test.go +++ b/internal/monitoring/canonical_guardrails_test.go @@ -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()", } { diff --git a/internal/monitoring/monitor.go b/internal/monitoring/monitor.go index 486079b4d..b37b64020 100644 --- a/internal/monitoring/monitor.go +++ b/internal/monitoring/monitor.go @@ -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. diff --git a/internal/monitoring/monitor_state_reset_test.go b/internal/monitoring/monitor_state_reset_test.go new file mode 100644 index 000000000..46fa8399f --- /dev/null +++ b/internal/monitoring/monitor_state_reset_test.go @@ -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") + } +}