diff --git a/internal/alerts/alerts_system.go b/internal/alerts/alerts_system.go index 11226b8c..edba9454 100644 --- a/internal/alerts/alerts_system.go +++ b/internal/alerts/alerts_system.go @@ -39,14 +39,7 @@ func cpuStateAlertValue(name string, breakdown []float64) (float64, bool) { func (am *AlertManager) HandleSystemAlerts(systemRecord *core.Record, data *system.CombinedData) error { // Systemd alerts are binary state, not numeric thresholds, so they're handled // separately. They read their own state from the database and don't use data. - // Read the confirmed empty state from the record being saved instead of data: - // dashboard polling can replace the system's in-memory payload concurrently. - var currentInfo system.Info - confirmedEmptySnapshot := false - if err := systemRecord.UnmarshalJSONField("info", ¤tInfo); err == nil { - confirmedEmptySnapshot = len(currentInfo.Services) > 0 && currentInfo.Services[0] == 0 - } - if err := am.HandleSystemdAlerts(systemRecord, confirmedEmptySnapshot); err != nil { + if err := am.HandleSystemdAlerts(systemRecord); err != nil { am.hub.Logger().Error("Error handling systemd alerts", "err", err) } diff --git a/internal/alerts/alerts_systemd.go b/internal/alerts/alerts_systemd.go index 567cbf4b..271cc255 100644 --- a/internal/alerts/alerts_systemd.go +++ b/internal/alerts/alerts_systemd.go @@ -4,6 +4,7 @@ import ( "fmt" "strings" + "github.com/henrygd/beszel/internal/entities/system" "github.com/henrygd/beszel/internal/entities/systemd" "github.com/pocketbase/dbx" @@ -22,7 +23,7 @@ const maxListedServices = 10 // service rather than using a delay. The agent only refreshes systemd state every // 10 minutes, so a shorter delay could never observe new data before expiring, and // that poll interval already hides services that fail and restart quickly. -func (am *AlertManager) HandleSystemdAlerts(systemRecord *core.Record, confirmedEmptySnapshot bool) error { +func (am *AlertManager) HandleSystemdAlerts(systemRecord *core.Record) error { alerts := am.alertsCache.GetAlertsByName(systemRecord.Id, alertNameSystemdFailed) if len(alerts) == 0 { return nil @@ -37,11 +38,17 @@ func (am *AlertManager) HandleSystemdAlerts(systemRecord *core.Record, confirmed if err != nil { return err } - // No rows normally means no systemd data for this system (agent without systemd, - // or not yet reported), which must not be treated as a recovery. A fresh snapshot - // marker disambiguates that case from an agent explicitly reporting zero services. - if total == 0 && !confirmedEmptySnapshot { - return nil + if total == 0 { + // No rows normally means no systemd data for this system (agent without + // systemd, or not yet reported), which must not be treated as a recovery. + // Read info only in this ambiguous case. The record being saved is used + // instead of data because dashboard polling can replace the system's + // in-memory payload concurrently. + var currentInfo system.Info + if err := systemRecord.UnmarshalJSONField("info", ¤tInfo); err != nil || + len(currentInfo.Services) == 0 || currentInfo.Services[0] != 0 { + return nil + } } systemName := systemRecord.GetString("name") diff --git a/internal/alerts/alerts_systemd_test.go b/internal/alerts/alerts_systemd_test.go index b6e6ee28..7d8a119b 100644 --- a/internal/alerts/alerts_systemd_test.go +++ b/internal/alerts/alerts_systemd_test.go @@ -91,7 +91,7 @@ func TestSystemdAlertFiresImmediately(t *testing.T) { am := alerts.NewTestAlertManagerWithoutWorker(hub) seedServices(t, hub, system.Id, systemd.StatusFailed, systemd.StatusActive) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) assert.Equal(t, initialEmailCount+1, hub.TestMailer.TotalSend(), "failed service should notify on first observation") @@ -120,9 +120,9 @@ func TestSystemdAlertFullCycle(t *testing.T) { // Fail, then recover. seedServices(t, hub, system.Id, systemd.StatusFailed) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) seedServices(t, hub, system.Id, systemd.StatusActive) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) assert.Equal(t, initialEmailCount+2, hub.TestMailer.TotalSend(), "should send a failure and a recovery notification") @@ -149,7 +149,7 @@ func TestSystemdAlertSendsRecoveryWhenTriggered(t *testing.T) { am := alerts.NewTestAlertManagerWithoutWorker(hub) seedServices(t, hub, system.Id, systemd.StatusActive, systemd.StatusInactive) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) assert.Equal(t, initialEmailCount+1, hub.TestMailer.TotalSend(), "recovery notification should be sent") messages := hub.TestMailer.Messages() @@ -171,7 +171,7 @@ func TestSystemdAlertDoesNotResendWhileTriggered(t *testing.T) { // Still failing across several cycles — should not re-notify. for range 3 { seedServices(t, hub, system.Id, systemd.StatusFailed) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) } assert.Equal(t, initialEmailCount, hub.TestMailer.TotalSend(), "should not re-notify while still triggered") @@ -186,7 +186,7 @@ func TestSystemdAlertRepeatedFailureNotifiesOnce(t *testing.T) { for range 3 { seedServices(t, hub, system.Id, systemd.StatusFailed) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) } assert.Equal(t, initialEmailCount+1, hub.TestMailer.TotalSend(), "repeated failures should only notify once") @@ -208,7 +208,7 @@ func TestSystemdAlertIgnoresServicesNoLongerReported(t *testing.T) { // Current batch reports only healthy services. seedServicesAt(t, hub, system.Id, now, systemd.StatusActive, systemd.StatusActive) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) assert.Equal(t, initialEmailCount+1, hub.TestMailer.TotalSend(), "stale failed row should not block recovery") alertRecord, err := hub.FindRecordById("alerts", alert.Id) @@ -240,8 +240,8 @@ func TestSystemdAlertNoSystemdDataIsIgnored(t *testing.T) { // A system with no systemd_services rows (agent without systemd, or nothing // reported yet) must not be treated as a recovery. - require.NoError(t, am.HandleSystemdAlerts(system, false)) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) + require.NoError(t, am.HandleSystemdAlerts(system)) assert.Equal(t, initialEmailCount, hub.TestMailer.TotalSend(), "missing systemd data should not send a recovery") alertRecord, err := hub.FindRecordById("alerts", alert.Id) @@ -279,7 +279,7 @@ func TestSystemdAlertNoAlertRecord(t *testing.T) { am := alerts.NewTestAlertManagerWithoutWorker(hub) seedServices(t, hub, system.Id, systemd.StatusFailed) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) assert.Equal(t, initialEmailCount, hub.TestMailer.TotalSend(), "no email when no alert record exists") } @@ -376,7 +376,7 @@ func TestSystemdAlertMultipleUsersRespectOwnAlerts(t *testing.T) { am := alerts.NewTestAlertManagerWithoutWorker(hub) seedServices(t, hub, system.Id, systemd.StatusFailed) - require.NoError(t, am.HandleSystemdAlerts(system, false)) + require.NoError(t, am.HandleSystemdAlerts(system)) messages := hub.TestMailer.Messages() require.Len(t, messages, 2, "each user should receive their own alert")