also looking at this
fix(db): name a dump once, so the file written is the file reported
#186
2 files
+37
-2
| @@ -119,7 +119,7 @@ describe('buildListScript + parseDbList', () => { | ||
| 119 | 119 | it('builds a per-database dump script (mysqldump / pg_dump) to a timestamped file', () => { |
| 120 | 120 | const my = buildBackupScript('mysql', 'acme').join('\n') |
| 121 | 121 | expect(my).toContain('mysqldump --socket=') |
| 122 | expect(my).toContain('acme-$(date +%Y%m%d-%H%M%S).sql.gz') | |
| 122 | expect(my).toContain('acme-$TS_CLOUD_BACKUP_STAMP.sql.gz') | |
| 123 | 123 | // Local pantry engine: socket (no -h) — TCP loopback demands md5. |
| 124 | 124 | const pg = buildBackupScript('postgres', 'acme').join('\n') |
| 125 | 125 | expect(pg).toContain('pg_dump -p 5432 -U postgres acme') |
| @@ -135,6 +135,32 @@ describe('buildListScript + parseDbList', () => { | ||
| 135 | 135 | expect(ext).toContain(`PGPASSWORD='pw' pg_dump -h db.example.com -p 5432 -U admin -w acme`) |
| 136 | 136 | }) |
| 137 | 137 | |
| 138 | /** | |
| 139 | * The timestamp has to be taken ONCE. Written inline as `$(date …)` it runs | |
| 140 | * again in every command that mentions the filename, so a dump crossing a | |
| 141 | * second boundary writes one file and then reports — and lists — a different | |
| 142 | * one that does not exist: a backup that succeeds and names a path nobody can | |
| 143 | * find. Caught on CI by the docker end-to-end test, which is slower than a | |
| 144 | * laptop and so actually crossed the boundary. | |
| 145 | */ | |
| 146 | it('names the dump once, so the file it writes is the file it reports', () => { | |
| 147 | for (const engine of ['postgres', 'mysql', 'mariadb'] as const) { | |
| 148 | const script = buildBackupScript(engine, 'acme') | |
| 149 | expect(script.filter(line => line.includes('$(date'))).toHaveLength(1) | |
| 150 | expect(script.some(line => line.startsWith('TS_CLOUD_BACKUP_STAMP='))).toBe(true) | |
| 151 | ||
| 152 | // The write, the report and the listing must all name the same thing. | |
| 153 | const named = script.filter(line => line.includes('acme-')) | |
| 154 | expect(named).toHaveLength(3) | |
| 155 | for (const line of named) expect(line).toContain('acme-$TS_CLOUD_BACKUP_STAMP.sql.gz') | |
| 156 | ||
| 157 | // And the stamp is set before anything uses it. | |
| 158 | expect(script.findIndex(l => l.startsWith('TS_CLOUD_BACKUP_STAMP='))) | |
| 159 | .toBeLessThan(script.findIndex(l => l.includes('acme-$TS_CLOUD_BACKUP_STAMP'))) | |
| 160 | } | |
| 161 | }) | |
| 162 | ||
| 163 | ||
| 138 | 164 | it('parses BACKUP= lines into database + file, deriving the db from the filename', () => { |
| 139 | 165 | const parsed = parseBackups( |
| 140 | 166 | 'BACKUP=/var/backups/ts-cloud/databases/acme-20260702-101500.sql.gz\nnoise\nBACKUP=/var/backups/ts-cloud/databases/blog-20260701-090000.sql.gz', |
| @@ -440,11 +440,19 @@ export function buildBackupScript( | ||
| 440 | 440 | destDir: string = DB_BACKUP_DIR, |
| 441 | 441 | database?: DatabaseConfig, |
| 442 | 442 | ): string[] { |
| 443 | const file = `${destDir}/${name}-$(date +%Y%m%d-%H%M%S).sql.gz` | |
| 443 | // The timestamp is taken ONCE into a variable rather than embedded as a | |
| 444 | // command substitution. Written inline, `$(date …)` runs again in every | |
| 445 | // command that mentions the filename — so a dump that happens to cross a | |
| 446 | // second boundary writes one file and then reports, and lists, a different | |
| 447 | // one that does not exist. The backup succeeds and names a path nobody can | |
| 448 | // find, which is the worst way for a backup to fail. | |
| 449 | const stamp = 'TS_CLOUD_BACKUP_STAMP="$(date +%Y%m%d-%H%M%S)"' | |
| 450 | const file = `${destDir}/${name}-$TS_CLOUD_BACKUP_STAMP.sql.gz` | |
| 444 | 451 | const mkdir = `mkdir -p ${destDir}` |
| 445 | 452 | if (engine === 'postgres') |
| 446 | 453 | return [ |
| 447 | 454 | mkdir, |
| 455 | stamp, | |
| 448 | 456 | `${pgAdminCommand(database, 'pg_dump')} ${name} | gzip > "${file}"`, |
| 449 | 457 | `echo "BACKUP=${file}"`, |
| 450 | 458 | `ls -l "${file}"`, |
| @@ -452,6 +460,7 @@ export function buildBackupScript( | ||
| 452 | 460 | const sock = engine === 'mariadb' ? SOCKETS.mariadb : SOCKETS.mysql |
| 453 | 461 | return [ |
| 454 | 462 | mkdir, |
| 463 | stamp, | |
| 455 | 464 | `mysqldump --socket=${sock} -u root ${name} | gzip > "${file}"`, |
| 456 | 465 | `echo "BACKUP=${file}"`, |
| 457 | 466 | `ls -l "${file}"`, |