fix: pre-landing review fixes (TTL checks only on create, leftover line via UBI_LEFT_FILE, pregate TTL from its deadline)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>
This commit is contained in:
garrytanandcapy-ai[bot] committed 2026-10-30 17:06:42 +00:00
1 parent d3788096ac
commit 19231513a9
7 files changed
+42 -19

No files matched your search

+3 -2
View File
@@ -990,7 +990,8 @@ recorded there before the create call and forgotten only after a confirmed
destroy, so a killed runner leaves a name to clean up. `run` and `verify` end
with `UBICLOUD: left=<0|1|unknown> owner=<o> vm=<name> others=<n>`, checked
against the project's VM list after teardown; anything but `left=0` turns a
passing command into exit 1. No owner-age sweep runs by default (the TTL reaper
passing command into exit 1. pregate-remote.sh reads that line from the file
named by `UBI_LEFT_FILE`, never from the command's output. No owner-age sweep runs by default (the TTL reaper
below covers every owner); set
`UBI_GC_HOURS` to a positive number (or run `gc HOURS`; `gc 0` is a no-op) to
sweep only this owner's VMs older than that. For other commands, use the runner
@@ -999,7 +1000,7 @@ or its `up` / `ssh` / `sync` / `pull` / `down` steps (`--help` lists them).
The runner is also the `/ubicloud` skill's engine (`bin/gstack-ubi` execs it).
A generated name ends in `-t<hours>` (`UBI_TTL_HOURS`, 1-999, default 3; pregate-remote.sh
uses `-t1`), the creator's own statement of how long the VM may live: `reap`
uses its stop deadline plus 15 minutes, rounded up: `-t1` today), the creator's own statement of how long the VM may live: `reap`
destroys every VM in the project whose TTL ended more than ten minutes ago,
whichever owner made it, and every `ubirun-*` name without a TTL (older runner
copies) once it is `UBI_LEGACY_HOURS` old (default 4); every `up` runs it
+5 -4
View File
@@ -216,11 +216,11 @@ while :; do
attempt=$(( attempt + 1 ))
: >"$log"
: >"$stream"
rm -f "$log.pid" "$log.timedout"
rm -f "$log.pid" "$log.timedout" "$log.left"
t0=$(date +%s)
vm_name="ubirun-$(bash "$here/ubi-runner.sh" owner)-$(date +%s)-$(head -c4 /dev/urandom | od -An -tx1 | tr -d ' \n')-t1"
vm_name="ubirun-$(bash "$here/ubi-runner.sh" owner)-$(date +%s)-$(head -c4 /dev/urandom | od -An -tx1 | tr -d ' \n')-t$(( (stop_after + 900 + 3599) / 3600 ))"
say "VM $vm_name; if this run is killed before its teardown line, destroy it with: bash scripts/ubicloud/ubi-runner.sh down $vm_name"
python3 -c 'import os, sys; os.setsid(); os.execvp(sys.argv[1], sys.argv[1:])' \
UBI_LEFT_FILE="$log.left" python3 -c 'import os, sys; os.setsid(); os.execvp(sys.argv[1], sys.argv[1:])' \
bash -c "$vm_session" pregate-remote-vm "$stop_after" "$log" "$stream" "$here/ubi-runner.sh" "${args[@]}" -n "$vm_name" &
vmrun=$!
tail -n +1 -f "$stream" &
@@ -254,7 +254,8 @@ span() { if [ -n "$1" ] && [ -n "$2" ]; then echo "$(( $2 - $1 ))s"; else echo "
say "phases: create $(span "$t0" "$t_ready"), setup $(span "$t_setup" "$t_run"), run $(span "$t_run" "$t_exit"), teardown $(span "$t_exit" "$t_end"); total $(( t_end - started ))s (target 600s)"
say "log: $log"
left=$(sed -n 's/^[0-9]* UBICLOUD: left=\([^ ]*\) .*/\1/p' "$log" | tail -1)
left=""
[ ! -f "$log.left" ] || left=$(sed -n 's/^UBICLOUD: left=\([^ ]*\) .*/\1/p' "$log.left" | tail -1)
if [ "$left" != 0 ]; then
stop 4 PREGATE_REMOTE_INFRA "VM cleanup not confirmed (UBICLOUD: left=${left:-missing}); no result reported" \
"VM: $vm_name; destroy it with: bash scripts/ubicloud/ubi-runner.sh down $vm_name" "log: $log" "spent=$spent ($billed billed min on $size)"
+21 -8
View File
@@ -37,7 +37,7 @@
# that `down`; further signals are ignored until it finishes. A create refused
# for vCPU quota prints the project's usage per owner.
#
# Stale-VM sweeps are off by default. With UBI_GC_HOURS set to a positive
# The owner-age sweep is off by default (the TTL reap below always runs). With UBI_GC_HOURS set to a positive
# number, every `up` first destroys this owner's VMs older than that; `gc HOURS`
# runs the same sweep on demand. Neither ever touches another owner's VMs.
#
@@ -54,7 +54,9 @@
# UBICLOUD: left=<0|1|unknown> owner=<owner> vm=<name> others=<n>
# after teardown: left counts this invocation's own VM, checked against the
# project's VM list, and others counts this owner's other VMs (information only).
# A left other than 0 turns a passing command into exit 1.
# A left other than 0 turns a passing command into exit 1; `run --keep` reports
# its live VM as left=1 kept=1 and keeps its exit status. With UBI_LEFT_FILE set,
# the line is also appended to that file, out of reach of the command's output.
#
# Quota preflight: when the project's vCPU ceiling is known (UBI_QUOTA_VCPUS, or
# the maximum a refusal printed, cached in $STATE_ROOT/quota-max), `up` adds the
@@ -107,8 +109,6 @@ for bin in curl python3 ssh ssh-keygen tar; do
done
UBICLOUD_API_KEY="${UBICLOUD_API_KEY:-${UBICLOUD_API_TOKEN:-}}"
[[ "$GC_HOURS" =~ ^[0-9]+$ ]] || die "UBI_GC_HOURS must be a whole number of hours (unset or 0 disables the sweep)"
[[ "$TTL_HOURS" =~ ^[1-9][0-9]{0,2}$ ]] || die "UBI_TTL_HOURS must be a whole number of hours from 1 to 999 with no leading zero (every VM carries a TTL; set it above your longest run)"
[[ "$LEGACY_HOURS" =~ ^[1-9][0-9]{0,3}$ ]] || die "UBI_LEGACY_HOURS must be a whole number of hours from 1 to 9999 with no leading zero (how old a TTL-less ubirun-* VM must be before reap destroys it)"
for v in IDLE_TIMEOUT RUN_TIMEOUT HEARTBEAT QUOTA_WAIT; do
[[ "${!v}" =~ ^[0-9]+$ ]] || die "UBI_$v must be a whole number of seconds (0 disables it)"
done
@@ -133,9 +133,19 @@ owner_id() {
}
OWNER=$(owner_id)
# require_owner: up, run and verify create VMs, and a VM must name its owner.
# require_owner: up, run and verify create VMs, and a VM must name its owner
# and carry a TTL. Only creates check UBI_TTL_HOURS, so an old value never
# blocks down, reap or list.
require_owner() {
[ -n "${UBI_OWNER:-}" ] || die "UBI_OWNER is not set: every VM must carry its owner; export UBI_OWNER=<your thread or session code> (for example gsta164) and rerun (UBI_OWNER_REQUIRED)"
[[ "$TTL_HOURS" =~ ^[1-9][0-9]{0,2}$ ]] || die "UBI_TTL_HOURS must be a whole number of hours from 1 to 999 with no leading zero (every VM carries a TTL; set it above your longest run)"
}
# leftover FIELDS...: print the UBICLOUD: leftover line, and append it to
# UBI_LEFT_FILE when set, so a caller can read it apart from the command's output.
leftover() {
echo "UBICLOUD: $*" >&2
[ -z "${UBI_LEFT_FILE:-}" ] || echo "UBICLOUD: $*" >>"$UBI_LEFT_FILE"
}
# api OUT ARGS...: send one Ubicloud CLI command, write the body to OUT, and
@@ -312,6 +322,7 @@ vm_rows() {
# (nobody declared a lifetime for it). Names outside the prefix are never touched.
cmd_reap() {
local now rows loc name owner expires created n=0 failed=0
[[ "$LEGACY_HOURS" =~ ^[1-9][0-9]{0,3}$ ]] || die "UBI_LEGACY_HOURS must be a whole number of hours from 1 to 9999 with no leading zero (how old a TTL-less ubirun-* VM must be before reap destroys it)"
now=$(date +%s)
rows=$(vm_rows) || { log "reap: could not list the project's VMs; nothing destroyed"; return 1; }
while read -r loc name owner expires created; do
@@ -752,7 +763,7 @@ run_teardown() {
else
left=unknown others=unknown
fi
echo "UBICLOUD: left=$left owner=$OWNER vm=$RUN_VM others=$others" >&2
leftover "left=$left owner=$OWNER vm=$RUN_VM others=$others"
[ "$left" = 0 ] && return 0
log "VM cleanup not confirmed for $RUN_VM (left=$left); destroy it with: $0 down $RUN_VM"
[ "$rc" -ne 0 ] || exit 1
@@ -794,12 +805,14 @@ cmd_run() {
cmd_up "${up_args[@]}" >/dev/null
if [[ "$name" =~ -t([0-9]+)$ ]]; then
log "$name carries a ${BASH_REMATCH[1]}h TTL (any later run of this script reaps it); if this process dies first: $0 down $name"
else
elif [[ "$name" =~ ^$PREFIX-[a-z][a-z0-9]{0,11}-[0-9]{10}- ]]; then
log "$name has no TTL (reap destroys it once it is ${LEGACY_HOURS}h old); if this process dies before its teardown line, destroy it with: $0 down $name"
else
log "$name has no TTL and no reapable name, so nothing reaps it; if this process dies before its teardown line, destroy it with: $0 down $name"
fi
if [ "$keep" = 1 ]; then
trap - EXIT
echo "UBICLOUD: left=1 owner=$OWNER vm=$name kept=1" >&2
leftover "left=1 owner=$OWNER vm=$name kept=1"
log "--keep: leaving $name running; destroy with: $0 down $name"
fi
+1 -1
View File
@@ -40,7 +40,7 @@ const UBI_RUNNER_STUB = `#!/usr/bin/env bash
# Every teardown is appended to $STUB_CALLS.down.
set -uo pipefail
[ "$1" != owner ] || { echo stub; exit 0; }
left() { [ "\${STUB_LEFT:-0}" = none ] || echo "UBICLOUD: left=\${STUB_LEFT:-0} owner=stub vm=$name others=0" >&2; }
left() { [ "\${STUB_LEFT:-0}" = none ] || echo "UBICLOUD: left=\${STUB_LEFT:-0} owner=stub vm=$name others=0" | tee -a "\${UBI_LEFT_FILE:-/dev/null}" >&2; }
[ "$1" = run ] || exit 2
shift
printf '%s\\n' "$*" >>"$STUB_CALLS"
+10 -4
View File
@@ -484,11 +484,16 @@ describe('ubi-runner run, verify and quota against a fake API and a fake ssh', (
test('run: ends with UBICLOUD: left=0 for its own VM; a same-owner sibling is counted as information and does not fail the run', () => {
const f = fakeVm({ listed: 'eu-central-h1 ubirun-thrd1-9999999999-sib1-t3\n' });
try {
const r = f.run(['run', '--src', f.root, '--', 'true'], { UBI_REAP: '0' });
const leftFile = join(f.root, 'left');
const r = f.run(['run', '--src', f.root, '--', 'true'], { UBI_REAP: '0', UBI_LEFT_FILE: leftFile });
const err = r.stderr.toString();
expect(r.exitCode, err).toBe(0);
expect(err).toMatch(/UBICLOUD: left=0 owner=thrd1 vm=ubirun-thrd1-\d{10}-[0-9a-f]{8}-t3 others=1\n/);
expect(f.listed()).toBe('eu-central-h1 ubirun-thrd1-9999999999-sib1-t3\n');
// Value: protects=UBI_LEFT_FILE carries the same leftover line pregate-remote.sh parses, apart from the command's output; fails_when=the file is not written or its format drifts from pregate-remote.sh's parser; why_new=the pregate stub hand-copies the format; seam=none; no_claim=real VM teardown (fake API and ssh)
expect(readFileSync(leftFile, 'utf8')).toMatch(/^UBICLOUD: left=0 owner=thrd1 vm=ubirun-thrd1-\d{10}-[0-9a-f]{8}-t3 others=1\n$/);
const parser = read('scripts/ubicloud/pregate-remote.sh').match(/left=\$\(sed -n '([^']+)' "\$log\.left"/)![1];
expect(Bun.spawnSync(['sed', '-n', parser, leftFile]).stdout.toString()).toBe('0\n');
// Value: protects=run --keep reports its live VM as UBICLOUD: left=1 ... kept=1 and never a clean left=0; fails_when=the kept line is dropped or --keep still prints the teardown line; why_new=no test runs run --keep; seam=none; no_claim=real VM lifetime (fake API and ssh)
const kept = f.run(['run', '--src', f.root, '--keep', '--', 'true'], { UBI_REAP: '0' });
const keptErr = kept.stderr.toString();
@@ -731,13 +736,14 @@ describe('ubi-runner run, verify and quota against a fake API and a fake ssh', (
expect(listFails.exitCode).toBe(1);
expect(listFails.stderr.toString()).toContain("reap: could not list the project's VMs; nothing destroyed");
for (const ttl of ['0', '08', '1000', 'abc']) {
const r = run(f, ['owner'], { UBI_TTL_HOURS: ttl });
const r = run(f, ['up'], { UBI_TTL_HOURS: ttl });
expect(r.exitCode).toBe(1);
expect(r.stderr.toString()).toContain('UBI_TTL_HOURS must be a whole number of hours from 1 to 999');
}
expect(run(f, ['owner'], { UBI_TTL_HOURS: '999' }).exitCode).toBe(0);
// Value: protects=an old UBI_TTL_HOURS=0 or bad UBI_LEGACY_HOURS never blocks commands that create nothing (down, list, owner); fails_when=the TTL checks move back to startup; why_new=the checks were global and blocked cleanup; seam=none; no_claim=real down against the API (owner stands in for every read-only command)
expect(run(f, ['owner'], { UBI_TTL_HOURS: '0', UBI_LEGACY_HOURS: '0' }).exitCode).toBe(0);
for (const legacy of ['0', '04', 'x']) {
const r = run(f, ['owner'], { UBI_LEGACY_HOURS: legacy });
const r = run(f, ['reap'], { UBI_LEGACY_HOURS: legacy });
expect(r.exitCode).toBe(1);
expect(r.stderr.toString()).toContain('UBI_LEGACY_HOURS must be a whole number of hours from 1 to 9999');
}
+1
View File
@@ -246,6 +246,7 @@ and for browser tests `xvfb` plus
it with every result: `left=0` means this run's VM is confirmed gone. `left=1`
or `left=unknown` turns a passing command into exit 1; run the printed `down`
command now. `others` counts this owner's other VMs, for information.
`run --keep` prints `left=1 ... kept=1` instead and keeps its exit status.
- Exit 124 with `watchdog: <phase> printed nothing for <n>s` (or `ran <n>s`)
means the remote hung or exceeded its budget; the last 40 lines follow.
Diagnose from them before rerunning; a hang is a failure, not infrastructure.
+1
View File
@@ -172,6 +172,7 @@ and for browser tests `xvfb` plus
it with every result: `left=0` means this run's VM is confirmed gone. `left=1`
or `left=unknown` turns a passing command into exit 1; run the printed `down`
command now. `others` counts this owner's other VMs, for information.
`run --keep` prints `left=1 ... kept=1` instead and keeps its exit status.
- Exit 124 with `watchdog: <phase> printed nothing for <n>s` (or `ran <n>s`)
means the remote hung or exceeded its budget; the last 40 lines follow.
Diagnose from them before rerunning; a hang is a failure, not infrastructure.