Skip to content

fix(ateapi): tear a crashed actor's workload down before it is deleted #256

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
teemow wants to merge 2 commits into from
Closed

fix(ateapi): tear a crashed actor's workload down before it is deleted #256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
812a098
fix(ateapi): tear a crashed actor's workload down before it is deleted
teemow Oct 10, 2026
a46592b
docs(fork): the crashed-actor teardown row
teemow Oct 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions FORK.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ Everything on `giantswarm` that is not in the pin (`git log v0.4.0-alpha1..giant
| The egress gateway takes further credential providers (`credentialProvider.additionalProviders`: `uriAuthority`, a lowercase DNS name, and `host`, `<service>.<namespace>.svc:<port>`, rendered on both egress listeners beside the bundled `k8s.io` provider, dialed with the gateway's pod identity and verified against the `servicedns.podcert.ate.dev` trust bundle; an entry cannot replace `k8s.io` or repeat an authority; `charts/substrate/tests/atenet_egress_credential_providers_test.yaml`) | kagent injects the caller of the current turn at egress through its own provider, so the caller's bearer never enters the actor: agentgateway already selects a provider by URI authority and fetches the actor's egress policy on every request, but the chart rendered one provider only (giantswarm/giantswarm#38054) | [#248](https://github.com/giantswarm/substrate/pull/248) (carries [#104](https://github.com/giantswarm/substrate/pull/104) onto the v0.4.0-alpha1 line) | to file for kagent-dev/substrate (chart only); giantswarm/giantswarm#37742 row to add |
| The Kubernetes credential provider mints Google access tokens (`ate-secret://google-access-token.k8s.io/default/<namespace>/<secret>/<key>`: the entry must hold a service account key; the provider signs its JWT assertion, exchanges it at the key's https `token_uri` for a `cloud-platform` token, caches the token per key while at least fifteen minutes remain, so the gateway's five-minute cache never serves an expired one, and returns the token, never the key; the chart and the kustomize component route both authorities, bundled names of `substrate.egressCredentialProviders` that `additionalProviders` cannot replace, on the HTTP and HTTPS egress listeners to the one provider Deployment) | Vertex AI takes OAuth 2.0 access tokens only, and minting one means signing with the key, which the egress gateway cannot do and an actor must not hold: no kagent agent with a Vertex `ModelConfig` (Gemini or Anthropic on Vertex AI) ran on Substrate ([#142](https://github.com/giantswarm/substrate/issues/142); giantswarm/kagent-upstream#178; [#37742](https://github.com/giantswarm/giantswarm/issues/37742) row 119) | [#250](https://github.com/giantswarm/substrate/pull/250): `git cherry-pick -x` of the upstream-ready branch's commit, which is kagent-dev/substrate's `5ac16e7c` adapted to the `k8s.io/default` URI grammar (upstream's commit predates it and names the authority `google-access-token.kubernetes.io`) | [kagent-dev/substrate#47](https://github.com/kagent-dev/substrate/pull/47) (open); the adapted commit on the pin is branch [`upstream/google-access-token-provider`](https://github.com/giantswarm/substrate/tree/upstream/google-access-token-provider) (`4453a6f4`), offered to #47 |
| Require `golang.org/x/net` v0.61.0 in every module that had it below (`go.mod`, `hack/tools/{code-generator,controller-gen,go-licenses,ko}`, `internal/plugins/gcp-secret-manager`, `tools/apitool`; `go get golang.org/x/net@v0.61.0 && go mod tidy` in each, then `go mod vendor` for the root module the CircleCI builds compile with `-mod=vendor`; `x/sync`, `x/sys`, `x/term`, `x/text` and `x/tools` move along, and tidy drops the stale indirect requirements `code-generator`'s module file still listed) | CVE-2026-97032 and CVE-2026-78663, two HTTP/2 vulnerabilities in `x/net` below v0.61.0; the platform's Go repositories move together, their CircleCI nancy step refusing the older module | [#252](https://github.com/giantswarm/substrate/pull/252) (`fix(deps): bump golang.org/x/net to v0.61.0 for two HTTP/2 CVEs`) | not for upstream as such: changes versions only and falls away at the re-pin onto the first upstream commit that requires `x/net` ≥ v0.61.0 (upstream's dependency automation moves it) |
| `crashActor` terminates the sandbox on atelet, detaches the actor's volumes and only then releases the worker (a failed step leaves the actor `CRASHED` with its worker assignment, for `DeleteActor` or `RevertActor` to retry); `DeleteActor` refuses with `FailedPrecondition` while the sandbox cannot be stopped or a volume released, keeping the assignment for the retry; `CreateActorTemplate` refuses a DATA `on_pause` or `on_commit` scope unless a container mounts a durable-dir volume (`InvalidArgument` naming the field); the existing-volumes e2e suite covers the delete after a failed suspend | a failed checkpoint crashed the actor and released its worker with the sandbox still running on it, so `DeleteActor` found no worker to reach and removed the record: the sandbox, its existing-volume NFS mounts and the worker's capacity leaked until the worker pod restarted, and the PersistentVolume stayed `Released` on `directory not empty` ([giantswarm/substrate#237](https://github.com/giantswarm/substrate/issues/237); giantswarm/giantswarm#37742 row 162) | [#256](https://github.com/giantswarm/substrate/pull/256) (`fix(ateapi): tear a crashed actor's workload down before it is deleted`) | [agent-substrate/substrate#1953](https://github.com/agent-substrate/substrate/pull/1953) (open: the crash tears the workload down before `CRASHED` and keeps the assignment when it fails, the shape adapted here) and [agent-substrate/substrate#1972](https://github.com/agent-substrate/substrate/pull/1972) (open: refuses the DATA scope without a durable dir, for agent-substrate/substrate#1936); falls away at the re-pin onto the first release that carries both |

Twenty-three patches change Substrate ahead of upstream — egress for an actor while it resumes, without which no
skill-carrying agent of the platform boots, the atelet scheduling knobs, the keep policy on the CRD chart's
Expand Down
26 changes: 24 additions & 2 deletions cmd/ateapi/internal/apivalidation/actor_template.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,13 +62,18 @@ func ValidateActorTemplateUpdate(ctx context.Context, fldPath *field.Path, newVa
}

// ValidateCustom_CreateActorTemplateRequest_ActorTemplate rejects container
// volume mounts that reference volumes the template does not declare, and a
// sub_path or read_only on a mount of any volume but an existing one.
// volume mounts that reference volumes the template does not declare, a
// sub_path or read_only on a mount of any volume but an existing one, and a
// DATA snapshot scope on a template no container of which mounts a
// durable-dir volume: a DATA snapshot is the durable-dir volumes and nothing
// else, so without one every pause or suspend of its actors would fail on the
// node and crash them.
func ValidateCustom_CreateActorTemplateRequest_ActorTemplate(_ context.Context, _ operation.Operation, fldPath *field.Path, value, _ *ateapipb.ActorTemplate) field.ErrorList {
declared := make(map[string]*ateapipb.Volume, len(value.GetVolumes()))
for _, vol := range value.GetVolumes() {
declared[vol.GetName()] = vol
}
mountsDurableDir := false
var errs field.ErrorList
for i, ctr := range value.GetContainers() {
for j, mount := range ctr.GetVolumeMounts() {
Expand All @@ -82,6 +87,9 @@ func ValidateCustom_CreateActorTemplateRequest_ActorTemplate(_ context.Context,
errs = append(errs, field.Invalid(mountPath.Child("name"), name, "must reference a volume declared in the template"))
continue
}
if vol.GetDurableDir() != nil {
mountsDurableDir = true
}
if vol.GetExistingVolume() != nil {
continue
}
Expand All @@ -93,6 +101,20 @@ func ValidateCustom_CreateActorTemplateRequest_ActorTemplate(_ context.Context,
}
}
}
if !mountsDurableDir {
snapshotConfig := fldPath.Child("snapshot_config")
for _, scope := range []struct {
field string
value ateapipb.SnapshotContentScope
}{
{"on_pause", value.GetSnapshotConfig().GetOnPause()},
{"on_commit", value.GetSnapshotConfig().GetOnCommit()},
} {
if scope.value == ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA {
errs = append(errs, field.Invalid(snapshotConfig.Child(scope.field), scope.value.String(), "DATA snapshots capture only durable-dir volumes, and no container mounts one"))
}
}
}
return errs
}

Expand Down
95 changes: 92 additions & 3 deletions cmd/ateapi/internal/apivalidation/actor_template_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ func TestValidateCreateActorTemplateRequest(t *testing.T) {
field.ErrorList{field.Invalid(field.NewPath("actor_template", "metadata", "name"), "Tmpl_A", "").WithOrigin("format=k8s-short-name")},
}, {
"valid data-scoped snapshots",
&ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(func(tmpl *ateapipb.ActorTemplate) {
&ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(mountDurableDir, func(tmpl *ateapipb.ActorTemplate) {
tmpl.SnapshotConfig.OnPause = ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA
tmpl.SnapshotConfig.OnCommit = ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA
})},
Expand Down Expand Up @@ -200,7 +200,7 @@ func TestValidateCreateActorTemplateRequest(t *testing.T) {
field.ErrorList{field.Invalid(field.NewPath("actor_template", "snapshot_config", "storage_location"), "gs://my-bucket/snapshots?versions=true", "")},
}, {
"on_commit broader than on_pause",
&ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(func(tmpl *ateapipb.ActorTemplate) {
&ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(mountDurableDir, func(tmpl *ateapipb.ActorTemplate) {
tmpl.SnapshotConfig.OnPause = ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA
tmpl.SnapshotConfig.OnCommit = ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL
})},
Expand All @@ -210,7 +210,7 @@ func TestValidateCreateActorTemplateRequest(t *testing.T) {
// violation (on_commit has no default of its own) and a subset
// violation (UNSPECIFIED is not DATA).
"on_commit unset with data on_pause",
&ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(func(tmpl *ateapipb.ActorTemplate) {
&ateapipb.CreateActorTemplateRequest{ActorTemplate: validActorTemplate(mountDurableDir, func(tmpl *ateapipb.ActorTemplate) {
tmpl.SnapshotConfig.OnPause = ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA
tmpl.SnapshotConfig.OnCommit = ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_UNSPECIFIED
})},
Expand Down Expand Up @@ -244,6 +244,95 @@ func TestValidateCreateActorTemplateRequest(t *testing.T) {
}
}

// A DATA snapshot scope is accepted only when a container mounts a
// durable-dir volume: a DATA snapshot captures those volumes and nothing else,
// so an actor of a template without one has nothing to snapshot, and its
// pause or suspend fails on the node and crashes it. Declaring the volume is
// not enough; the node snapshots the mounted directory.
func TestValidateCreateActorTemplateRequest_DataScopeNeedsMountedDurableDir(t *testing.T) {
onPause := field.NewPath("actor_template", "snapshot_config", "on_pause")
onCommit := field.NewPath("actor_template", "snapshot_config", "on_commit")
full := ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_FULL
data := ateapipb.SnapshotContentScope_SNAPSHOT_CONTENT_SCOPE_DATA

volumes := []struct {
name string
mutate func(*ateapipb.ActorTemplate)
mounted bool
}{
{name: "no volumes"},
{
name: "durable dir declared but not mounted",
mutate: func(tmpl *ateapipb.ActorTemplate) {
tmpl.Volumes = []*ateapipb.Volume{{Name: "data", DurableDir: &ateapipb.DurableDirVolumeSource{}}}
},
},
{
name: "existing volume mounted",
mutate: func(tmpl *ateapipb.ActorTemplate) {
tmpl.Volumes = []*ateapipb.Volume{{Name: "workspace", ExistingVolume: &ateapipb.ExistingVolumeSource{}}}
tmpl.Containers[0].VolumeMounts = []*ateapipb.VolumeMount{{Name: "workspace", MountPath: "/workspace"}}
},
},
{name: "durable dir mounted", mutate: mountDurableDir, mounted: true},
{
name: "durable dir mounted by a second container",
mutate: func(tmpl *ateapipb.ActorTemplate) {
tmpl.Volumes = []*ateapipb.Volume{{Name: "data", DurableDir: &ateapipb.DurableDirVolumeSource{}}}
tmpl.Containers = append(tmpl.Containers, &ateapipb.Container{
Name: "sidecar",
Image: tmpl.Containers[0].Image,
VolumeMounts: []*ateapipb.VolumeMount{{Name: "data", MountPath: "/var/data"}},
})
},
mounted: true,
},
}
scopes := []struct {
name string
onPause, onCommit ateapipb.SnapshotContentScope
wantUnmounted field.ErrorList
}{
{name: "FULL/FULL", onPause: full, onCommit: full},
{
name: "FULL/DATA", onPause: full, onCommit: data,
wantUnmounted: field.ErrorList{field.Invalid(onCommit, "SNAPSHOT_CONTENT_SCOPE_DATA", "")},
},
{
name: "DATA/DATA", onPause: data, onCommit: data,
wantUnmounted: field.ErrorList{
field.Invalid(onPause, "SNAPSHOT_CONTENT_SCOPE_DATA", ""),
field.Invalid(onCommit, "SNAPSHOT_CONTENT_SCOPE_DATA", ""),
},
},
}
for _, vol := range volumes {
for _, scope := range scopes {
t.Run(vol.name+" "+scope.name, func(t *testing.T) {
tmpl := validActorTemplate(func(tmpl *ateapipb.ActorTemplate) {
tmpl.SnapshotConfig.OnPause = scope.onPause
tmpl.SnapshotConfig.OnCommit = scope.onCommit
})
if vol.mutate != nil {
vol.mutate(tmpl)
}
var want field.ErrorList
if !vol.mounted {
want = scope.wantUnmounted
}
assertValidateErr(t, ValidateCreateActorTemplateRequest(context.Background(), &ateapipb.CreateActorTemplateRequest{ActorTemplate: tmpl}), want)
})
}
}
}

// mountDurableDir declares a durable-dir volume and mounts it in the
// template's first container, as a template with DATA snapshot scopes must.
func mountDurableDir(tmpl *ateapipb.ActorTemplate) {
tmpl.Volumes = append(tmpl.Volumes, &ateapipb.Volume{Name: "data", DurableDir: &ateapipb.DurableDirVolumeSource{}})
tmpl.Containers[0].VolumeMounts = append(tmpl.Containers[0].VolumeMounts, &ateapipb.VolumeMount{Name: "data", MountPath: "/var/data"})
}

func TestValidateGetActorTemplateRequest(t *testing.T) {
tests := []struct {
name string
Expand Down
Loading
Loading