diff --git a/.gitea/workflows/cleanup-actions.yml b/.gitea/workflows/cleanup-actions.yml new file mode 100644 index 0000000..3f45a9c --- /dev/null +++ b/.gitea/workflows/cleanup-actions.yml @@ -0,0 +1,53 @@ +name: Clean up old workflow runs + +# Manual only, and dry run by default (#324). +# +# Gitea 1.27.3 expires a run's logs and artifacts but never the run record +# itself, so the Actions list grows without limit and fills with entries whose +# logs are already gone. This removes those entries. +# +# Deliberately not on a schedule. Deleting a run cannot be undone, the list is +# an annoyance rather than a problem, and a cron that quietly removes history +# should be a decision taken on its own rather than the default that arrives +# with the tool. + +on: + workflow_dispatch: + inputs: + keep_days: + description: 'Keep runs newer than this many days' + required: false + default: '7' + apply: + description: 'Type true to actually delete. Anything else reports only.' + required: false + default: 'false' + +jobs: + cleanup: + runs-on: ubuntu-latest + steps: + - name: Checkout + uses: actions/checkout@v4 + + - name: Setup Node + uses: actions/setup-node@v4 + with: + node-version: '20' + + # No npm install: the script uses only node's own https module, so there + # is nothing to fetch and nothing that can break when a dependency moves. + - name: Delete old runs + env: + # A dedicated secret rather than the automatic per-job token, because + # deleting a run may be beyond what that token is allowed to do. If it + # turns out to be sufficient, this and the secret can both go. + GITEA_ACCESS_TOKEN: ${{ secrets.ACTIONS_CLEANUP_TOKEN }} + # Taken from the run's own context so this file carries no hostname + # and works unchanged if the instance ever moves — which #313 may yet + # make happen. + GITEA_HOST: ${{ github.server_url }} + GITEA_REPO: ${{ github.repository }} + KEEP_DAYS: ${{ github.event.inputs.keep_days }} + APPLY: ${{ github.event.inputs.apply }} + run: node scripts/cleanup-workflow-runs.js diff --git a/backend/tests/integration/setup/globalSetup.ts b/backend/tests/integration/setup/globalSetup.ts index 83ff035..fbdd4c6 100755 --- a/backend/tests/integration/setup/globalSetup.ts +++ b/backend/tests/integration/setup/globalSetup.ts @@ -23,9 +23,52 @@ async function waitForDb(retries = 20): Promise { ); } +/** + * Every address the database host resolves to, and which server answered (#154). + * + * This is the measurement that issue has needed twice and never had. The + * schema in a failing run comes back — a suite fails on a missing `orders`, + * and a later suite truncating that same table passes — which a database + * losing its schema cannot do. More than one server answering to one name can, + * and Docker's embedded DNS round-robins every container sharing an alias, so + * a leftover service container from an earlier run fits every observation + * including the empty `dmesg`. + * + * **More than one address printed here is the answer outright.** One address, + * and this reading is wrong too — the next suspect is a single container being + * restarted with a fresh data directory, which the postmaster start time + * recorded alongside will show. + * + * Logged unconditionally rather than only on failure: a passing run's addresses + * are the control, and without them a failing run's have nothing to be compared + * against. Never throws — a diagnostic that can fail the run it was added to + * explain is worse than none. + */ +async function reportDatabaseIdentity(): Promise { + const host = process.env.TEST_PGHOST || 'localhost'; + try { + const { lookup } = await import('dns/promises'); + const addresses = await lookup(host, { all: true }); + const rendered = addresses.map((a) => a.address).join(', '); + console.info( + `[#154] "${host}" resolves to ${addresses.length} address(es): ${rendered}` + + (addresses.length > 1 ? ' <-- more than one server can answer; this is the bug' : '') + ); + } catch (err) { + console.info(`[#154] could not resolve "${host}": ${err instanceof Error ? err.message : String(err)}`); + } +} + export default async function globalSetup(): Promise { await waitForDb(); - const { migrate, closeDb, assertSchemaPresent } = await import('./testDb'); + const { migrate, closeDb, assertSchemaPresent, describeBackend } = await import('./testDb'); + + await reportDatabaseIdentity(); + // Recorded before migrating so the run's baseline is in the log even if the + // migration is the thing that fails. A suite that later reports a different + // postmaster start time is talking to a different instance. + console.info(`[#154] ${await describeBackend('globalSetup reached')}`); + await migrate(); // Cheap, once, and it establishes the fact the rest of the run depends on: diff --git a/backend/tests/integration/setup/testDb.ts b/backend/tests/integration/setup/testDb.ts index 98018a5..bf30b09 100755 --- a/backend/tests/integration/setup/testDb.ts +++ b/backend/tests/integration/setup/testDb.ts @@ -75,21 +75,68 @@ async function missingTables(): Promise { * can be fixed from here: whatever the cause, the next occurrence should read as * "the database lost its schema" on the first line. */ +/** + * Which Postgres actually answered, rather than which one we asked for (#154). + * + * The reason this exists: the schema in a failing run **comes back**. A suite + * fails because `orders` does not exist, and a later suite truncating the same + * table passes. A dropped database does not un-drop itself, so more than one + * server must be answering to the same name — Docker's embedded DNS round-robins + * every container sharing an alias, so a leftover service container from an + * earlier run would produce exactly this. + * + * `pg_postmaster_start_time()` is what settles it and needs no special rights: + * two Postgres instances cannot share one. Differing values across a single run + * are proof outright, where a differing `inet_server_addr` alone could be argued + * to be one container that moved. + * + * Never throws. This is a diagnostic, and a diagnostic that can fail a run it + * was added to explain is worse than no diagnostic. + */ +export async function describeBackend(label: string): Promise { + try { + const { rows } = await testPool.query<{ + addr: string | null; + started: string; + pid: number; + db: string; + }>( + `SELECT inet_server_addr()::text AS addr, + pg_postmaster_start_time()::text AS started, + pg_backend_pid() AS pid, + current_database() AS db` + ); + const row = rows[0]; + if (!row) return `${label}: no row returned`; + return `${label}: db=${row.db} addr=${row.addr ?? 'local'} postmaster_start=${row.started} pid=${row.pid}`; + } catch (err) { + return `${label}: could not be identified (${err instanceof Error ? err.message : String(err)})`; + } +} + export async function assertSchemaPresent(context: string): Promise { const missing = await missingTables(); if (missing.length === 0) return; + // Gathered only on the failure path, which is the one worth paying for, and + // is where #154 has repeatedly lacked the one fact that would identify it. + const backend = await describeBackend('Answering server'); + throw new Error( `The test database has no schema (${context}). ` + `Missing ${missing.length} of ${REQUIRED_TABLES.length} tables: ${missing.join(', ')}. +` + + `${backend} + ` + `Migrations ran at the start of this run, so the schema existed and has since gone. ` + - `Nothing in this suite drops tables — TRUNCATE does not — so the database itself was ` + - `replaced or restarted underneath the run. On CI the likeliest cause is the Postgres ` + - `service container being recreated, which comes back with an empty data directory. ` + + `Nothing in this suite drops tables — TRUNCATE does not. Compare the postmaster start ` + + `time above against the one globalSetup logged: if they differ, this is a different ` + + `Postgres instance answering to the same name rather than one database losing its ` + + `schema, and the schema was never lost at all. ` + `See #154.` ); } diff --git a/scripts/cleanup-workflow-runs.js b/scripts/cleanup-workflow-runs.js new file mode 100644 index 0000000..b58b228 --- /dev/null +++ b/scripts/cleanup-workflow-runs.js @@ -0,0 +1,176 @@ +/** + * Deletes completed Actions runs older than a given age. + * + * Gitea 1.27.3 has retention for a run's logs (`LOG_RETENTION_DAYS`) and its + * artifacts, but none for the run record itself — so the Actions list grows + * without limit while the entries on it become empty shells once their logs + * expire. This removes the shells. See #324. + * + * Dry run unless APPLY is exactly "true". Deleting a run cannot be undone and + * there is no confirmation step once this starts, so the default has to be the + * harmless one. + * + * Environment: + * GITEA_HOST origin of the instance, e.g. https://gitea.example.com + * GITEA_REPO "owner/name" + * GITEA_ACCESS_TOKEN token permitted to delete runs + * KEEP_DAYS keep runs newer than this many days (default 7) + * APPLY "true" to delete; anything else reports only + */ + +const https = require('https'); +const { URL } = require('url'); + +const HOST = process.env.GITEA_HOST; +const REPO = process.env.GITEA_REPO; +const TOKEN = process.env.GITEA_ACCESS_TOKEN; +const KEEP_DAYS = Number(process.env.KEEP_DAYS || 7); +const APPLY = process.env.APPLY === 'true'; + +for (const [name, value] of Object.entries({ GITEA_HOST: HOST, GITEA_REPO: REPO, GITEA_ACCESS_TOKEN: TOKEN })) { + if (!value) { + console.error(`${name} is required`); + process.exit(1); + } +} +if (!Number.isFinite(KEEP_DAYS) || KEEP_DAYS < 0) { + console.error(`KEEP_DAYS must be a non-negative number, got ${process.env.KEEP_DAYS}`); + process.exit(1); +} + +const origin = new URL(HOST); +const BASE = `/api/v1/repos/${REPO}/actions/runs`; + +function call(method, path) { + return new Promise((resolve, reject) => { + const req = https.request( + { hostname: origin.hostname, port: origin.port || 443, path, method, headers: { Authorization: `token ${TOKEN}` } }, + (res) => { + let body = ''; + res.on('data', (d) => (body += d)); + res.on('end', () => resolve({ status: res.statusCode, body })); + } + ); + req.on('error', reject); + req.end(); + }); +} + +async function listAllRuns() { + const runs = []; + // Bounded rather than `while (true)`: a paging bug against an API that keeps + // answering would otherwise loop until the job times out. + for (let page = 1; page <= 200; page++) { + const res = await call('GET', `${BASE}?page=${page}&limit=50`); + if (res.status >= 400) throw new Error(`listing runs failed: ${res.status} ${res.body.slice(0, 200)}`); + const batch = JSON.parse(res.body).workflow_runs || []; + if (batch.length === 0) break; + runs.push(...batch); + if (batch.length < 50) break; + } + return runs; +} + +const EPOCH_GUARD = new Date('2000-01-01').getTime(); + +/** + * When a run happened, from whichever timestamp it actually has. + * + * `started_at` is preferred and is usually right, but a run **cancelled before + * it ever started** reports an epoch value there while carrying a real + * `completed_at`. Reading only `started_at` therefore made every cancelled run + * look undateable, and a first pass at this left 19 of them — from three weeks + * earlier — sitting in a list that was supposed to hold seven days. + * + * Returns null when neither timestamp is usable, which is the case that must + * stay conservative: an epoch date treated as "1969, therefore old" would + * delete precisely the runs that have not happened yet. + */ +function runTimeMs(run) { + for (const stamp of [run.started_at, run.completed_at]) { + const ms = stamp ? new Date(stamp).getTime() : NaN; + if (Number.isFinite(ms) && ms >= EPOCH_GUARD) return ms; + } + return null; +} + +/** + * Runs old enough to remove, and the reasons the others were left. + * + * A run that is not completed is never a candidate — which is also what stops + * this deleting the very run it is executing in. + */ +function selectDoomed(runs, cutoffMs) { + const doomed = []; + const kept = { recent: 0, unfinished: 0, undated: 0 }; + + for (const run of runs) { + if (run.status !== 'completed') { + kept.unfinished++; + continue; + } + const ms = runTimeMs(run); + if (ms === null) { + kept.undated++; + continue; + } + if (ms >= cutoffMs) { + kept.recent++; + continue; + } + doomed.push({ id: run.id, startedAt: run.started_at, when: new Date(ms).toISOString() }); + } + + doomed.sort((a, b) => a.id - b.id); + return { doomed, kept }; +} + +(async () => { + const runs = await listAllRuns(); + const cutoffMs = Date.now() - KEEP_DAYS * 86400000; + const { doomed, kept } = selectDoomed(runs, cutoffMs); + + console.log(`repository : ${REPO}`); + console.log(`total runs : ${runs.length}`); + console.log(`keeping : ${kept.recent} newer than ${KEEP_DAYS}d, ${kept.unfinished} unfinished, ${kept.undated} undated`); + console.log(`to delete : ${doomed.length}`); + if (doomed.length > 0) { + console.log(` oldest : run ${doomed[0].id} (${doomed[0].when})`); + console.log(` newest : run ${doomed[doomed.length - 1].id} (${doomed[doomed.length - 1].when})`); + } + console.log(''); + + if (!APPLY) { + console.log('DRY RUN — nothing deleted. Re-run with apply set to "true".'); + return; + } + if (doomed.length === 0) { + console.log('Nothing to delete.'); + return; + } + + let deleted = 0; + const failures = []; + for (const run of doomed) { + const res = await call('DELETE', `${BASE}/${run.id}`); + if (res.status >= 200 && res.status < 300) deleted++; + else failures.push(`${run.id}:${res.status}`); + + const done = deleted + failures.length; + if (done % 50 === 0) console.log(` ...${done}/${doomed.length}`); + } + + console.log(''); + console.log(`deleted : ${deleted}`); + console.log(`failed : ${failures.length}`); + if (failures.length > 0) { + console.log(` ${failures.slice(0, 20).join(' ')}${failures.length > 20 ? ' …' : ''}`); + // A partial delete is not a success. Most likely the token cannot delete + // runs, and reporting green here would hide that behind a job that + // appeared to work. + process.exitCode = 1; + } +})().catch((err) => { + console.error(`FAILED: ${err.message}`); + process.exit(1); +});