Skip to content

Commit 8091f66

Browse files
authored
feat(sandbox): write agent output to the container log (#4005)
Since #2726 the canonical main process's stdout and stderr are captured in pipes that feed only the in-memory replay buffer used by sandbox connect. Agent output therefore never reaches the container's own stdout and stderr, so it is missing from kubectl logs, docker logs, and podman logs and from anything that collects container logs. Before #2726 the entrypoint inherited the container's descriptors and its output appeared there. Copy the main process's output to the launcher's stdout and stderr in addition to the replay buffer, restoring the earlier behavior: - Output is copied byte for byte to the matching stream from a forwarder thread per stream, after it is published to the replay buffer. When the container runtime falls behind on a stream, that stream's reader waits instead of dropping output, so backpressure reaches the agent as it did with inherited descriptors, while the other stream and attachments keep receiving output. - Before the main process's exit is published, the output readers finish and queued output is drained to the container log, so an agent's final lines are not lost at shutdown. A 30 second deadline covers both; when it expires, readers waiting on the container log are released and drain the pipes into the replay buffer only, so a stalled container log cannot block exit reporting. - PTY-mode processes are not copied. The terminal stream carries escape sequences and echoed input, and terminal commands never reached the container log before #2726. - Exec, SSH, and SFTP sessions are not copied. Launcher log lines keep their existing format and remain in the container's stderr. They are written as whole lines, and a newline is inserted first when the agent left stderr mid-line, so launcher and agent lines do not merge. The Docker and VM drivers appended the tail of the workload's output to failure messages: Docker the workload container's log, and the VM driver the guest console, which carries the launcher's stdout and stderr. Those messages land in the sandbox's Ready condition and in platform events that the gateway republishes to the sandbox event stream. With agent output in that log, those messages would carry arbitrary agent output, including anything sensitive the agent prints, into gateway status and events. The supervisor starts its health endpoint only after the agent starts, so every Docker failure path could include agent output, and the VM driver reports one whenever the VM or host supervisor exits. Forward only the supervisor's log tail, matching the Podman driver, which reads the workload log solely to match fixed launcher markers and never forwards raw workload output. The workload's output remains available through docker logs and the VM's rootfs-console.log. Document where main process output appears in the logging docs and the cluster debugging skill. Closes #3928 Signed-off-by: Kris Hicks <khicks@nvidia.com>
1 parent 71440b2 commit 8091f66

10 files changed

Lines changed: 747 additions & 62 deletions

File tree

‎crates/openshell-driver-docker/src/lib.rs‎

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -5288,12 +5288,6 @@ async fn spawn_docker_control_process(
52885288
if !log_tail.is_empty() {
52895289
write!(message, "; log tail: {log_tail}").ok();
52905290
}
5291-
let sandbox_log_tail =
5292-
docker_container_log_tail(&monitored_docker, &failure_context.container_id)
5293-
.await;
5294-
if !sandbox_log_tail.is_empty() {
5295-
write!(message, "; sandbox log tail: {sandbox_log_tail}").ok();
5296-
}
52975291
let _ = monitored_docker.remove_container(
52985292
&monitored_supervisor_id,
52995293
Some(RemoveContainerOptionsBuilder::default().force(true).build()),
@@ -5344,11 +5338,9 @@ async fn wait_for_docker_supervisor_ready(
53445338
Status::internal(format!("inspect Docker sandbox container: {error}"))
53455339
})?;
53465340
if sandbox.state.unwrap_or_default().running == Some(false) {
5347-
let sandbox_log_tail = docker_container_log_tail(docker, sandbox_id).await;
5348-
return Err(Status::unavailable(format!(
5349-
"Docker sandbox exited before supervisor became ready{}",
5350-
format_named_log_tail("sandbox log tail", &sandbox_log_tail)
5351-
)));
5341+
return Err(Status::unavailable(
5342+
"Docker sandbox exited before supervisor became ready",
5343+
));
53525344
}
53535345
let inspected = docker
53545346
.inspect_container(supervisor_id, None)
@@ -5361,12 +5353,10 @@ async fn wait_for_docker_supervisor_ready(
53615353
Some(HealthStatusEnum::HEALTHY) => return Ok(()),
53625354
_ if state.running == Some(false) => {
53635355
let log_tail = docker_container_log_tail(docker, supervisor_id).await;
5364-
let sandbox_log_tail = docker_container_log_tail(docker, sandbox_id).await;
5365-
warn!(sandbox_id, supervisor_id, supervisor_logs = %log_tail, sandbox_logs = %sandbox_log_tail, "Docker supervisor exited before becoming ready");
5356+
warn!(sandbox_id, supervisor_id, supervisor_logs = %log_tail, "Docker supervisor exited before becoming ready");
53665357
return Err(Status::unavailable(format!(
5367-
"Docker supervisor exited before becoming ready{}{}",
5368-
format_log_tail(&log_tail),
5369-
format_named_log_tail("sandbox log tail", &sandbox_log_tail)
5358+
"Docker supervisor exited before becoming ready{}",
5359+
format_log_tail(&log_tail)
53705360
)));
53715361
}
53725362
_ => tokio::time::sleep(Duration::from_millis(100)).await,
@@ -5375,13 +5365,9 @@ async fn wait_for_docker_supervisor_ready(
53755365
}
53765366

53775367
fn format_log_tail(log_tail: &str) -> String {
5378-
format_named_log_tail("log tail", log_tail)
5379-
}
5380-
5381-
fn format_named_log_tail(label: &str, log_tail: &str) -> String {
5382-
// gRPC status messages travel in HTTP/2 headers. Two 16 KiB container
5383-
// tails exceed the client's 16 KiB header budget and hide the real error
5384-
// behind PROTOCOL_ERROR. Allow for up to 3x percent-encoding expansion.
5368+
// gRPC status messages travel in HTTP/2 headers. A full 16 KiB container
5369+
// tail can exceed the client's 16 KiB header budget and hide the real
5370+
// error behind PROTOCOL_ERROR. Allow for up to 3x percent-encoding expansion.
53855371
const MAX_STATUS_LOG_TAIL_BYTES: usize = 1024;
53865372
if log_tail.is_empty() {
53875373
String::new()
@@ -5390,9 +5376,9 @@ fn format_named_log_tail(label: &str, log_tail: &str) -> String {
53905376
while !log_tail.is_char_boundary(start) {
53915377
start += 1;
53925378
}
5393-
format!("; {label}: [truncated] {}", &log_tail[start..])
5379+
format!("; log tail: [truncated] {}", &log_tail[start..])
53945380
} else {
5395-
format!("; {label}: {log_tail}")
5381+
format!("; log tail: {log_tail}")
53965382
}
53975383
}
53985384

‎crates/openshell-driver-docker/src/tests.rs‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,15 +27,14 @@ use tempfile::TempDir;
2727
#[test]
2828
fn startup_error_log_tails_fit_grpc_header_budget() {
2929
// Multibyte text exercises both the UTF-8 cut and worst-case gRPC message
30-
// percent encoding. Preserve the final diagnostic from each container.
30+
// percent encoding. Preserve the supervisor's final diagnostic.
3131
let logs = format!("{}\nstartup timed out", "🦀".repeat(8192));
3232
let message = format!(
33-
"Docker supervisor exited before becoming ready{}{}",
33+
"Docker supervisor exited before becoming ready{}",
3434
format_log_tail(&logs),
35-
format_named_log_tail("sandbox log tail", &logs),
3635
);
37-
assert_eq!(message.matches("[truncated]").count(), 2);
38-
assert_eq!(message.matches("startup timed out").count(), 2);
36+
assert_eq!(message.matches("[truncated]").count(), 1);
37+
assert_eq!(message.matches("startup timed out").count(), 1);
3938
let response = Status::unavailable(message).into_http::<()>();
4039
let header_bytes: usize = response
4140
.headers()

‎crates/openshell-driver-vm/src/driver.rs‎

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4171,16 +4171,6 @@ impl VmDriver {
41714171
|| format!("{component} process exited"),
41724172
|code| format!("{component} process exited with status {code}"),
41734173
);
4174-
if component == "VM"
4175-
&& let Some(state_dir) = state_dir.as_deref()
4176-
&& let Some(console) = read_vm_console_tail(
4177-
&state_dir.join("rootfs-console.log"),
4178-
VM_CONSOLE_DIAGNOSTIC_BYTES,
4179-
)
4180-
{
4181-
write!(message, "; guest console tail:\n{console}")
4182-
.expect("writing to String cannot fail");
4183-
}
41844174
if component == "host supervisor"
41854175
&& let Some(state_dir) = state_dir.as_deref()
41864176
&& let Some(stderr) = read_vm_console_tail(
@@ -4191,16 +4181,6 @@ impl VmDriver {
41914181
write!(message, "; supervisor stderr tail:\n{stderr}")
41924182
.expect("writing to String cannot fail");
41934183
}
4194-
if component == "host supervisor"
4195-
&& let Some(state_dir) = state_dir.as_deref()
4196-
&& let Some(console) = read_vm_console_tail(
4197-
&state_dir.join("rootfs-console.log"),
4198-
VM_CONSOLE_DIAGNOSTIC_BYTES,
4199-
)
4200-
{
4201-
write!(message, "; guest console tail:\n{console}")
4202-
.expect("writing to String cannot fail");
4203-
}
42044184
if let Some(snapshot) = self
42054185
.set_snapshot_condition(
42064186
&sandbox_id,
@@ -7517,6 +7497,63 @@ mod tests {
75177497
}));
75187498
}
75197499

7500+
#[tokio::test]
7501+
async fn process_exit_status_omits_guest_console_output() {
7502+
let running_child = || {
7503+
Command::new("sleep")
7504+
.arg("30")
7505+
.stdin(Stdio::null())
7506+
.stdout(Stdio::null())
7507+
.stderr(Stdio::null())
7508+
.kill_on_drop(true)
7509+
.spawn()
7510+
.unwrap()
7511+
};
7512+
for (component, child) in [
7513+
("VM", spawn_exited_child()),
7514+
("host supervisor", running_child()),
7515+
] {
7516+
let temp = tempfile::tempdir().unwrap();
7517+
std::fs::write(temp.path().join("rootfs-console.log"), "agent output\n").unwrap();
7518+
std::fs::write(temp.path().join("supervisor.err.log"), "supervisor error\n").unwrap();
7519+
let driver = test_driver_with_extensions(LifecycleExtensionRegistry::new());
7520+
let mut events = driver.events.subscribe();
7521+
insert_test_record(&driver, "sb-exit", temp.path().to_path_buf(), child).await;
7522+
7523+
driver.monitor_sandbox("sb-exit".to_string()).await;
7524+
7525+
let condition = driver.registry.lock().await["sb-exit"]
7526+
.snapshot
7527+
.status
7528+
.as_ref()
7529+
.and_then(|status| {
7530+
status
7531+
.conditions
7532+
.iter()
7533+
.find(|condition| condition.reason == "ProcessExited")
7534+
.cloned()
7535+
})
7536+
.expect("ProcessExited condition");
7537+
let mut event_message = None;
7538+
while let Ok(event) = events.try_recv() {
7539+
if let Some(watch_sandboxes_event::Payload::PlatformEvent(platform)) = event.payload
7540+
&& let Some(event) = platform.event
7541+
&& event.reason == "ProcessExited"
7542+
{
7543+
event_message = Some(event.message);
7544+
}
7545+
}
7546+
let event_message = event_message.expect("ProcessExited platform event");
7547+
for message in [&condition.message, &event_message] {
7548+
assert!(message.starts_with(&format!("{component} process exited")));
7549+
assert!(!message.contains("agent output"), "{component}: {message}");
7550+
}
7551+
if component == "host supervisor" {
7552+
assert!(condition.message.contains("supervisor error"));
7553+
}
7554+
}
7555+
}
7556+
75207557
#[tokio::test]
75217558
async fn background_provisioning_does_not_extend_the_rpc_span_lifetime() {
75227559
let traced = TestTracing::new();

0 commit comments

Comments
 (0)