From 51af23e879cca629dd3b2c63a2f104869917056c Mon Sep 17 00:00:00 2001 From: garrytan Date: Fri, 30 Oct 2026 18:35:17 +0000 Subject: [PATCH] fix(ubicloud): legacy sweep reaps only the caller's own TTL-less VMs (GST-324) Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com> --- CHANGELOG.md | 4 ++-- docs/TESTING_INTERNALS.md | 6 ++++-- docs/skills.md | 2 +- scripts/ubicloud/ubi-runner.sh | 21 +++++++++++---------- test/ubicloud-runner.test.ts | 20 ++++++++++++-------- ubicloud/SKILL.md | 5 +++-- ubicloud/SKILL.md.tmpl | 5 +++-- 7 files changed, 36 insertions(+), 27 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index deab7b9e1..9516f5cd4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,13 +5,13 @@ **gstack no longer leaves Ubicloud VMs running after a lane dies (GST-118).** A runner killed mid-run, a copy of the runner without a TTL, or a VM with no owner could keep billing until someone noticed. Now every VM names its owner and its lifetime, the next `up` anywhere destroys anything past its lifetime, and every run says in one line whether its VM is gone. ### Fixed -- **`scripts/ubicloud/ubi-runner.sh`**: `up`, `run` and `verify` refuse without `UBI_OWNER` (`UBI_OWNER_REQUIRED`), so no VM is created that nobody can name. The default TTL drops from 4 hours to 3 (`UBI_TTL_HOURS`, 1-999), checked only when a VM is created, so an old setting never blocks `down` or `reap`. `reap`, which every `up` runs first, now also destroys `ubirun-*` VMs that carry no TTL (made by older runner copies) once they are 4 hours old (`UBI_LEGACY_HOURS`); names outside the `ubirun-` prefix are still never touched. +- **`scripts/ubicloud/ubi-runner.sh`**: `up`, `run` and `verify` refuse without `UBI_OWNER` (`UBI_OWNER_REQUIRED`), so no VM is created that nobody can name. The default TTL drops from 4 hours to 3 (`UBI_TTL_HOURS`, 1-999), checked only when a VM is created, so an old setting never blocks `down` or `reap`. `reap`, which every `up` runs first, now also destroys the caller's own `ubirun--*` VMs that carry no TTL (made by older runner copies) once they are 4 hours old (`UBI_LEGACY_HOURS`). Another owner's TTL-less VM, an untagged one and names outside the `ubirun-` prefix are never touched, because only a TTL lets reap judge another lane's VM. - **`UBICLOUD: left=<0|1|unknown> owner= vm= others=`**: `run` and `verify` end with this line, checked against the project's VM list after teardown. Anything but `left=0` turns a passing command into exit 1 and prints the `down` command to run; `run --keep` prints `left=1 ... kept=1` and keeps its exit status. - **`scripts/ubicloud/pregate-remote.sh`**: the remote pregate refuses without `UBI_OWNER` (`PREGATE_REMOTE_OWNER_REQUIRED`), names its VM with a TTL taken from its own stop deadline plus 15 minutes (`-t1` at today's 2,700-second timeout), and fails with `PREGATE_REMOTE_INFRA` unless the runner reports `left=0`. It reads that line from the file named by `UBI_LEFT_FILE`, never from the remote command's output. - docs/TESTING_INTERNALS.md, docs/pregate.md, docs/skills.md and the `/ubicloud` skill describe the owner rule, the new TTL default, the legacy sweep and the `left=` line. ### Tests -- `test/ubicloud-runner.test.ts`: refusal without an owner, the TTL bounds, the legacy sweep at the default and at `UBI_LEGACY_HOURS=2`, `left=0`, `left=1` on a failed destroy (with a failing command keeping exit 7), `run --keep`, and read-only commands accepting an old `UBI_TTL_HOURS=0`. +- `test/ubicloud-runner.test.ts`: refusal without an owner, the TTL bounds, the legacy sweep at the default and at `UBI_LEGACY_HOURS=2` (never another owner's, an untagged or an owner-less caller's), `left=0`, `left=1` on a failed destroy (with a failing command keeping exit 7), `run --keep`, and read-only commands accepting an old `UBI_TTL_HOURS=0`. - `test/ubicloud-pregate-remote.test.ts`: the owner refusal, the `-t1` TTL at today's timeout, and `left=1`, `left=unknown` and a missing line each failing closed with the `down` hint. ## [1.91.335.0] - 2026-10-30 diff --git a/docs/TESTING_INTERNALS.md b/docs/TESTING_INTERNALS.md index 5c3cd0052..cb477888d 100644 --- a/docs/TESTING_INTERNALS.md +++ b/docs/TESTING_INTERNALS.md @@ -1002,8 +1002,10 @@ The runner is also the `/ubicloud` skill's engine (`bin/gstack-ubi` execs it). A generated name ends in `-t` (`UBI_TTL_HOURS`, 1-999, default 3; pregate-remote.sh 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 +whichever owner made it, and the caller's own `ubirun--*` names +without a TTL (older runner copies) once they are `UBI_LEGACY_HOURS` old +(default 4). Another owner's TTL-less VM and an untagged one are never reaped, +because only a TTL lets reap judge another lane's VM. Every `up` runs it first (`UBI_REAP=0` skips it). Names outside the `ubirun-` prefix are never reaped. The quota preflight adds the size's vCPUs to the project's running VMs and refuses before the create request when that exceeds the known ceiling diff --git a/docs/skills.md b/docs/skills.md index 739408308..03f4ef9b2 100644 --- a/docs/skills.md +++ b/docs/skills.md @@ -991,7 +991,7 @@ VM. `/ubicloud` runs it there through one entry point, `bin/gstack-ubi` every VM is named `ubirun----t`, so its owner (`UBI_OWNER`, required to create one) and TTL (`UBI_TTL_HOURS`, default 3) are visible to everyone; every `up` first reaps VMs whose TTL ended, whichever owner -made them, and `ubirun-*` VMs with no TTL once they are 4 hours old; every run +made them, and its own owner's VMs with no TTL once they are 4 hours old; every run ends with `UBICLOUD: left=0` or fails; the quota preflight refuses before the create request when the project's running VMs plus the new size exceed the known ceiling (`UBI_QUOTA_VCPUS`, or the diff --git a/scripts/ubicloud/ubi-runner.sh b/scripts/ubicloud/ubi-runner.sh index 0936b9fab..5d15ee8d2 100755 --- a/scripts/ubicloud/ubi-runner.sh +++ b/scripts/ubicloud/ubi-runner.sh @@ -44,9 +44,9 @@ # TTL and reaper: a name this script generates ends in -t (UBI_TTL_HOURS, # 1-999, default 3), the creator's own declaration of how long the VM may live. # `reap` destroys every VM in the project whose TTL expired more than ten -# minutes ago, whichever owner made it, and every ubirun-* name with no TTL -# (older runner copies, hand-built names) once it is UBI_LEGACY_HOURS old -# (default 4); every `up` runs it first (UBI_REAP=0 skips it), so a runner +# minutes ago, whichever owner made it, and this owner's own ubirun--* +# names with no TTL once they are UBI_LEGACY_HOURS old (default 4; another +# owner's or an untagged TTL-less VM is never reaped); every `up` runs it first (UBI_REAP=0 skips it), so a runner # killed by a sleeping machine leaves nothing behind for longer than its TTL. # Names outside the ubirun- prefix are never reaped. # @@ -318,8 +318,9 @@ vm_rows() { # reap: destroy every VM whose name carries a TTL that ended more than # REAP_GRACE seconds ago, whichever owner created it (the TTL is that owner's -# own declaration), and every TTL-less ubirun-* name at least LEGACY_HOURS old -# (nobody declared a lifetime for it). Names outside the prefix are never touched. +# own declaration), and the caller's own TTL-less ubirun--* names at +# least LEGACY_HOURS old. Another owner's TTL-less VM, an untagged one, and names +# outside the prefix are never touched: only a TTL lets reap judge another lane's VM. 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)" @@ -331,7 +332,7 @@ cmd_reap() { [ "$now" -ge $(( expires + REAP_GRACE )) ] || continue log "reaping expired $loc/$name (owner $owner; its TTL ended $(( (now - expires) / 60 )) min ago)" else - [ "$owner" != . ] && [ "$created" != - ] || continue + [ -n "${UBI_OWNER:-}" ] && [ "$owner" = "$OWNER" ] && [ "$created" != - ] || continue [ $(( now - 10#$created )) -ge $(( LEGACY_HOURS * 3600 )) ] || continue log "reaping TTL-less $loc/$name (owner $owner; $(( (now - 10#$created) / 3600 ))h old, legacy sweep at ${LEGACY_HOURS}h)" fi @@ -805,7 +806,7 @@ 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" - elif [[ "$name" =~ ^$PREFIX-[a-z][a-z0-9]{0,11}-[0-9]{10}- ]]; then + elif [[ "$name" =~ ^$PREFIX-$OWNER-[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" @@ -960,15 +961,15 @@ usage: ubi-runner.sh [args] owner print this caller's owner tag ($OWNER) records VMs this machine created and has not confirmed gone ($VM_RECORD) gc HOURS destroy this owner's VMs older than HOURS (never other owners') - reap destroy every VM whose -t TTL has ended (any owner) and every - TTL-less ubirun-* VM older than UBI_LEGACY_HOURS + reap destroy every VM whose -t TTL has ended (any owner) and this + owner's TTL-less ubirun-* VMs older than UBI_LEGACY_HOURS cli ARGS... raw Ubicloud CLI passthrough (e.g. cli vm list) VMs are named $PREFIX----t; up, run and verify need UBI_OWNER (e.g. your thread code); run and verify end with a UBICLOUD: left= line. defaults: size=$DEFAULT_SIZE location=$DEFAULT_LOCATION (env UBI_SIZE, UBI_LOCATION) env: UBI_TTL_HOURS (${TTL_HOURS}h in the name, 1-999), UBI_REAP (0: up skips the reap), - UBI_LEGACY_HOURS (${LEGACY_HOURS}h: reap's bound for TTL-less ubirun-* names), + UBI_LEGACY_HOURS (${LEGACY_HOURS}h: reap's bound for this owner's TTL-less ubirun-* names), UBI_IDLE_TIMEOUT (${IDLE_TIMEOUT}s of silence kills the remote command, exit 124; 0 disables), UBI_RUN_TIMEOUT (${RUN_TIMEOUT}s wall clock; 0 unlimited), UBI_HEARTBEAT (${HEARTBEAT}s between heartbeat lines), UBI_QUOTA_VCPUS (project vCPU ceiling for the preflight; else the last refusal's maximum), diff --git a/test/ubicloud-runner.test.ts b/test/ubicloud-runner.test.ts index c8cc0f824..aa19c5372 100644 --- a/test/ubicloud-runner.test.ts +++ b/test/ubicloud-runner.test.ts @@ -222,27 +222,31 @@ esac } finally { rmSync(f.root, { recursive: true, force: true }); } }); - test('UBI_REAP=0 skips the reap and UBI_TTL_HOURS sets the name\'s TTL; reap destroys expired TTLs and old TTL-less ubirun names, never unexpired, recent or foreign ones', () => { + test('UBI_REAP=0 skips the reap and UBI_TTL_HOURS sets the name\'s TTL; reap destroys expired TTLs and this owner\'s old TTL-less names, never unexpired, recent, another owner\'s TTL-less, untagged or foreign ones', () => { const now = Math.floor(Date.now() / 1000); - const listed = `eu-central-h1 ubirun-other-1000000000-bbbb-t1\neu-central-h1 ubirun-thrd1-9999999999-dddd-t1\neu-central-h1 ubirun-thrd1-1000000000-aaaa\neu-central-h1 ubirun-1000000000-cccc\neu-central-h1 ubirun-peer-${now - 3 * 3600}-eeee\neu-central-h1 team-db-1000000000-ffff\n`; + const listed = `eu-central-h1 ubirun-other-1000000000-bbbb-t1\neu-central-h1 ubirun-thrd1-9999999999-dddd-t1\neu-central-h1 ubirun-thrd1-1000000000-aaaa\neu-central-h1 ubirun-1000000000-cccc\neu-central-h1 ubirun-thrd1-${now - 3 * 3600}-eeee\neu-central-h1 ubirun-gbra240-1000000000-gggg\neu-central-h1 team-db-1000000000-ffff\n`; const f = fakeApi({ destroy: 200, listed }); try { - const up = Bun.spawnSync(['bash', join(DIR, 'ubi-runner.sh'), 'up'], { env: { ...process.env, PATH: `${join(f.root, 'bin')}:${process.env.PATH}`, UBICLOUD_API_KEY: 'offline', UBI_RUNNER_STATE: join(f.root, 'state'), UBI_POLL_SECONDS: '1', UBI_VM_RECORD: f.record, UBI_OWNER: 'thrd1', UBI_QUOTA_VCPUS: '96', UBI_REAP: '0', UBI_TTL_HOURS: '7' }, timeout: 90_000 }); + const up = Bun.spawnSync(['bash', join(DIR, 'ubi-runner.sh'), 'up'], { env: { ...process.env, PATH: `${join(f.root, 'bin')}:${process.env.PATH}`, UBICLOUD_API_KEY: 'offline', UBI_RUNNER_STATE: join(f.root, 'state'), UBI_POLL_SECONDS: '1', UBI_VM_RECORD: f.record, UBI_OWNER: 'thrd1', UBI_QUOTA_VCPUS: '112', UBI_REAP: '0', UBI_TTL_HOURS: '7' }, timeout: 90_000 }); expect(up.exitCode).toBe(1); - expect(up.stderr.toString()).toMatch(/quota preflight refused eu-central-h1\/ubirun-thrd1-\d{10}-[0-9a-f]{8}-t7 \(standard-16\): it needs 16 vCPUs and the project's VMs already use 96 of 96/); + expect(up.stderr.toString()).toMatch(/quota preflight refused eu-central-h1\/ubirun-thrd1-\d{10}-[0-9a-f]{8}-t7 \(standard-16\): it needs 16 vCPUs and the project's VMs already use 112 of 112/); expect(f.calls()).not.toContain('destroy'); const reap = f.run('reap'); expect(reap.exitCode).toBe(0); expect(reap.stderr.toString()).toMatch(/reaping TTL-less eu-central-h1\/ubirun-thrd1-1000000000-aaaa \(owner thrd1; \d+h old, legacy sweep at 4h\)/); const destroyed = f.calls().split('\n').filter(l => l.includes('destroy')).map(l => l.match(/ubirun-[a-z0-9-]+|team-db-[a-z0-9-]+/)?.[0]).sort(); - expect(destroyed).toEqual(['ubirun-1000000000-cccc', 'ubirun-other-1000000000-bbbb-t1', 'ubirun-thrd1-1000000000-aaaa']); + expect(destroyed).toEqual(['ubirun-other-1000000000-bbbb-t1', 'ubirun-thrd1-1000000000-aaaa']); expect(f.run('reap').stderr.toString()).toContain('reap: no expired VMs'); // Value: protects=UBI_LEGACY_HOURS sets reap's bound for TTL-less ubirun names (a 3h-old name is spared at 4h, reaped at 2h); fails_when=reap ignores UBI_LEGACY_HOURS and keeps a fixed bound; why_new=only the default bound and the validation are tested; seam=none; no_claim=real Ubicloud list and destroy (fake curl) const lowered = Bun.spawnSync(['bash', join(DIR, 'ubi-runner.sh'), 'reap'], { env: { ...process.env, PATH: `${join(f.root, 'bin')}:${process.env.PATH}`, UBICLOUD_API_KEY: 'offline', UBI_RUNNER_STATE: join(f.root, 'state'), UBI_POLL_SECONDS: '1', UBI_VM_RECORD: f.record, UBI_OWNER: 'thrd1', UBI_LEGACY_HOURS: '2' }, timeout: 90_000 }); expect(lowered.exitCode).toBe(0); - expect(lowered.stderr.toString()).toMatch(/reaping TTL-less eu-central-h1\/ubirun-peer-\d{10}-eeee \(owner peer; 3h old, legacy sweep at 2h\)/); - expect(f.calls()).toMatch(/ubirun-peer-\d{10}-eeee[^\n]*destroy/); - expect(f.calls()).not.toMatch(/(team-db|dddd)[^\n]*destroy/); + expect(lowered.stderr.toString()).toMatch(/reaping TTL-less eu-central-h1\/ubirun-thrd1-\d{10}-eeee \(owner thrd1; 3h old, legacy sweep at 2h\)/); + expect(f.calls()).toMatch(/ubirun-thrd1-\d{10}-eeee[^\n]*destroy/); + // Value: protects=the legacy sweep never destroys another lane's or an untagged TTL-less VM (GST-324: 22 gbra240 eval VMs lost); fails_when=the sweep matches any owner again; why_new=the cross-owner sweep destroyed a paid eval; seam=none; no_claim=real Ubicloud list and destroy (fake curl) + expect(f.calls()).not.toMatch(/(team-db|dddd|gggg|cccc)[^\n]*destroy/); + const ownerless = Bun.spawnSync(['bash', join(DIR, 'ubi-runner.sh'), 'reap'], { env: { ...process.env, PATH: `${join(f.root, 'bin')}:${process.env.PATH}`, UBICLOUD_API_KEY: 'offline', UBI_RUNNER_STATE: join(f.root, 'state'), UBI_POLL_SECONDS: '1', UBI_VM_RECORD: f.record, UBI_OWNER: '', UBI_LEGACY_HOURS: '1' }, timeout: 90_000 }); + expect(ownerless.exitCode).toBe(0); + expect(ownerless.stderr.toString()).not.toContain('reaping TTL-less'); } finally { rmSync(f.root, { recursive: true, force: true }); } }, 30_000); diff --git a/ubicloud/SKILL.md b/ubicloud/SKILL.md index 2dffeca09..4b56c9f53 100644 --- a/ubicloud/SKILL.md +++ b/ubicloud/SKILL.md @@ -146,8 +146,9 @@ A VM in `records` but absent from `list` is already gone: `$U down ` forgets it. A VM in the project under another owner is theirs; leave it (its TTL will reap it). Every `up` runs `$U reap` first, destroying any VM whose `-t` TTL ended more than ten minutes ago, whichever owner made it, and -any `ubirun-*` name with no TTL once it is `UBI_LEGACY_HOURS` old (default 4); -names outside the `ubirun-` prefix are never reaped. +this owner's own `ubirun--*` names with no TTL once they are +`UBI_LEGACY_HOURS` old (default 4). Another owner's TTL-less VM, an untagged +one and names outside the `ubirun-` prefix are never reaped. Quota: with `UBI_QUOTA_VCPUS=` exported (or after one refusal, whose maximum the runner remembers), `up` adds the size's vCPUs to the project's diff --git a/ubicloud/SKILL.md.tmpl b/ubicloud/SKILL.md.tmpl index feff69cfd..cfcf9fa52 100644 --- a/ubicloud/SKILL.md.tmpl +++ b/ubicloud/SKILL.md.tmpl @@ -72,8 +72,9 @@ A VM in `records` but absent from `list` is already gone: `$U down ` forgets it. A VM in the project under another owner is theirs; leave it (its TTL will reap it). Every `up` runs `$U reap` first, destroying any VM whose `-t` TTL ended more than ten minutes ago, whichever owner made it, and -any `ubirun-*` name with no TTL once it is `UBI_LEGACY_HOURS` old (default 4); -names outside the `ubirun-` prefix are never reaped. +this owner's own `ubirun--*` names with no TTL once they are +`UBI_LEGACY_HOURS` old (default 4). Another owner's TTL-less VM, an untagged +one and names outside the `ubirun-` prefix are never reaped. Quota: with `UBI_QUOTA_VCPUS=` exported (or after one refusal, whose maximum the runner remembers), `up` adds the size's vCPUs to the project's