Per-container CPU and memory limits for micro-VM actors - #859
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces per-container CPU and memory limits for micro-VM actors, propagating these limits from the ActorTemplate API down to the Kata agent's OCI runtime spec. It includes API schema validations, guest envelope checks, and extensive unit tests. The review feedback highlights a critical bug in the resource envelope validation where negative memory limits could bypass checks and offset other container limits, as well as a broken anchor link in the API documentation.
|
Have you seen #679 ? |
Sorry I just saw this comment, yes let's do this as a follow-up? I will check back next week for rebase, given the timing, apologies. |
An ActorTemplate container can declare limits so it cannot starve or kill its siblings in the same actor. Only limits are expressible: a request is a scheduling hint, and scheduling happens at the pool level, so per-container limits subdivide a budget that is already held. Gated to sandboxClass microvm. gVisor applies cgroup limits at the sandbox level, where one sentry backs every container in the actor, so a per-container cgroup is created and then stays empty (google/gvisor#190). The gate lifts per runtime. Only cpu and memory are accepted and each must be greater than zero, so a template cannot declare a limit that is silently discarded downstream. Limits is a bounded named type because a MaxProperties marker on the field lands at the wrong schema level for a named map, and without a bound the CEL cost estimator rejects the whole schema. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
Scalar bytes and millis rather than a ResourceList map, so ate-api-server parses each resource.Quantity once and every consumer downstream compares numbers. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
toAteletResources is the only place a resource.Quantity is parsed. The wiring into the ateletpb.Container is covered by a test, verified by deleting the line and watching it fail: without it, removing the field left every test in the repository green while limits never left ate-api-server. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
Carrying the limits as a standard OCI field means a runtime that gains per-container enforcement picks them up with no new plumbing. A container that declares none produces an unchanged spec. The CFS quota is clamped to the kernel's 1ms minimum, as kubelet's MilliCPUToQuota does: tg_set_cfs_bandwidth rejects anything smaller, so a cpu limit under 10m would otherwise produce a spec the guest refuses at container create, with an error naming a cgroup write rather than the template field. Quota is derived from the period constant rather than a second literal, and the test asserts quota/period against the declared milli-cores, so retuning the period cannot silently change every container's share. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
Four changes, all on the path from the bundle's OCI spec to the guest cgroup. SpecToAgentPB converted only Devices and CPU.Shares out of Linux.Resources, so a memory limit reached config.json and was dropped on the way to the agent: in the guest memory.max read "max" and a container limited to 64Mi allocated 128MB without being killed. It now carries Memory and CPU quota, and defaults the period when a quota is set, because period is optional in OCI but a plain uint64 on the wire where an unset one is indistinguishable from zero. ensureKataCompatibleSpec applied kata's defaults only when Resources was nil, so the moment atelet set the field the device allowlist and CPU shares would have vanished. mergeKataResources now fills the gaps the defaults cover and leaves everything else alone, so a field added upstream reaches the guest rather than being silently dropped here. checkResourceEnvelope rejects limits the guest can never satisfy, summed across the actor's containers because they share one guest. Errors carry InvalidArgument: ActorTemplateSpec is immutable, so the failure is permanent and must not read as a server fault. CreateCarrier and StartOverlayWorkload are handed the same spec, so the limits were applied to the carrier as well as the workload. The carrier is created and never started, so a limit there bounds nothing, and one small enough to exclude its init fails the create with an error naming neither the limit nor the container. Verified on hardware: with the limit in place a container declaring 64Mi reads 67108864 in its guest cgroup and is OOM-killed allocating 128MB, while a sibling that declares nothing keeps running. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
A non-positive limit means "unlimited" in the OCI spec, so it is not a claim on the guest. Summing it let a negative offset a sibling's real limit and slip the total past the envelope check: 1536Mi + 1024Mi + (-1536Mi) reads as 1024Mi against a 2048Mi guest, so a 2560Mi overrun was accepted. cpuLimitMillis already skipped a non-positive quota; memory now matches. Also repoints the SandboxConfig anchor in api-guide.md, which went stale when agent-substrate#848 renamed the section. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
agent-substrate#679 sizes the sandbox from ActorTemplate.spec.resources and stamps that size onto every container, which overwrites the per-container limits PR agent-substrate#859 writes into the same OCI field. Records the composition model (per-container subdivides a declared actor envelope, no inheritance), the chosen fix (stop stamping user containers on the micro-VM path), and the evidence for keeping over-subscription validation at runtime: an admission-time CEL sum exceeds the cost budget by more than 100x and takes the existing rules down with it. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
Six tasks from the rebase onto the merged agent-substrate#679 through to the PR update, each ending in a runnable test. Records that micro-VM tests are linux-only and must run in Docker, since a green run on macOS silently skips them. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
8a09fac to
c756707
Compare
The actor-level size sizes the VM. Stamping it onto every container replaced each container's declared limit with the actor total, so a container that asked for 64Mi got the whole guest and could never be OOM-killed on its own. A container now gets a cgroup limit only when it declares one; an undeclared container is bounded by guest RAM, which is the real ceiling. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
Nothing applies the actor-level size to a container spec any more, so the size threaded from RunWorkload and RestoreWorkload through buildActorContainers into ensureKataCompatibleSpec had no reader. resolveGuestMemMiB still sizes the VM. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
These were working notes for composing per-container limits with actor-level sizing, not documentation the project publishes. The behaviour they describe is documented in docs/api-guide.md, where readers of the API will find it. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
… the ceiling When the actor declares its own size the guest is that limit minus the VMM reserve, so advising a larger SandboxConfig sends the user to a knob that has no effect. The error now names spec.resources.limits.memory and the reserve when the actor declared a limit, and keeps the SandboxConfig advice when it did not. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
The guide said a micro-VM guest is sized by the SandboxConfig and not by the actor, which is backwards now that the guest is sized from the declared actor limit. Also states where over-subscription is caught, so nobody assumes admission rejects it. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
An operator hitting a limits error or reading the sizing docs needs to be routed to the knob that actually governs the value they hit. A CPU-shortfall error named spec.resources.limits.memory, a field that cannot change the vCPU count, and cited a VMM reserve that applies only to memory. CPU shortfalls now name spec.resources.limits.cpu. Since only ateom-gvisor reaches sizing.ApplyToOCISpec, the sizing docs sent anyone tracing a micro-VM container's cgroup limit to a function the micro-VM binary never calls; they now point at spec.containers[].resources. The api-guide preamble made the same claim for both runtimes. The atelet quota floor comment stated a threshold of 100 milli-cores while the code, its test, the CRD doc and the api-guide all say 10. Records that a micro-VM container's rootfs upper is a guest tmpfs charged to its own cgroup, so writing a large file to the rootfs, /tmp or /run OOM-kills a container that would survive the same manifest on Kubernetes. Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
Per-container CPU and memory limits for micro-VM actors. First slice of #752.
An
ActorTemplatecontainer can cap its own cpu and memory so it cannot starve or kill its siblings in the same actor. A container that exceeds its memory limit is OOM-killed on its own; the rest of the actor is unaffected.#752 asks for per-container device selection as well. This PR does the cpu and memory half; devices are the remaining half, and when they land, GPU assignment should follow the same field rather than staying actor-wide as it is today.
Why micro-VM only
The two sandbox classes support opposite halves of #752. Micro-VM actors have a real guest kernel, so each container gets its own cgroup and the limits bind. gVisor applies cgroup limits at the sandbox level: one sentry backs every container in the actor, so a per-container cgroup is created and then stays empty (google/gvisor#190). Measured on a running actor, the workload container's cgroup reported
memory.current=0while all 20 sandbox processes sat in the pause leaf. A template that setsresourceswithsandboxClass: gvisoris rejected at admission.How a limit travels
ActorTemplate→ CEL validation →ate-api-serverresolves eachresource.Quantityonce →ateletpb→ atelet writes OCIlinux.resources→ateom-microvmmerges kata's defaults and checks the guest envelope →SpecToAgentPB→ kata agent → guest cgroup.The limit is carried as a standard OCI field rather than a substrate-private concept, so a runtime that gains per-container enforcement picks it up without new plumbing.
Composition with #679
#679 sizes the sandbox itself from
ActorTemplate.spec.resources: guest RAM and vCPUs for a micro-VM, the sentry for gVisor. This PR subdivides that sandbox. The two are different fields and complementary layers.They collided in one place. #679 applied the actor-level size to every container's OCI spec, overwriting the per-container limits atelet writes into the same field, so a container asking for 64Mi silently received the whole guest. This PR removes that call on the micro-VM path. A container now gets a cgroup limit only when it declares one; an undeclared container is bounded by guest RAM, which is the real ceiling. On micro-VM the stamp was close to a no-op regardless, since a cap equal to the whole guest can never bind.
gVisor is untouched. There the actor-level size is applied to the
pausecontainer, and since one sentry backs every container, that is the only cgroup that binds.Two consequences worth calling out for reviewers of #679:
sizing.SandboxSizethreaded intobuildActorContainersandensureKataCompatibleSpechad no remaining reader, so it and theguestSizehelper are removed.resolveGuestMemMiBstill sizes the VM and still rejects a declared limit too small to boot.internal/sizing's package doc claimed both runtimes sharedApplyToOCISpecfor container cgroups. That is now gVisor-only, and the comments say so. No code in that file changed.checkResourceEnveloperuns afterresolveGuestMemMiB, so it validates the per-container sum against the post-reserve guest rather than the SandboxConfig default. Its error now namesspec.resources.limits.memory(or.cpu) when the actor declared a size, andSandboxConfigwhen it did not.internal/e2e/suites/sizingis unaffected: it asserts onnum_cpuandmem_total_bytes, both of which come from VM sizing, and only logs the cgroup files.Verified on hardware
Same template, run twice, one commit apart on a micro-VM actor:
hog_ovl/memory.maxmax67108864(the declared 64Mi)bystander_ovl/memory.maxmaxmaxThe bug in between:
SpecToAgentPBconverted onlyDevicesandCPU.Sharesout ofLinux.Resources, so a memory limit reached the bundle'sconfig.jsonand was dropped on the way to the agent. Every unit test passed and the on-disk spec was correct while the feature did nothing.Notes for review
ContainerResourcesdeliberately does not reusecorev1.ResourceRequirements, which also carriesrequestsandclaims. There is no scheduler inside an actor to hint at, and for memory a soft request cannot express "must have this much to come back at all". Right-size actor sandboxes to declared ActorTemplate resource limits #679 reached the same conclusion from the other direction and now rejectsspec.resources.requestsand.claimsat admission.Limitsis a bounded named type. AMaxPropertiesmarker on the field lands at the wrong schema level for a named map, and without a bound the CEL cost estimator rejects the whole schema. This CRD is close to its schema-wide CEL cost ceiling; a rule that iterates containers and parses quantities exceeds it by more than 100x and takes the existing rules down with it.10mis raised to10m: the kernel rejects a CFS quota under 1ms.mergeKataResourcesfills the gaps kata's defaults cover rather than allowlisting known fields, so a field added upstream reaches the guest instead of being dropped.Known gaps
STATUS_RUNNINGand nothing records which container died. Raised on Feature Request: Resource-usage telemetry at actor / ActorTemplate granularity #550, where the guest cgroup'smemory.eventsfits the per-container telemetry being designed there./tmpis OOM-killed. Documented inapi-guide.md; separating the two budgets needs anephemeral-storagefield.ctr.GetResources()withnilleaves the whole suite green. Closing it needs an imagecache fixture forprepareOCIDirectory.ActorTemplateSpecis immutable.api-guide.mdstill saysActorTemplatehas no per-container resource fields, which this PR makes false. Correcting it is left to the devices half of Select devices per-container in the ActorTemplate #752, which is what will change the GPU behaviour that paragraph describes.