diff --git a/cmd/ateom-microvm/checkpoint.go b/cmd/ateom-microvm/checkpoint.go index f55dfe2f9..8141ecc1f 100644 --- a/cmd/ateom-microvm/checkpoint.go +++ b/cmd/ateom-microvm/checkpoint.go @@ -257,6 +257,13 @@ func listFiles(dir string) ([]string, error) { // snapshot is already on disk, so this only needs to release resources. ra may be // nil (e.g. ateom restarted and lost in-memory state). func (s *AteomService) teardownActor(ctx context.Context, id string, ra *runningActor, client *ch.Client) { + // Stop offering the guest to GetWorkloadStats first, before anything below + // makes it stop answering. Clearing it here rather than alongside the + // attribution is what keeps a poll that lands mid-teardown on the + // FAILED_PRECONDITION path ("no numbers right now") instead of surfacing a + // closed connection as a failed read. + s.guestStats.Store(nil) + if client != nil { tShutdown := time.Now() shutCtx, cancel := context.WithTimeout(ctx, 10*time.Second) @@ -272,9 +279,9 @@ func (s *AteomService) teardownActor(ctx context.Context, id string, ra *running // fails the forwarding goroutines' in-flight ReadStdout/ReadStderr calls, so // they return io.EOF and exit (no goroutine leak). Guarded so a second // teardown / a never-forwarded actor is a no-op. - if ra.logAgent != nil { - _ = ra.logAgent.Close() - ra.logAgent = nil + if ra.guestAgent != nil { + _ = ra.guestAgent.Close() + ra.guestAgent = nil } // Kill the CH process ateom launched. diff --git a/cmd/ateom-microvm/internal/agentstats/agentstats.go b/cmd/ateom-microvm/internal/agentstats/agentstats.go new file mode 100644 index 000000000..abe853d7c --- /dev/null +++ b/cmd/ateom-microvm/internal/agentstats/agentstats.go @@ -0,0 +1,155 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// Package agentstats turns the kata guest agent's per-container cgroup +// accounting into the resource-usage sample ateom reports. +// +// The micro-VM ateom uses it to answer ateompb.Ateom/GetWorkloadStats. The host +// cgroup is the wrong place to look on this runtime: the guest's RAM is a fixed +// allocation cloud-hypervisor takes at boot, so the host cgroup reads roughly +// the same whether the actor is idle or saturated. The numbers that move with +// the workload are the ones the guest kernel keeps, and the agent is what can +// read them. +// +// This package is deliberately pure — it converts an already-fetched +// agentpb.CgroupStats and never talks to a guest — which keeps it testable +// without a live micro-VM and, unlike the rest of the micro-VM ateom, without +// the linux build tag. +package agentstats + +import ( + "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/third_party/kata/agentpb" +) + +// Sample is a point-in-time reading for one container, or the sum of several. +// It carries the same four numbers as the gVisor ateom's cgroupstats.Sample, +// because both feed the same four fields of GetWorkloadStatsResponse. +// +// Anything the guest did not report reads as zero rather than failing the whole +// sample, for the same reason as there: a partial reading is more useful than +// none. FromCgroupStats says which fields can do that and why. +type Sample struct { + // MemoryCurrentBytes is what is currently charged to the container's guest + // cgroup, page cache included. + MemoryCurrentBytes uint64 + + // MemoryPeakBytes is the high-water mark of MemoryCurrentBytes. Zero when + // the guest kernel does not expose one: the agent fills it from the cgroup's + // max-usage file, which cgroup v2 only grew in Linux 5.19. + MemoryPeakBytes uint64 + + // MemoryWorkingSetBytes is MemoryCurrentBytes less the reclaimable page + // cache, floored at zero — the figure to compare against a memory limit, + // since MemoryCurrentBytes drifts upward with cache the kernel would drop + // for free under pressure. + MemoryWorkingSetBytes uint64 + + // CPUUsageUsec is cumulative CPU time consumed by the container, as seen by + // the guest kernel. + CPUUsageUsec uint64 +} + +// Keys of the memory.stat entry holding reclaimable file-backed pages. The +// agent passes the guest's memory.stat through verbatim, so which one is +// present depends on the cgroup version the guest kernel gave the container: +// v2 names it inactive_file, v1 has a per-cgroup inactive_file and the +// hierarchical total_inactive_file, and the total is the one that matches what +// v2's figure means. +const ( + inactiveFileV2 = "inactive_file" + inactiveFileV1 = "total_inactive_file" +) + +// FromCgroupStats converts one container's guest cgroup accounting. +// +// It never fails. Every field the agent left out reads as zero, and cs itself +// may be nil — the agent answers without cgroup stats for a container it has no +// accounting for, which is a normal state for one that has exited rather than +// an error. The caller decides what an all-zero container means; see the +// summing in the micro-VM ateom's GetWorkloadStats. +func FromCgroupStats(cs *agentpb.CgroupStats) Sample { + mem := cs.GetMemoryStats().GetUsage() + current := mem.GetUsage() + + // Saturating rather than wrapping. The guest reads usage and memory.stat a + // moment apart, so the reclaimable figure can legitimately exceed the usage + // read beside it; on uint64 the naive subtraction gives an absurd number + // instead of the near-zero the reading means. + workingSet := current + if inactiveFile, ok := inactiveFileBytes(cs); ok { + workingSet = 0 + if inactiveFile < current { + workingSet = current - inactiveFile + } + } + + return Sample{ + MemoryCurrentBytes: current, + MemoryPeakBytes: mem.GetMaxUsage(), + + MemoryWorkingSetBytes: workingSet, + + // The agent reports CPU time in nanoseconds, matching the runc stats + // struct its own is modeled on; the proto wants microseconds. Truncating + // division loses at most a microsecond per sample, and the field is + // cumulative, so the error does not accumulate across samples. + CPUUsageUsec: cs.GetCpuStats().GetCpuUsage().GetTotalUsage() / 1000, + } +} + +// inactiveFileBytes returns the reclaimable page cache the guest reported, and +// whether it reported one at all. Absent, the working set collapses to usage, +// which over-reports by however much reclaimable cache the container holds — +// the safe direction, since it never claims the workload is using less than it +// is. +func inactiveFileBytes(cs *agentpb.CgroupStats) (uint64, bool) { + stats := cs.GetMemoryStats().GetStats() + for _, key := range []string{inactiveFileV2, inactiveFileV1} { + if v, ok := stats[key]; ok { + return v, true + } + } + return 0, false +} + +// Plus returns the sum of two samples, for accumulating an actor's containers +// into the one figure the proto reports. +// +// Summing the peaks is an upper bound on the peak of the sum, not the peak of +// the sum itself: two containers that peaked at different moments add up to a +// total the actor never actually reached. Reporting the true figure would need +// the guest to track the actor as a unit, which it does not — each container +// gets its own cgroup (see StartOverlayWorkload). The bound is the honest +// approximation, and for the single-container actors this runtime mostly serves +// it is exact. +// +// Saturating on overflow, so a nonsensical reading from one container cannot +// wrap the total to a small number and read as healthy. +func (s Sample) Plus(o Sample) Sample { + return Sample{ + MemoryCurrentBytes: addSaturating(s.MemoryCurrentBytes, o.MemoryCurrentBytes), + MemoryPeakBytes: addSaturating(s.MemoryPeakBytes, o.MemoryPeakBytes), + MemoryWorkingSetBytes: addSaturating(s.MemoryWorkingSetBytes, o.MemoryWorkingSetBytes), + CPUUsageUsec: addSaturating(s.CPUUsageUsec, o.CPUUsageUsec), + } +} + +// addSaturating returns a+b, or the maximum uint64 if that would wrap. +func addSaturating(a, b uint64) uint64 { + if sum := a + b; sum >= a { + return sum + } + const maxUint64 = ^uint64(0) + return maxUint64 +} diff --git a/cmd/ateom-microvm/internal/agentstats/agentstats_test.go b/cmd/ateom-microvm/internal/agentstats/agentstats_test.go new file mode 100644 index 000000000..6384763d8 --- /dev/null +++ b/cmd/ateom-microvm/internal/agentstats/agentstats_test.go @@ -0,0 +1,190 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package agentstats + +import ( + "testing" + + "github.com/google/go-cmp/cmp" + + "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/third_party/kata/agentpb" +) + +// cgroupStats builds an agent reading: usage/max bytes, the memory.stat entries +// the guest reported, and cumulative CPU nanoseconds. +func cgroupStats(usage, maxUsage uint64, stats map[string]uint64, cpuNanos uint64) *agentpb.CgroupStats { + return &agentpb.CgroupStats{ + MemoryStats: &agentpb.MemoryStats{ + Usage: &agentpb.MemoryData{Usage: usage, MaxUsage: maxUsage}, + Stats: stats, + }, + CpuStats: &agentpb.CpuStats{ + CpuUsage: &agentpb.CpuUsage{TotalUsage: cpuNanos}, + }, + } +} + +func TestFromCgroupStats(t *testing.T) { + for _, tc := range []struct { + name string + cs *agentpb.CgroupStats + want Sample + }{ + { + name: "cgroup v2 guest", + cs: cgroupStats(157286400, 209715200, map[string]uint64{"inactive_file": 20971520}, 1234567000), + want: Sample{ + MemoryCurrentBytes: 157286400, + MemoryPeakBytes: 209715200, + MemoryWorkingSetBytes: 136314880, + CPUUsageUsec: 1234567, + }, + }, + { + // A v1 guest names the hierarchical figure differently. Dropping to + // the working-set-equals-usage fallback here would over-report every + // sample from such a guest rather than fail visibly, so it is worth + // pinning that the alternate key is understood. + name: "cgroup v1 guest reports total_inactive_file", + cs: cgroupStats(1000, 2000, map[string]uint64{"total_inactive_file": 400}, 0), + want: Sample{MemoryCurrentBytes: 1000, MemoryPeakBytes: 2000, MemoryWorkingSetBytes: 600}, + }, + { + // v1 reports both: a per-cgroup figure and the hierarchical total. + // The total is the one that means what v2's inactive_file means. + name: "both keys present prefers the v2 name", + cs: cgroupStats(1000, 0, map[string]uint64{"inactive_file": 100, "total_inactive_file": 400}, 0), + want: Sample{MemoryCurrentBytes: 1000, MemoryWorkingSetBytes: 900}, + }, + { + // The two figures are not a consistent snapshot, so this is + // reachable; on uint64 the naive subtraction would report ~1.8e19. + name: "reclaimable cache above usage floors at zero", + cs: cgroupStats(1000, 0, map[string]uint64{"inactive_file": 4000}, 0), + want: Sample{MemoryCurrentBytes: 1000, MemoryWorkingSetBytes: 0}, + }, + { + name: "equal usage and reclaimable cache floors at zero", + cs: cgroupStats(1000, 0, map[string]uint64{"inactive_file": 1000}, 0), + want: Sample{MemoryCurrentBytes: 1000, MemoryWorkingSetBytes: 0}, + }, + { + // Nothing to subtract, so the working set collapses to usage: an + // over-report, which is the safe direction. + name: "no memory.stat entries", + cs: cgroupStats(1000, 2000, nil, 0), + want: Sample{MemoryCurrentBytes: 1000, MemoryPeakBytes: 2000, MemoryWorkingSetBytes: 1000}, + }, + { + name: "memory.stat without a reclaimable-cache entry", + cs: cgroupStats(1000, 0, map[string]uint64{"anon": 900}, 0), + want: Sample{MemoryCurrentBytes: 1000, MemoryWorkingSetBytes: 1000}, + }, + { + // Guests below Linux 5.19 have no cgroup v2 high-water mark. The rest + // of the sample must survive that. + name: "no peak reported", + cs: cgroupStats(1000, 0, map[string]uint64{"inactive_file": 100}, 5000), + want: Sample{MemoryCurrentBytes: 1000, MemoryWorkingSetBytes: 900, CPUUsageUsec: 5}, + }, + { + // The agent reports nanoseconds and the proto wants microseconds. + // A sub-microsecond total truncates to zero rather than rounding up. + name: "sub-microsecond cpu time truncates", + cs: cgroupStats(0, 0, nil, 999), + want: Sample{}, + }, + { + name: "cpu time rounds down to whole microseconds", + cs: cgroupStats(0, 0, nil, 1999), + want: Sample{CPUUsageUsec: 1}, + }, + { + name: "memory reported without cpu", + cs: &agentpb.CgroupStats{MemoryStats: &agentpb.MemoryStats{Usage: &agentpb.MemoryData{Usage: 4096}}}, + want: Sample{MemoryCurrentBytes: 4096, MemoryWorkingSetBytes: 4096}, + }, + { + name: "cpu reported without memory", + cs: &agentpb.CgroupStats{CpuStats: &agentpb.CpuStats{CpuUsage: &agentpb.CpuUsage{TotalUsage: 2000}}}, + want: Sample{CPUUsageUsec: 2}, + }, + { + // What the agent answers for a container it has no accounting for — + // one that has exited, most often. Must be a zero sample, not a + // panic: this is parsed on a timer for the life of every workload. + name: "nil cgroup stats", + cs: nil, + want: Sample{}, + }, + { + name: "empty cgroup stats", + cs: &agentpb.CgroupStats{}, + want: Sample{}, + }, + { + // The reclaimable figure is there but the usage it subtracts from is + // not, so the working set floors rather than wrapping. + name: "reclaimable cache without a usage message", + cs: &agentpb.CgroupStats{MemoryStats: &agentpb.MemoryStats{Stats: map[string]uint64{"inactive_file": 100}}}, + want: Sample{}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if diff := cmp.Diff(tc.want, FromCgroupStats(tc.cs)); diff != "" { + t.Errorf("FromCgroupStats() mismatch (-want +got):\n%s", diff) + } + }) + } +} + +func TestSamplePlus(t *testing.T) { + maxUint64 := ^uint64(0) + + for _, tc := range []struct { + name string + a, b Sample + want Sample + }{ + { + name: "adds every field", + a: Sample{MemoryCurrentBytes: 100, MemoryPeakBytes: 200, MemoryWorkingSetBytes: 90, CPUUsageUsec: 10}, + b: Sample{MemoryCurrentBytes: 1, MemoryPeakBytes: 2, MemoryWorkingSetBytes: 3, CPUUsageUsec: 4}, + want: Sample{MemoryCurrentBytes: 101, MemoryPeakBytes: 202, MemoryWorkingSetBytes: 93, CPUUsageUsec: 14}, + }, + { + // The accumulator starts here, so a zero left operand must be the + // identity or every actor's first container would be dropped. + name: "zero is the identity", + a: Sample{}, + b: Sample{MemoryCurrentBytes: 7, MemoryPeakBytes: 8, MemoryWorkingSetBytes: 9, CPUUsageUsec: 10}, + want: Sample{MemoryCurrentBytes: 7, MemoryPeakBytes: 8, MemoryWorkingSetBytes: 9, CPUUsageUsec: 10}, + }, + { + // A wrapped total would read as a nearly idle actor, which is the one + // wrong answer that looks plausible. + name: "saturates instead of wrapping", + a: Sample{MemoryCurrentBytes: maxUint64, MemoryPeakBytes: maxUint64, MemoryWorkingSetBytes: maxUint64, CPUUsageUsec: maxUint64}, + b: Sample{MemoryCurrentBytes: 1, MemoryPeakBytes: 2, MemoryWorkingSetBytes: 3, CPUUsageUsec: 4}, + want: Sample{MemoryCurrentBytes: maxUint64, MemoryPeakBytes: maxUint64, MemoryWorkingSetBytes: maxUint64, CPUUsageUsec: maxUint64}, + }, + } { + t.Run(tc.name, func(t *testing.T) { + if diff := cmp.Diff(tc.want, tc.a.Plus(tc.b)); diff != "" { + t.Errorf("Sample.Plus() mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/cmd/ateom-microvm/internal/kata/agentclient.go b/cmd/ateom-microvm/internal/kata/agentclient.go index f7a242694..a365c8b6f 100644 --- a/cmd/ateom-microvm/internal/kata/agentclient.go +++ b/cmd/ateom-microvm/internal/kata/agentclient.go @@ -230,6 +230,27 @@ func (a *AgentClient) ReadStderr(ctx context.Context, containerID, execID string return resp.GetData(), nil } +// StatsContainer returns the guest cgroup accounting for one container, as the +// kata-agent reads it from inside the guest. Mirrors +// grpc.AgentService/StatsContainer. +// +// It returns only the cgroup half of the response; the network counters +// alongside it are per-guest-interface rather than per-container and are not +// what ateom reports. A nil return with a nil error means the agent answered +// without cgroup stats, which callers should read as "no numbers", not zero. +// +// Safe to call while the stdout/stderr forwarding goroutines are reading over +// the same client: ttrpc multiplexes concurrent calls over the one connection, +// which is already what those goroutines rely on. +func (a *AgentClient) StatsContainer(ctx context.Context, containerID string) (*agentpb.CgroupStats, error) { + resp := &agentpb.StatsContainerResponse{} + req := &agentpb.StatsContainerRequest{ContainerId: containerID} + if err := a.client.Call(ctx, "grpc.AgentService", "StatsContainer", req, resp); err != nil { + return nil, fmt.Errorf("agent StatsContainer %q: %w", containerID, err) + } + return resp.GetCgroupStats(), nil +} + // StreamReader adapts the agent's repeated ReadStdout/ReadStderr unary calls into // an io.Reader, so the consumer can pump the container's output through the shared // actorlog forwarder like any other stream. Each Read issues one RPC with Len set diff --git a/cmd/ateom-microvm/main.go b/cmd/ateom-microvm/main.go index b46d750be..ccedb0c63 100644 --- a/cmd/ateom-microvm/main.go +++ b/cmd/ateom-microvm/main.go @@ -303,7 +303,7 @@ type AteomService struct { // // Kept here rather than on runningActor, even though that struct already // exists per actor: runningActor holds processes that do not exist until the - // guest is up (chCmd, vfsdCmd, logAgent), so it cannot be built before the + // guest is up (chCmd, vfsdCmd, guestAgent), so it cannot be built before the // boot, and an entry in running is what tells CheckpointWorkload a live VM is // there. Attribution has to outlive both of those constraints — it is needed // from the moment the ateom accepts the actor, including for a boot that @@ -317,6 +317,23 @@ type AteomService struct { // reader. As there, the type makes a lock-free read possible without making // one happen — GetWorkloadStats must not take lock at all. activeActor atomic.Pointer[ateomstats.ActorAttribution] + + // guestStats is what GetWorkloadStats measures with: the kata-agent client + // and the guest containers to sum. Nil whenever there is no guest to ask — + // before the containers are up, after teardownActor, and for the rest of an + // activation whose post-restore agent dial failed. + // + // Separate from activeActor because the two become true at different points: + // the attribution is retained from the moment the ateom accepts the actor, + // deliberately including a boot that never finishes, while this can only + // exist once the guest is answering. Non-nil here implies activeActor is + // set, never the reverse. + // + // Atomic for the same reason as activeActor, and it is the other half of the + // same rule: GetWorkloadStats must not take lock, so it cannot reach into + // running for the agent client the way a lifecycle RPC does. Written under + // lock like every other transition; the atomic is for the reader. + guestStats atomic.Pointer[guestStatsTarget] } var _ ateompb.AteomServer = (*AteomService)(nil) diff --git a/cmd/ateom-microvm/restore.go b/cmd/ateom-microvm/restore.go index 93d6518a9..1136412d3 100644 --- a/cmd/ateom-microvm/restore.go +++ b/cmd/ateom-microvm/restore.go @@ -307,14 +307,14 @@ func (s *AteomService) restoreFullScope(ctx context.Context, p actorBootParams, // _ovl (same as the cold run). Best-effort — a failed dial must not fail the // restore (the actor is already running); forwarding is just skipped. vsockPath := kata.VsockSocketPath(actorUID) - logAC, dialErr := dialAgentRetry(ctx, vsockPath, 15*time.Second) + guestAC, dialErr := dialAgentRetry(ctx, vsockPath, 15*time.Second) if dialErr != nil { - slog.WarnContext(ctx, "post-restore agent dial failed; actor log forwarding disabled for this restore", + slog.WarnContext(ctx, "post-restore agent dial failed; actor log forwarding and guest stats disabled for this restore", slog.String("id", actorUID), slog.Any("err", dialErr)) } else { - ra.logAgent = logAC + ra.guestAgent = guestAC for _, c := range containers { - s.startActorLogForwarding(logAC, p.actorRef, actorUID, templateNS, templateName, overlayWorkloadID(c.GetName()), c.GetName()) + s.startActorLogForwarding(guestAC, p.actorRef, actorUID, templateNS, templateName, overlayWorkloadID(c.GetName()), c.GetName()) } } @@ -322,6 +322,21 @@ func (s *AteomService) restoreFullScope(ctx context.Context, p actorBootParams, return err } s.running[actorUID] = ra + + // Publish the guest to GetWorkloadStats, past the last error return above + // for the same reason as in coldBootActor. Skipped when the dial failed: + // telemetry rides on the forwarding connection, so that activation answers + // FAILED_PRECONDITION until its next checkpoint. Not worth a second dial of + // its own — whatever kept the agent from answering a 15s retry loop would + // keep it from answering that one too. + if ra.guestAgent != nil { + workloadIDs := make([]string, 0, len(containers)) + for _, c := range containers { + workloadIDs = append(workloadIDs, overlayWorkloadID(c.GetName())) + } + s.guestStats.Store(&guestStatsTarget{actorUID: actorUID, agent: ra.guestAgent, workloadIDs: workloadIDs}) + } + slog.InfoContext(ctx, "Actor restored (overlay rootfs)", slog.String("id", actorUID), slog.Duration("total", time.Since(tStart))) return nil diff --git a/cmd/ateom-microvm/run.go b/cmd/ateom-microvm/run.go index eb4b34740..1ea149013 100644 --- a/cmd/ateom-microvm/run.go +++ b/cmd/ateom-microvm/run.go @@ -76,13 +76,17 @@ type runningActor struct { // un-faulted pages). Empty for cold-run actors (their snapshot is already complete). restoreSourceDir string - // logAgent is the kata-agent ttrpc client kept open for the lifetime of the - // stdout/stderr forwarding goroutines (they pump the container's output via - // ReadStdout/ReadStderr on this connection). It is NOT closed when RunWorkload / - // RestoreWorkload return — teardownActor closes it, which makes the in-flight - // ReadStdout/ReadStderr calls fail and the forwarding goroutines exit (io.EOF). - // nil if forwarding was not started (e.g. a best-effort post-restore dial failed). - logAgent *kata.AgentClient + // guestAgent is the kata-agent ttrpc client retained past boot. Two things + // share it: the stdout/stderr forwarding goroutines (they pump the + // container's output via ReadStdout/ReadStderr on this connection for the + // actor's lifetime) and GetWorkloadStats (via s.guestStats, which points at + // this same client). It is NOT closed when RunWorkload / RestoreWorkload + // return — teardownActor closes it, which makes the in-flight + // ReadStdout/ReadStderr calls fail and the forwarding goroutines exit + // (io.EOF). nil if the post-boot dial failed (e.g. a best-effort + // post-restore dial), which loses both log forwarding and guest stats for + // this activation. + guestAgent *kata.AgentClient } // baseIDFile is a tiny snapshot file (under the checkpoint/restore dir) holding @@ -489,7 +493,7 @@ func (s *AteomService) coldBootActor(ctx context.Context, p actorBootParams) (re return fmt.Errorf("while waiting for container readyz: %w", err) } - ra := &runningActor{chCmd: chCmd, vfsdCmd: vfsdCmd, durableVfsdCmd: durableVfsdCmd, apiSocket: apiSocket, baseID: actorUID, logAgent: ac} + ra := &runningActor{chCmd: chCmd, vfsdCmd: vfsdCmd, durableVfsdCmd: durableVfsdCmd, apiSocket: apiSocket, baseID: actorUID, guestAgent: ac} if err := s.activateActorNetworking(p.actorRef.Atespace, p.actorRef.Name, egress); err != nil { return err } @@ -499,10 +503,19 @@ func (s *AteomService) coldBootActor(ctx context.Context, p actorBootParams) (re // container/exec id is _ovl (see startOverlayContainer), so key the streams by // that and tag with the display container name. The goroutines read over ac for the // actor's lifetime and exit (io.EOF) when teardownActor closes ac. + workloadIDs := make([]string, 0, len(ctrs)) for _, c := range ctrs { s.startActorLogForwarding(ac, p.actorRef, actorUID, templateNS, templateName, overlayWorkloadID(c.name), c.name) + workloadIDs = append(workloadIDs, overlayWorkloadID(c.name)) } + // Publish the guest to GetWorkloadStats, past every error return above: a + // failing attempt closes ac on its way out (and coldBootActorRetrying may + // then try the whole boot again), so a target published earlier would leave + // the handler polling a connection nobody owns. Same client the forwarding + // above reads over — ttrpc multiplexes, and teardownActor ends both. + s.guestStats.Store(&guestStatsTarget{actorUID: actorUID, agent: ac, workloadIDs: workloadIDs}) + return nil } diff --git a/cmd/ateom-microvm/stats.go b/cmd/ateom-microvm/stats.go index cb17e6270..f8548ff8b 100644 --- a/cmd/ateom-microvm/stats.go +++ b/cmd/ateom-microvm/stats.go @@ -18,21 +18,192 @@ package main import ( "context" + "errors" + "time" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/agentstats" + "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/third_party/kata/agentpb" "github.com/agent-substrate/substrate/internal/proto/ateompb" ) +// statsCallTimeout bounds one container's guest-agent call. The RPC is polled +// on a timer, so it must fail fast rather than pile pollers up behind a guest +// that has stopped answering: a missed sample is cheap, a stuck handler is not. +// Generous next to a healthy read, which is a vsock round trip and four small +// file reads inside the guest, and far short of the lifecycle calls' 20-30s. +const statsCallTimeout = 2 * time.Second + +// containerStatsReader is the one guest-agent call GetWorkloadStats makes. +// *kata.AgentClient satisfies it; the narrow interface is what lets the handler +// be tested without a live micro-VM, which is otherwise the only way to get an +// agent to talk to. +type containerStatsReader interface { + StatsContainer(ctx context.Context, containerID string) (*agentpb.CgroupStats, error) +} + +// guestStatsTarget is everything GetWorkloadStats needs to sample a live guest. +// +// It exists so the handler never reads AteomService.running, which lock guards: +// the handler must not take lock (see below), and a map read racing a lifecycle +// RPC's write is a data race whatever the read is for. The fields it holds are +// the ones RunWorkload and RestoreWorkload already produce. +type guestStatsTarget struct { + // actorUID is the actor these containers belong to. Checked against the + // attribution before anything is reported, so a target left behind by a + // transition cannot file one actor's numbers under another's name. + actorUID string + + // agent is the kata-agent client the actor's log forwarding already keeps + // open for its lifetime, borrowed rather than dialed again. ttrpc + // multiplexes, so a poll and the forwarding reads share it safely. + agent containerStatsReader + + // workloadIDs are the guest containers to sum: the overlay WORKLOADS, one + // per actor container. Their carriers are deliberately absent — a carrier is + // created and never started (see CreateCarrier), so it runs no process and + // its cgroup has nothing in it to add. + workloadIDs []string +} + // GetWorkloadStats implements ateompb.Ateom/GetWorkloadStats. // -// The attribution half is wired up (see AteomService.activeActor); the measurement -// half is not. The micro-VM read goes to the guest agent's StatsContainer -// rather than the host cgroup — guest RAM is a fixed allocation, so the host -// cgroup barely moves with the workload — and lands in the follow-up to -// https://github.com/agent-substrate/substrate/issues/594, at which point this -// stops returning Unimplemented. +// The sample comes from inside the guest, not from the host cgroup. On this +// runtime the host cgroup holds cloud-hypervisor, whose memory is the guest RAM +// allocation it took at boot: near-constant, and near-identical for an idle +// actor and a saturated one. The guest kernel is what accounts for the +// workload, and the kata-agent is what can read it out. +// +// Unlike the three lifecycle RPCs this does not take s.lock, and must not +// start: it is polled on a timer for the whole life of a workload, while lock +// is held across an entire cold boot with its retry, across a snapshot write, +// and across a restore. Blocking on it would silence the poller through exactly +// the phases whose usage is most interesting. Both pieces of state it reads — +// the attribution and the guest target — are atomics for that reason. func (s *AteomService) GetWorkloadStats(ctx context.Context, req *ateompb.GetWorkloadStatsRequest) (*ateompb.GetWorkloadStatsResponse, error) { - return nil, status.Error(codes.Unimplemented, "GetWorkloadStats is not implemented yet") + if req.GetActorUid() == "" { + return nil, status.Error(codes.InvalidArgument, "actor_uid is required") + } + + // Both of these are NOT_FOUND rather than FAILED_PRECONDITION: they tell the + // caller the requested actor is not here, which no amount of retrying on the + // same timer will change. Its worker-to-actor mapping wants re-resolving. + active := s.activeActor.Load() + if active == nil { + return nil, status.Errorf(codes.NotFound, "ateom is available; it is not executing actor %q", req.GetActorUid()) + } + if active.UID != req.GetActorUid() { + return nil, status.Errorf(codes.NotFound, "ateom is executing actor %q, not the requested %q", active.UID, req.GetActorUid()) + } + + // The requested actor is the one here, but there is no guest to ask yet. + // Usually that is a poll landing in the boot or the restore: the ateom + // retains the attribution from the moment it accepts the actor, and the + // target is only published once the containers are up. It is also what a + // teardown looks like from here, since teardownActor clears the target + // before it closes the connection, and what a restore whose post-restore + // agent dial failed looks like for the rest of that activation. + // + // FAILED_PRECONDITION in all three: the answer is "no numbers right now", + // the caller should take the next sample, and for the teardown the next + // sample is the NOT_FOUND above. + target := s.guestStats.Load() + if target == nil { + return nil, status.Error(codes.FailedPrecondition, "no guest agent connection to measure yet") + } + // Belt and braces against the one thing that must never happen. The target + // is published and cleared under lock alongside the attribution, so this + // should be unreachable; if the two ever disagree, decline rather than + // report a stale guest's numbers under the requested actor's name. + if target.actorUID != active.UID { + return nil, status.Errorf(codes.FailedPrecondition, "guest agent connection belongs to actor %q, not %q", target.actorUID, active.UID) + } + + observedAt := time.Now() + sample, err := sumContainerStats(ctx, target) + if err != nil { + // Not Internal: a guest that has stopped answering is a routine state + // here, not a bug. Either the sandbox is going away — which the next + // CheckpointWorkload turns into the NOT_FOUND above — or the agent is + // briefly unreachable, and the next poll gets a number. + return nil, status.Errorf(codes.FailedPrecondition, "no container stats from the guest agent: %v", err) + } + + // Re-check that the same workload is still the active one. The calls above + // hold no lock, so a checkpoint plus a fresh run can complete underneath + // them, and the numbers would then belong to an actor other than the one + // being reported. Pointer identity is enough: activeActor is stored as a new + // pointer on every Run and Restore and never mutated in place, so an + // unchanged pointer means no transition happened across the read. + // + // NOT_FOUND, like the two checks above and for the same reason: the + // requested actor is no longer the one here, so a retry lands on one of them + // and gets that answer anyway. The same state should not report two + // different codes depending on where in the handler it was noticed. + if s.activeActor.Load() != active { + return nil, status.Errorf(codes.NotFound, "ateom stopped executing actor %q while the sample was being taken", req.GetActorUid()) + } + + return &ateompb.GetWorkloadStatsResponse{ + Atespace: active.Ref.Atespace, + ActorName: active.Ref.Name, + ActorUid: active.UID, + ActorTemplateNamespace: active.TemplateNamespace, + ActorTemplateName: active.TemplateName, + + SandboxClass: ateompb.SandboxClass_SANDBOX_CLASS_MICROVM, + Source: ateompb.StatsSource_STATS_SOURCE_GUEST_AGENT, + + MemoryCurrentBytes: sample.MemoryCurrentBytes, + MemoryPeakBytes: sample.MemoryPeakBytes, + MemoryWorkingSetBytes: sample.MemoryWorkingSetBytes, + CpuUsageUsec: sample.CPUUsageUsec, + + ObservedAtUnixNano: observedAt.UnixNano(), + }, nil +} + +// sumContainerStats reads every container of the actor and adds them up. +// +// A container the agent cannot report contributes nothing instead of failing +// the sample. That is not only the "a partial reading beats none" trade the +// gVisor side makes for a missing cgroup file — for the common way this +// happens, zero is the correct contribution rather than a fallback: a container +// that has exited took its guest cgroup with it and consumes nothing from here +// on. Failing the actor's telemetry because one sidecar is gone would be the +// wrong answer. +// +// It fails only when no container could be read at all, which is the guest as a +// whole not answering rather than one container being gone, and returns the +// last error so the caller can say why. +func sumContainerStats(ctx context.Context, target *guestStatsTarget) (agentstats.Sample, error) { + var ( + total agentstats.Sample + read int + lastErr error + ) + for _, id := range target.workloadIDs { + callCtx, cancel := context.WithTimeout(ctx, statsCallTimeout) + cs, err := target.agent.StatsContainer(callCtx, id) + cancel() + if err != nil { + lastErr = err + continue + } + read++ + total = total.Plus(agentstats.FromCgroupStats(cs)) + } + + if read == 0 { + if lastErr == nil { + // No containers to read. The actor is up with an empty container + // list, which the boot path does not produce, so treat it as "no + // numbers" rather than reporting a confident zero. + lastErr = errors.New("no containers to measure") + } + return agentstats.Sample{}, lastErr + } + return total, nil } diff --git a/cmd/ateom-microvm/stats_test.go b/cmd/ateom-microvm/stats_test.go index 449453ecf..7345f1380 100644 --- a/cmd/ateom-microvm/stats_test.go +++ b/cmd/ateom-microvm/stats_test.go @@ -18,12 +18,16 @@ package main import ( "context" + "errors" "testing" + "time" "github.com/google/go-cmp/cmp" "google.golang.org/grpc/codes" "google.golang.org/grpc/status" + "google.golang.org/protobuf/testing/protocmp" + "github.com/agent-substrate/substrate/cmd/ateom-microvm/internal/third_party/kata/agentpb" "github.com/agent-substrate/substrate/internal/ateomstats" "github.com/agent-substrate/substrate/internal/proto/ateompb" "github.com/agent-substrate/substrate/internal/resources" @@ -97,36 +101,316 @@ func TestActorBootParamsAttributionMatchesRequest(t *testing.T) { } } -// TestGetWorkloadStatsUnimplemented pins the stub's advertised contract. The -// guest agent read replaces this body; until then a caller that gets any other code -// back would be reading numbers that are not there. -// -// The retention this stub will eventually read — s.activeActor, set by -// RunWorkload / RestoreWorkload and cleared by CheckpointWorkload — has no unit -// test, for the same reason as on the gVisor side: those three RPCs reach for -// netlink, cloud-hypervisor, and the worker pod's netns within a few lines of -// entry and cannot be driven from `go test`. Its mapping is covered above and -// in internal/ateomstats; the transitions are verified end to end once -// GetWorkloadStats returns real data. -func TestGetWorkloadStatsUnimplemented(t *testing.T) { +// The lifecycle transitions that maintain s.activeActor and s.guestStats — set +// by RunWorkload and RestoreWorkload, cleared by CheckpointWorkload's teardown — +// have no unit test, because those three RPCs each reach for netlink, +// cloud-hypervisor, and the worker pod's netns within a few lines of entry and +// cannot be driven from `go test`. The mapping they use is covered above and in +// internal/ateomstats; the transitions are verified end to end. What is testable +// here is everything GetWorkloadStats does with the result, which is where the +// polling loop will actually live. + +var testActor = ateomstats.ActorAttribution{ + Ref: resources.ActorRef{Atespace: "space-a", Name: "actor-a"}, + UID: "uid-a", + TemplateNamespace: "ns-a", + TemplateName: "template-a", +} + +// fakeAgent stands in for the kata-agent client: a canned reply per container +// id, or a canned error for the ones that are meant to fail. +type fakeAgent struct { + stats map[string]*agentpb.CgroupStats + errs map[string]error + + // calls records the container ids asked for, in order, so a test can tell + // "summed two containers" from "read one twice". + calls []string + // deadlines records whether each call's context carried one, which is how + // the per-call timeout is pinned. + deadlines []bool +} + +func (f *fakeAgent) StatsContainer(ctx context.Context, containerID string) (*agentpb.CgroupStats, error) { + f.calls = append(f.calls, containerID) + _, ok := ctx.Deadline() + f.deadlines = append(f.deadlines, ok) + if err, ok := f.errs[containerID]; ok { + return nil, err + } + return f.stats[containerID], nil +} + +// containerStats is one container's guest reading: usage bytes, peak bytes, +// reclaimable page cache, and cumulative CPU nanoseconds. +func containerStats(usage, peak, inactiveFile, cpuNanos uint64) *agentpb.CgroupStats { + return &agentpb.CgroupStats{ + MemoryStats: &agentpb.MemoryStats{ + Usage: &agentpb.MemoryData{Usage: usage, MaxUsage: peak}, + Stats: map[string]uint64{"inactive_file": inactiveFile}, + }, + CpuStats: &agentpb.CpuStats{CpuUsage: &agentpb.CpuUsage{TotalUsage: cpuNanos}}, + } +} + +// newStatsService builds a service executing testActor with the given guest +// containers published to GetWorkloadStats. +func newStatsService(agent containerStatsReader, workloadIDs ...string) *AteomService { s := &AteomService{} + s.activeActor.Store(&testActor) + s.guestStats.Store(&guestStatsTarget{actorUID: testActor.UID, agent: agent, workloadIDs: workloadIDs}) + return s +} + +func TestGetWorkloadStats(t *testing.T) { + agent := &fakeAgent{stats: map[string]*agentpb.CgroupStats{ + "app_ovl": containerStats(157286400, 209715200, 20971520, 1234567000), + }} + s := newStatsService(agent, "app_ovl") + + before := time.Now().UnixNano() + got, err := s.GetWorkloadStats(context.Background(), &ateompb.GetWorkloadStatsRequest{ActorUid: "uid-a"}) + after := time.Now().UnixNano() + if err != nil { + t.Fatalf("GetWorkloadStats() error = %v, want nil", err) + } + + if got.GetObservedAtUnixNano() < before || got.GetObservedAtUnixNano() > after { + t.Errorf("GetWorkloadStats() observed_at_unix_nano = %d, want within [%d, %d]", got.GetObservedAtUnixNano(), before, after) + } + // Checked above; zeroed so the rest can be compared as a whole. + got.ObservedAtUnixNano = 0 - resp, err := s.GetWorkloadStats(context.Background(), &ateompb.GetWorkloadStatsRequest{ActorUid: "uid-c"}) - if resp != nil { - t.Errorf("GetWorkloadStats() returned response %v, want nil", resp) + want := &ateompb.GetWorkloadStatsResponse{ + Atespace: "space-a", + ActorName: "actor-a", + ActorUid: "uid-a", + ActorTemplateNamespace: "ns-a", + ActorTemplateName: "template-a", + SandboxClass: ateompb.SandboxClass_SANDBOX_CLASS_MICROVM, + Source: ateompb.StatsSource_STATS_SOURCE_GUEST_AGENT, + MemoryCurrentBytes: 157286400, + MemoryPeakBytes: 209715200, + MemoryWorkingSetBytes: 136314880, + CpuUsageUsec: 1234567, + } + if diff := cmp.Diff(want, got, protocmp.Transform()); diff != "" { + t.Errorf("GetWorkloadStats() mismatch (-want +got):\n%s", diff) + } + + // A poll must not be able to hang on a guest that has stopped answering, + // which is only true if the handler bounds each call rather than passing the + // caller's context straight through. + if want := []bool{true}; !cmp.Equal(want, agent.deadlines) { + t.Errorf("StatsContainer call contexts had deadlines %v, want %v", agent.deadlines, want) + } +} + +// TestGetWorkloadStatsSumsContainers covers the multi-container actor: the +// guest gives one cgroup per container (see StartOverlayWorkload), and the +// proto reports one figure for the actor. +func TestGetWorkloadStatsSumsContainers(t *testing.T) { + agent := &fakeAgent{stats: map[string]*agentpb.CgroupStats{ + "app_ovl": containerStats(1000, 4000, 100, 7000), + "sidecar_ovl": containerStats(500, 800, 200, 3000), + }} + s := newStatsService(agent, "app_ovl", "sidecar_ovl") + + got, err := s.GetWorkloadStats(context.Background(), &ateompb.GetWorkloadStatsRequest{ActorUid: "uid-a"}) + if err != nil { + t.Fatalf("GetWorkloadStats() error = %v, want nil", err) + } + + if want := uint64(1500); got.GetMemoryCurrentBytes() != want { + t.Errorf("memory_current_bytes = %d, want %d", got.GetMemoryCurrentBytes(), want) + } + // The sum of the peaks, which is an upper bound on the peak of the sum: the + // two containers need not have peaked at the same moment. + if want := uint64(4800); got.GetMemoryPeakBytes() != want { + t.Errorf("memory_peak_bytes = %d, want %d", got.GetMemoryPeakBytes(), want) + } + if want := uint64(1200); got.GetMemoryWorkingSetBytes() != want { + t.Errorf("memory_working_set_bytes = %d, want %d", got.GetMemoryWorkingSetBytes(), want) + } + if want := uint64(10); got.GetCpuUsageUsec() != want { + t.Errorf("cpu_usage_usec = %d, want %d", got.GetCpuUsageUsec(), want) + } + + if want := []string{"app_ovl", "sidecar_ovl"}; !cmp.Equal(want, agent.calls) { + t.Errorf("StatsContainer called for %v, want %v", agent.calls, want) + } +} + +// TestGetWorkloadStatsSkipsUnreadableContainer pins the partial-reading trade: +// one container the agent cannot report must not silence the actor's telemetry. +// The usual way in is a container that has exited, which took its guest cgroup +// with it and consumes nothing from here on — so contributing zero is the +// correct answer for it, not merely a tolerable one. +func TestGetWorkloadStatsSkipsUnreadableContainer(t *testing.T) { + agent := &fakeAgent{ + stats: map[string]*agentpb.CgroupStats{"app_ovl": containerStats(1000, 2000, 100, 5000)}, + errs: map[string]error{"exited_ovl": errors.New("no such container")}, + } + s := newStatsService(agent, "app_ovl", "exited_ovl") + + got, err := s.GetWorkloadStats(context.Background(), &ateompb.GetWorkloadStatsRequest{ActorUid: "uid-a"}) + if err != nil { + t.Fatalf("GetWorkloadStats() error = %v, want nil", err) + } + if want := uint64(1000); got.GetMemoryCurrentBytes() != want { + t.Errorf("memory_current_bytes = %d, want %d", got.GetMemoryCurrentBytes(), want) } - if got := status.Code(err); got != codes.Unimplemented { - t.Errorf("GetWorkloadStats() error code = %v, want %v (err: %v)", got, codes.Unimplemented, err) + if want := uint64(5); got.GetCpuUsageUsec() != want { + t.Errorf("cpu_usage_usec = %d, want %d", got.GetCpuUsageUsec(), want) + } +} + +// TestGetWorkloadStatsCountsAnsweredContainer covers the boundary of the rule +// above: a container the agent answers for without cgroup stats is a reading of +// zero, not a failure, so the sample stands even when it is the only container. +func TestGetWorkloadStatsCountsAnsweredContainer(t *testing.T) { + agent := &fakeAgent{stats: map[string]*agentpb.CgroupStats{"app_ovl": nil}} + s := newStatsService(agent, "app_ovl") + + got, err := s.GetWorkloadStats(context.Background(), &ateompb.GetWorkloadStatsRequest{ActorUid: "uid-a"}) + if err != nil { + t.Fatalf("GetWorkloadStats() error = %v, want nil", err) + } + if got.GetMemoryCurrentBytes() != 0 || got.GetCpuUsageUsec() != 0 { + t.Errorf("GetWorkloadStats() = %v, want an all-zero measurement", got) + } +} + +func TestGetWorkloadStatsErrors(t *testing.T) { + healthy := &fakeAgent{stats: map[string]*agentpb.CgroupStats{"app_ovl": containerStats(1000, 2000, 100, 5000)}} + + for _, tc := range []struct { + name string + // service builds the service under test, so each case can put the two + // atomics in exactly the state it means to exercise. + service func() *AteomService + actorUID string + want codes.Code + }{ + { + // A required field the caller left off: a client bug, distinct from + // the races below, so it gets a distinct code. + name: "empty actor_uid", + service: func() *AteomService { return newStatsService(healthy, "app_ovl") }, + actorUID: "", + want: codes.InvalidArgument, + }, + { + // Not here at all. NOT_FOUND rather than FAILED_PRECONDITION, because + // what the caller should do about it is re-resolve, not retry. + name: "ateom is available", + service: func() *AteomService { return &AteomService{} }, + actorUID: "uid-a", + want: codes.NotFound, + }, + { + // The worker was recycled between the caller's view of the world and + // this call. Reporting anyway would file one actor's numbers under + // another's name, and it is the same "not here" as the case above. + name: "actor_uid does not match the executing workload", + service: func() *AteomService { return newStatsService(healthy, "app_ovl") }, + actorUID: "uid-b", + want: codes.NotFound, + }, + { + // The requested actor is the one here, but the guest is not up yet — + // a poll landing in the boot or the restore, or one landing after + // teardownActor cleared the target. The transient case. + name: "no guest agent connection yet", + service: func() *AteomService { + s := &AteomService{} + s.activeActor.Store(&testActor) + return s + }, + actorUID: "uid-a", + want: codes.FailedPrecondition, + }, + { + // Should be unreachable — the two atomics are written together under + // lock — so what is pinned here is that disagreeing state declines + // instead of misattributing. + name: "guest agent connection belongs to another actor", + service: func() *AteomService { + s := newStatsService(healthy, "app_ovl") + s.guestStats.Store(&guestStatsTarget{actorUID: "uid-b", agent: healthy, workloadIDs: []string{"app_ovl"}}) + return s + }, + actorUID: "uid-a", + want: codes.FailedPrecondition, + }, + { + // Not one container gone but the guest as a whole not answering: the + // sandbox is going away, which the next CheckpointWorkload turns into + // the NOT_FOUND above, or the agent is briefly unreachable. Either way + // it is "no numbers right now" rather than Internal — the guest not + // answering is a routine state here, not a bug in this ateom. + name: "no container answers", + service: func() *AteomService { + agent := &fakeAgent{errs: map[string]error{ + "app_ovl": errors.New("ttrpc: closed"), + "sidecar_ovl": errors.New("ttrpc: closed"), + }} + return newStatsService(agent, "app_ovl", "sidecar_ovl") + }, + actorUID: "uid-a", + want: codes.FailedPrecondition, + }, + { + // The boot path does not produce an actor with no containers, so a + // confident zero would be reporting a state we do not understand. + name: "no containers to measure", + service: func() *AteomService { return newStatsService(healthy) }, + actorUID: "uid-a", + want: codes.FailedPrecondition, + }, + } { + t.Run(tc.name, func(t *testing.T) { + resp, err := tc.service().GetWorkloadStats(context.Background(), &ateompb.GetWorkloadStatsRequest{ActorUid: tc.actorUID}) + if resp != nil { + t.Errorf("GetWorkloadStats() returned response %v, want nil", resp) + } + if got := status.Code(err); got != tc.want { + t.Errorf("GetWorkloadStats() error code = %v, want %v (err: %v)", got, tc.want, err) + } + }) + } +} + +// TestGetWorkloadStatsDoesNotTakeLock is the regression test for the property +// the design turns on: a stats poll must not queue behind a lifecycle RPC. +// s.lock is held for the duration of the call here, so a handler that reached +// for it — or that looked up the agent client in s.running, which lock guards — +// would deadlock and fail this test by timing out rather than by assertion. +func TestGetWorkloadStatsDoesNotTakeLock(t *testing.T) { + agent := &fakeAgent{stats: map[string]*agentpb.CgroupStats{"app_ovl": containerStats(1000, 2000, 100, 5000)}} + s := newStatsService(agent, "app_ovl") + + // Stands in for a RunWorkload or CheckpointWorkload in flight, which hold + // the lock across their entire bodies. + s.lock.Lock() + defer s.lock.Unlock() + + if _, err := s.GetWorkloadStats(context.Background(), &ateompb.GetWorkloadStatsRequest{ActorUid: "uid-a"}); err != nil { + t.Errorf("GetWorkloadStats() error = %v, want nil", err) } } // TestAteomServiceStartsAvailable checks that a freshly constructed service -// retains no attribution, mirroring the gVisor ateom's test of the same name. -// GetWorkloadStats's NOT_FOUND-when-available behavior is built on this: a -// non-nil zero value here would make an idle ateom report an empty actor's -// usage instead of refusing. +// retains no attribution and offers no guest, mirroring the gVisor ateom's test +// of the same name. GetWorkloadStats's NOT_FOUND-when-available behavior is +// built on the first: a non-nil zero value would make an idle ateom report an +// empty actor's usage instead of refusing. func TestAteomServiceStartsAvailable(t *testing.T) { - if s := (&AteomService{}); s.activeActor.Load() != nil { - t.Errorf("new AteomService.activeActor = %v, want nil", s.activeActor.Load()) + s := &AteomService{} + if got := s.activeActor.Load(); got != nil { + t.Errorf("new AteomService.activeActor = %v, want nil", got) + } + if got := s.guestStats.Load(); got != nil { + t.Errorf("new AteomService.guestStats = %v, want nil", got) } } diff --git a/internal/proto/ateompb/ateom.pb.go b/internal/proto/ateompb/ateom.pb.go index cfe6b3ed5..3ff384612 100644 --- a/internal/proto/ateompb/ateom.pb.go +++ b/internal/proto/ateompb/ateom.pb.go @@ -162,7 +162,12 @@ const ( StatsSource_STATS_SOURCE_UNSPECIFIED StatsSource = 0 // Read from the sandbox's cgroup on the host. StatsSource_STATS_SOURCE_CGROUP StatsSource = 1 - // Read from inside the guest, over the guest agent's vsock connection. + // Read from inside the guest, over the guest agent's vsock connection. What + // it counts is the workload's own containers, as the guest kernel accounts + // for them: the guest kernel itself, the agent, and the VMM process on the + // host are overhead this source cannot see. The cgroup source above is the + // other way round -- the sandbox's host process is one process, so its + // runtime's overhead is charged along with the workload's. StatsSource_STATS_SOURCE_GUEST_AGENT StatsSource = 2 ) @@ -1080,8 +1085,11 @@ func (x *GetWorkloadStatsRequest) GetActorUid() string { // GetWorkloadStatsResponse is one resource-usage sample for the executing // workload. The unit of measurement is the SANDBOX, which today equals the -// actor; per-container attribution needs the gVisor sentry's own accounting and -// is not reported here. +// actor. Per-container attribution is not reported: the micro-VM source could +// give it, since the guest keeps a cgroup per container and ateom sums them, +// but the gVisor source cannot split one at all without the sentry's own +// accounting, and a field only one runtime could ever fill would be worse than +// none. type GetWorkloadStatsResponse struct { state protoimpl.MessageState `protogen:"open.v1"` // Identity of the measured actor, retained by ateom from the diff --git a/internal/proto/ateompb/ateom.proto b/internal/proto/ateompb/ateom.proto index 2d6a8d8be..f190fcd15 100644 --- a/internal/proto/ateompb/ateom.proto +++ b/internal/proto/ateompb/ateom.proto @@ -273,14 +273,22 @@ enum StatsSource { STATS_SOURCE_UNSPECIFIED = 0; // Read from the sandbox's cgroup on the host. STATS_SOURCE_CGROUP = 1; - // Read from inside the guest, over the guest agent's vsock connection. + // Read from inside the guest, over the guest agent's vsock connection. What + // it counts is the workload's own containers, as the guest kernel accounts + // for them: the guest kernel itself, the agent, and the VMM process on the + // host are overhead this source cannot see. The cgroup source above is the + // other way round -- the sandbox's host process is one process, so its + // runtime's overhead is charged along with the workload's. STATS_SOURCE_GUEST_AGENT = 2; } // GetWorkloadStatsResponse is one resource-usage sample for the executing // workload. The unit of measurement is the SANDBOX, which today equals the -// actor; per-container attribution needs the gVisor sentry's own accounting and -// is not reported here. +// actor. Per-container attribution is not reported: the micro-VM source could +// give it, since the guest keeps a cgroup per container and ateom sums them, +// but the gVisor source cannot split one at all without the sentry's own +// accounting, and a field only one runtime could ever fill would be worse than +// none. message GetWorkloadStatsResponse { // Identity of the measured actor, retained by ateom from the // RunWorkloadRequest / RestoreWorkloadRequest that started it. Echoed back so