review fixes: exec children get oom_score_adj=+500 + memory.oom.group; tier logged per spawn; memwatch reuses IMDS helpers, logs put failures on change; module owns a same-region alerts topic (alerts_email)
11 files changed,
+234
−74
.gitignore+1 −0go.mod+1 −1infra/modules/README.md+1 −1infra/modules/region-host/.terraform.lock.hcl+0 −39infra/modules/region-host/main.tf+22 −4infra/modules/region-host/variables.tf+2 −2internal/cgroup/cgroup.go+52 −0internal/cgroup/cgroup_test.go+38 −2internal/host/memwatch.go+55 −23internal/host/memwatch_test.go+57 −0internal/host/units/carlos-edge.service+5 −2
CI passed — run details
diff --git a/.gitignore b/.gitignore| index 6b209b585..42d71f76e 100644 |
| --- a/.gitignore |
| +++ b/.gitignore |
| @@ -8,3 +8,4 @@ platform |
| # amadan reuses one workspace per repo across jobs, so an untracked |
| # directory in the checkout is that step's cache — see the step for why. |
| .amadan-cache/ |
| +infra/modules/**/.terraform.lock.hcl |
diff --git a/go.mod b/go.mod| index 6f666b88e..addca88a5 100644 |
| --- a/go.mod |
| +++ b/go.mod |
| @@ -6,7 +6,6 @@ require ( |
| github.com/aws/aws-sdk-go-v2 v1.44.0 |
| github.com/aws/aws-sdk-go-v2/config v1.32.6 |
| github.com/aws/aws-sdk-go-v2/credentials v1.19.6 |
| - github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.16 |
| github.com/aws/aws-sdk-go-v2/service/cloudwatch v1.68.0 |
| github.com/aws/aws-sdk-go-v2/service/costexplorer v1.67.5 |
| github.com/aws/aws-sdk-go-v2/service/iam v1.58.2 |
| @@ -23,6 +22,7 @@ require ( |
| require ( |
| github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.7.14 // indirect |
| + github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.18.16 // indirect |
| github.com/aws/aws-sdk-go-v2/feature/s3/manager v1.20.18 // indirect |
| github.com/aws/aws-sdk-go-v2/internal/configsources v1.4.40 // indirect |
| github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.7.40 // indirect |
diff --git a/infra/modules/README.md b/infra/modules/README.md| index a788ee4fe..1765debb2 100644 |
| --- a/infra/modules/README.md |
| +++ b/infra/modules/README.md |
| @@ -70,7 +70,7 @@ bucket, so a wrong-region wake fails loudly at the IAM layer. |
| | `steering_record_names` | Name bound on that grant, e.g. `["*.oncarlos.com"]`. Empty (default) grants A-record UPSERT/DELETE on the WHOLE zone, apex included — set it. | |
| | `ami_name_pattern` | AMI name filter (default `debian-13-arm64-*`). | |
| | `ami_owner` | AMI owner account (default `136693071363`, Debian). | |
| -| `alerts_topic_arn` | SNS topic for the host's `MemAvailablePercent < 15% for 3 min` alarm (memory-pressure spec §2.4). Empty (default): no alarm. The `cloudwatch:PutMetricData` grant on namespace `Carlos/Host` is unconditional, so the agent's memwatch duty publishes either way. | |
| +| `alerts_email` | Email for the host's `MemAvailablePercent < 15% for 3 min` alarm (memory-pressure spec §2.4). The module creates a same-region SNS topic + subscription per host (an alarm cannot notify a topic in another region). Empty (default): no topic, no alarm. Set it only once the box is publishing — missing data is treated as breaching. The `cloudwatch:PutMetricData` grant on namespace `Carlos/Host` is unconditional, so the agent's memwatch duty publishes either way. | |
| | `enable_auto_recovery` | Arm the EC2 auto-recovery alarm (HA ladder rung 0) — a system-status-check failure recovers the instance onto healthy hardware, same instance id, EIP and volumes. Default `true`; set `false` only where an external supervisor owns recovery. | |
| Outputs: `eip_public_ip` (point your DNS here), `instance_id`, |
diff --git a/infra/modules/region-host/.terraform.lock.hcl b/infra/modules/region-host/.terraform.lock.hcl| deleted file mode 100644 |
| index f81693a7c..000000000 |
| --- a/infra/modules/region-host/.terraform.lock.hcl |
| +++ /dev/null |
| @@ -1,39 +0,0 @@ |
| -# This file is maintained automatically by "tofu init". |
| -# Manual edits may be lost in future updates. |
| - |
| -provider "registry.opentofu.org/hashicorp/aws" { |
| - version = "6.62.0" |
| - constraints = ">= 6.0.0" |
| - hashes = [ |
| - "h1:5x/4mMlIqSyeMJQ8tD0A+FTNynpdHW8IA7F2zqrgpwU=", |
| - "h1:Etlx1tUvlB7sqFWPciSn9Yz1g50ctM+dGlyVMpu720I=", |
| - "h1:FBnaqFZDf3DdVb4SVpCPoT+TZQUSChNmmDViEGPV1rw=", |
| - "h1:MRIBAtFWiQAyo4kpbBUtIvxlSmpQ9Eel0nLMYCb6MH4=", |
| - "h1:MeMP80kzq1meAJQ0l+kU6KVW1S+4EhIRSy6q3L++LPU=", |
| - "h1:N8W8KgcjlG1kb9mPapfieH/vYzyNrJBsb4RS0axwg5Y=", |
| - "h1:OB5obEZKuaX4gQ7DYxYvAMaf4I6v2keeKOPfOpFMkgI=", |
| - "h1:WnhkO4yQc0QvVgx/xJFI+UbM1y8BC8yvApoDUlhb5XY=", |
| - "h1:fN9PvxrT2/rAiT86Hen0vicyMbKXAZG6VC/TEP9ZpD8=", |
| - "h1:kRrdLje5ab/tmObt9lOKOi2KCyIB8B3xZCKhcfdS2K0=", |
| - "h1:l5VASLhVAOCr2Q+7ywGqWb+JSEIJ5UjilOMBjFMdQ7U=", |
| - "h1:mugvZRK3/kSysUmpt664tMNpOZtGbQd6nGKaM4p3kwo=", |
| - "h1:nY4ct0BTUQ7se8gbnzXsPCN2tiP4mZsGnEw2juSpXNs=", |
| - "h1:qRFHk1ksyfMj2KOrs9pEntzagM+Tzqm+qznJfxVrJj0=", |
| - "h1:szVx4GPJr2hJt9t82VilK2Gu/N9WR/KbdeLZ9wb0Nek=", |
| - "zh:072d542e40ca0b8c5e081c9f834e3e41aa2ad31c01219522f314685d5c482823", |
| - "zh:0b2643473f5bb154d724e64e75632814d74acda5fd81b6488d0ed13f413152b2", |
| - "zh:108b3e175886e45c4e955f1ef323aa849dfd82fc450108238be2ab7c0d1f0c2e", |
| - "zh:3586ca03a5d07e40a67b01b7fdebc6f7636a42c2a2f6ab88635ffca8f6928dfe", |
| - "zh:47bd626153ee94e6b45533c7c02f579dc6d586d6d9580e03a040643f647be50b", |
| - "zh:49aafbe6f665b3c4c97522905fd09229f2758a782c7cfc508280607030134ef8", |
| - "zh:6e3397227c3f8f3becc04f95ebcb977d7012f025c9921f9ef5e603fe6d5da665", |
| - "zh:70f4478658a13eeb57725e3b424329494a0ffbd78ac2734effea4e60d802cfa2", |
| - "zh:774d4357447f94a5d4981c7569f85cd140ac577974174c5e7b477d834c6bfb43", |
| - "zh:7971ef7bc532ab0edb3ee739ac3010b46321ec8541f873956a20dabbd6a0f189", |
| - "zh:8ec18fba906468a9e6ce15329aa098dfb45ab1b9dca446a8fc2079a0d6ef590c", |
| - "zh:98328661edd4dfc4e4782fd03a7010854218d3fbf35435837ac33e7ee8aafe7e", |
| - "zh:9cbbf5033116f72749ebd4cf9add6aadf7e9e4bc7704eec86a8f4c139c16c120", |
| - "zh:9faf944474f9233ddbb3606bc495570d4b952fea21f09c28676f9847d619e50f", |
| - "zh:d0636d6de8a7b0b0bcc2add80556ee7118981d738998d4772558b9de0df99af1", |
| - ] |
| -} |
diff --git a/infra/modules/region-host/main.tf b/infra/modules/region-host/main.tf| index d8f94946b..5af1188d0 100644 |
| --- a/infra/modules/region-host/main.tf |
| +++ b/infra/modules/region-host/main.tf |
| @@ -340,9 +340,27 @@ resource "aws_cloudwatch_metric_alarm" "host_auto_recover" { |
| # On 2026-08-28 the flagship thrashed for 15 minutes with EC2 status ok/ok; |
| # MemAvailable would have crossed 15% minutes earlier. Missing data is |
| # BREACHING: a box that stops reporting is a box in trouble (or an agent |
| -# rolled without CARLOS_HOST_METRICS — either way, look). |
| +# rolled without CARLOS_HOST_METRICS — either way, look). Which is also |
| +# why the caller should set alerts_email only once the box is publishing |
| +# (spec §4): wiring it first buys days of ALARM mail about nothing. |
| +# |
| +# The topic lives here, in the host's region, because an alarm can only |
| +# notify a same-region topic — one account-wide topic passed to four |
| +# regional hosts would apply and then never deliver for three of them. |
| +resource "aws_sns_topic" "alerts" { |
| + count = var.alerts_email != "" ? 1 : 0 |
| + name = "${var.name}-alerts" |
| +} |
| + |
| +resource "aws_sns_topic_subscription" "alerts_email" { |
| + count = var.alerts_email != "" ? 1 : 0 |
| + topic_arn = aws_sns_topic.alerts[0].arn |
| + protocol = "email" |
| + endpoint = var.alerts_email |
| +} |
| + |
| resource "aws_cloudwatch_metric_alarm" "host_memory_low" { |
| - count = var.alerts_topic_arn != "" ? 1 : 0 |
| + count = var.alerts_email != "" ? 1 : 0 |
| alarm_name = "${var.name}-memory-low" |
| alarm_description = "MemAvailable below 15% for 3 minutes on ${var.name} — a tenant is eating the box; see the memory-pressure spec §4 for what to look at." |
| namespace = "Carlos/Host" |
| @@ -354,8 +372,8 @@ resource "aws_cloudwatch_metric_alarm" "host_memory_low" { |
| threshold = 15 |
| comparison_operator = "LessThanThreshold" |
| treat_missing_data = "breaching" |
| - alarm_actions = [var.alerts_topic_arn] |
| - ok_actions = [var.alerts_topic_arn] |
| + alarm_actions = [aws_sns_topic.alerts[0].arn] |
| + ok_actions = [aws_sns_topic.alerts[0].arn] |
| dimensions = { |
| InstanceId = aws_instance.host.id |
| } |
diff --git a/infra/modules/region-host/variables.tf b/infra/modules/region-host/variables.tf| index 78d3dbc2f..3f0a898c0 100644 |
| --- a/infra/modules/region-host/variables.tf |
| +++ b/infra/modules/region-host/variables.tf |
| @@ -109,8 +109,8 @@ variable "enable_cost_explorer" { |
| default = false |
| } |
| -variable "alerts_topic_arn" { |
| - description = "SNS topic that receives this host's memory alarm (memory-pressure spec §2.4). Empty: no alarm is created; the PutMetricData grant is unconditional so the agent's memwatch duty can publish either way." |
| +variable "alerts_email" { |
| + description = "Email that receives this host's memory alarm (memory-pressure spec §2.4). A CloudWatch alarm can only notify an SNS topic in its own region, so the module creates a regional topic + subscription per host rather than taking a topic ARN. Empty: no topic, no alarm; the PutMetricData grant is unconditional so the agent's memwatch duty can publish either way." |
| type = string |
| default = "" |
| } |
diff --git a/internal/cgroup/cgroup.go b/internal/cgroup/cgroup.go| index 64bd89d5c..58f5491df 100644 |
| --- a/internal/cgroup/cgroup.go |
| +++ b/internal/cgroup/cgroup.go |
| @@ -65,6 +65,27 @@ var ( |
| // throttling start well before the kill. |
| func highOf(max int64) int64 { return max / 10 * 9 } |
| +// ExecOOMScoreAdj is written to every exec child's /proc/<pid>/oom_score_adj |
| +// right after it starts. The kernel's OOM killer — the memcg one that fires |
| +// when carlos-edge.service's own MemoryMax is hit included — scores per |
| +// PROCESS by RSS, and the agent (~110 MB) outweighs any single tenant |
| +// process on the flagship today, so without this the outer wall's victim |
| +// would be the agent, and Restart=always would then cycle every awake |
| +// instance: precisely what OOMPolicy=continue exists to prevent. +500 adds |
| +// half the box's RAM to a child's score; the agent stays at 0 and is never |
| +// the first pick. Raising one's own-uid process's score needs no privilege |
| +// (lowering does), which is why this is done here rather than by a unit |
| +// directive. Grandchildren inherit the value at fork; a family that forks |
| +// in the microseconds between exec and this write keeps 0 for those early |
| +// forks — the safe direction is still "a tenant process before the agent" |
| +// for everything forked after. |
| +const ExecOOMScoreAdj = "500" |
| + |
| +// oomScoreAdjPath is a seam so a test can point the write at a temp dir. |
| +var oomScoreAdjPath = func(pid int) string { |
| + return filepath.Join("/proc", strconv.Itoa(pid), "oom_score_adj") |
| +} |
| + |
| // ParseSelfCgroup extracts the cgroup v2 path from /proc/self/cgroup |
| // content — the "0::" unified-hierarchy line — relative (no leading slash), |
| // ready to join under /sys/fs/cgroup. Not-ok when there is no v2 line (a |
| @@ -241,6 +262,7 @@ func (m *Manager) Start(host string, limit Limit, build func() *exec.Cmd) (*exec |
| err = startCmd(cmd) |
| f.Close() |
| if err == nil { |
| + m.biasOOM(host, cmd) |
| return cmd, nil |
| } |
| firstErr := err |
| @@ -267,6 +289,16 @@ func (m *Manager) Start(host string, limit Limit, build func() *exec.Cmd) (*exec |
| // control file that isn't ours) is one logged line and an uncapped child. |
| // Written BEFORE the child is in the cgroup, so the cap holds from its |
| // first page. memory.max first: if that one fails, high is pointless. |
| +// |
| +// The cap bounds the cgroup's TOTAL charge — anonymous memory plus the page |
| +// cache its reads pull in — not RSS. A tenant whose cache alone exceeds |
| +// memory.high lives under constant reclaim: slow, and nothing in the |
| +// journal says why. Hence the log line: the tier a host landed on is the |
| +// first thing to read when an app "got slow after the roll". |
| +// |
| +// memory.oom.group=1 makes an OOM kill inside this cgroup take the whole |
| +// family: a hub whose children outlive it would otherwise leave orphans |
| +// holding the socket while the activator respawns a fresh hub beside them. |
| func (m *Manager) applyLimit(host, dir string, limit Limit) { |
| if limit.MemoryMax <= 0 { |
| return |
| @@ -278,6 +310,10 @@ func (m *Manager) applyLimit(host, dir string, limit Limit) { |
| if err := os.WriteFile(filepath.Join(dir, "memory.high"), []byte(strconv.FormatInt(highOf(limit.MemoryMax), 10)), 0o644); err != nil { |
| m.logf("cgroup: %s: memory.high: %v — child is capped but not throttled first", host, err) |
| } |
| + if err := os.WriteFile(filepath.Join(dir, "memory.oom.group"), []byte("1"), 0o644); err != nil { |
| + m.logf("cgroup: %s: memory.oom.group: %v — an OOM kill here takes one process, not the family", host, err) |
| + } |
| + m.logf("cgroup: %s: memory.max=%dM memory.high=%dM", host, limit.MemoryMax>>20, highOf(limit.MemoryMax)>>20) |
| } |
| // startAndMove is the fallback spawn: plain start, then write the child's |
| @@ -304,13 +340,29 @@ func (m *Manager) startAndMove(host string, build func() *exec.Cmd) (*exec.Cmd, |
| if err := os.WriteFile(procs, []byte(strconv.Itoa(cmd.Process.Pid)), 0o644); err != nil { |
| m.logf("cgroup: %s: move pid %d: %v — child stays unaccounted", host, cmd.Process.Pid, err) |
| } |
| + m.biasOOM(host, cmd) |
| return cmd, nil |
| } |
| +// biasOOM writes ExecOOMScoreAdj for a just-started child. Best-effort: a |
| +// refused write is one line, and the child runs with the agent's score. |
| +func (m *Manager) biasOOM(host string, cmd *exec.Cmd) { |
| + if cmd == nil || cmd.Process == nil { |
| + return |
| + } |
| + if err := os.WriteFile(oomScoreAdjPath(cmd.Process.Pid), []byte(ExecOOMScoreAdj), 0o644); err != nil { |
| + m.logf("cgroup: %s: oom_score_adj for pid %d: %v — the agent and this child score alike at an edge-tree OOM", host, cmd.Process.Pid, err) |
| + } |
| +} |
| + |
| // CleanupHost removes host's per-host cgroup dir after its children exit. |
| // Nil-safe and rmdir-tolerant: while a sibling (instance vs sidecar) is |
| // still alive the dir is non-empty and the remove fails — the LAST exiter's |
| // call succeeds, and the next agent start's sweep catches anything missed. |
| +// (On real cgroupfs the control files applyLimit wrote don't count: rmdir |
| +// ignores interface files. In the temp-dir test fixture they are ordinary |
| +// files, so a test that caps a host and then expects CleanupHost to remove |
| +// the dir must delete them first — that failure has no production analogue.) |
| func (m *Manager) CleanupHost(host string) { |
| if m == nil { |
| return |
diff --git a/internal/cgroup/cgroup_test.go b/internal/cgroup/cgroup_test.go| index 25debd0d6..b7858b343 100644 |
| --- a/internal/cgroup/cgroup_test.go |
| +++ b/internal/cgroup/cgroup_test.go |
| @@ -284,8 +284,9 @@ func TestStartWritesMemoryLimit(t *testing.T) { |
| } |
| dir := m.hostDir("h.example.com") |
| for file, want := range map[string]string{ |
| - "memory.max": "1073741824", |
| - "memory.high": "966367638", // max/10*9, integer arithmetic |
| + "memory.max": "1073741824", |
| + "memory.high": "966367638", // max/10*9, integer arithmetic |
| + "memory.oom.group": "1", |
| } { |
| got, err := os.ReadFile(filepath.Join(dir, file)) |
| if err != nil { |
| @@ -339,6 +340,41 @@ func TestStartLimitWriteFailureStillSpawns(t *testing.T) { |
| if err != nil || cmd == nil { |
| t.Fatalf("Start = (%v, %v), want a started cmd despite the limit failure", cmd, err) |
| } |
| + // max failed, so high must NOT have been written: a high with no max |
| + // is a throttle with no wall, and the doc comment promises the short |
| + // circuit. |
| + if _, err := os.Stat(filepath.Join(dir, "memory.high")); !os.IsNotExist(err) { |
| + t.Errorf("memory.high written after memory.max failed: %v", err) |
| + } |
| +} |
| + |
| +// TestStartBiasesChildOOMScore: a started child gets ExecOOMScoreAdj |
| +// written to its oom_score_adj (redirected into a temp dir here), on both |
| +// spawn paths. Without it the agent — the largest process in the edge |
| +// tree — is the memcg OOM killer's first pick when the outer wall fires. |
| +func TestStartBiasesChildOOMScore(t *testing.T) { |
| + root := newDelegatedRoot(t) |
| + m, err := SetupAt(root, t.Logf) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + adj := t.TempDir() |
| + orig := oomScoreAdjPath |
| + oomScoreAdjPath = func(pid int) string { return filepath.Join(adj, strconv.Itoa(pid)) } |
| + t.Cleanup(func() { oomScoreAdjPath = orig }) |
| + // A real start: /bin/true gives the child a pid to write for. |
| + cmd, err := m.startAndMove("h.example.com", buildTrue) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + _ = cmd.Wait() |
| + got, err := os.ReadFile(filepath.Join(adj, strconv.Itoa(cmd.Process.Pid))) |
| + if err != nil { |
| + t.Fatalf("oom_score_adj not written: %v", err) |
| + } |
| + if string(got) != ExecOOMScoreAdj { |
| + t.Errorf("oom_score_adj = %q, want %q", got, ExecOOMScoreAdj) |
| + } |
| } |
| func TestTiers(t *testing.T) { |
diff --git a/internal/host/memwatch.go b/internal/host/memwatch.go| index bf2ac4a8a..54781fad8 100644 |
| --- a/internal/host/memwatch.go |
| +++ b/internal/host/memwatch.go |
| @@ -2,7 +2,9 @@ package host |
| import ( |
| "context" |
| + "errors" |
| "log" |
| + "net/http" |
| "os" |
| "strconv" |
| "strings" |
| @@ -10,7 +12,6 @@ import ( |
| "github.com/aws/aws-sdk-go-v2/aws" |
| awsconfig "github.com/aws/aws-sdk-go-v2/config" |
| - "github.com/aws/aws-sdk-go-v2/feature/ec2/imds" |
| "github.com/aws/aws-sdk-go-v2/service/cloudwatch" |
| "github.com/aws/aws-sdk-go-v2/service/cloudwatch/types" |
| ) |
| @@ -49,28 +50,50 @@ type memwatch struct { |
| mem func() (*int64, *int64) // MemTotal, MemAvailable in bytes |
| pressure func() (string, error) // /proc/pressure/memory content |
| logf func(string, ...any) |
| + // lastPutErr is the text of the put failure last logged, "" while |
| + // puts succeed. Failures log on CHANGE (and recovery logs once), not |
| + // once a minute: a box rolled before its IAM grant lands would |
| + // otherwise write 1,440 identical lines a day. |
| + lastPutErr string |
| +} |
| + |
| +// memwatchArmed is the host.env gate, split out so the three answers it |
| +// can give are table-testable: armed, off (unset), or off with a value |
| +// this duty doesn't understand — the last is the one an operator wants to |
| +// see in the journal, because they typed it. |
| +func memwatchArmed(getenv func(string) string) (armed bool, why string) { |
| + switch v := strings.TrimSpace(getenv("CARLOS_HOST_METRICS")); v { |
| + case "cloudwatch": |
| + return true, "" |
| + case "": |
| + return false, "CARLOS_HOST_METRICS unset" |
| + default: |
| + return false, "CARLOS_HOST_METRICS=" + strconv.Quote(v) + ` (the only value is "cloudwatch")` |
| + } |
| } |
| // memwatchLoop arms the duty when host.env asks for it. One log line either |
| // way; a box that can't resolve its instance id or build a client logs why |
| // and stays quiet rather than retrying — the operator set the env, so a |
| // silent duty is a bug they'd want to see at arm time, in the journal. |
| +// Process-lifetime, like statusLoop: nothing stops the ticker. |
| func memwatchLoop() { |
| - if v := strings.TrimSpace(os.Getenv("CARLOS_HOST_METRICS")); v != "cloudwatch" { |
| - if v != "" { |
| - log.Printf("agent: memwatch disabled: CARLOS_HOST_METRICS=%q (the only value is \"cloudwatch\")", v) |
| - } else { |
| - log.Printf("agent: memwatch disabled (CARLOS_HOST_METRICS unset)") |
| - } |
| + if armed, why := memwatchArmed(os.Getenv); !armed { |
| + log.Printf("agent: memwatch disabled (%s)", why) |
| return |
| } |
| ctx := context.Background() |
| + // Region comes from the default chain — AWS_REGION in host.env, else |
| + // IMDS. A compatible-store deployment that points AWS_REGION at its |
| + // own object store (internal/lease/endpoint.go's note) would aim the |
| + // CloudWatch client at nonsense, which is one more reason this duty |
| + // is opt-in per box rather than on wherever credentials resolve. |
| cfg, err := awsconfig.LoadDefaultConfig(ctx) |
| if err != nil { |
| log.Printf("agent: memwatch disabled: load aws config: %v", err) |
| return |
| } |
| - id, err := imdsInstanceID(ctx, cfg) |
| + id, err := imdsInstanceID() |
| if err != nil { |
| log.Printf("agent: memwatch disabled: instance id: %v", err) |
| return |
| @@ -83,15 +106,14 @@ func memwatchLoop() { |
| logf: log.Printf, |
| } |
| log.Printf("agent: memwatch armed (%s, every %s, instance %s)", memwatchNamespace, memwatchPeriod, id) |
| - t := time.NewTicker(memwatchPeriod) |
| - defer t.Stop() |
| - for now := range t.C { |
| + for now := range time.Tick(memwatchPeriod) { |
| w.tick(ctx, now) |
| } |
| } |
| // tick reads, builds, puts. A failed read skips the tick (a zero would look |
| -// like a full box); a failed put is one line and the next tick tries again. |
| +// like a full box); a failed put is logged when it first happens or changes, |
| +// and the next tick tries again. |
| func (w *memwatch) tick(ctx context.Context, now time.Time) { |
| total, avail := w.mem() |
| if total == nil || avail == nil || *total <= 0 { |
| @@ -105,8 +127,14 @@ func (w *memwatch) tick(ctx context.Context, now time.Time) { |
| } |
| ctx, cancel := context.WithTimeout(ctx, 20*time.Second) |
| defer cancel() |
| - if _, err := w.put.PutMetricData(ctx, memwatchData(w.instanceID, availPct, psi, hasPSI, now)); err != nil { |
| - w.logf("memwatch: put: %v", err) |
| + _, err := w.put.PutMetricData(ctx, memwatchData(w.instanceID, availPct, psi, hasPSI, now)) |
| + switch { |
| + case err != nil && err.Error() != w.lastPutErr: |
| + w.lastPutErr = err.Error() |
| + w.logf("memwatch: put: %v (logged once until it changes)", err) |
| + case err == nil && w.lastPutErr != "": |
| + w.lastPutErr = "" |
| + w.logf("memwatch: put recovered") |
| } |
| } |
| @@ -136,16 +164,20 @@ func memwatchData(instanceID string, availPct, psi float64, hasPSI bool, now tim |
| } |
| // imdsInstanceID asks the metadata service which instance this is — the |
| -// dimension the alarm keys on. The IMDS uid-gate (harden-host) lets the |
| -// carlos user through by construction. |
| -func imdsInstanceID(ctx context.Context, cfg aws.Config) (string, error) { |
| - ctx, cancel := context.WithTimeout(ctx, 5*time.Second) |
| - defer cancel() |
| - out, err := imds.NewFromConfig(cfg).GetInstanceIdentityDocument(ctx, &imds.GetInstanceIdentityDocumentInput{}) |
| - if err != nil { |
| - return "", err |
| +// dimension the alarm keys on — through the same IMDSv2 token dance |
| +// readPlacementRegion uses (imdsToken/imdsLeaf). The IMDS uid-gate |
| +// (harden-host) lets the carlos user through by construction. |
| +func imdsInstanceID() (string, error) { |
| + client := &http.Client{Timeout: 2 * time.Second} |
| + token := imdsToken(client) |
| + if token == "" { |
| + return "", errors.New("no IMDSv2 token (not on EC2, or IMDS unreachable)") |
| + } |
| + id, code := imdsLeaf(client, token, "/latest/meta-data/instance-id") |
| + if code != http.StatusOK || id == "" { |
| + return "", errors.New("instance-id leaf did not answer 200") |
| } |
| - return out.InstanceID, nil |
| + return id, nil |
| } |
| // readPressureFile is the /proc/pressure/memory read; absent on a kernel |
diff --git a/internal/host/memwatch_test.go b/internal/host/memwatch_test.go| index 24ef0e436..9068a4730 100644 |
| --- a/internal/host/memwatch_test.go |
| +++ b/internal/host/memwatch_test.go |
| @@ -2,6 +2,8 @@ package host |
| import ( |
| "context" |
| + "errors" |
| + "strings" |
| "testing" |
| "time" |
| @@ -96,3 +98,58 @@ func TestMemwatchTick(t *testing.T) { |
| t.Errorf("a tick with unreadable meminfo must not put: %d calls", len(f.calls)) |
| } |
| } |
| + |
| +// TestMemwatchArmed: the gate's three answers. The unknown-value case is |
| +// the one that matters — an operator who typed CARLOS_HOST_METRICS=true |
| +// gets told what the only value is, in the journal, at arm time. |
| +func TestMemwatchArmed(t *testing.T) { |
| + for v, want := range map[string]bool{"cloudwatch": true, " cloudwatch ": true, "": false, "true": false} { |
| + armed, why := memwatchArmed(func(string) string { return v }) |
| + if armed != want { |
| + t.Errorf("%q: armed=%v, want %v", v, armed, want) |
| + } |
| + if !armed && why == "" { |
| + t.Errorf("%q: disabled with no reason", v) |
| + } |
| + } |
| + if _, why := memwatchArmed(func(string) string { return "true" }); !strings.Contains(why, `"cloudwatch"`) { |
| + t.Errorf("unknown value's reason %q does not name the only valid value", why) |
| + } |
| +} |
| + |
| +type failPutter struct{ err error } |
| + |
| +func (f *failPutter) PutMetricData(context.Context, *cloudwatch.PutMetricDataInput, ...func(*cloudwatch.Options)) (*cloudwatch.PutMetricDataOutput, error) { |
| + return nil, f.err |
| +} |
| + |
| +// TestMemwatchPutFailureLogsOnChange: the same failure minute after minute |
| +// is ONE line; a different failure is another; recovery is one more. |
| +func TestMemwatchPutFailureLogsOnChange(t *testing.T) { |
| + var lines []string |
| + fp := &failPutter{err: errors.New("AccessDenied")} |
| + w := memwatch{ |
| + put: fp, |
| + instanceID: "i-0123", |
| + mem: func() (*int64, *int64) { tot, av := int64(4000), int64(1000); return &tot, &av }, |
| + pressure: func() (string, error) { return "", errors.New("no psi") }, |
| + logf: func(f string, a ...any) { lines = append(lines, f) }, |
| + } |
| + for i := 0; i < 3; i++ { |
| + w.tick(context.Background(), time.Unix(0, 0)) |
| + } |
| + if len(lines) != 1 { |
| + t.Fatalf("3 identical failures logged %d lines, want 1: %v", len(lines), lines) |
| + } |
| + fp.err = errors.New("Throttling") |
| + w.tick(context.Background(), time.Unix(0, 0)) |
| + if len(lines) != 2 { |
| + t.Fatalf("a changed failure did not log: %v", lines) |
| + } |
| + w.put = &fakePutter{} |
| + w.tick(context.Background(), time.Unix(0, 0)) |
| + w.tick(context.Background(), time.Unix(0, 0)) |
| + if len(lines) != 3 || !strings.Contains(lines[2], "recovered") { |
| + t.Fatalf("recovery logged wrong: %v", lines) |
| + } |
| +} |
diff --git a/internal/host/units/carlos-edge.service b/internal/host/units/carlos-edge.service| index 461c6a01e..018304cec 100644 |
| --- a/internal/host/units/carlos-edge.service |
| +++ b/internal/host/units/carlos-edge.service |
| @@ -62,8 +62,11 @@ SystemCallArchitectures=native |
| # so one file fits a t4g.micro and a t4g.medium; 80% leaves what the |
| # box's other units (keymail, console, ssm, litestream, journald) use. |
| # Past High the kernel reclaims the tree (page cache first, then zram); |
| -# at Max it kills the largest process in the tree — a tenant child, by |
| -# two orders of magnitude over the agent. Per-instance caps below this |
| +# at Max the memcg OOM killer picks the highest-scoring PROCESS in the |
| +# tree. By RSS alone that would be the agent itself (~110 MB against |
| +# 20-60 MB tenant processes), so the agent writes oom_score_adj=+500 on |
| +# every exec child it starts (internal/cgroup ExecOOMScoreAdj) — a tenant |
| +# process is always the first pick. Per-instance caps below this |
| # (carlos-exec@<host> memory.max, internal/cgroup) are the inner walls. |
| MemoryHigh=70% |
| MemoryMax=80% |