diff --git a/docs/plans/M3b/11-brokerd-container.md b/docs/plans/M3b/11-brokerd-container.md index e891468..47bce27 100644 --- a/docs/plans/M3b/11-brokerd-container.md +++ b/docs/plans/M3b/11-brokerd-container.md @@ -92,12 +92,15 @@ process kills the sleep and nothing keeps the pipes open. ## How to work (read this first) -Two earlier sessions on this task ended with nothing written: each tried to plan the whole file -in one turn and ran out of room while still deciding. So: +Three earlier sessions on this task ended without a commit: each tried to plan the whole file +(the third, the whole of `run`) in one turn and ran out of room while still deciding. So: -- **Start from the skeleton.** It compiles. Replace the `todo!()`s **one function at a time**, in - this order: `read_capped`, `new`, `egress_dir`, `podman`, `wait`, `run`. After each, run - `cargo check -p brokerd` and fix what it says before going on. +- **Start from the skeleton.** It compiles. `run` and `run_container` are **already written**: they + only call the helpers. Replace the `todo!()`s **one function at a time**, in this order: + `read_capped`, `new`, `egress_dir`, `podman`, `wait`, `spawn` (step 3), `Io::start` (step 4), + `Io::finish` (step 6), `answer` (step 7, the table). After each, run `cargo check -p brokerd` + and fix what it says before going on. Each helper is a few lines; none needs the others' + details beyond its signature. - **Let the compiler answer API questions.** If you are unsure whether something compiles (how to call the log, which trait a type has), write it and run `cargo check`. The log is called as `(self.log)(&line)`. diff --git a/docs/plans/M3b/12-brokerd-egress.md b/docs/plans/M3b/12-brokerd-egress.md index b0ce907..332b2a4 100644 --- a/docs/plans/M3b/12-brokerd-egress.md +++ b/docs/plans/M3b/12-brokerd-egress.md @@ -33,9 +33,13 @@ impl Podman { After step 2 of task 11 (name, input, limit): -- `spec.egress()` is `None` → exactly as before: `tool_args(…, None)` and run the container. -- `spec.egress()` is `Some(hosts)` → start the proxy (below), and on success run the container - with `tool_args(spec, &runner, &name, Some(&dir))`. +- `spec.egress()` is `None` → exactly as before: `tool_args(…, None)` and `run_container`. +- `spec.egress()` is `Some(hosts)` → start the proxy (below), and on success call + `run_container` with `tool_args(spec, &runner, &name, Some(&dir))`. + +`run_container` (from task 11) does steps 3 to 7 for any argument list, so the only change to +`run` is this choice. Put the proxy's start in its own function (`start_egress`), returning the +guard or the error. ### Starting the proxy: every step and exit diff --git a/docs/plans/M3b/README.md b/docs/plans/M3b/README.md index c8db2a0..a5b2cdd 100644 --- a/docs/plans/M3b/README.md +++ b/docs/plans/M3b/README.md @@ -74,6 +74,13 @@ At the end: `make gate` prints `gate: ok` with about 638 tests. compiling skeleton of `container.rs` (signatures, constants, the steps as comments, `todo!()` bodies), checked against the given tests (11 red), and says to fill one function at a time with `cargo check` between. Resume from task 11. +- 2026-09-23, task 11, third attempt: it filled five of the six functions from the skeleton, with + `cargo check` between, then planned all of `run` in one turn and was cut off (two compile errors + left). Saved in `.state/runs/M3b/11-third-attempt.diff`. The skeleton now has `run` and + `run_container` written as glue, and the rest as small helpers (`spawn`, `Io::start`, + `Io::finish`, `answer`); the design model filled them in a scratch copy to check the split can + pass (11 of 11, five runs, clippy clean) and removed that. Task 12 now calls `run_container`. + Resume from task 11. ## Running it diff --git a/docs/plans/M3b/files/crates/brokerd/src/container.rs b/docs/plans/M3b/files/crates/brokerd/src/container.rs index 3ce0cfb..619eecd 100644 --- a/docs/plans/M3b/files/crates/brokerd/src/container.rs +++ b/docs/plans/M3b/files/crates/brokerd/src/container.rs @@ -2,12 +2,15 @@ //! tool prints is the result's content; every failure is a fixed sentence, and what Podman itself //! said goes only to `brokerd`'s log. M3b spec, section 6. //! -//! SKELETON from task 11: replace every `todo!()`, one function at a time, running -//! `cargo check -p brokerd` after each. Then delete this paragraph. +//! SKELETON from task 11: replace every `todo!()`, one function at a time, in the order the task +//! gives, running `cargo check -p brokerd` after each. `run` and `run_container` are already +//! written. Then delete this paragraph. +use std::ffi::OsString; use std::io::{Read, Write}; use std::path::{Path, PathBuf}; use std::process::{Child, Command, ExitStatus, Stdio}; +use std::thread::JoinHandle; use std::sync::Arc; use std::sync::atomic::{AtomicU64, Ordering}; use std::time::{Duration, Instant}; @@ -61,16 +64,79 @@ impl Podman { } } +impl Podman { + /// Steps 3 to 7 for one container: spawn, feed and read it, wait, answer. Task 12 calls it too. + /// Already written: it only joins the helpers below. + fn run_container( + &self, + name: &str, + args: Vec, + input: String, + limit: Duration, + ) -> Result { + let Some(mut child) = self.spawn(args) else { + return Err(RunError::Unavailable(CANNOT_START.to_string())); + }; + let started = Instant::now(); + let cap = usize::try_from(self.runner.output_cap).unwrap_or(usize::MAX); + let io = Io::start(&mut child, input, cap); + let status = self.wait(&mut child, name, started, limit); + let (out, truncated, err) = io.finish(); + self.answer(name, status, out, truncated, &err) + } + + /// Step 3: spawn `podman` with `args` and all three standard streams piped. On failure, log + /// `brokerd: cannot start {podman path}: {e}` + "\n" + RUNBOOK and return `None`. + fn spawn(&self, args: Vec) -> Option { + todo!() + } + + /// Step 7: the answer, by the table in the task. `status` is `None` when the time limit was + /// passed. `out` is the kept standard output, `err` the kept standard error (for the log only: + /// it never goes into a `RunError`). + fn answer( + &self, + name: &str, + status: Option, + out: Vec, + truncated: bool, + err: &str, + ) -> Result { + todo!() + } +} + impl Runtime for Podman { + /// Steps 1 and 2, then `run_container`. Already written. fn run(&self, spec: &RunSpec) -> Result { - // 1. `n` and `name`. - // 2. `args`, `input`, `limit`. - // 3. Spawn podman with all three standard streams piped; a failure is Unavailable. - // 4. Three threads: write `input` then drop stdin; `read_capped` stdout with the cap; - // `read_capped` stderr with STDERR_KEPT. - // 5. `self.wait(…)`. - // 6. Join the three threads (`join().ok()`, `unwrap_or_default()`). - // 7. The answer, by the table in the task. + let n = self.next.fetch_add(1, Ordering::SeqCst); + let name = podman::container_name(spec.session(), spec.call(), n); + let args = podman::tool_args(spec, &self.runner, &name, None); + let input = spec.arguments().canonical_json(); + let limit = self.runner.time_limit(spec.tool()); + self.run_container(&name, args, input, limit) + } +} + +/// Step 4: the three threads that feed and read one container, so no pipe can block another. +struct Io { + writer: Option>, + stdout: Option, bool)>>, + stderr: Option, bool)>>, +} + +impl Io { + /// Take `child`'s three pipes (`child.stdin.take()` and so on) and start one thread for each: + /// write `input` to standard input and then drop it; `read_capped` standard output with `cap`; + /// `read_capped` standard error with `STDERR_KEPT`. A pipe that is `None` gets no thread. + fn start(child: &mut Child, input: String, cap: usize) -> Io { + todo!() + } + + /// Step 6: join the three threads. A thread that is missing or panicked counts as empty + /// (`join().ok()`, `unwrap_or_default()`). Returns the kept standard output, whether there was + /// more, and the kept standard error decoded with `from_utf8_lossy`. + fn finish(self) -> (Vec, bool, String) { todo!() } }