restarts alert: one rule per unit, a watch that survives resets, gaps and the agent (#284)
Summary
Answers Astra's code review of slice A (4 P1, 3 P2).
- The restarts rule is per unit ("restarts@<unit>"), so a second unit
that starts looping is its own page. RuleState/Event lose the detail
field; the unit is in the rule name.
- The watch counts dated rises of the counter. A manual restart's reset
adds the post-reset count instead of wiping the history; a gap in
sampling longer than the window starts again from the present.
- The watch is saved after each tick and sampled once at agent start,
so the agent's own restart loop is counted.
- A unit clears only when settled for a whole window and not failed or
mid-restart.
- systemctl is called with a 5s deadline.
- The drop-in is written atomically through safefile, and a failed
daemon-reload puts the previous file back so the next run retries.
Census overlay regenerated: seven rows, seal d17b4d9b.Agent notes
The verb's two warning strings became one, and "a platform unit" is gone; the copy review must be relaunched with the new strings.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
15 files changed,
+628
−282
internal/console/alertdetail_test.go+27 −23internal/console/alertwatch.go+10 −8internal/console/metricsview.go+4 −1internal/host/alertsink.go+8 −14internal/host/alertsink_test.go+2 −2internal/host/metrics_loop.go+31 −15internal/host/restartwatch.go+155 −68internal/host/restartwatch_test.go+128 −61internal/metrics/alert.go+75 −27internal/metrics/alert_test.go+88 −23internal/ops/litestream_memory.go+37 −9internal/ops/ops.go+5 −6internal/ops/ops_test.go+44 −15internal/provision/plan14_root_reads_test.go+2 −2internal/provision/testdata/plan14-root-reads-ls284.tsv+12 −8
CI passed — run details
CI log
=== gate ===
cd launchctx && go vet ./... && go test ./... -count=1
ok amadan.net/carlos/platform.git/launchctx 0.011s
? amadan.net/carlos/platform.git/launchctx/controlname [no test files]
? amadan.net/carlos/platform.git/launchctx/wireid [no test files]
go vet ./...
gofmt -l .: clean
go test ./... -count=1 -timeout 30m
ok github.com/carlosframework/platform 334.834s
? github.com/carlosframework/platform/cmd/changelog [no test files]
ok github.com/carlosframework/platform/cmd/console 3.195s
ok github.com/carlosframework/platform/cmd/moveprobe 92.269s
ok github.com/carlosframework/platform/cmd/zddprobe 0.380s
ok github.com/carlosframework/platform/internal/activator 227.170s
ok github.com/carlosframework/platform/internal/agentsock 0.017s
ok github.com/carlosframework/platform/internal/alerting 0.004s
ok github.com/carlosframework/platform/internal/analytics 0.022s
ok github.com/carlosframework/platform/internal/appconfig 2.343s
ok github.com/carlosframework/platform/internal/apperr 0.075s
ok github.com/carlosframework/platform/internal/applog 0.131s
ok github.com/carlosframework/platform/internal/awsinv 0.004s
ok github.com/carlosframework/platform/internal/boxchoice 0.006s
ok github.com/carlosframework/platform/internal/boxid 0.989s
? github.com/carlosframework/platform/internal/boxid/boxidtest [no test files]
? github.com/carlosframework/platform/internal/boxid/internal/signtest [no test files]
ok github.com/carlosframework/platform/internal/boxvolumes 0.008s
ok github.com/carlosframework/platform/internal/caps 0.011s
ok github.com/carlosframework/platform/internal/certwarm 0.002s
ok github.com/carlosframework/platform/internal/cgroup 0.694s
ok github.com/carlosframework/platform/internal/changelog 0.297s
ok github.com/carlosframework/platform/internal/clamd 0.055s
? github.com/carlosframework/platform/internal/cliflag [no test files]
ok github.com/carlosframework/platform/internal/console 364.061s
? github.com/carlosframework/platform/internal/controlname [no test files]
? github.com/carlosframework/platform/internal/controlsock [no test files]
ok github.com/carlosframework/platform/internal/dbdoor 6.238s
ok github.com/carlosframework/platform/internal/dbdoor/dbdoortest 0.008s
ok github.com/carlosframework/platform/internal/dbverify 0.004s
ok github.com/carlosframework/platform/internal/dnsrec 0.002s
ok github.com/carlosframework/platform/internal/docsite 0.005s
ok github.com/carlosframework/platform/internal/domains 2.144s
ok github.com/carlosframework/platform/internal/doorkey 0.006s
ok github.com/carlosframework/platform/internal/economics 0.004s
ok github.com/carlosframework/platform/internal/edge 26.013s
? github.com/carlosframework/platform/internal/edgeproof [no test files]
ok github.com/carlosframework/platform/internal/edgesfile 0.007s
ok github.com/carlosframework/platform/internal/fleetapi 0.119s
ok github.com/carlosframework/platform/internal/gate 1.655s
ok github.com/carlosframework/platform/internal/genselect 0.015s
ok github.com/carlosframework/platform/internal/geo 0.021s
ok github.com/carlosframework/platform/internal/geoip 0.029s
ok github.com/carlosframework/platform/internal/host 126.714s
ok github.com/carlosframework/platform/internal/hostactors 0.560s
ok github.com/carlosframework/platform/internal/hostbox 2.147s
ok github.com/carlosframework/platform/internal/hostclaim 0.519s
ok github.com/carlosframework/platform/internal/hostevidence 0.014s
ok github.com/carlosframework/platform/internal/hostexec 4.913s
ok github.com/carlosframework/platform/internal/hostfence 0.033s
ok github.com/carlosframework/platform/internal/hostfiles 10.478s
ok github.com/carlosframework/platform/internal/hostfloor 0.014s
ok github.com/carlosframework/platform/internal/hostguard 0.967s
ok github.com/carlosframework/platform/internal/hostops 1.463s
? github.com/carlosframework/platform/internal/hostops/fencerelease [no test files]
ok github.com/carlosframework/platform/internal/hostops/internal/fencerelease 0.015s
ok github.com/carlosframework/platform/internal/hostquarantine 0.014s
ok github.com/carlosframework/platform/internal/hostrelocate 0.445s
ok github.com/carlosframework/platform/internal/hostresolve 0.136s
ok github.com/carlosframework/platform/internal/hostwrite 0.015s
ok github.com/carlosframework/platform/internal/infraview 0.014s
ok github.com/carlosframework/platform/internal/instancectl 1.832s
ok github.com/carlosframework/platform/internal/instanceexec 2.707s
ok github.com/carlosframework/platform/internal/instancefs 0.019s
ok github.com/carlosframework/platform/internal/instancekey 0.016s
ok github.com/carlosframework/platform/internal/instanceupdates 0.020s
ok github.com/carlosframework/platform/internal/layout 1.427s
ok github.com/carlosframework/platform/internal/lease 9.447s
ok github.com/carlosframework/platform/internal/lease/leasetest 0.005s
ok github.com/carlosframework/platform/internal/ledger 0.012s
ok github.com/carlosframework/platform/internal/mail 0.182s
ok github.com/carlosframework/platform/internal/maintenance 0.088s
ok github.com/carlosframework/platform/internal/maintenanceexec 5.551s
ok github.com/carlosframework/platform/internal/maintenanceguard 0.082s
ok github.com/carlosframework/platform/internal/metrics 0.007s
ok github.com/carlosframework/platform/internal/moves 21.069s
ok github.com/carlosframework/platform/internal/moves/archive 0.751s
ok github.com/carlosframework/platform/internal/objstore 0.183s
ok github.com/carlosframework/platform/internal/ops 0.165s
ok github.com/carlosframework/platform/internal/park 0.197s
ok github.com/carlosframework/platform/internal/privatefile 0.004s
ok github.com/carlosframework/platform/internal/provision 13.054s
ok github.com/carlosframework/platform/internal/publicrelease 0.009s
? github.com/carlosframework/platform/internal/regionname [no test files]
ok github.com/carlosframework/platform/internal/registry 4.074s
ok github.com/carlosframework/platform/internal/relay 7.233s
ok github.com/carlosframework/platform/internal/release 19.607s
ok github.com/carlosframework/platform/internal/rollout 3.785s
ok github.com/carlosframework/platform/internal/rolloutwords 0.003s
ok github.com/carlosframework/platform/internal/rootaws 0.007s
ok github.com/carlosframework/platform/internal/rootparticipation 0.007s
ok github.com/carlosframework/platform/internal/rootstate 0.025s
ok github.com/carlosframework/platform/internal/s3compat 0.004s
ok github.com/carlosframework/platform/internal/safefile 0.003s
ok github.com/carlosframework/platform/internal/schedule 2.324s
ok github.com/carlosframework/platform/internal/sim/hostlife 9.552s
ok github.com/carlosframework/platform/internal/sim/litestreamfacts 3.729s
ok github.com/carlosframework/platform/internal/status 0.039s
ok github.com/carlosframework/platform/internal/steering 0.005s
ok github.com/carlosframework/platform/internal/stopevidence 0.171s
ok github.com/carlosframework/platform/internal/svcrole 0.002s
ok github.com/carlosframework/platform/internal/telemetry 0.004s
ok github.com/carlosframework/platform/internal/termfence 0.004s
ok github.com/carlosframework/platform/internal/tunnelclient 0.004s
ok github.com/carlosframework/platform/internal/unixdial 0.002s
ok github.com/carlosframework/platform/internal/updatewords 0.004s
ok github.com/carlosframework/platform/internal/usage 0.005s
ok github.com/carlosframework/platform/internal/version 0.001s
ok github.com/carlosframework/platform/internal/vet 1.547s
=== cross ===
--- shard 0 of 5 ---
cross-compiling darwin/arm64
--- shard 1 of 5 ---
cross-compiling darwin/amd64
--- shard 2 of 5 ---
cross-compiling linux/arm64
--- shard 3 of 5 ---
cross-compiling windows/amd64
--- shard 4 of 5 ---
cross-compiling windows/arm64
=== minio-contract ===
minio contract: arm64, minio RELEASE.2025-09-07T16-13-09Z, mc RELEASE.2025-08-13T08-35-41Z
minio: downloading https://github.com/minio/minio/releases/download/RELEASE.2025-09-07T16-13-09Z/minio.linux-arm64.RELEASE.2025-09-07T16-13-09Z
minio: downloaded, digest verified
mc: downloading https://github.com/minio/mc/releases/download/RELEASE.2025-08-13T08-35-41Z/mc.linux-arm64.RELEASE.2025-08-13T08-35-41Z
mc: downloaded, digest verified
minio healthy on 127.0.0.1:20999
Added `local` successfully.
Bucket created successfully `local/contract-bucket`.
Bucket created successfully `local/denied-bucket`.
Created policy `contract-only` successfully.
Added user `scopeduser` successfully.
Attached Policies: [contract-only]
To User: scopeduser
buckets and scoped user ready
=== RUN TestDeliveredContractMinio
=== RUN TestDeliveredContractMinio/put
=== RUN TestDeliveredContractMinio/get_round-trips_the_body
=== RUN TestDeliveredContractMinio/list_shows_the_key
=== RUN TestDeliveredContractMinio/head_and_list_agree_on_the_version
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_its_own_object
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/not_by_PutObjectTagging
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/and_not_by_x-amz-tagging_on_upload
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CreateMultipartUpload
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CopyObject_with_a_REPLACE_tagging_directive
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/but_multipart_itself_is_allowed
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_copy_itself_is_allowed
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/a_POST-policy_forged_tag_with_a_matching_version_is_still_not_a_verdict
=== RUN TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_an_ordinary_tagged-with-something-else_upload_is_allowed
=== RUN TestDeliveredContractMinio/delete_removes_it
=== RUN TestDeliveredContractMinio/cross-bucket_denial
--- PASS: TestDeliveredContractMinio (0.02s)
--- PASS: TestDeliveredContractMinio/put (0.00s)
--- PASS: TestDeliveredContractMinio/get_round-trips_the_body (0.00s)
--- PASS: TestDeliveredContractMinio/list_shows_the_key (0.00s)
--- PASS: TestDeliveredContractMinio/head_and_list_agree_on_the_version (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/not_by_PutObjectTagging (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_its_own_object/and_not_by_x-amz-tagging_on_upload (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route (0.01s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CreateMultipartUpload (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/not_by_CopyObject_with_a_REPLACE_tagging_directive (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/but_multipart_itself_is_allowed (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_copy_itself_is_allowed (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/a_POST-policy_forged_tag_with_a_matching_version_is_still_not_a_verdict (0.00s)
--- PASS: TestDeliveredContractMinio/the_app_cannot_tag_by_any_other_route/and_an_ordinary_tagged-with-something-else_upload_is_allowed (0.00s)
--- PASS: TestDeliveredContractMinio/delete_removes_it (0.00s)
--- PASS: TestDeliveredContractMinio/cross-bucket_denial (0.00s)
PASS
ok github.com/carlosframework/platform/internal/objstore 0.022s
=== RUN TestT4StorageProbeMinio
2026/10/14 00:12:29 moves: T4: STORE: list: operation error S3: ListObjectsV2, https response error StatusCode: 403, RequestID: 18DE3CFAB9E5A451, HostID: 7c420053e0fbcd7f41653fdd92ffa2802c903d05303b1aa15adc2e8bb65d183f, api error SignatureDoesNotMatch: The request signature we calculated does not match the signature you provided. Check your key and signing method.
--- PASS: TestT4StorageProbeMinio (0.00s)
PASS
ok github.com/carlosframework/platform/internal/host 0.022s
minio contract: PASS
diff --git a/internal/console/alertdetail_test.go b/internal/console/alertdetail_test.go| index 2af025d52..2c83b1ccd 100644 |
| --- a/internal/console/alertdetail_test.go |
| +++ b/internal/console/alertdetail_test.go |
| @@ -26,40 +26,44 @@ func TestAlertTextForRestarts(t *testing.T) { |
| } |
| } |
| -// TestAlertMessageDetailLine: a firing alert that carries a detail names it |
| -// right after the value, worded for its rule; a clear (the box zeroes the |
| -// state) and a rule with no detail add no line. |
| -func TestAlertMessageDetailLine(t *testing.T) { |
| +// TestAlertMessageNamesTheUnit: a per-unit rule reads as its kind, with the |
| +// unit on a line of its own right after the value, firing and cleared alike; |
| +// a rule with no subject adds no line. |
| +func TestAlertMessageNamesTheUnit(t *testing.T) { |
| at := time.Date(2026, 10, 13, 9, 0, 0, 0, time.UTC) |
| - firing := func(detail string) metrics.RuleState { |
| - return metrics.RuleState{Firing: true, Value: 3, FiredAt: at, Detail: detail} |
| + firing := metrics.RuleState{Firing: true, Value: 5, FiredAt: at} |
| + |
| + subject, body := alertMessage("box-a", "restarts@carlos-litestream.service", firing, at) |
| + if subject != "[carlos] box-a: restarts alert" { |
| + t.Errorf("subject = %q", subject) |
| } |
| - for _, c := range []struct { |
| - rule, detail, want string |
| - }{ |
| - {"restarts", "carlos-litestream.service", "Value at the transition: 3 restarts in 10 minutes\nUnit: carlos-litestream.service\nTime (UTC)"}, |
| - {"somefuturerule", "thing", "Value at the transition: 3\nDetail: thing\nTime (UTC)"}, |
| - {"restarts", "", "Value at the transition: 3 restarts in 10 minutes\nTime (UTC)"}, |
| - } { |
| - _, body := alertMessage("box-a", c.rule, firing(c.detail), at) |
| - if !strings.Contains(body, c.want) { |
| - t.Errorf("%s/%q body missing %q:\n%s", c.rule, c.detail, c.want, body) |
| + for _, want := range []string{"The restarts alert is firing on box-a.", "Value at the transition: 5 restarts in 10 minutes\nUnit: carlos-litestream.service\nTime (UTC)"} { |
| + if !strings.Contains(body, want) { |
| + t.Errorf("firing body missing %q:\n%s", want, body) |
| } |
| } |
| - _, body := alertMessage("box-a", "restarts", metrics.RuleState{}, at) |
| - if strings.Contains(body, "Unit:") { |
| - t.Errorf("a clear invented a detail line:\n%s", body) |
| + subject, body = alertMessage("box-a", "restarts@carlos-litestream.service", metrics.RuleState{}, at) |
| + if subject != "[carlos] box-a: restarts cleared" || !strings.Contains(body, "Unit: carlos-litestream.service\n") { |
| + t.Errorf("a clear lost the unit: %q\n%s", subject, body) |
| + } |
| + if _, body = alertMessage("box-a", "somefuturerule@thing", firing, at); !strings.Contains(body, "Value at the transition: 5\nDetail: thing\nTime (UTC)") { |
| + t.Errorf("unknown per-subject rule:\n%s", body) |
| + } |
| + if _, body = alertMessage("box-a", "disk", metrics.RuleState{Firing: true, Value: 0.92, FiredAt: at}, at); !strings.Contains(body, "Value at the transition: 92%\nTime (UTC)") { |
| + t.Errorf("a rule with no subject grew a line:\n%s", body) |
| } |
| } |
| -// TestSweepAlertsCarriesDetail: the dashboard's row keeps the rule's detail. |
| -func TestSweepAlertsCarriesDetail(t *testing.T) { |
| +// TestSweepAlertsSplitsPerUnitRules: the dashboard's row shows a per-unit |
| +// rule as its kind with the unit beside it. |
| +func TestSweepAlertsSplitsPerUnitRules(t *testing.T) { |
| af := newAlertFixture(t) |
| af.writeAlert(t, metrics.AlertState{ |
| V: 1, Box: "box-a", UpdatedAt: time.Now(), |
| Rules: map[string]metrics.RuleState{ |
| - "restarts": {Firing: true, Value: 4, FiredAt: time.Now(), Detail: "carlos-litestream.service"}, |
| - "disk": {Firing: true, Value: 0.9, FiredAt: time.Now()}, |
| + "restarts@carlos-litestream.service": {Firing: true, Value: 4, FiredAt: time.Now()}, |
| + "restarts@carlos-relay.service": {}, |
| + "disk": {Firing: true, Value: 0.9, FiredAt: time.Now()}, |
| }, |
| }) |
| rows, err := af.srv.sweepAlerts(context.Background()) |
diff --git a/internal/console/alertwatch.go b/internal/console/alertwatch.go| index 025702121..20f38b810 100644 |
| --- a/internal/console/alertwatch.go |
| +++ b/internal/console/alertwatch.go |
| @@ -324,22 +324,24 @@ func alertMessage(box, rule string, rs metrics.RuleState, updated time.Time) (su |
| at = rs.FiredAt |
| } |
| stamp := at.UTC().Format(time.RFC3339) |
| - detail := alertDetailLine(rule, rs.Detail) |
| + // A per-subject rule ("restarts@<unit>") reads as its kind, with what it |
| + // is about on a line of its own. |
| + name, about := metrics.SplitRule(rule) |
| + detail := alertDetailLine(name, about) |
| if rs.Firing { |
| - subject = fmt.Sprintf("[carlos] %s: %s alert", box, rule) |
| + subject = fmt.Sprintf("[carlos] %s: %s alert", box, name) |
| body = fmt.Sprintf("The %s alert is firing on %s.\n\nValue at the transition: %s\n%sTime (UTC): %s\n\n"+ |
| - "You are getting this because you hold the CARLOS fleet-operator role.\n", rule, box, alertValueText(rule, rs.Value), detail, stamp) |
| + "You are getting this because you hold the CARLOS fleet-operator role.\n", name, box, alertValueText(name, rs.Value), detail, stamp) |
| return subject, body |
| } |
| - subject = fmt.Sprintf("[carlos] %s: %s cleared", box, rule) |
| + subject = fmt.Sprintf("[carlos] %s: %s cleared", box, name) |
| body = fmt.Sprintf("The %s alert has cleared on %s.\n\nValue at the transition: %s\n%sTime (UTC): %s\n\n"+ |
| - "You are getting this because you hold the CARLOS fleet-operator role.\n", rule, box, alertValueText(rule, rs.Value), detail, stamp) |
| + "You are getting this because you hold the CARLOS fleet-operator role.\n", name, box, alertValueText(name, rs.Value), detail, stamp) |
| return subject, body |
| } |
| -// alertDetailLine is the mail's line naming what a rule's value was about, |
| -// with its newline, or "" when the rule carried no detail (a clear zeroes it, |
| -// and most rules never have one). DRAFT copy, pending the owner's copy review. |
| +// alertDetailLine is the mail's line naming what a per-subject rule is about, |
| +// with its newline, or "" for a rule with no subject, which is most of them. |
| func alertDetailLine(rule, detail string) string { |
| switch { |
| case detail == "": |
diff --git a/internal/console/metricsview.go b/internal/console/metricsview.go| index 2a1028146..848f498a6 100644 |
| --- a/internal/console/metricsview.go |
| +++ b/internal/console/metricsview.go |
| @@ -843,7 +843,10 @@ func (s *Server) sweepAlerts(ctx context.Context) ([]AlertRow, error) { |
| if !rs.Firing { |
| continue |
| } |
| - rows = append(rows, AlertRow{Box: box, Rule: name, Value: rs.Value, FiredAt: rs.FiredAt, Detail: rs.Detail}) |
| + // A per-subject rule ("restarts@<unit>") shows as its kind with |
| + // the subject beside it. |
| + kind, about := metrics.SplitRule(name) |
| + rows = append(rows, AlertRow{Box: box, Rule: kind, Value: rs.Value, FiredAt: rs.FiredAt, Detail: about}) |
| } |
| } |
| return rows, nil |
diff --git a/internal/host/alertsink.go b/internal/host/alertsink.go| index 6b91e2b0d..7d4d795f8 100644 |
| --- a/internal/host/alertsink.go |
| +++ b/internal/host/alertsink.go |
| @@ -354,7 +354,8 @@ func alertSubject(ev metrics.Event) string { |
| // alertSentence is the lower-case "what is wrong" clause, shared by the |
| // subject and the body. |
| func alertSentence(ev metrics.Event) string { |
| - switch ev.Rule { |
| + name, subject := metrics.SplitRule(ev.Rule) |
| + switch name { |
| case "disk": |
| return fmt.Sprintf("disk is %s full on %s", pct(ev.Value), ev.Box) |
| case "mem": |
| @@ -366,14 +367,15 @@ func alertSentence(ev metrics.Event) string { |
| case "geoip": |
| return geoipAlertSentence(ev) |
| case "restarts": |
| - return fmt.Sprintf("%s restarted %d times in %d minutes on %s", restartsAlertUnit(ev), int64(ev.Value), int(restartWindow.Minutes()), ev.Box) |
| + return fmt.Sprintf("%s restarted %d times in %d minutes on %s", subject, int64(ev.Value), int(restartWindow.Minutes()), ev.Box) |
| default: |
| return fmt.Sprintf("%s is firing on %s", ev.Rule, ev.Box) |
| } |
| } |
| func alertClearSentence(ev metrics.Event) string { |
| - switch ev.Rule { |
| + name, subject := metrics.SplitRule(ev.Rule) |
| + switch name { |
| case "disk": |
| return fmt.Sprintf("disk is back to %s full on %s", pct(ev.Value), ev.Box) |
| case "mem": |
| @@ -385,14 +387,15 @@ func alertClearSentence(ev metrics.Event) string { |
| case "geoip": |
| return fmt.Sprintf("geoip on %s is back to normal", ev.Box) |
| case "restarts": |
| - return fmt.Sprintf("%s has stopped restarting on %s", restartsAlertUnit(ev), ev.Box) |
| + return fmt.Sprintf("%s has stopped restarting on %s", subject, ev.Box) |
| default: |
| return fmt.Sprintf("%s has cleared on %s", ev.Rule, ev.Box) |
| } |
| } |
| func alertLevel(r metrics.Rules, rule string) (string, bool) { |
| - switch rule { |
| + name, _ := metrics.SplitRule(rule) |
| + switch name { |
| case "disk": |
| return pct(r.DiskFrac), true |
| case "mem": |
| @@ -407,15 +410,6 @@ func alertLevel(r metrics.Rules, rule string) (string, bool) { |
| return "", false |
| } |
| -// restartsAlertUnit is the unit a restarts event is about. An event with no |
| -// detail (one persisted by an older build) still has to read as a sentence. |
| -func restartsAlertUnit(ev metrics.Event) string { |
| - if ev.Detail == "" { |
| - return "a platform unit" |
| - } |
| - return ev.Detail |
| -} |
| - |
| func alertSustain(r metrics.Rules, rule string) time.Duration { |
| switch rule { |
| case "mem": |
diff --git a/internal/host/alertsink_test.go b/internal/host/alertsink_test.go| index 5853e0aae..d9f5da537 100644 |
| --- a/internal/host/alertsink_test.go |
| +++ b/internal/host/alertsink_test.go |
| @@ -58,13 +58,13 @@ func TestFormatAlertFireAndClear(t *testing.T) { |
| }, |
| { |
| name: "restarts fires and names the unit", |
| - ev: metrics.Event{Rule: "restarts", Firing: true, Value: 6, Box: "box-a", Detail: "carlos-litestream.service"}, |
| + ev: metrics.Event{Rule: "restarts@carlos-litestream.service", Firing: true, Value: 6, Box: "box-a"}, |
| subjectHas: []string{"carlos alert", "carlos-litestream.service", "restarted 6 times", "10 minutes", "box-a"}, |
| bodyHas: []string{"carlos-litestream.service", "more than 3 restarts in 10 minutes", "clears"}, |
| }, |
| { |
| name: "restarts clears", |
| - ev: metrics.Event{Rule: "restarts", Firing: false, Value: 0, Box: "box-a", Detail: "carlos-litestream.service"}, |
| + ev: metrics.Event{Rule: "restarts@carlos-litestream.service", Firing: false, Value: 0, Box: "box-a"}, |
| subjectHas: []string{"carlos alert cleared", "carlos-litestream.service", "stopped restarting", "box-a"}, |
| bodyHas: []string{"carlos-litestream.service", "stopped restarting"}, |
| }, |
diff --git a/internal/host/metrics_loop.go b/internal/host/metrics_loop.go| index a52b6c7c4..e0e5678df 100644 |
| --- a/internal/host/metrics_loop.go |
| +++ b/internal/host/metrics_loop.go |
| @@ -35,7 +35,7 @@ type sampleProbes struct { |
| units func() map[string]unitRaw |
| // restarts is each platform unit's systemd restart counter (see |
| // restartWatch). A nil func, or a nil map from it, is "could not ask". |
| - restarts func() map[string]int64 |
| + restarts func() map[string]unitRestartState |
| now func() time.Time |
| } |
| @@ -488,14 +488,13 @@ func alertTick(ctx context.Context, s release.Store, box string, rules metrics.R |
| sinks *alertSinks) error { |
| obs := metrics.Observation{ |
| - At: latest.At, |
| - CPUFrac: latest.Host.CPUFrac, |
| - MemFrac: latest.Host.MemFrac, |
| - DiskFrac: latest.Host.DiskFrac, |
| - LagSecs: nil, // v1: no lag reading feeds the agent loop yet |
| - GeoIP: geoIPAlertReading(latest.At), |
| - Restarts: extra.restarts, |
| - RestartsUnit: extra.restartsUnit, |
| + At: latest.At, |
| + CPUFrac: latest.Host.CPUFrac, |
| + MemFrac: latest.Host.MemFrac, |
| + DiskFrac: latest.Host.DiskFrac, |
| + LagSecs: nil, // v1: no lag reading feeds the agent loop yet |
| + GeoIP: geoIPAlertReading(latest.At), |
| + Restarts: extra.restarts, |
| } |
| newState, events := metrics.Evaluate(rules, *st, obs) |
| @@ -534,8 +533,7 @@ func alertTick(ctx context.Context, s release.Store, box string, rules metrics.R |
| // and are not stored in the raw chunks. The zero value is "no reading", which |
| // leaves every rule they feed exactly as it was. |
| type alertReadings struct { |
| - restarts *float64 // see restartWatch.tick |
| - restartsUnit string |
| + restarts map[string]metrics.UnitRestarts // see restartWatch.tick |
| } |
| // persistAlertState writes st to path as indented JSON via the repo's |
| @@ -818,7 +816,27 @@ func metricsLoop(execRoot string, stampedUnits func() []string, sinks *alertSink |
| // alerted is what keeps a standing refusal from paging every minute. |
| // dataDir() is the bake dir the agent hands release.BakeDir. |
| var refusals refusalWatch |
| - var restarts restartWatch |
| + |
| + // The restart watch is the one reading taken before the first sleep. It |
| + // is loaded from the last run and sampled at once, so an agent that is |
| + // itself restarting every minute or so still adds each restart to the |
| + // count and gets a chance to say so. An agent that dies within seconds of |
| + // starting cannot report on itself from the inside at all. |
| + watchPath := restartWatchPath() |
| + restarts := loadRestartWatch(watchPath) |
| + restartTick := func(at time.Time) alertReadings { |
| + extra := alertReadings{restarts: restarts.tick(at, probes.restarts())} |
| + if err := saveRestartWatch(watchPath, restarts); err != nil { |
| + log.Printf("agent: restart watch: save %s: %v", watchPath, err) |
| + } |
| + return extra |
| + } |
| + // A sample with no host readings: every rule but restarts sees nil and |
| + // is left exactly as it was. |
| + started := time.Now() |
| + if err := alertTick(ctx, s, box, rules, &st, metrics.Sample{At: started}, restartTick(started), statePath, sinks); err != nil { |
| + log.Printf("agent: alert tick: %v", err) |
| + } |
| for { |
| time.Sleep(60 * time.Second) |
| tick++ |
| @@ -828,9 +846,7 @@ func metricsLoop(execRoot string, stampedUnits func() []string, sinks *alertSink |
| sm.tick(probes) |
| latest := sm.buf[len(sm.buf)-1] |
| - var extra alertReadings |
| - extra.restarts, extra.restartsUnit = restarts.tick(latest.At, probes.restarts()) |
| - if err := alertTick(ctx, s, box, rules, &st, latest, extra, statePath, sinks); err != nil { |
| + if err := alertTick(ctx, s, box, rules, &st, latest, restartTick(latest.At), statePath, sinks); err != nil { |
| log.Printf("agent: alert tick: %v", err) |
| } |
diff --git a/internal/host/restartwatch.go b/internal/host/restartwatch.go| index 14b89822d..01160b67d 100644 |
| --- a/internal/host/restartwatch.go |
| +++ b/internal/host/restartwatch.go |
| @@ -8,108 +8,191 @@ package host |
| // only thing that noticed was a person asking whether backups were running. |
| // |
| // systemd counts those restarts itself (NRestarts), so the agent asks it once |
| -// a tick and alerts on how fast the number is moving. |
| +// a tick and alerts, per unit, on how fast the number is moving. |
| import ( |
| + "context" |
| + "encoding/json" |
| + "os" |
| "os/exec" |
| + "path/filepath" |
| "strconv" |
| "strings" |
| "time" |
| + |
| + "github.com/carlosframework/platform/internal/metrics" |
| ) |
| -// restartWindow is how far back a unit's restarts are counted. The restarts |
| -// rule fires on a count inside it (metrics.Rules.RestartCount) and clears |
| -// after the whole of it passes with none. |
| +// restartWindow is how far back a unit's restarts are counted. A unit's |
| +// restarts rule fires on a count inside it (metrics.Rules.RestartCount) and |
| +// can clear only after the whole of it passes with none. |
| const restartWindow = 10 * time.Minute |
| -// restartWatch remembers each watched unit's restart counter over |
| -// restartWindow. It lives for the life of the metrics loop; an agent restart |
| -// starts it empty, which is why a reading of zero waits for a full window. |
| +// restartProbeTimeout bounds the one systemctl call a tick makes. The call |
| +// shares a goroutine with every other alert rule, so a systemd that does not |
| +// answer must cost a reading, not the disk alert. |
| +const restartProbeTimeout = 5 * time.Second |
| + |
| +// unitRestartState is one unit as systemd reports it. |
| +type unitRestartState struct { |
| + restarts int64 // NRestarts |
| + down bool // ActiveState is failed, or activating between restarts |
| +} |
| + |
| +// restartWatch remembers each watched unit's restarts over restartWindow. It |
| +// is saved after every tick (saveRestartWatch) and loaded at agent start, so |
| +// the agent's own restarts are counted: carlos-edge.service is one of the |
| +// units, and a watch held only in memory would die with each of them. |
| type restartWatch struct { |
| - marks map[string][]restartMark |
| - // quietSince is when the watch began, or last saw any unit restart or |
| - // have its counter reset, whichever is later. |
| - quietSince time.Time |
| + // At is the last tick that got an answer from systemd. |
| + At time.Time `json:"at"` |
| + Units map[string]*unitRestartHistory `json:"units"` |
| +} |
| + |
| +type unitRestartHistory struct { |
| + // Last is the counter at the last tick. |
| + Last int64 `json:"last"` |
| + // Bumps are the counter's rises inside the window, each dated by the |
| + // tick that saw it. |
| + Bumps []restartBump `json:"bumps,omitempty"` |
| + // QuietSince is when the watch first met this unit, or last saw it |
| + // restart or have its counter reset, whichever is later. |
| + QuietSince time.Time `json:"quiet_since"` |
| } |
| -type restartMark struct { |
| - at time.Time |
| - n int64 |
| +type restartBump struct { |
| + At time.Time `json:"at"` |
| + N int64 `json:"n"` |
| } |
| -// tick records counts (unit name to systemd's NRestarts) as of now and |
| -// returns the most restarts any one unit made inside the window, with that |
| -// unit's name. nil counts means systemd could not be asked: the reading is |
| -// nil and the history is kept. |
| +// tick records units as of now and returns each unit's reading. nil units |
| +// means systemd could not be asked: the reading is nil and the history is |
| +// kept. |
| // |
| -// A zero reading is only given after a whole window in which the watch saw |
| -// nothing restart. Zero is what clears a firing alert, and neither a watch |
| -// that has just started nor a unit an operator has just restarted by hand has |
| -// shown that the loop is over. |
| -func (w *restartWatch) tick(now time.Time, counts map[string]int64) (reading *float64, unit string) { |
| - if counts == nil { |
| - return nil, "" |
| +// A first sighting is a baseline, not a restart: a unit that had restarted |
| +// 900 times before the watch met it has not restarted "now". |
| +// |
| +// systemd zeroes NRestarts when a unit is started by hand. Restarts already |
| +// counted stay counted, and whatever the counter shows after the reset is |
| +// added to them, so an operator's restart in the middle of a loop neither |
| +// hides the loop nor reads as a negative number. |
| +// |
| +// A gap longer than the window between two answers leaves nothing to count: |
| +// the watch knows how far each counter moved and not when, so it starts |
| +// again from the present rather than date old restarts to this tick. |
| +func (w *restartWatch) tick(now time.Time, units map[string]unitRestartState) map[string]metrics.UnitRestarts { |
| + if units == nil { |
| + return nil |
| } |
| - if w.marks == nil { |
| - w.marks = map[string][]restartMark{} |
| - w.quietSince = now |
| + if w.Units == nil || (!w.At.IsZero() && now.Sub(w.At) > restartWindow) { |
| + w.Units = map[string]*unitRestartHistory{} |
| } |
| - for name := range w.marks { |
| - if _, ok := counts[name]; !ok { |
| - delete(w.marks, name) // unloaded: nothing left to watch |
| + for name := range w.Units { |
| + if _, ok := units[name]; !ok { |
| + delete(w.Units, name) // unloaded: nothing left to watch |
| } |
| } |
| - var worst int64 |
| - for name, n := range counts { |
| - marks := w.marks[name] |
| - if last := len(marks) - 1; last >= 0 && n != marks[last].n { |
| - w.quietSince = now |
| - if n < marks[last].n { |
| - // The counter went backwards: systemd zeroes it when the unit |
| - // is started by hand, so what came before is another run's |
| - // history. |
| - marks = nil |
| - } |
| + out := make(map[string]metrics.UnitRestarts, len(units)) |
| + for name, u := range units { |
| + h := w.Units[name] |
| + if h == nil { |
| + h = &unitRestartHistory{Last: u.restarts, QuietSince: now} |
| + w.Units[name] = h |
| } |
| - marks = append(marks, restartMark{at: now, n: n}) |
| - // Keep one mark at or beyond the window's edge as the baseline. |
| - for len(marks) > 1 && now.Sub(marks[1].at) >= restartWindow { |
| - marks = marks[1:] |
| + rose := u.restarts - h.Last |
| + if u.restarts < h.Last { |
| + rose = u.restarts // reset by a manual start; these came after it |
| + h.QuietSince = now |
| } |
| - w.marks[name] = marks |
| - if d := n - marks[0].n; d > worst || (d == worst && d > 0 && name < unit) { |
| - worst, unit = d, name |
| + if rose > 0 { |
| + h.Bumps = append(h.Bumps, restartBump{At: now, N: rose}) |
| + h.QuietSince = now |
| } |
| + h.Last = u.restarts |
| + |
| + var count int64 |
| + kept := h.Bumps[:0] |
| + for _, b := range h.Bumps { |
| + if now.Sub(b.At) < restartWindow { |
| + kept = append(kept, b) |
| + count += b.N |
| + } |
| + } |
| + h.Bumps = kept |
| + out[name] = metrics.UnitRestarts{ |
| + Count: float64(count), |
| + Settled: now.Sub(h.QuietSince) >= restartWindow, |
| + Down: u.down, |
| + } |
| + } |
| + w.At = now |
| + return out |
| +} |
| + |
| +// restartWatchPath is where the watch is kept between agent runs, beside the |
| +// alert state it feeds. |
| +func restartWatchPath() string { return filepath.Join(dataDir(), "monitor", "restarts.json") } |
| + |
| +// loadRestartWatch reads a saved watch. Anything unreadable is an empty |
| +// watch, which is always a safe place to start from: it can delay an alert by |
| +// a few ticks and can never invent one. |
| +func loadRestartWatch(path string) restartWatch { |
| + var w restartWatch |
| + b, err := os.ReadFile(path) |
| + if err != nil || json.Unmarshal(b, &w) != nil { |
| + return restartWatch{} |
| + } |
| + return w |
| +} |
| + |
| +// saveRestartWatch writes the watch by temp file and rename, like |
| +// persistAlertState. Best effort: a failed save costs history across the |
| +// next agent restart, nothing more. |
| +func saveRestartWatch(path string, w restartWatch) error { |
| + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { |
| + return err |
| + } |
| + b, err := json.Marshal(w) |
| + if err != nil { |
| + return err |
| } |
| - if worst == 0 && now.Sub(w.quietSince) < restartWindow { |
| - return nil, "" |
| + tmp := path + ".tmp" |
| + if err := os.WriteFile(tmp, b, 0o644); err != nil { |
| + return err |
| } |
| - v := float64(worst) |
| - return &v, unit |
| + return os.Rename(tmp, path) |
| } |
| -// platformUnitRestarts asks systemd for the restart counter of every loaded |
| -// platform service. nil when systemd cannot be asked. |
| +// platformUnitRestarts asks systemd for the restart counter and state of |
| +// every loaded platform service. nil when systemd cannot be asked in time. |
| // |
| // Template instances (carlos-app@<host>, carlos-instance@<id>) are left out: |
| // those are tenants' processes, and a tenant's crash loop is not the fleet |
| // operator's page. |
| -func platformUnitRestarts() map[string]int64 { |
| - out, err := exec.Command("systemctl", "show", "--property=Id", "--property=NRestarts", "carlos-*.service").Output() |
| +func platformUnitRestarts() map[string]unitRestartState { |
| + ctx, cancel := context.WithTimeout(context.Background(), restartProbeTimeout) |
| + defer cancel() |
| + out, err := exec.CommandContext(ctx, "systemctl", "show", |
| + "--property=Id", "--property=NRestarts", "--property=ActiveState", "carlos-*.service").Output() |
| if err != nil { |
| return nil |
| } |
| return parseUnitRestarts(string(out)) |
| } |
| -// parseUnitRestarts reads `systemctl show --property=Id --property=NRestarts` |
| -// output: one blank-line-separated block per unit, the two properties in |
| -// whatever order systemd prints them. A block missing either is skipped. |
| -// Never nil: no loaded units is an answer, an empty one. |
| -func parseUnitRestarts(out string) map[string]int64 { |
| - counts := map[string]int64{} |
| +// parseUnitRestarts reads `systemctl show --property=Id --property=NRestarts |
| +// --property=ActiveState` output: one blank-line-separated block per unit, |
| +// the properties in whatever order systemd prints them. A block with no Id |
| +// or no counter is skipped. Never nil: no loaded units is an answer, an |
| +// empty one. |
| +func parseUnitRestarts(out string) map[string]unitRestartState { |
| + units := map[string]unitRestartState{} |
| for _, block := range strings.Split(out, "\n\n") { |
| - id, n, haveN := "", int64(0), false |
| + var ( |
| + id string |
| + u unitRestartState |
| + haveN bool |
| + ) |
| for _, line := range strings.Split(block, "\n") { |
| k, v, ok := strings.Cut(strings.TrimSpace(line), "=") |
| if !ok { |
| @@ -119,15 +202,19 @@ func parseUnitRestarts(out string) map[string]int64 { |
| case "Id": |
| id = v |
| case "NRestarts": |
| - if parsed, err := strconv.ParseInt(v, 10, 64); err == nil { |
| - n, haveN = parsed, true |
| + if n, err := strconv.ParseInt(v, 10, 64); err == nil { |
| + u.restarts, haveN = n, true |
| } |
| + case "ActiveState": |
| + // "inactive" is a oneshot between runs or a unit an operator |
| + // stopped: not running, and not in trouble either. |
| + u.down = v == "failed" || v == "activating" || v == "deactivating" |
| } |
| } |
| if id == "" || !haveN || strings.Contains(id, "@") { |
| continue |
| } |
| - counts[id] = n |
| + units[id] = u |
| } |
| - return counts |
| + return units |
| } |
diff --git a/internal/host/restartwatch_test.go b/internal/host/restartwatch_test.go| index a58129801..9efda2ccd 100644 |
| --- a/internal/host/restartwatch_test.go |
| +++ b/internal/host/restartwatch_test.go |
| @@ -1,113 +1,180 @@ |
| package host |
| import ( |
| + "path/filepath" |
| "testing" |
| "time" |
| + |
| + "github.com/carlosframework/platform/internal/metrics" |
| ) |
| -// The #284 shape: one unit restarting every few seconds among quiet ones. The |
| -// reading is that unit's restarts inside the window, with its name. |
| -func TestRestartWatchCountsTheWorstUnitInsideTheWindow(t *testing.T) { |
| +const lsUnit = "carlos-litestream.service" |
| + |
| +func restartsAt(t0 time.Time, m int) time.Time { return t0.Add(time.Duration(m) * time.Minute) } |
| + |
| +func oneUnit(n int64) map[string]unitRestartState { |
| + return map[string]unitRestartState{lsUnit: {restarts: n}} |
| +} |
| + |
| +// The #284 shape: one unit restarting every few seconds among quiet ones. |
| +// Each unit gets its own count, and a count of zero is only "settled" after a |
| +// whole window with no restart. |
| +func TestRestartWatchCountsEachUnitInsideTheWindow(t *testing.T) { |
| var w restartWatch |
| t0 := time.Date(2026, 10, 13, 12, 0, 0, 0, time.UTC) |
| - counts := func(ls, relay int64) map[string]int64 { |
| - return map[string]int64{"carlos-litestream.service": ls, "carlos-relay.service": relay, "carlos-edge.service": 0} |
| + units := func(ls, relay int64) map[string]unitRestartState { |
| + return map[string]unitRestartState{lsUnit: {restarts: ls}, "carlos-relay.service": {restarts: relay}, "carlos-edge.service": {}} |
| } |
| // First sight of a counter is a baseline, not a restart: a unit that has |
| // restarted 900 times before the agent looked has not restarted "now". |
| - if r, _ := w.tick(t0, counts(900, 2)); r != nil { |
| - t.Fatalf("first tick gave a reading of %v, want none", *r) |
| + got := w.tick(t0, units(900, 2)) |
| + if got[lsUnit].Count != 0 || got[lsUnit].Settled { |
| + t.Fatalf("first tick: %+v, want no restarts and not settled", got[lsUnit]) |
| } |
| - r, unit := w.tick(t0.Add(time.Minute), counts(909, 2)) |
| - if r == nil || *r != 9 || unit != "carlos-litestream.service" { |
| - t.Fatalf("after one minute: reading=%v unit=%q, want 9 restarts of carlos-litestream.service", r, unit) |
| + got = w.tick(restartsAt(t0, 1), units(909, 2)) |
| + if got[lsUnit].Count != 9 || got["carlos-relay.service"].Count != 0 { |
| + t.Fatalf("after one minute: %+v", got) |
| } |
| - r, unit = w.tick(t0.Add(2*time.Minute), counts(912, 3)) |
| - if r == nil || *r != 12 || unit != "carlos-litestream.service" { |
| - t.Fatalf("after two minutes: reading=%v unit=%q, want 12", r, unit) |
| + got = w.tick(restartsAt(t0, 2), units(912, 3)) |
| + if got[lsUnit].Count != 12 || got["carlos-relay.service"].Count != 1 { |
| + t.Fatalf("after two minutes: %+v", got) |
| } |
| - // The loop stops. Restarts age out of the window one tick at a time, and |
| - // zero arrives only when the whole window has been quiet. |
| - var last *float64 |
| - for m := 3; m <= 11; m++ { |
| - last, _ = w.tick(t0.Add(time.Duration(m)*time.Minute), counts(912, 3)) |
| - if last == nil || *last == 0 { |
| - t.Fatalf("minute %d: reading=%v, but minute 2's restarts are still inside the window", m, last) |
| + // The loop stops. Restarts age out of the window by the tick that saw |
| + // them, and the unit settles only when the whole window has been quiet. |
| + for m := 3; m <= 10; m++ { |
| + got = w.tick(restartsAt(t0, m), units(912, 3)) |
| + if got[lsUnit].Count == 0 || got[lsUnit].Settled { |
| + t.Fatalf("minute %d: %+v, but minute 1's restarts are still inside the window", m, got[lsUnit]) |
| } |
| } |
| - last, _ = w.tick(t0.Add(12*time.Minute), counts(912, 3)) |
| - if last == nil || *last != 0 { |
| - t.Fatalf("minute 12: reading=%v, want 0 after a quiet window", last) |
| + if got = w.tick(restartsAt(t0, 11), units(912, 3)); got[lsUnit].Count != 3 || got[lsUnit].Settled { |
| + t.Fatalf("minute 11: %+v, want only minute 2's three restarts left", got[lsUnit]) |
| + } |
| + if got = w.tick(restartsAt(t0, 12), units(912, 3)); got[lsUnit].Count != 0 || !got[lsUnit].Settled { |
| + t.Fatalf("minute 12: %+v, want settled at zero", got[lsUnit]) |
| } |
| } |
| -// Zero clears a firing alert, so a watch that has only just started (the |
| -// agent restarted mid-incident) must not say it. |
| -func TestRestartWatchGivesNoZeroBeforeAFullWindow(t *testing.T) { |
| +// systemd zeroes NRestarts when a unit is started by hand. Restarts already |
| +// seen stay counted and the ones after the reset are added: an operator's |
| +// restart in the middle of a loop must not hide the loop (100, 103, then 2 is |
| +// five restarts, not two and not minus 101). |
| +func TestRestartWatchCountsThroughACounterReset(t *testing.T) { |
| var w restartWatch |
| t0 := time.Date(2026, 10, 13, 12, 0, 0, 0, time.UTC) |
| - quiet := map[string]int64{"carlos-litestream.service": 40} |
| - for m := 0; m < 10; m++ { |
| - if r, _ := w.tick(t0.Add(time.Duration(m)*time.Minute), quiet); r != nil { |
| - t.Fatalf("minute %d: reading=%v before the watch has seen a whole window", m, *r) |
| - } |
| + w.tick(t0, oneUnit(100)) |
| + if got := w.tick(restartsAt(t0, 1), oneUnit(103)); got[lsUnit].Count != 3 { |
| + t.Fatalf("before the reset: %+v, want 3", got[lsUnit]) |
| + } |
| + got := w.tick(restartsAt(t0, 2), oneUnit(2)) |
| + if got[lsUnit].Count != 5 || got[lsUnit].Settled { |
| + t.Fatalf("after the reset: %+v, want 5 and not settled", got[lsUnit]) |
| } |
| - if r, _ := w.tick(t0.Add(10*time.Minute), quiet); r == nil || *r != 0 { |
| - t.Fatalf("minute 10: reading=%v, want 0", r) |
| + // A reset to zero with nothing after it adds nothing, and still is not |
| + // proof of quiet. |
| + got = w.tick(restartsAt(t0, 3), oneUnit(0)) |
| + if got[lsUnit].Count != 5 || got[lsUnit].Settled { |
| + t.Fatalf("a second manual start: %+v, want 5 and not settled", got[lsUnit]) |
| } |
| } |
| -// systemd zeroes NRestarts when a unit is started by hand. That is neither a |
| -// negative number of restarts nor proof the loop is over. |
| -func TestRestartWatchTreatsACounterResetAsANewRun(t *testing.T) { |
| +// When systemd cannot be asked there is no reading, and a short gap keeps |
| +// what the watch knew. A gap longer than the window does not: the counter |
| +// moved, but nothing says it moved recently. |
| +func TestRestartWatchAcrossGapsInSampling(t *testing.T) { |
| var w restartWatch |
| t0 := time.Date(2026, 10, 13, 12, 0, 0, 0, time.UTC) |
| - at := func(m int) time.Time { return t0.Add(time.Duration(m) * time.Minute) } |
| - unit := func(n int64) map[string]int64 { return map[string]int64{"carlos-litestream.service": n} } |
| - |
| - w.tick(at(0), unit(500)) |
| - if r, _ := w.tick(at(1), unit(510)); r == nil || *r != 10 { |
| - t.Fatalf("reading=%v, want 10", r) |
| + w.tick(t0, oneUnit(1)) |
| + if got := w.tick(restartsAt(t0, 1), nil); got != nil { |
| + t.Fatalf("reading %+v with no answer from systemd", got) |
| } |
| - // An operator restarts it by hand: the counter is 0 again. |
| - if r, _ := w.tick(at(2), unit(0)); r != nil { |
| - t.Fatalf("a counter reset gave a reading of %v, want none", *r) |
| + if got := w.tick(restartsAt(t0, 2), oneUnit(7)); got[lsUnit].Count != 6 { |
| + t.Fatalf("after a two-minute gap: %+v, want 6 against the baseline from before it", got[lsUnit]) |
| } |
| - // Still broken: it loops again from zero, and is counted from zero. |
| - if r, _ := w.tick(at(3), unit(5)); r == nil || *r != 5 { |
| - t.Fatalf("after the reset: reading=%v, want 5", r) |
| + |
| + // Four more restarts, then no answer for eighteen minutes. |
| + got := w.tick(restartsAt(t0, 20), oneUnit(11)) |
| + if got[lsUnit].Count != 0 || got[lsUnit].Settled { |
| + t.Fatalf("after an eighteen-minute gap: %+v, want no dated restarts and not settled", got[lsUnit]) |
| + } |
| + if got = w.tick(restartsAt(t0, 21), oneUnit(12)); got[lsUnit].Count != 1 { |
| + t.Fatalf("the watch did not start again from the present: %+v", got[lsUnit]) |
| } |
| } |
| -// systemd unavailable is "could not ask": no reading, and what the watch |
| -// already knew is kept for when it can ask again. |
| -func TestRestartWatchKeepsItsHistoryWhenSystemdCannotBeAsked(t *testing.T) { |
| +// A unit that goes away is forgotten, and one that comes back is met afresh. |
| +func TestRestartWatchForgetsAnUnloadedUnit(t *testing.T) { |
| var w restartWatch |
| t0 := time.Date(2026, 10, 13, 12, 0, 0, 0, time.UTC) |
| - w.tick(t0, map[string]int64{"carlos-litestream.service": 1}) |
| - if r, _ := w.tick(t0.Add(time.Minute), nil); r != nil { |
| - t.Fatalf("reading=%v with no answer from systemd", *r) |
| + w.tick(t0, oneUnit(5)) |
| + if got := w.tick(restartsAt(t0, 1), map[string]unitRestartState{}); len(got) != 0 { |
| + t.Fatalf("an unloaded unit still has a reading: %+v", got) |
| + } |
| + if got := w.tick(restartsAt(t0, 2), oneUnit(50)); got[lsUnit].Count != 0 { |
| + t.Fatalf("a reloaded unit was counted against its old baseline: %+v", got[lsUnit]) |
| + } |
| +} |
| + |
| +// The watch survives the agent: the agent's own unit is one of those it |
| +// watches, so its restarts are only countable from what the last run saved. |
| +func TestRestartWatchSavedAcrossAnAgentRestart(t *testing.T) { |
| + path := filepath.Join(t.TempDir(), "monitor", "restarts.json") |
| + t0 := time.Date(2026, 10, 13, 12, 0, 0, 0, time.UTC) |
| + edge := func(n int64) map[string]unitRestartState { |
| + return map[string]unitRestartState{"carlos-edge.service": {restarts: n}} |
| + } |
| + |
| + // An agent that lives about ninety seconds: each run loads, samples at |
| + // start, and saves. |
| + var last map[string]metrics.UnitRestarts |
| + for run := 0; run < 5; run++ { |
| + w := loadRestartWatch(path) |
| + last = w.tick(t0.Add(time.Duration(run)*90*time.Second), edge(int64(run))) |
| + if err := saveRestartWatch(path, w); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + if got := last["carlos-edge.service"].Count; got != 4 { |
| + t.Fatalf("after five runs: %v restarts counted, want 4", got) |
| + } |
| + |
| + // A saved watch older than the window is not history any more. |
| + w := loadRestartWatch(path) |
| + if got := w.tick(t0.Add(2*time.Hour), edge(40)); got["carlos-edge.service"].Count != 0 { |
| + t.Fatalf("a two-hour-old watch dated its restarts to now: %+v", got) |
| } |
| - if r, _ := w.tick(t0.Add(2*time.Minute), map[string]int64{"carlos-litestream.service": 7}); r == nil || *r != 6 { |
| - t.Fatalf("reading=%v, want 6 against the baseline from before the gap", r) |
| + |
| + // An unreadable file is an empty watch. |
| + if w := loadRestartWatch(filepath.Join(t.TempDir(), "absent.json")); w.Units != nil || !w.At.IsZero() { |
| + t.Fatalf("absent file loaded as %+v", w) |
| } |
| } |
| func TestParseUnitRestarts(t *testing.T) { |
| - // Verbatim shape from systemd 257: NRestarts before Id, a blank line |
| + // The shape systemd 257 prints: properties in its own order, a blank line |
| // between units. |
| - out := "NRestarts=0\nId=carlos-edge.service\n\nNRestarts=912\nId=carlos-litestream.service\n\n" + |
| - "NRestarts=3\nId=carlos-app@shop.example.com.service\n\nId=carlos-adopt.service\n\nNRestarts=x\nId=carlos-relay.service\n" |
| + out := "NRestarts=0\nId=carlos-edge.service\nActiveState=active\n\n" + |
| + "NRestarts=912\nId=carlos-litestream.service\nActiveState=activating\n\n" + |
| + "NRestarts=5\nId=carlos-relay.service\nActiveState=failed\n\n" + |
| + "NRestarts=0\nId=carlos-stage.service\nActiveState=inactive\n\n" + |
| + "NRestarts=3\nId=carlos-app@shop.example.com.service\nActiveState=active\n\n" + |
| + "Id=carlos-adopt.service\nActiveState=inactive\n\n" + |
| + "NRestarts=x\nId=carlos-restart.service\n" |
| got := parseUnitRestarts(out) |
| - want := map[string]int64{"carlos-edge.service": 0, "carlos-litestream.service": 912} |
| + want := map[string]unitRestartState{ |
| + "carlos-edge.service": {}, |
| + "carlos-litestream.service": {restarts: 912, down: true}, |
| + "carlos-relay.service": {restarts: 5, down: true}, |
| + "carlos-stage.service": {}, |
| + } |
| if len(got) != len(want) { |
| t.Fatalf("got %v, want %v (a tenant's unit, a block with no counter and one with a bad counter are all skipped)", got, want) |
| } |
| for k, v := range want { |
| if got[k] != v { |
| - t.Errorf("%s = %d, want %d", k, got[k], v) |
| + t.Errorf("%s = %+v, want %+v", k, got[k], v) |
| } |
| } |
| if got := parseUnitRestarts(""); got == nil || len(got) != 0 { |
diff --git a/internal/metrics/alert.go b/internal/metrics/alert.go| index 489952dc4..7afe219c2 100644 |
| --- a/internal/metrics/alert.go |
| +++ b/internal/metrics/alert.go |
| @@ -1,7 +1,9 @@ |
| package metrics |
| import ( |
| + "sort" |
| "strconv" |
| + "strings" |
| "time" |
| ) |
| @@ -20,8 +22,8 @@ type Rules struct { |
| // fires (geoip spec §10.4); it clears on the first reading of 0. |
| GeoIPSustain time.Duration // default 30 * time.Minute |
| // RestartCount is how many restarts one unit may make inside the |
| - // observation's window before the restarts rule fires. It is a count, not |
| - // a fraction, and the rule clears only on a reading of 0. |
| + // reporter's window before that unit's restarts rule fires. It is a |
| + // count, not a fraction; see UnitRestarts for what clears it. |
| RestartCount float64 // fire >, default 3 |
| } |
| @@ -91,12 +93,41 @@ type Observation struct { |
| // 1 stale, 2 expiring, 3 expired, 4 not loaded, 5 feed failing. nil on |
| // a box whose edge has not published one. |
| GeoIP *float64 |
| - // Restarts is the most restarts any one watched unit made inside the |
| - // reporter's window, and RestartsUnit names that unit. A reading of 0 |
| - // says a whole window passed with none; nil says the reporter could not |
| - // tell yet. |
| - Restarts *float64 |
| - RestartsUnit string |
| + // Restarts is every watched unit's reading, keyed by unit name. nil is |
| + // "the reporter could not ask this tick" and leaves every restarts rule |
| + // as it was. A unit missing from a non-nil map is no longer loaded. |
| + Restarts map[string]UnitRestarts |
| +} |
| + |
| +// UnitRestarts is one unit's restart reading. |
| +type UnitRestarts struct { |
| + // Count is how many times the unit restarted inside the reporter's |
| + // window. |
| + Count float64 |
| + // Settled says the reporter has watched this unit for a whole window in |
| + // which it neither restarted nor was restarted by hand. Only then is a |
| + // Count of 0 evidence that a loop is over. |
| + Settled bool |
| + // Down says systemd reports the unit failed, or between restarts. A unit |
| + // that stopped restarting because it gave up has not recovered. A unit |
| + // that is merely not running (a oneshot between runs, one an operator |
| + // stopped) is not down. |
| + Down bool |
| +} |
| + |
| +// restartsRule is the per-unit rule name: one unit's loop must not hide |
| +// another's, so each watched unit fires and clears by itself. |
| +const restartsRule = "restarts" |
| + |
| +// RuleFor is the rule name for one subject of a per-subject rule, e.g. |
| +// RuleFor("restarts", "carlos-relay.service"). |
| +func RuleFor(name, subject string) string { return name + "@" + subject } |
| + |
| +// SplitRule undoes RuleFor: the rule's kind and what it is about. A rule |
| +// with no subject ("disk") comes back with an empty one. |
| +func SplitRule(rule string) (name, subject string) { |
| + name, subject, _ = strings.Cut(rule, "@") |
| + return name, subject |
| } |
| // RuleState is one rule's sustain/hysteresis bookkeeping. Since and FiredAt |
| @@ -114,9 +145,6 @@ type RuleState struct { |
| Since time.Time `json:"since,omitempty"` // continuously-above-threshold since (sustain tracking) |
| FiredAt time.Time `json:"fired_at,omitempty"` // set on fire, cleared on clear |
| Value float64 `json:"value,omitempty"` // reading at last transition |
| - // Detail names what the reading was about, for the rules whose value |
| - // alone does not say: the unit for "restarts". Empty for the rest. |
| - Detail string `json:"detail,omitempty"` |
| } |
| // AlertState is the JSON persisted to disk by the agent loop and published |
| @@ -126,7 +154,7 @@ type AlertState struct { |
| V int `json:"v"` // 1 |
| Box string `json:"box"` |
| UpdatedAt time.Time `json:"updated_at"` |
| - Rules map[string]RuleState `json:"rules"` // "disk","mem","cpu","lag","geoip","restarts" |
| + Rules map[string]RuleState `json:"rules"` // "disk","mem","cpu","lag","geoip", and "restarts@<unit>" per unit |
| } |
| // Event is emitted on a firing or clearing transition — never on every |
| @@ -136,7 +164,6 @@ type Event struct { |
| Firing bool `json:"firing"` // true=fired, false=cleared |
| Value float64 `json:"value"` |
| Box string `json:"box"` |
| - Detail string `json:"detail,omitempty"` // see RuleState.Detail |
| } |
| // AlertKey is the bucket key an AlertState for box is stored/published |
| @@ -289,22 +316,43 @@ func Evaluate(r Rules, st AlertState, obs Observation) (AlertState, []Event) { |
| } |
| } |
| - // restarts: a count inside a window the reporter keeps, so there is no |
| - // sustain here. It fires above RestartCount and clears only at 0, which is |
| - // a whole window without a restart: a unit that has merely slowed down |
| - // (systemd backs its restarts off) has not recovered. |
| + // restarts, one rule per unit: a count inside a window the reporter |
| + // keeps, so there is no sustain here. A unit fires above RestartCount. It |
| + // clears when the reporter has seen it quiet for a whole window and it is |
| + // not down, or when it is no longer loaded: a unit that has merely slowed |
| + // down (systemd backs its restarts off), or stopped because it gave up, |
| + // has not recovered. |
| if obs.Restarts != nil { |
| - val := *obs.Restarts |
| - cur := out.Rules["restarts"] |
| - switch { |
| - case cur.Firing && val == 0: |
| - unit := cur.Detail |
| - out.Rules["restarts"] = RuleState{} |
| - events = append(events, Event{Rule: "restarts", Firing: false, Value: val, Box: st.Box, Detail: unit}) |
| - case !cur.Firing && val > r.RestartCount: |
| - out.Rules["restarts"] = RuleState{Firing: true, Since: obs.At, FiredAt: obs.At, Value: val, Detail: obs.RestartsUnit} |
| - events = append(events, Event{Rule: "restarts", Firing: true, Value: val, Box: st.Box, Detail: obs.RestartsUnit}) |
| + // A cleared rule stays in the map as a quiet RuleState rather than |
| + // being deleted: the console learns of a clear by reading firing |
| + // false, and a key that vanished would leave it believing the alert |
| + // is still up. The set of platform units is small and fixed. |
| + var changed []Event |
| + for unit, u := range obs.Restarts { |
| + rule := RuleFor(restartsRule, unit) |
| + cur := out.Rules[rule] |
| + switch { |
| + case !cur.Firing && u.Count > r.RestartCount: |
| + out.Rules[rule] = RuleState{Firing: true, Since: obs.At, FiredAt: obs.At, Value: u.Count} |
| + changed = append(changed, Event{Rule: rule, Firing: true, Value: u.Count, Box: st.Box}) |
| + case cur.Firing && u.Count == 0 && u.Settled && !u.Down: |
| + out.Rules[rule] = RuleState{} |
| + changed = append(changed, Event{Rule: rule, Firing: false, Box: st.Box}) |
| + } |
| + } |
| + for rule, cur := range out.Rules { |
| + name, unit := SplitRule(rule) |
| + if name != restartsRule || unit == "" || !cur.Firing { |
| + continue |
| + } |
| + if _, loaded := obs.Restarts[unit]; !loaded { |
| + out.Rules[rule] = RuleState{} |
| + changed = append(changed, Event{Rule: rule, Firing: false, Box: st.Box}) |
| + } |
| } |
| + // Map order is random; the events of one tick are not. |
| + sort.Slice(changed, func(i, j int) bool { return changed[i].Rule < changed[j].Rule }) |
| + events = append(events, changed...) |
| } |
| return out, events |
diff --git a/internal/metrics/alert_test.go b/internal/metrics/alert_test.go| index 92e0a6d4c..71f2f0b79 100644 |
| --- a/internal/metrics/alert_test.go |
| +++ b/internal/metrics/alert_test.go |
| @@ -248,44 +248,109 @@ func TestGeoIPReasonChangeWhileFiring(t *testing.T) { |
| } |
| } |
| -// The restarts rule fires above the count, names the unit, holds while the |
| -// unit is still restarting (however slowly), and clears only on a reading of |
| -// zero. A nil reading changes nothing in either direction. |
| -func TestRestartsRuleFiresAboveTheCountAndClearsOnlyAtZero(t *testing.T) { |
| +// One unit's restarts rule fires above the count, holds while the unit is |
| +// still restarting (however slowly) or is not running, and clears only once |
| +// it has been quiet for a settled window and is not down. A nil reading |
| +// changes nothing. |
| +func TestRestartsRuleFiresAboveTheCountAndClearsOnlyWhenSettledAndUp(t *testing.T) { |
| r := DefaultRules() |
| t0 := time.Date(2026, 10, 13, 12, 0, 0, 0, time.UTC) |
| - f := func(v float64) *float64 { return &v } |
| + const ls = "carlos-litestream.service" |
| + rule := RuleFor("restarts", ls) |
| + one := func(u UnitRestarts) map[string]UnitRestarts { return map[string]UnitRestarts{ls: u} } |
| st := AlertState{V: 1, Box: "b"} |
| - st, ev := Evaluate(r, st, Observation{At: t0, Restarts: f(3), RestartsUnit: "carlos-litestream.service"}) |
| - if len(ev) != 0 || st.Rules["restarts"].Firing { |
| + st, ev := Evaluate(r, st, Observation{At: t0, Restarts: one(UnitRestarts{Count: 3})}) |
| + if len(ev) != 0 || st.Rules[rule].Firing { |
| t.Fatalf("fired at the count, not above it: %v", ev) |
| } |
| - st, ev = Evaluate(r, st, Observation{At: t0.Add(time.Minute), Restarts: f(4), RestartsUnit: "carlos-litestream.service"}) |
| - if len(ev) != 1 || !ev[0].Firing || ev[0].Rule != "restarts" || ev[0].Value != 4 || ev[0].Detail != "carlos-litestream.service" { |
| + st, ev = Evaluate(r, st, Observation{At: t0.Add(time.Minute), Restarts: one(UnitRestarts{Count: 4})}) |
| + if len(ev) != 1 || !ev[0].Firing || ev[0].Rule != rule || ev[0].Value != 4 { |
| t.Fatalf("above the count: %+v", ev) |
| } |
| - if rs := st.Rules["restarts"]; !rs.Firing || rs.Detail != "carlos-litestream.service" || !rs.FiredAt.Equal(t0.Add(time.Minute)) { |
| + if rs := st.Rules[rule]; !rs.Firing || !rs.FiredAt.Equal(t0.Add(time.Minute)) { |
| t.Fatalf("state after firing: %+v", rs) |
| } |
| - // Still restarting, now once a minute, and another unit is the worst. |
| - st, ev = Evaluate(r, st, Observation{At: t0.Add(2 * time.Minute), Restarts: f(1), RestartsUnit: "carlos-relay.service"}) |
| - if len(ev) != 0 || !st.Rules["restarts"].Firing || st.Rules["restarts"].Detail != "carlos-litestream.service" { |
| - t.Fatalf("a slower loop cleared or renamed the alert: %v %+v", ev, st.Rules["restarts"]) |
| + for i, u := range []UnitRestarts{ |
| + {Count: 1}, // still restarting, now once a minute |
| + {Count: 0}, // quiet, but not yet for a whole window |
| + {Count: 0, Settled: true, Down: true}, // quiet because it gave up |
| + } { |
| + st, ev = Evaluate(r, st, Observation{At: t0.Add(time.Duration(2+i) * time.Minute), Restarts: one(u)}) |
| + if len(ev) != 0 || !st.Rules[rule].Firing { |
| + t.Fatalf("reading %+v cleared the alert: %v", u, ev) |
| + } |
| } |
| - st, ev = Evaluate(r, st, Observation{At: t0.Add(3 * time.Minute)}) |
| - if len(ev) != 0 || !st.Rules["restarts"].Firing { |
| + st, ev = Evaluate(r, st, Observation{At: t0.Add(6 * time.Minute)}) |
| + if len(ev) != 0 || !st.Rules[rule].Firing { |
| t.Fatalf("a nil reading changed the rule: %v", ev) |
| } |
| - st, ev = Evaluate(r, st, Observation{At: t0.Add(13 * time.Minute), Restarts: f(0)}) |
| - if len(ev) != 1 || ev[0].Firing || ev[0].Detail != "carlos-litestream.service" { |
| - t.Fatalf("a zero reading must clear and name the unit that had fired: %+v", ev) |
| + st, ev = Evaluate(r, st, Observation{At: t0.Add(16 * time.Minute), Restarts: one(UnitRestarts{Count: 0, Settled: true})}) |
| + if len(ev) != 1 || ev[0].Firing || ev[0].Rule != rule { |
| + t.Fatalf("settled, up and quiet must clear: %+v", ev) |
| } |
| - if rs := st.Rules["restarts"]; rs.Firing || rs.Detail != "" || !rs.FiredAt.IsZero() { |
| - t.Fatalf("state after clearing: %+v", rs) |
| + // The key stays, quiet: that is how the console reads the clear. |
| + if rs, kept := st.Rules[rule]; !kept || rs.Firing || !rs.FiredAt.IsZero() { |
| + t.Fatalf("state after clearing: %+v (kept %v)", rs, kept) |
| } |
| - // Quiet stays quiet. |
| - if _, ev = Evaluate(r, st, Observation{At: t0.Add(14 * time.Minute), Restarts: f(0)}); len(ev) != 0 { |
| + if _, ev = Evaluate(r, st, Observation{At: t0.Add(17 * time.Minute), Restarts: one(UnitRestarts{Count: 0, Settled: true})}); len(ev) != 0 { |
| t.Fatalf("a quiet rule emitted: %v", ev) |
| } |
| } |
| + |
| +// Each unit has its own rule: a second unit that starts looping while the |
| +// first alert is up is its own page, and the first recovering does not clear |
| +// the second. |
| +func TestRestartsRuleIsPerUnit(t *testing.T) { |
| + r := DefaultRules() |
| + t0 := time.Date(2026, 10, 13, 12, 0, 0, 0, time.UTC) |
| + relay, ls := RuleFor("restarts", "carlos-relay.service"), RuleFor("restarts", "carlos-litestream.service") |
| + st := AlertState{V: 1, Box: "b"} |
| + |
| + st, ev := Evaluate(r, st, Observation{At: t0, Restarts: map[string]UnitRestarts{ |
| + "carlos-relay.service": {Count: 5}, |
| + "carlos-litestream.service": {Count: 0, Settled: true}, |
| + }}) |
| + if len(ev) != 1 || ev[0].Rule != relay || !ev[0].Firing { |
| + t.Fatalf("relay looping: %+v", ev) |
| + } |
| + st, ev = Evaluate(r, st, Observation{At: t0.Add(time.Minute), Restarts: map[string]UnitRestarts{ |
| + "carlos-relay.service": {Count: 6}, |
| + "carlos-litestream.service": {Count: 9}, |
| + }}) |
| + if len(ev) != 1 || ev[0].Rule != ls || !ev[0].Firing || ev[0].Value != 9 { |
| + t.Fatalf("a second unit looping must fire its own rule: %+v", ev) |
| + } |
| + st, ev = Evaluate(r, st, Observation{At: t0.Add(20 * time.Minute), Restarts: map[string]UnitRestarts{ |
| + "carlos-relay.service": {Count: 0, Settled: true}, |
| + "carlos-litestream.service": {Count: 2}, |
| + }}) |
| + if len(ev) != 1 || ev[0].Rule != relay || ev[0].Firing { |
| + t.Fatalf("relay recovering must clear only relay: %+v", ev) |
| + } |
| + if !st.Rules[ls].Firing { |
| + t.Fatalf("litestream's alert went with relay's: %+v", st.Rules) |
| + } |
| + // The unit is unloaded (an operator removed it): nothing left to watch. |
| + _, ev = Evaluate(r, st, Observation{At: t0.Add(21 * time.Minute), Restarts: map[string]UnitRestarts{ |
| + "carlos-relay.service": {Count: 0, Settled: true}, |
| + }}) |
| + if len(ev) != 1 || ev[0].Rule != ls || ev[0].Firing { |
| + t.Fatalf("an unloaded unit's alert must clear: %+v", ev) |
| + } |
| +} |
| + |
| +func TestSplitRule(t *testing.T) { |
| + for rule, want := range map[string][2]string{ |
| + "disk": {"disk", ""}, |
| + "restarts@carlos-litestream.service": {"restarts", "carlos-litestream.service"}, |
| + "restarts@carlos-app@shop.service": {"restarts", "carlos-app@shop.service"}, |
| + } { |
| + if n, s := SplitRule(rule); n != want[0] || s != want[1] { |
| + t.Errorf("SplitRule(%q) = %q, %q; want %q, %q", rule, n, s, want[0], want[1]) |
| + } |
| + } |
| + if got := RuleFor("restarts", "carlos-relay.service"); got != "restarts@carlos-relay.service" { |
| + t.Errorf("RuleFor = %q", got) |
| + } |
| +} |
diff --git a/internal/ops/litestream_memory.go b/internal/ops/litestream_memory.go| index 16726e5ae..c503cfa2a 100644 |
| --- a/internal/ops/litestream_memory.go |
| +++ b/internal/ops/litestream_memory.go |
| @@ -16,9 +16,12 @@ package ops |
| // this verb keeps the unit's own default. |
| import ( |
| + "errors" |
| "fmt" |
| "os" |
| "path/filepath" |
| + |
| + "github.com/carlosframework/platform/internal/safefile" |
| ) |
| const ( |
| @@ -73,17 +76,42 @@ func LitestreamMemoryDropIn(n int) string { |
| fmt.Sprintf("Environment=GOMEMLIMIT=%dMiB\n", litestreamGoMemLimit(max)>>20) |
| } |
| -// writeMemoryDropIn writes the drop-in for n databases to path and, when the |
| -// file changed, has systemd re-read its units through reload. A reload that |
| -// fails is returned with wrote still true: the file is in place and the |
| -// caller says how to finish the job by hand. |
| -func writeMemoryDropIn(path string, n int, reload func() error) (wrote bool, err error) { |
| +// writeMemoryDropIn makes the drop-in at path state the cap for n databases |
| +// and has systemd re-read its units through reload. It reports whether the |
| +// cap systemd holds changed. |
| +// |
| +// The file is replaced whole (safefile: staged beside it, synced, renamed), |
| +// because systemd may re-read the directory at any moment, on a self-update's |
| +// reload for one, and a half-written drop-in falls back to the unit's smaller |
| +// default under a replicator that is already using more. |
| +// |
| +// A reload that fails puts the previous file back. The file on disk then |
| +// still differs from what this verb wants, so the next run tries again; |
| +// leaving the new file in place would have every later run find nothing to |
| +// change and never reload. |
| +func writeMemoryDropIn(path string, n int, reload func() error) (changed bool, err error) { |
| + want := LitestreamMemoryDropIn(n) |
| + prior, readErr := os.ReadFile(path) |
| + switch { |
| + case readErr == nil && string(prior) == want: |
| + return false, nil |
| + case readErr != nil && !os.IsNotExist(readErr): |
| + return false, readErr |
| + } |
| if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { |
| return false, err |
| } |
| - wrote, err = writeIfChanged(path, LitestreamMemoryDropIn(n)) |
| - if err != nil || !wrote { |
| - return wrote, err |
| + if err := safefile.WriteFile(path, []byte(want), 0o644); err != nil { |
| + return false, err |
| + } |
| + if err := reload(); err != nil { |
| + var undo error |
| + if readErr == nil { |
| + undo = safefile.WriteFile(path, prior, 0o644) |
| + } else { |
| + undo = os.Remove(path) |
| + } |
| + return false, errors.Join(fmt.Errorf("systemd did not reload: %w", err), undo) |
| } |
| - return true, reload() |
| + return true, nil |
| } |
diff --git a/internal/ops/ops.go b/internal/ops/ops.go| index e4192b0ec..e0855a3ba 100644 |
| --- a/internal/ops/ops.go |
| +++ b/internal/ops/ops.go |
| @@ -164,14 +164,13 @@ should no longer be replicated.`, |
| } |
| if dropIn != "" && dropIn != "none" { |
| capped, err := writeMemoryDropIn(dropIn, len(dbs), systemdDaemonReload) |
| - switch { |
| - case err != nil && !capped: |
| + if err != nil { |
| // Loud, not fatal: the config above is written and correct, and |
| // refusing the restart it needs would trade a cap that may be too |
| - // small for a database set that is certainly stale. |
| - fmt.Fprintf(os.Stderr, "litestream: WARNING: could not write the memory cap to %s: %v\n The replicator keeps its current cap.\n", dropIn, err) |
| - case err != nil: |
| - fmt.Fprintf(os.Stderr, "litestream: WARNING: wrote the memory cap to %s but systemd did not reload: %v\n Run: systemctl daemon-reload && systemctl restart carlos-litestream\n", dropIn, err) |
| + // small for a database set that is certainly stale. Nothing is |
| + // left half-done: the next run finds the cap unwritten and tries |
| + // again. |
| + fmt.Fprintf(os.Stderr, "litestream: WARNING: could not set the memory cap in %s: %v\n The replicator keeps its current cap. The next run will try again.\n", dropIn, err) |
| } |
| if capped { |
| fmt.Fprintf(os.Stderr, "litestream: memory cap %dM for %d databases (%s)\n", LitestreamMemoryMax(len(dbs))>>20, len(dbs), dropIn) |
diff --git a/internal/ops/ops_test.go b/internal/ops/ops_test.go| index 190e8b784..a3a414636 100644 |
| --- a/internal/ops/ops_test.go |
| +++ b/internal/ops/ops_test.go |
| @@ -924,9 +924,9 @@ func TestWriteMemoryDropInReloadsOnlyOnChange(t *testing.T) { |
| reloads := 0 |
| reload := func() error { reloads++; return nil } |
| - wrote, err := writeMemoryDropIn(path, 100, reload) |
| - if err != nil || !wrote || reloads != 1 { |
| - t.Fatalf("first write: wrote=%v err=%v reloads=%d, want true, nil, 1", wrote, err, reloads) |
| + changed, err := writeMemoryDropIn(path, 100, reload) |
| + if err != nil || !changed || reloads != 1 { |
| + t.Fatalf("first write: changed=%v err=%v reloads=%d, want true, nil, 1", changed, err, reloads) |
| } |
| body, err := os.ReadFile(path) |
| if err != nil { |
| @@ -937,26 +937,55 @@ func TestWriteMemoryDropInReloadsOnlyOnChange(t *testing.T) { |
| t.Errorf("drop-in for 100 databases lacks %q:\n%s", strings.TrimSpace(want), body) |
| } |
| } |
| + if fi, err := os.Stat(path); err != nil || fi.Mode().Perm() != 0o644 { |
| + t.Errorf("drop-in mode = %v (%v), want 0644: systemd reads it as root, operators as anyone", fi.Mode().Perm(), err) |
| + } |
| + if ents, _ := os.ReadDir(filepath.Dir(path)); len(ents) != 1 { |
| + t.Errorf("the drop-in directory holds %d entries, want only the drop-in (no staging left behind)", len(ents)) |
| + } |
| // The same count changes no byte, so it reloads nothing. |
| - if wrote, err := writeMemoryDropIn(path, 100, reload); err != nil || wrote || reloads != 1 { |
| - t.Fatalf("same count: wrote=%v err=%v reloads=%d, want false, nil, 1", wrote, err, reloads) |
| + if changed, err := writeMemoryDropIn(path, 100, reload); err != nil || changed || reloads != 1 { |
| + t.Fatalf("same count: changed=%v err=%v reloads=%d, want false, nil, 1", changed, err, reloads) |
| } |
| - if wrote, err := writeMemoryDropIn(path, 101, reload); err != nil || !wrote || reloads != 2 { |
| - t.Fatalf("one more database: wrote=%v err=%v reloads=%d, want true, nil, 2", wrote, err, reloads) |
| + if changed, err := writeMemoryDropIn(path, 101, reload); err != nil || !changed || reloads != 2 { |
| + t.Fatalf("one more database: changed=%v err=%v reloads=%d, want true, nil, 2", changed, err, reloads) |
| } |
| } |
| -// A reload that fails leaves the file written and says so, so the verb can |
| -// tell the operator how to finish rather than report nothing changed. |
| -func TestWriteMemoryDropInReportsAFailedReload(t *testing.T) { |
| +// A reload that fails must not strand the new cap on disk unloaded: every |
| +// later run would find the file already right and never reload it. The |
| +// previous file comes back, so the next run tries again and, when the reload |
| +// works, reports the change. |
| +func TestWriteMemoryDropInRetriesAfterAFailedReload(t *testing.T) { |
| path := filepath.Join(t.TempDir(), "d", "50-memory.conf") |
| - wrote, err := writeMemoryDropIn(path, 200, func() error { return errors.New("no systemd here") }) |
| - if !wrote || err == nil { |
| - t.Fatalf("wrote=%v err=%v, want true and the reload error", wrote, err) |
| + broken := func() error { return errors.New("no systemd here") } |
| + working := func() error { return nil } |
| + |
| + // No file before: a failed reload leaves none. |
| + if changed, err := writeMemoryDropIn(path, 200, broken); changed || err == nil { |
| + t.Fatalf("changed=%v err=%v, want false and the reload error", changed, err) |
| + } |
| + if _, err := os.Stat(path); !os.IsNotExist(err) { |
| + t.Fatalf("a failed first reload left the drop-in in place (stat err %v)", err) |
| + } |
| + if changed, err := writeMemoryDropIn(path, 200, working); !changed || err != nil { |
| + t.Fatalf("retry: changed=%v err=%v, want true, nil", changed, err) |
| + } |
| + |
| + // A file before: a failed reload puts its bytes back. |
| + before, _ := os.ReadFile(path) |
| + if changed, err := writeMemoryDropIn(path, 400, broken); changed || err == nil { |
| + t.Fatalf("changed=%v err=%v, want false and the reload error", changed, err) |
| + } |
| + if after, _ := os.ReadFile(path); string(after) != string(before) { |
| + t.Fatalf("a failed reload left the new cap on disk:\n%s", after) |
| + } |
| + if changed, err := writeMemoryDropIn(path, 400, working); !changed || err != nil { |
| + t.Fatalf("retry over a previous file: changed=%v err=%v, want true, nil", changed, err) |
| } |
| - if _, statErr := os.Stat(path); statErr != nil { |
| - t.Fatalf("the drop-in was not left in place: %v", statErr) |
| + if after, _ := os.ReadFile(path); !strings.Contains(string(after), "MemoryMax="+strconv.FormatInt(LitestreamMemoryMax(400)>>20, 10)+"M\n") { |
| + t.Fatalf("the retried cap is not the one for 400 databases:\n%s", after) |
| } |
| } |
diff --git a/internal/provision/plan14_root_reads_test.go b/internal/provision/plan14_root_reads_test.go| index 7de4dc0b6..41ccd4f28 100644 |
| --- a/internal/provision/plan14_root_reads_test.go |
| +++ b/internal/provision/plan14_root_reads_test.go |
| @@ -665,9 +665,9 @@ func TestPlan14RootReadCensus(t *testing.T) { |
| t.Fatal(err) |
| } |
| // Census ruling ls284 (#284): the litestream memory drop-in, the unit's |
| - // restart backoff and the agent's restart-counter probe add five frozen |
| + // restart backoff and the agent's restart-counter probe and its saved state add seven frozen |
| // declarations. |
| - seal = "e22293f6cf4c0ab793a82f1f1c94d6f118aac578228289184c981736b76ff3d1" |
| + seal = "d17b4d9b8522d2a671b4e0ecdb010a6eeb2dc4c32d2e79a70266cc66845b56ca" |
| errs := append(compareRootReads(got, inv), checkRootReadLegacy(inv, seal)...) |
| errs = append(errs, checkRootReadDeclarations(got, inv)...) |
| if len(errs) > 0 { |
diff --git a/internal/provision/testdata/plan14-root-reads-ls284.tsv b/internal/provision/testdata/plan14-root-reads-ls284.tsv| index 5686913c3..9bcbdfcf1 100644 |
| --- a/internal/provision/testdata/plan14-root-reads-ls284.tsv |
| +++ b/internal/provision/testdata/plan14-root-reads-ls284.tsv |
| @@ -2,17 +2,21 @@ |
| # The ruling is requested from the census owner on amadan #280; these rows state the branch's own account. Earlier overlays are immutable. |
| @legacy f942faf6b1bda4b171de3930c7a9f5627d4db32a2f802c5ecb10ef26410e0af9 internal/console/templates/metrics.html external legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=The alerts table shows a rule's detail beside its name. Same alert state objects, read by the same console sweep; no new source or authority. file:external@0 8732b0e3a4e60029b6c13109be12b15f0a48f51f0fb3c7578e6a52fc26e93140 |
| internal/console/templates/metrics.html external legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=The alerts table shows a rule's detail beside its name. Same alert state objects, read by the same console sweep; no new source or authority. file:external@0 8732b0e3a4e60029b6c13109be12b15f0a48f51f0fb3c7578e6a52fc26e93140 |
| -@legacy e7cbc320e7a7bf386dfba3b4e937f9096e66da05b291fc09291cde981e02ed70 internal/host/restartwatch.go platformUnitRestarts legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=The agent, as carlos, asks systemd for the restart counters of loaded carlos-*.service units; a fixed command with no registry or tenant input, the same class as internal/host/probes.go adoptTimerActive under J4 root-input. Feeds an alert only; no root decision. file:Command@60 774285fff720047ba8ece6d33027b61a80aa3c38aae803c7e8da4711f423fb9e |
| -internal/host/restartwatch.go platformUnitRestarts legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=The agent, as carlos, asks systemd for the restart counters of loaded carlos-*.service units; a fixed command with no registry or tenant input, the same class as internal/host/probes.go adoptTimerActive under J4 root-input. Feeds an alert only; no root decision. file:Command@60 774285fff720047ba8ece6d33027b61a80aa3c38aae803c7e8da4711f423fb9e |
| +@legacy 94e5b066d93dbc5769523cae394e58c44b75643afdd2af60a76b91a35a506a99 internal/host/restartwatch.go loadRestartWatch legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=The agent, as carlos, reads back its own restart-watch state from the data directory's monitor folder, beside the alert state it already keeps there. Feeds an alert only; never read by root and no root decision. file:ReadFile@81 96d67295e3e0842253c19e7df5dcefd15485b38cb48b4e249cc45a6ee193d976 |
| +internal/host/restartwatch.go loadRestartWatch legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=The agent, as carlos, reads back its own restart-watch state from the data directory's monitor folder, beside the alert state it already keeps there. Feeds an alert only; never read by root and no root decision. file:ReadFile@81 96d67295e3e0842253c19e7df5dcefd15485b38cb48b4e249cc45a6ee193d976 |
| +@legacy bc86270eeccc35765f5ecd7771202ed99605065e5e703c23bc14934aa79ddb3d internal/host/restartwatch.go platformUnitRestarts legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=The agent, as carlos, asks systemd for the restart counters of loaded carlos-*.service units; a fixed command with no registry or tenant input, the same class as internal/host/probes.go adoptTimerActive under J4 root-input. Feeds an alert only; no root decision. file:CommandContext@166 fde6a651c0a8a66c324124a2bd3085959c9b575cae08e2d65d2eb666761b0dcc |
| +internal/host/restartwatch.go platformUnitRestarts legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=The agent, as carlos, asks systemd for the restart counters of loaded carlos-*.service units; a fixed command with no registry or tenant input, the same class as internal/host/probes.go adoptTimerActive under J4 root-input. Feeds an alert only; no root decision. file:CommandContext@166 fde6a651c0a8a66c324124a2bd3085959c9b575cae08e2d65d2eb666761b0dcc |
| @legacy e39e5045ef43883a78c327f56fbd90e501ff3c012c8ddf512b0f7585b75623c1 internal/host/units/carlos-litestream.service external legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Memory cap default 512M with GOMEMLIMIT, and restart backoff (RestartSteps, RestartMaxDelaySec). User, paths, command and sandboxing unchanged; no new source or authority. file:external@0 2fc82f963202103a16a89aaecdcaeb89c6b9fee8e9d0e09cb76389a2326c1714 |
| internal/host/units/carlos-litestream.service external legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Memory cap default 512M with GOMEMLIMIT, and restart backoff (RestartSteps, RestartMaxDelaySec). User, paths, command and sandboxing unchanged; no new source or authority. file:external@0 2fc82f963202103a16a89aaecdcaeb89c6b9fee8e9d0e09cb76389a2326c1714 |
| -@legacy 0996b7dc2ceda55b3f4dadaadbb5048c625eeb77ba1924602cb512f34d8538d8 internal/ops/ops.go runLitestream legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Same registry open and prior-config read. Adds one root-owned write, the memory drop-in beside the unit, sized from the database count this run computed, and a daemon-reload; no new input is trusted. data-literal@214,file:Open@1383,file:ReadFile@2357 9e392f103ef25698243e9cf44f5e9164ae751c58c6a536f6185641c04c3aec30 |
| -internal/ops/ops.go runLitestream legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Same registry open and prior-config read. Adds one root-owned write, the memory drop-in beside the unit, sized from the database count this run computed, and a daemon-reload; no new input is trusted. data-literal@214,file:Open@1383,file:ReadFile@2357 9e392f103ef25698243e9cf44f5e9164ae751c58c6a536f6185641c04c3aec30 |
| +@legacy 63b9984a01df41c70dfd5364238390d2a78d6b814b46f03f5f7c25f799d386e7 internal/ops/litestream_memory.go writeMemoryDropIn legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Root reads the root-owned drop-in this verb itself wrote, to decide whether to rewrite it; the same compare-before-write as writeIfChanged under J4 root-input. The path is fixed or an operator-supplied flag. file:ReadFile@146 9ba23f0626a3f649027d809610c6fa0e02584f9e501517de69b9d57767e06d90 |
| +internal/ops/litestream_memory.go writeMemoryDropIn legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Root reads the root-owned drop-in this verb itself wrote, to decide whether to rewrite it; the same compare-before-write as writeIfChanged under J4 root-input. The path is fixed or an operator-supplied flag. file:ReadFile@146 9ba23f0626a3f649027d809610c6fa0e02584f9e501517de69b9d57767e06d90 |
| +@legacy dc2175dd0445552045146497dd473327ec0ddca2de728f6b8a2c5c755b82420d internal/ops/ops.go runLitestream legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Same registry open and prior-config read. Adds one root-owned write, the memory drop-in beside the unit, sized from the database count this run computed, and a daemon-reload; no new input is trusted. data-literal@214,file:Open@1383,file:ReadFile@2357 380a01961cc02f2a18673bc4ad0a0bc1112905d4726f8ab9e7b370da36686306 |
| +internal/ops/ops.go runLitestream legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Same registry open and prior-config read. Adds one root-owned write, the memory drop-in beside the unit, sized from the database count this run computed, and a daemon-reload; no new input is trusted. data-literal@214,file:Open@1383,file:ReadFile@2357 380a01961cc02f2a18673bc4ad0a0bc1112905d4726f8ab9e7b370da36686306 |
| @legacy cb8030cd13f53803aca45d166742f194f1cb00e1d881263a776b79a334a37189 internal/ops/ops.go systemdDaemonReload legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Root runs a fixed systemctl daemon-reload with no caller input, the same class as internal/host/bootstrap.go systemctl under J4 root-input. file:Command@48 3edadda9e71b806a78d14160f2da0b6091212c4ee55933a327856be811290a74 |
| internal/ops/ops.go systemdDaemonReload legacy accepted-legacy accepted-legacy;rev3=census-ruling-ls284;basis=Root runs a fixed systemctl daemon-reload with no caller input, the same class as internal/host/bootstrap.go systemctl under J4 root-input. file:Command@48 3edadda9e71b806a78d14160f2da0b6091212c4ee55933a327856be811290a74 |
| -@coverage internal/console c74e3c3856736aa66b02d24eee814db9323230ea06f3d5a5c89d0ecf5da6d0c2 |
| +@coverage internal/console 85d4f9521a2acc08aa1276f414be41cea32aa4275f761546e77e951d0d58b558 |
| @coverage internal/console/templates e622e1e901447dc067aba32e4e520eef4279ffcdd807695f012fd38f646d48b4 |
| -@coverage internal/host 7db3cdd4a2116f43a0ac38ed95f3d5fa0521aca5b9d7541590900ed01b8b761b |
| +@coverage internal/host 53769084266f3ae76e999ed6f7f01f3f362618535af4bf283c607e41aa51a3be |
| @coverage internal/host/units e06e4a1a0d9d45f8e7494f0419834bf23e14fb2cd4861b94ef905bfaf795a7d1 |
| -@coverage internal/metrics 68118aefc1c0088fafd6e7eb6ce825f3e6413ce6997c4eb0d9e1444227300ce1 |
| -@coverage internal/ops 0070d8bd997630d6644a52f12662fae91e80fc7874e00f8a4db4bf07450d468c |
| +@coverage internal/metrics 1adc51fc485ba895970c22277b7bff997c24ebededbd19f48a10e75cf531323a |
| +@coverage internal/ops 1c36eec5ac001ce556030e557579be93dd082d57d47c5c002ee93bdc44cf12f8 |