Sweep leftover path identity probes in startup recovery (#181)

agents/container/storage.ts

@@ -12,7 +12,7 @@ import { MAXIMUM_TIMER_MS, runInProcessGroup, type ProcessGroup } from '../proce
 import { runTrackedProcess, type ProcessGroupLifecycle } from '../tracked-docker.ts';
 import { MAXIMUM_DECLARED_LINKS, MAXIMUM_NAME_BYTES, TREE_SCRIPT } from './tree-script.ts';
 import { ALLOCATION_IN_USE, allocationListCommands, assertResourceOwner, claimAllocationId, hasOwnerLabels,
-  ownerLabelArgs, releaseAllocationId, type ResourceOwner, UUID_V4 } from '../labels.ts';
+  ownerLabelArgs, releaseAllocationId, type ResourceOwner, runnerProbeLabels, UUID_V4 } from '../labels.ts';
 
 export interface TaskFilesystems {
   readonly keeper: string;
@@ -415,24 +415,37 @@ export const PROBE_VOLUME_MOUNT = ['type=volume', 'target=/work', 'volume-driver
   ...TASK_VOLUME_OPTIONS(1024 * 1024, 64).map(option => `volume-opt=${option}`)]
   // A field with a comma in it is quoted, as Docker's CSV parser reads --mount.
   .map(field => field.includes(',') ? `"${field}"` : field).join(',');
+/** The probe container's name: a fixed prefix and a UUID v4. Recovery removes only containers named so (#181). */
+export const PATH_IDENTITY_PROBE_NAME = new RegExp(`^codeboost-path-identity-${UUID_V4}$`);
+/**
+ * The probe's `docker run` arguments. The container and its anonymous volume carry the runner's token and the probe kind
+ * (#181), so startup recovery finds what a failed or interrupted probe left.
+ */
+export function pathIdentityProbeArgs(imageId: string, runnerOwner: string, name: string): string[] {
+  if (!PATH_IDENTITY_PROBE_NAME.test(name)) throw new Error('The path identity probe needs a name D generates.');
+  const labels = Object.entries(runnerProbeLabels(runnerOwner, 'path-identity')).map(([key, value]) => `${key}=${value}`);
+  return ['run', '--rm', '--name', name, ...labels.flatMap(label => ['--label', label]), '--read-only', '--user', '10001:10001',
+    '--network=none', '--cap-drop=ALL', '--security-opt=no-new-privileges', '--security-opt=seccomp=builtin', '--runtime=runc',
+    '--pids-limit=32', '--memory=128m', '--cpus=.25',
+    '--mount', [PROBE_VOLUME_MOUNT, ...labels.map(label => `volume-label=${label}`)].join(','),
+    '--entrypoint', 'sh', imageId, '-c', PATH_IDENTITY_SCRIPT, 'probe', '/work'];
+}
 /**
  * The path identity of task storage (#162): how the filesystem the agent edits, where the runner inspects task changes, tells two
  * spellings of a path apart. It is measured, not assumed, on a volume made with the same options as task storage, in a
  * container hardened like the keeper. The volume is anonymous and the container `--rm`, so once the container has
  * started the daemon removes both when it exits, even if this process does not live to see it. A client stopped before
  * then (its deadline) may leave the container created but never started, which `--rm` does not remove, so a failed
- * probe removes its container, and that volume, by the unique name it gave it. Not covered: a create that lands after
- * the settle window, or a runner that dies before the removal; the probe carries no owner labels, so recovery does not
- * find that container (`docker ps -a --filter name=codeboost-path-identity-` does). Nor is the volume's removal
- * confirmed: `--rm` and `rm --volumes` remove it, but Docker only logs a failure to. A recovery sweep is #181.
+ * probe removes its container, and that volume, by the unique name it gave it. What this cannot remove (a create that
+ * lands after the settle window, a runner that dies before the removal, a volume Docker failed to remove, since it
+ * only logs that) carries `runnerOwner` and the probe kind, and the next startup's recovery removes it (#181).
  */
-export async function probeTaskStoragePathIdentity(imageId: string, timeoutMs = 60_000, cleanupMs = PROBE_CLEANUP_MS): Promise<PathIdentity> {
+export async function probeTaskStoragePathIdentity(imageId: string, runnerOwner: string, timeoutMs = 60_000,
+  cleanupMs = PROBE_CLEANUP_MS): Promise<PathIdentity> {
   if (!/^sha256:[0-9a-f]{64}$/.test(imageId)) throw new Error('The path identity probe requires the immutable built image ID.');
   assertBuiltAgentImage(imageId);
   const name = `codeboost-path-identity-${randomUUID()}`;
-  const args = ['run', '--rm', '--name', name, '--read-only', '--user', '10001:10001', '--network=none', '--cap-drop=ALL',
-    '--security-opt=no-new-privileges', '--security-opt=seccomp=builtin', '--runtime=runc', '--pids-limit=32', '--memory=128m',
-    '--cpus=.25', '--mount', PROBE_VOLUME_MOUNT, '--entrypoint', 'sh', imageId, '-c', PATH_IDENTITY_SCRIPT, 'probe', '/work'];
+  const args = pathIdentityProbeArgs(imageId, runnerOwner, name);
   const outcome = await runInProcessGroup('docker', args, { env: dockerEnvironment(), timeoutMs });
   if (outcome.status !== 0) {
     const failure = new DockerError(args, outcome);
@@ -445,7 +458,7 @@ export async function probeTaskStoragePathIdentity(imageId: string, timeoutMs =
     const refused = outcome.status !== null && /Error response from daemon/.test(outcome.stderr);
     try { await removeProbeContainer(name, refused ? 0 : PROBE_SETTLE_MS, cleanupMs); }
     catch (error) {
-      throw new Error(`${failure.message} The probe container ${name} may remain; remove it with docker rm --force --volumes ${name}.`,
+      throw new Error(`${failure.message} The probe container ${name} may remain; the next start's recovery removes it, or remove it now with docker rm --force --volumes ${name}.`,
         { cause: new AggregateError([failure, error]) });
     }
     throw failure;

agents/labels.ts

@@ -87,3 +87,23 @@ export const ownerLabels = (owner: ResourceOwner): Readonly<Record<string, strin
 export const hasOwnerLabels = (labels: Readonly<Record<string, string>> | null | undefined, owner: ResourceOwner) =>
   labels?.[RUNNER_LABEL] === owner.runnerOwner && labels?.[ATTEMPT_LABEL] === owner.attemptId
   && labels?.[ALLOCATION_LABEL] === owner.allocationId;
+
+/**
+ * The kind label of a runner-scoped probe (#181): an object that belongs to the runner as a whole, not to an attempt or
+ * an allocation, so it carries only the runner label and this one. Startup recovery, under the runner lock, removes
+ * this runner's leftover probes: only one process holds a runner token at a time, so a probe found then is an earlier
+ * process's. A Docker client that process left may still be creating one: if the create lands between recovery's
+ * container and volume listings, recovery keeps its volume, which the new container holds; if it lands after them,
+ * recovery does not see it. Either way the next start removes both.
+ */
+export const PROBE_LABEL = 'io.codeboost.probe';
+/** The probes D runs: `path-identity` measures task storage's path identity at runner startup (#162). */
+export type RunnerProbe = 'path-identity';
+export const RUNNER_PROBES: readonly RunnerProbe[] = Object.freeze(['path-identity']);
+
+/** The labels on a runner-scoped probe's container and volume. Validates the runner token before it is written. */
+export function runnerProbeLabels(runnerOwner: string, probe: RunnerProbe): Readonly<Record<string, string>> {
+  if (!isRunnerOwner(runnerOwner)) throw new Error('runnerOwner must be 32 lowercase hex characters.');
+  if (!RUNNER_PROBES.includes(probe)) throw new Error('Unknown runner probe.');
+  return Object.freeze({ [RUNNER_LABEL]: runnerOwner, [PROBE_LABEL]: probe });
+}

agents/recovery.ts

@@ -1,9 +1,9 @@
 import { DOCKER_ID } from './client-outcome.ts';
 import { runDocker, type DockerOutcome } from './docker.ts';
-import { ALLOCATION_LABEL, ATTEMPT_LABEL, isAllocationId, isLabelAttemptId, isRunnerOwner,
+import { ALLOCATION_LABEL, ATTEMPT_LABEL, isAllocationId, isLabelAttemptId, isRunnerOwner, PROBE_LABEL,
   RUNNER_LABEL } from './labels.ts';
-import { adoptRecoveredTaskStorage, hasLiveTaskStorage, RecoveredStorageRejected, type RecoveredTaskStorage,
-  type TaskStorageParts } from './container/storage.ts';
+import { adoptRecoveredTaskStorage, hasLiveTaskStorage, PATH_IDENTITY_PROBE_NAME, RecoveredStorageRejected,
+  type RecoveredTaskStorage, type TaskStorageParts } from './container/storage.ts';
 
 /** A Docker object recovery found, with the labels it carries. */
 export interface RecoveredResource {
@@ -26,7 +26,7 @@ export interface UnownedResource extends RecoveredResource {
 export interface RecoveryReport {
   /**
    * Agent containers, egress proxies, seeders, export, inspection and commit containers and networks of this runner,
-   * now confirmed gone.
+   * and its path identity probes' containers and volumes (#181), now confirmed gone.
    */
   readonly removed: readonly RecoveredResource[];
   /** One handle per task-storage allocation of this runner, kept whole for export and `removeTaskFilesystems`. */
@@ -48,7 +48,9 @@ const EGRESS_LABEL = 'io.codeboost.egress';
 const STORAGE_LABEL = 'io.codeboost.task-storage';
 // Every label a codeboost object of any build carries at least one of; used to find objects without a runner label.
 // Each Docker type is scanned for all of them, so an object with a label D never puts on its type is still found.
-const KIND_LABELS = [INVOCATION_LABEL, EGRESS_LABEL, STORAGE_LABEL, ALLOCATION_LABEL] as const;
+const KIND_LABELS = [INVOCATION_LABEL, EGRESS_LABEL, STORAGE_LABEL, ALLOCATION_LABEL, PROBE_LABEL] as const;
+// The name Docker gives an anonymous volume, as the path identity probe's is.
+const ANONYMOUS_VOLUME = /^[0-9a-f]{64}$/;
 const DIAGNOSTIC_LIMIT = 1_000;
 // Objects per `docker inspect`, so a daemon with many codeboost objects cannot exceed the argument-size limit.
 const INSPECT_BATCH = 200;
@@ -111,19 +113,27 @@ const inspectBatch = async (kind: Kind, refs: readonly string[], remaining: () =
 };
 const unique = (values: readonly string[]) => [...new Set(values)];
 
-// Remove one object by its full ID, and confirm it is gone.
+/** Docker refused to remove a volume because a container mounts it. */
+class VolumeInUse extends Error {}
+// Remove one object by its full ID (a volume by its name, which is its identity), and confirm it is gone.
 const removeById = async (resource: RecoveredResource, remaining: () => number) => {
-  const args = resource.kind === 'network' ? ['network', 'rm', resource.id!] : ['rm', '--force', resource.id!];
+  const ref = resource.kind === 'volume' ? resource.name : resource.id!;
+  const args = resource.kind === 'network' ? ['network', 'rm', ref]
+    : resource.kind === 'volume' ? ['volume', 'rm', '--force', ref] : ['rm', '--force', ref];
   const result = await runDocker(args, { timeoutMs: remaining() });
   if (result.status === 0) return;
-  if (!absent(await runDocker([resource.kind, 'inspect', resource.id!], { timeoutMs: remaining() })))
+  // Docker 27.4: "remove <name>: volume is in use - [<container IDs>]".
+  if (resource.kind === 'volume' && /volume is in use/.test(result.stderr))
+    throw new VolumeInUse(`Volume ${resource.name} is in use.`);
+  if (!absent(await runDocker([resource.kind, 'inspect', ref], { timeoutMs: remaining() })))
     throw new Error(`Could not remove ${resource.kind} ${resource.name}: ${(result.error?.message ?? result.stderr).trim()}`);
 };
 
 /**
  * Crash recovery for one database (#51 item 4). Acts only on objects whose `io.codeboost.runner` label is
  * `runnerOwner`: it removes agent containers, egress proxies, seeders, export, inspection and commit containers and
- * networks, and resolves only once they are gone. It keeps task storage whole (volumes and keeper) and returns a recovery handle per
+ * networks, and the containers and anonymous volumes path identity probes left (#181), and resolves only once they are
+ * gone. It keeps task storage whole (volumes and keeper) and returns a recovery handle per
  * allocation. Objects from older builds without a runner label, and anything it does not recognise, are reported and
  * never touched.
  *
@@ -151,7 +161,10 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000,
     network: [] as RecoveredResource[] };
   const unowned: UnownedResource[] = [];
   for (const kind of ['container', 'volume', 'network'] as const) {
-    owned[kind] = await inspect(kind, await list(kind, `${RUNNER_LABEL}=${runnerOwner}`, remaining), remaining);
+    // An object inspected with another runner's label replaced the one listed under the same name: it is not this
+    // runner's, so recovery neither removes nor reports it.
+    owned[kind] = (await inspect(kind, await list(kind, `${RUNNER_LABEL}=${runnerOwner}`, remaining), remaining))
+      .filter(resource => resource.labels[RUNNER_LABEL] === runnerOwner);
     if (options.unowned === false) continue;
     // Docker cannot filter on a missing label, so list every codeboost object of this kind and keep those without one.
     const candidates = unique((await Promise.all(KIND_LABELS.map(label => list(kind, label, remaining)))).flat());
@@ -173,10 +186,15 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000,
     storage.set(allocationId, group);
   };
   // D writes exactly one kind label on each object, with complete owner labels (an egress object's egress label is its
-  // allocation ID). Recovery has no creation-time ID to prove ownership, so any other combination is not D's and is
-  // left alone.
+  // allocation ID). A runner-scoped probe carries the probe label and no attempt or allocation. Recovery has no
+  // creation-time ID to prove ownership, so any other combination is not D's and is left alone.
   const kindOf = (resource: RecoveredResource) => {
     const labels = resource.labels;
+    if (PROBE_LABEL in labels) {
+      const scoped = [ATTEMPT_LABEL, ALLOCATION_LABEL, INVOCATION_LABEL, EGRESS_LABEL, STORAGE_LABEL]
+        .some(label => label in labels);
+      return !scoped && labels[PROBE_LABEL] === 'path-identity' ? 'probe:path-identity' : undefined;
+    }
     if (!isLabelAttemptId(labels[ATTEMPT_LABEL]) || !isAllocationId(labels[ALLOCATION_LABEL])) return undefined;
     const present = [INVOCATION_LABEL, EGRESS_LABEL, STORAGE_LABEL].filter(label => label in labels);
     if (present.length !== 1) return undefined;
@@ -196,12 +214,17 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000,
       || kind === 'storage:inspect' || kind === 'storage:commit'
       || kind === 'agent' || kind === 'egress')
       remove.push(resource);
+    // Only one process holds this runner's token, and it probes after recovery, so a probe found now is an earlier
+    // process's. One that process's orphaned client creates during the scan holds its volume, which is then kept.
+    else if (kind === 'probe:path-identity' && PATH_IDENTITY_PROBE_NAME.test(resource.name)) remove.push(resource);
     else unknown(resource);
   }
+  const probeVolumes: RecoveredResource[] = [];
   for (const resource of owned.volume) {
     const kind = kindOf(resource);
     if (kind === 'storage:work') keep(resource, 'workVolume');
     else if (kind === 'storage:metadata') keep(resource, 'metadataVolume');
+    else if (kind === 'probe:path-identity' && ANONYMOUS_VOLUME.test(resource.name)) probeVolumes.push(resource);
     else unknown(resource);
   }
   const networks = owned.network.filter(resource => {
@@ -210,14 +233,20 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000,
     return false;
   });
 
-  // Containers first: a network cannot be removed while an agent or proxy is still attached to it.
+  // Containers first: a network cannot be removed while an agent or proxy is still attached to it, nor a probe's
+  // volume while its container still exists.
   const failures: unknown[] = [], confirmed: RecoveredResource[] = [];
-  for (const resource of [...remove, ...networks]) {
-    if (resource.kind === 'network' && failures.length) break;
+  for (const resource of [...remove, ...probeVolumes, ...networks]) {
+    if (resource.kind !== 'container' && failures.length) break;
     try {
       await removeById(resource, remaining);
       confirmed.push(resource);
-    } catch (error) { failures.push(error); }
+    } catch (error) {
+      // Every container recovery removes is gone by now, so the holder is one it leaves: one it reports as unowned, one
+      // without codeboost labels, or a probe an earlier process's orphaned client created between the container and
+      // volume listings. The volume is kept, and a start after its holder is gone removes it.
+      if (!(error instanceof VolumeInUse)) failures.push(error);
+    }
   }
   if (failures.length) {
     const detail = failures.map(error => (error as Error).message).join('; ');
@@ -242,6 +271,6 @@ export async function recoverLeftovers(runnerOwner: string, timeoutMs = 120_000,
       for (const resource of group.resources) unowned.push(Object.freeze({ ...resource, reason: 'inconsistent-storage' }));
     }
   }
-  return Object.freeze({ removed: Object.freeze([...remove, ...networks]), storage: Object.freeze(handles),
+  return Object.freeze({ removed: Object.freeze(confirmed), storage: Object.freeze(handles),
     unowned: Object.freeze(unowned) });
 }