mirror of
https://github.com/agent-substrate/substrate.git
synced 2026-11-10 10:13:50 +08:00
Per-container CPU and memory limits for micro-VM actors. First slice of #752. An `ActorTemplate` container 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. ```yaml sandboxClass: microvm containers: - name: trainer resources: limits: {memory: 1500Mi} - name: sidecar resources: limits: {memory: 256Mi, cpu: "0.2"} ``` #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=0` while all 20 sandbox processes sat in the pause leaf. A template that sets `resources` with `sandboxClass: gvisor` is rejected at admission. ## How a limit travels `ActorTemplate` → CEL validation → `ate-api-server` resolves each `resource.Quantity` once → `ateletpb` → atelet writes OCI `linux.resources` → `ateom-microvm` merges 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 never reached the guest at all: `SpecToAgentPB` carried neither `Memory` nor `CPU.Quota`, so it stopped at the bundle's `config.json`. Running the pre-PR pipeline shows `Quota=50000 Memory=2Gi` on disk arriving as `Quota=0 Memory=<nil>` on the wire. gVisor is untouched. There the actor-level size is applied to the `pause` container, and since one sentry backs every container, that is the only cgroup that binds. Two consequences worth calling out for reviewers of #679: - The `sizing.SandboxSize` threaded into `buildActorContainers` and `ensureKataCompatibleSpec` had no remaining reader, so it and the `guestSize` helper are removed. `resolveGuestMemMiB` still sizes the VM and still rejects a declared limit too small to boot. - `internal/sizing`'s package doc claimed both runtimes shared `ApplyToOCISpec` for container cgroups. That is now gVisor-only, and the comments say so. No code in that file changed. `checkResourceEnvelope` runs after `resolveGuestMemMiB`, so it validates the per-container sum against the post-reserve guest rather than the SandboxConfig default. Its error now names `spec.resources.limits.memory` (or `.cpu`) when the actor declared a size, and `SandboxConfig` when it did not. `internal/e2e/suites/sizing` is unaffected: it asserts on `num_cpu` and `mem_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: | | before | after | | :--- | :--- | :--- | | `hog_ovl/memory.max` | `max` | `67108864` (the declared 64Mi) | | `bystander_ovl/memory.max` | `max` | `max` | | hog allocates 128MB | survived | OOM-killed | | bystander | alive | alive | The bug in between: `SpecToAgentPB` converted only `Devices` and `CPU.Shares` out of `Linux.Resources`, so a memory limit reached the bundle's `config.json` and 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 - `ContainerResources` deliberately does not reuse `corev1.ResourceRequirements`, which also carries `requests` and `claims`. 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". #679 reached the same conclusion from the other direction and now rejects `spec.resources.requests` and `.claims` at admission. - `Limits` is a bounded named type. 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. 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. - A cpu limit below `10m` is raised to `10m`: the kernel rejects a CFS quota under 1ms. - `SpecToAgentPB` drops a non-positive quota and a zero period rather than forwarding them. A non-positive quota means unlimited in OCI but was sent as a literal zero, which the guest applies as no CPU at all; a non-nil zero period overwrote the CFS default set for a live quota. Both now read a spec the way `cpuLimitMillis` does, so the envelope check and the conversion agree on the same input. - `mergeKataResources` fills the gaps kata's defaults cover rather than allowlisting known fields, so a field it does not recognise survives the merge. That holds only as far as the merge: `SpecToAgentPB` converts `Devices`, `Memory` and `CPU` only, so `Pids`, `BlockIO`, `HugepageLimits` and `Network` stop at the ttrpc boundary. Nothing sets them today. ## Known gaps - An OOM-killed container is not reported above the guest. The actor stays `STATUS_RUNNING` and nothing records which container died. Raised on #550. Note that #961 settled workload stats at the sandbox level and deliberately does not attribute per container, so surfacing which container died needs a separate signal: the guest cgroup's `memory.events`, not the stats path. - A container's writable rootfs is a guest tmpfs, so filesystem writes are charged to its memory limit and a container writing more than its limit to `/tmp` is OOM-killed. Documented in `api-guide.md`; separating the two budgets needs an `ephemeral-storage` field. - Over-subscription across containers is caught when the actor starts, not at apply time. An admission-time sum check is not implementable within the CEL cost budget. - The atelet hop that attaches resources to the bundle spec has no test: replacing `ctr.GetResources()` with `nil` leaves the whole suite green. Closing it needs an imagecache fixture for `prepareOCIDirectory`. - Restored actors inherit the golden's cgroups rather than applying their own spec. Correct today only because `ActorTemplateSpec` is immutable. - The GPU section of `api-guide.md` still says `ActorTemplate` has no per-container resource fields, which this PR makes false. Correcting it is left to the devices half of #752, which is what will change the GPU behaviour that paragraph describes. --- - [x] Tests pass - [x] Appropriate changes to documentation are included in the PR --------- Signed-off-by: Eliran Wolff <eliranw@nvidia.com>
269 lines
8.6 KiB
Go
269 lines
8.6 KiB
Go
// 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 controlapi
|
|
|
|
import (
|
|
"fmt"
|
|
|
|
"github.com/agent-substrate/substrate/internal/proto/ateletpb"
|
|
atev1alpha1 "github.com/agent-substrate/substrate/pkg/api/v1alpha1"
|
|
"github.com/agent-substrate/substrate/pkg/proto/ateapipb"
|
|
corev1 "k8s.io/api/core/v1"
|
|
)
|
|
|
|
// toAteletResources resolves a container's declared limits into the scalars
|
|
// atelet and the ateoms consume; everything downstream compares numbers.
|
|
// Returns nil when the container declares no limits, so the OCI spec stays
|
|
// untouched for templates that do not use them.
|
|
func toAteletResources(r *atev1alpha1.ContainerResources) *ateletpb.ResourceLimits {
|
|
if r == nil || len(r.Limits) == 0 {
|
|
return nil
|
|
}
|
|
out := &ateletpb.ResourceLimits{}
|
|
if q, ok := r.Limits[corev1.ResourceMemory]; ok {
|
|
out.MemoryBytes = q.Value()
|
|
}
|
|
if q, ok := r.Limits[corev1.ResourceCPU]; ok {
|
|
out.CpuMillis = q.MilliValue()
|
|
}
|
|
if out.MemoryBytes == 0 && out.CpuMillis == 0 {
|
|
return nil
|
|
}
|
|
return out
|
|
}
|
|
|
|
// workloadSpecFromActorTemplate builds a WorkloadSpec from the template;
|
|
// container env is copied verbatim.
|
|
func workloadSpecFromActorTemplate(actorTemplate *atev1alpha1.ActorTemplate, actor *ateapipb.Actor) (*ateletpb.WorkloadSpec, error) {
|
|
workloadSpec := &ateletpb.WorkloadSpec{}
|
|
|
|
// Convert volumes to atelet's representation. ActorTemplate validation has
|
|
// already ensured that only one source is set.
|
|
for _, vol := range actorTemplate.Spec.Volumes {
|
|
switch {
|
|
case vol.VolumeSource.DurableDir != nil:
|
|
workloadSpec.Volumes = append(workloadSpec.Volumes, &ateletpb.Volume{
|
|
Name: vol.Name,
|
|
Source: &ateletpb.Volume_DurableDir{
|
|
DurableDir: &ateletpb.DurableDirVolume{},
|
|
},
|
|
})
|
|
|
|
case vol.VolumeSource.SystemInfo != nil:
|
|
ateletSystemInfo := &ateletpb.SystemInfoVolume{}
|
|
for _, dataSource := range vol.VolumeSource.SystemInfo.DataSources {
|
|
switch {
|
|
case dataSource.ActorMetadata != nil:
|
|
actorMetadata := &ateletpb.ActorMetadataDataSource{}
|
|
for _, item := range dataSource.ActorMetadata.Items {
|
|
actorMetadata.Items = append(actorMetadata.Items, &ateletpb.ActorMetadataItem{
|
|
Field: toAteletActorMetadataField(item.Field),
|
|
Path: item.Path,
|
|
})
|
|
}
|
|
ateletSystemInfo.DataSources = append(ateletSystemInfo.DataSources, &ateletpb.SystemInfoDataSource{
|
|
DataSource: &ateletpb.SystemInfoDataSource_ActorMetadata{
|
|
ActorMetadata: actorMetadata,
|
|
},
|
|
})
|
|
case dataSource.TrustBundle != nil:
|
|
// atelet resolves named trustBundles against its allowlist
|
|
// and ClusterTrustBundle informer at write time
|
|
ateletSystemInfo.DataSources = append(ateletSystemInfo.DataSources, &ateletpb.SystemInfoDataSource{
|
|
DataSource: &ateletpb.SystemInfoDataSource_TrustBundle{
|
|
TrustBundle: &ateletpb.TrustBundleDataSource{
|
|
Name: dataSource.TrustBundle.Name,
|
|
Path: dataSource.TrustBundle.Path,
|
|
},
|
|
},
|
|
})
|
|
default:
|
|
continue // Drop unrecognized data sources
|
|
}
|
|
}
|
|
workloadSpec.Volumes = append(workloadSpec.Volumes, &ateletpb.Volume{
|
|
Name: vol.Name,
|
|
Source: &ateletpb.Volume_SystemInfo{
|
|
SystemInfo: ateletSystemInfo,
|
|
},
|
|
})
|
|
|
|
case vol.VolumeSource.Image != nil:
|
|
workloadSpec.Volumes = append(workloadSpec.Volumes, &ateletpb.Volume{
|
|
Name: vol.Name,
|
|
Source: &ateletpb.Volume_Image{
|
|
Image: &ateletpb.ImageVolumeSource{
|
|
Reference: vol.VolumeSource.Image.Reference,
|
|
},
|
|
},
|
|
})
|
|
|
|
default:
|
|
continue // Drop unrecognized volumes.
|
|
}
|
|
}
|
|
|
|
// TODO: order may be important for nested mounts. Also need to think about
|
|
// nested mount support in general.
|
|
if err := appendExternalVolumes(workloadSpec, actorTemplate, actor); err != nil {
|
|
return nil, err
|
|
}
|
|
|
|
for _, ctr := range actorTemplate.Spec.Containers {
|
|
ateletCtr := &ateletpb.Container{
|
|
Name: ctr.Name,
|
|
Image: ctr.Image,
|
|
Command: ctr.Command,
|
|
Args: ctr.Args,
|
|
Readyz: toAteletReadyz(ctr.Readyz),
|
|
SecurityContext: toAteletSecurityContext(ctr.SecurityContext),
|
|
Resources: toAteletResources(ctr.Resources),
|
|
}
|
|
for _, env := range ctr.Env {
|
|
ateletCtr.Env = append(ateletCtr.Env, &ateletpb.EnvEntry{
|
|
Name: env.Name,
|
|
Value: env.Value,
|
|
})
|
|
}
|
|
for _, mount := range ctr.VolumeMounts {
|
|
ateletCtr.VolumeMounts = append(ateletCtr.VolumeMounts, &ateletpb.VolumeMount{
|
|
Name: mount.Name,
|
|
MountPath: mount.MountPath,
|
|
})
|
|
}
|
|
workloadSpec.Containers = append(workloadSpec.Containers, ateletCtr)
|
|
}
|
|
|
|
return workloadSpec, nil
|
|
}
|
|
|
|
// appendExternalVolumes maps template external volumes to resolved actor volumes and appends them to workloadSpec
|
|
// if they are referenced in container volumeMounts.
|
|
func appendExternalVolumes(workloadSpec *ateletpb.WorkloadSpec, template *atev1alpha1.ActorTemplate, actor *ateapipb.Actor) error {
|
|
if template == nil {
|
|
return nil
|
|
}
|
|
for _, vol := range template.Spec.Volumes {
|
|
if vol.ExternalVolumeTemplate != nil {
|
|
if !isVolumeMounted(vol.Name, template) {
|
|
continue
|
|
}
|
|
if actor == nil {
|
|
return fmt.Errorf("actor is required when externalVolumeTemplate is present")
|
|
}
|
|
|
|
var storageVolID string
|
|
var volType string
|
|
var volCtx map[string]string
|
|
for _, dbVol := range actor.GetStatus().GetActorVolumes() {
|
|
if dbVol.GetVolumeName() == vol.Name {
|
|
storageVolID = dbVol.GetStorageVolumeId()
|
|
volType = dbVol.GetVolumeType()
|
|
volCtx = dbVol.GetVolumeContext()
|
|
break
|
|
}
|
|
}
|
|
if storageVolID == "" {
|
|
return fmt.Errorf("volume %s not found for actor %s", vol.Name, actor.GetMetadata().GetName())
|
|
}
|
|
workloadSpec.Volumes = append(workloadSpec.Volumes, &ateletpb.Volume{
|
|
Name: vol.Name,
|
|
Source: &ateletpb.Volume_External{
|
|
External: &ateletpb.ExternalVolumeSource{
|
|
StorageVolumeId: storageVolID,
|
|
VolumeType: volType,
|
|
VolumeContext: volCtx,
|
|
},
|
|
},
|
|
})
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func isVolumeMounted(volumeName string, template *atev1alpha1.ActorTemplate) bool {
|
|
for _, ctr := range template.Spec.Containers {
|
|
for _, mount := range ctr.VolumeMounts {
|
|
if mount.Name == volumeName {
|
|
return true
|
|
}
|
|
}
|
|
}
|
|
return false
|
|
}
|
|
|
|
// toAteletReadyz projects the CRD readyz field onto the ateletpb wire type.
|
|
// Returns nil when the source is nil so containers without a probe stay
|
|
// unchanged on the wire.
|
|
// toAteletActorMetadataField projects the CRD field selector onto the atelet
|
|
// wire enum. Unknown values map to UNSPECIFIED, which atelet skips; CRD enum
|
|
// validation makes that unreachable for stored templates.
|
|
func toAteletActorMetadataField(in atev1alpha1.ActorMetadataField) ateletpb.ActorMetadataField {
|
|
switch in {
|
|
case atev1alpha1.ActorMetadataFieldName:
|
|
return ateletpb.ActorMetadataField_ACTOR_METADATA_FIELD_NAME
|
|
case atev1alpha1.ActorMetadataFieldAtespace:
|
|
return ateletpb.ActorMetadataField_ACTOR_METADATA_FIELD_ATESPACE
|
|
case atev1alpha1.ActorMetadataFieldUID:
|
|
return ateletpb.ActorMetadataField_ACTOR_METADATA_FIELD_UID
|
|
default:
|
|
return ateletpb.ActorMetadataField_ACTOR_METADATA_FIELD_UNSPECIFIED
|
|
}
|
|
}
|
|
|
|
func toAteletReadyz(in *atev1alpha1.ContainerReadyz) *ateletpb.Readyz {
|
|
if in == nil {
|
|
return nil
|
|
}
|
|
out := &ateletpb.Readyz{TimeoutSeconds: in.TimeoutSeconds}
|
|
if in.HTTPGet != nil {
|
|
out.HttpGet = &ateletpb.HTTPGetAction{
|
|
Path: in.HTTPGet.Path,
|
|
Port: in.HTTPGet.Port,
|
|
}
|
|
}
|
|
return out
|
|
}
|
|
|
|
// toAteletSecurityContext projects the CRD securityContext onto the ateletpb
|
|
// wire type. Returns nil when the source is nil or carries nothing, so
|
|
// containers that set no security settings stay unchanged on the wire.
|
|
func toAteletSecurityContext(in *atev1alpha1.SecurityContext) *ateletpb.SecurityContext {
|
|
if in == nil || in.Capabilities == nil {
|
|
return nil
|
|
}
|
|
caps := in.Capabilities
|
|
if len(caps.Add) == 0 && len(caps.Drop) == 0 {
|
|
return nil
|
|
}
|
|
return &ateletpb.SecurityContext{
|
|
Capabilities: &ateletpb.Capabilities{
|
|
Add: capabilityNames(caps.Add),
|
|
Drop: capabilityNames(caps.Drop),
|
|
},
|
|
}
|
|
}
|
|
|
|
func capabilityNames(in []atev1alpha1.Capability) []string {
|
|
if len(in) == 0 {
|
|
return nil
|
|
}
|
|
out := make([]string, 0, len(in))
|
|
for _, c := range in {
|
|
out = append(out, string(c))
|
|
}
|
|
return out
|
|
}
|