Skip to content

Handle graceful termination in microvm ateom - #893

Open
sfunkenhauser wants to merge 1 commit into
agent-substrate:mainfrom
sfunkenhauser:ateom_gt_mvm
Open

Handle graceful termination in microvm ateom#893
sfunkenhauser wants to merge 1 commit into
agent-substrate:mainfrom
sfunkenhauser:ateom_gt_mvm

Conversation

@sfunkenhauser

Copy link
Copy Markdown
Collaborator

Part of #23

Trap SIGTERM in the ateom, and forward SIGTERM to all application containers.
Once all application containers exit, ateom will exit itself.

@sfunkenhauser
sfunkenhauser force-pushed the ateom_gt_mvm branch 3 times, most recently from a584eb0 to d674bb8 Compare August 12, 2026 20:21
@sfunkenhauser
sfunkenhauser marked this pull request as ready for review August 12, 2026 20:41
@sfunkenhauser

Copy link
Copy Markdown
Collaborator Author

Benjamin Elder (@BenTheElder) PTAL

I've patched the e2e test fixes from #904 just to get a passing run, but will submit and rebase once that lands.

@BenTheElder

Copy link
Copy Markdown
Collaborator

that's in, taking a look at the other commit

// Obtain a kata-agent client to signal the guest: reuse the log-forwarding
// connection if it's open, else dial a fresh one (best-effort). A dial we open
// here is closed here; ra.guestAgent belongs to the log forwarder.
agent := ra.guestAgent

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 should-fix 🟡 – This reads ra.guestAgent with lock released, and a concurrent teardown writes it. gracefulShutdown drops the lock before calling here so a suspend can still land, and CheckpointWorkload is deliberately not gated by rejectIfDraining. Its teardownActor does ra.guestAgent.Close() followed by ra.guestAgent = nil, so the field is written while this goroutine reads it — an unsynchronised read/write that -race will flag.

The observable damage is mild: a stale pointer means signalling a closed client and logging the error, and a nil one means fifteen seconds of dialAgentRetry against a VM that is being destroyed. Both are bounded and already tolerated.

The comment above maps.Clone has the right instinct about the map; the same reasoning applies one level down to the struct it hands out. Capturing what the drain needs — the agent and workloadIDs — into a value while the lock is still held would close it, and workloadIDs is immutable after construction so only the agent really needs copying.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants