diff --git a/@xen-orchestra/backup-archive/src/BackupCache.mts b/@xen-orchestra/backup-archive/src/BackupCache.mts new file mode 100644 index 0000000000..9f921434ef --- /dev/null +++ b/@xen-orchestra/backup-archive/src/BackupCache.mts @@ -0,0 +1,49 @@ +import { RemoteHandlerAbstract } from '@xen-orchestra/fs' +import { promisify } from 'node:util' +import zlib from 'node:zlib' + +const gzip = promisify(zlib.gzip) +const gunzip = promisify(zlib.gunzip) + +type LogWarn = (message: any, opts?: object) => void + +/** + * Reads a gzip-compressed JSON cache file, tolerant of a missing or corrupt file. + * Shared with the legacy @xen-orchestra/backups RemoteAdapter. + */ +export async function readBackupCache( + handler: RemoteHandlerAbstract, + path: string, + logWarn: LogWarn +): Promise | undefined> { + try { + return JSON.parse((await gunzip(await handler.readFile(path))).toString()) + } catch (error) { + if (error?.code !== 'ENOENT') { + logWarn('failed to read cache', { error, path }) + } + } +} + +/** + * Writes data as a gzip-compressed JSON cache file. + * Shared with the legacy @xen-orchestra/backups RemoteAdapter. + * + * No-op on immutable remotes: the cache file may not be modifiable there, + * and callers should be regenerating/reading the data directly instead. + */ +export async function writeBackupCache( + handler: RemoteHandlerAbstract, + path: string, + data: Record, + logWarn: LogWarn +): Promise { + if (handler.isImmutable()) { + return + } + try { + await handler.writeFile(path, await gzip(JSON.stringify(data)), { flags: 'w' }) + } catch (error) { + logWarn('failed to write cache', { error, path }) + } +} diff --git a/@xen-orchestra/backup-archive/src/MetadataBackup.mts b/@xen-orchestra/backup-archive/src/MetadataBackup.mts new file mode 100644 index 0000000000..8579ab0ca8 --- /dev/null +++ b/@xen-orchestra/backup-archive/src/MetadataBackup.mts @@ -0,0 +1,16 @@ +import { RemoteHandlerAbstract } from '@xen-orchestra/fs' + +const METADATA_BACKUP_ID_REGEXP = + /^xo-(config|pool-metadata)-backups\/\w{8}(-\w{4}){3}-\w{12}(\/\w{8}(-\w{4}){3}-\w{12})?\/\d{8}T\d{6}Z/ + +/** + * Deletes an XO config or pool metadata backup directory (not a VM backup). + * Shared with the legacy @xen-orchestra/backups RemoteAdapter. + */ +export async function deleteMetadataBackup(handler: RemoteHandlerAbstract, backupId: string): Promise { + if (!METADATA_BACKUP_ID_REGEXP.test(backupId)) { + throw new Error(`The id (${backupId}) not correspond to a metadata folder`) + } + + await handler.rmtree(backupId) +} diff --git a/@xen-orchestra/backup-archive/src/VmBackupDirectory.mts b/@xen-orchestra/backup-archive/src/VmBackupDirectory.mts index 3af7663c7f..b9edb9177c 100644 --- a/@xen-orchestra/backup-archive/src/VmBackupDirectory.mts +++ b/@xen-orchestra/backup-archive/src/VmBackupDirectory.mts @@ -17,11 +17,8 @@ import { import { cleanOrphanDiskDirs } from '@xen-orchestra/backup-archive/disks' import { asyncEach } from '@vates/async-each' import { createLogger } from '@xen-orchestra/log' -import { promisify } from 'node:util' -import zlib from 'node:zlib' - -const gzip = promisify(zlib.gzip) -const gunzip = promisify(zlib.gunzip) +import { readBackupCache, writeBackupCache } from './BackupCache.mjs' +import { unlinkTolerant } from './_unlinkTolerant.mjs' const { info: logInfo, warn: logWarn } = createLogger('xo:backup-archive') @@ -30,7 +27,7 @@ const FILES_TO_KEEP = ['cache.json.gz', 'vdis'] export class VmBackupDirectory implements VmBackupInterface { handler: RemoteHandlerAbstract rootPath: string - files: Array = new Array() + files: Array = [] orphans: Set = new Set() backupArchives: Map = new Map() opts: ResolvedBackupCleanOptions @@ -43,6 +40,9 @@ export class VmBackupDirectory implements VmBackupInterface { // on disk, so clean() must regenerate it even when nothing was merged/removed. #cacheOutOfSync = false + // Set by #checkCacheCount(): whether cache.json.gz existed at the start of the run. + #cacheExisted = false + constructor( handler: RemoteHandlerAbstract, vmBackupPath: string, @@ -67,21 +67,11 @@ export class VmBackupDirectory implements VmBackupInterface { } async #readCache(path: string): Promise | undefined> { - try { - return JSON.parse((await gunzip(await this.handler.readFile(path))).toString()) - } catch (error) { - if (error?.code !== 'ENOENT') { - logWarn('failed to read cache', { error, path }) - } - } + return readBackupCache(this.handler, path, logWarn) } async #writeCache(path: string, data: Record): Promise { - try { - await this.handler.writeFile(path, await gzip(JSON.stringify(data)), { flags: 'w' }) - } catch (error) { - logWarn('failed to write cache', { error, path }) - } + return writeBackupCache(this.handler, path, data, logWarn) } async init() { @@ -90,7 +80,7 @@ export class VmBackupDirectory implements VmBackupInterface { this.#uniqueLineages = new Map() for (const fullPath of this.files.filter(path => path.endsWith('.json'))) { - let metadata: PartialBackupMetadata | undefined = undefined + let metadata: PartialBackupMetadata | undefined try { metadata = JSON.parse(await this.handler.readFile(fullPath)) satisfies PartialBackupMetadata } catch (error) { @@ -176,18 +166,28 @@ export class VmBackupDirectory implements VmBackupInterface { // Let each archive clean its own files (e.g. remove metadata for incomplete backups) // and update metadata with merged sizes if applicable + const goneArchives: string[] = [] await asyncEach( - Array.from(this.backupArchives.values()), - async (archive: VmBackupInterface) => { + Array.from(this.backupArchives.entries()), + async ([metadataPath, archive]: [string, VmBackupInterface]) => { const { removedFiles } = await archive.clean({ remove, mergedSizes: allMergedSizes }) if (removedFiles.length > 0) { cacheNeedsRegen = true } + if (removedFiles.includes(metadataPath)) { + goneArchives.push(metadataPath) + } }, { concurrency: 2 } ) - if (allMergedSizes.size > 0 || cacheNeedsRegen || this.#cacheOutOfSync) { + // an archive whose metadata has just been deleted is not a backup anymore: it must not be + // advertised by the regenerated cache + for (const metadataPath of goneArchives) { + this.backupArchives.delete(metadataPath) + } + + if (this.#cacheExisted && (allMergedSizes.size > 0 || cacheNeedsRegen || this.#cacheOutOfSync)) { await this.#regenerateCache() } @@ -215,8 +215,34 @@ export class VmBackupDirectory implements VmBackupInterface { async #checkCacheCount(): Promise { const cachePath = `${this.rootPath}/cache.json.gz` + + this.#cacheExisted = false + this.#cacheOutOfSync = false + + if (!this.files.includes(normalize(cachePath))) { + return + } + + if (this.handler.isImmutable()) { + //Best effort: some remotes (e.g. S3) may not normalize a permission error to EPERM, so tolerate any error. + try { + await this.handler.unlink(cachePath) + } catch (error) { + this.opts.logWarn('error while deleting leftover backup cache', { path: cachePath, error }) + } + return + } + const existingCache = await this.#readCache(cachePath) - const actual = existingCache === undefined ? 0 : Object.keys(existingCache).length + if (existingCache === undefined) { + // present but unreadable: drop it, RemoteAdapter will recreate a valid one + this.opts.logWarn('removing unreadable backup cache', { path: cachePath }) + await unlinkTolerant(this.handler, cachePath) + return + } + + this.#cacheExisted = true + const actual = Object.keys(existingCache).length const expected = this.backupArchives.size this.#cacheOutOfSync = actual !== expected if (this.#cacheOutOfSync) { diff --git a/@xen-orchestra/backup-archive/src/VmFullBackupArchive.integ.mjs b/@xen-orchestra/backup-archive/src/VmFullBackupArchive.integ.mjs index e26ff66dc9..e6cc57d3e1 100644 --- a/@xen-orchestra/backup-archive/src/VmFullBackupArchive.integ.mjs +++ b/@xen-orchestra/backup-archive/src/VmFullBackupArchive.integ.mjs @@ -10,8 +10,13 @@ import { rimraf } from 'rimraf' // eslint-disable-next-line n/no-missing-import import { VmBackupDirectory } from '../dist/VmBackupDirectory.mjs' import tar from 'tar-stream' +import { promisify } from 'node:util' +import zlib from 'node:zlib' const { beforeEach, afterEach, describe } = test +const gzip = promisify(zlib.gzip) +const gunzip = promisify(zlib.gunzip) + let tempDir, handler, vmBackupDir const vmUuid = 'test-vm-uuid' const rootPath = `xo-vm-backups/${vmUuid}` @@ -124,7 +129,10 @@ describe('VmBackupDirectory with full backups', { concurrency: 1 }, () => { test('clean() preserves cache.json.gz', async () => { await createFullBackupMetadata('backup1.json', 'backup1.xva') - await handler.writeFile(`${rootPath}/cache.json.gz`, 'cache') + await handler.writeFile( + `${rootPath}/cache.json.gz`, + await gzip(JSON.stringify({ [`/${rootPath}/backup1.json`]: {} })) + ) await VmBackupDirectory.cleanVm(handler, rootPath) @@ -133,6 +141,104 @@ describe('VmBackupDirectory with full backups', { concurrency: 1 }, () => { assert.ok(remainingFiles.includes('cache.json.gz')) }) + test('clean() removes an unreadable cache.json.gz', async () => { + await createFullBackupMetadata('backup1.json', 'backup1.xva') + await handler.writeFile(`${rootPath}/cache.json.gz`, 'not gzipped json') + + await VmBackupDirectory.cleanVm(handler, rootPath, { logWarn: () => {} }) + + const remainingFiles = await handler.list(rootPath) + assert.ok(!remainingFiles.includes('cache.json.gz'), 'an unreadable cache.json.gz should be removed, not kept') + }) + + test('clean() on an immutable remote never creates cache.json.gz', async () => { + await createFullBackupMetadata('backup1.json', 'backup1.xva') + await handler.writeFile(`${rootPath}/orphan.xva`, 'orphan-content') + handler.isImmutable = () => true + + await VmBackupDirectory.cleanVm(handler, rootPath, { remove: true }) + + const remainingFiles = await handler.list(rootPath) + assert.ok(!remainingFiles.includes('cache.json.gz'), 'cache.json.gz should never be created on an immutable remote') + }) + + test('clean() removes a leftover cache.json.gz found on an immutable remote', async () => { + await createFullBackupMetadata('backup1.json', 'backup1.xva') + await handler.writeFile(`${rootPath}/cache.json.gz`, await gzip(JSON.stringify({}))) + handler.isImmutable = () => true + + await VmBackupDirectory.cleanVm(handler, rootPath) + + const remainingFiles = await handler.list(rootPath) + assert.ok(!remainingFiles.includes('cache.json.gz'), 'erroneous cache.json.gz should be removed, not rewritten') + }) + + test('clean() removes an unreadable leftover cache.json.gz found on an immutable remote', async () => { + await createFullBackupMetadata('backup1.json', 'backup1.xva') + await handler.writeFile(`${rootPath}/cache.json.gz`, 'not gzipped json') + handler.isImmutable = () => true + + await VmBackupDirectory.cleanVm(handler, rootPath) + + const remainingFiles = await handler.list(rootPath) + assert.ok( + !remainingFiles.includes('cache.json.gz'), + 'an unreadable cache.json.gz should be removed from an immutable remote' + ) + }) + + test('clean() tolerates an EPERM while removing the cache of an immutable remote', async () => { + await createFullBackupMetadata('backup1.json', 'backup1.xva') + await handler.writeFile(`${rootPath}/cache.json.gz`, await gzip(JSON.stringify({}))) + handler.isImmutable = () => true + handler.unlink = () => { + const error = new Error('EPERM: operation not permitted') + error.code = 'EPERM' + throw error + } + + await VmBackupDirectory.cleanVm(handler, rootPath) + + const remainingFiles = await handler.list(rootPath) + assert.ok(remainingFiles.includes('cache.json.gz'), 'the immutable cache.json.gz could not be removed') + }) + + test('clean() keeps maintaining a pre-existing cache.json.gz across a remove', async () => { + await createFullBackupMetadata('backup1.json', 'backup1.xva') + await createFullBackupMetadata('backup2.json', 'backup2.xva') + + // a cache with the expected number of entries: only the removal can trigger the regeneration + const cachePath = `${rootPath}/cache.json.gz` + await handler.writeFile( + cachePath, + await gzip( + JSON.stringify({ + [`/${rootPath}/backup1.json`]: { stale: true }, + [`/${rootPath}/backup2.json`]: { stale: true }, + }) + ) + ) + + // backup2 loses its XVA: its metadata will be removed by clean() + await handler.unlink(`${rootPath}/backup2.xva`) + + await VmBackupDirectory.cleanVm(handler, rootPath, { remove: true, logInfo: () => {}, logWarn: () => {} }) + + const remainingFiles = await handler.list(rootPath) + assert.ok(remainingFiles.includes('backup1.json')) + assert.ok(remainingFiles.includes('backup1.xva')) + assert.ok(!remainingFiles.includes('backup2.json'), 'metadata of the backup with a missing XVA should be removed') + assert.ok(remainingFiles.includes('cache.json.gz'), 'an existing cache.json.gz must not be dropped by a remove') + + const cache = JSON.parse((await gunzip(await handler.readFile(cachePath))).toString()) + assert.deepEqual( + Object.keys(cache), + [`/${rootPath}/backup1.json`], + 'cache.json.gz should have been regenerated with the surviving backup only' + ) + assert.equal(cache[`/${rootPath}/backup1.json`].stale, undefined, 'the entry must come from the archive on disk') + }) + test('clean() removes an orphan XVA checksum file (no matching metadata or XVA)', async () => { await handler.writeFile(`${rootPath}/stray.xva.checksum`, 'sha256:deadbeef') diff --git a/@xen-orchestra/backup-archive/src/VmFullBackupArchive.mts b/@xen-orchestra/backup-archive/src/VmFullBackupArchive.mts index 99a384d63b..858c8bc964 100644 --- a/@xen-orchestra/backup-archive/src/VmFullBackupArchive.mts +++ b/@xen-orchestra/backup-archive/src/VmFullBackupArchive.mts @@ -1,6 +1,6 @@ -import { basename, normalize } from '@xen-orchestra/fs/path' +import { basename, normalize, resolveFromFile } from '@xen-orchestra/fs/path' import assert from 'node:assert' -import { FileDescriptor } from '@xen-orchestra/fs' +import { FileDescriptor, RemoteHandlerAbstract } from '@xen-orchestra/fs' import { ArchiveCleanOptions, CheckResult, @@ -8,8 +8,10 @@ import { VmBackupInterface, PartialBackupMetadata, ResolvedBackupCleanOptions, + DEFAULT_REMOVE_CONCURRENCY, } from './VmBackup.types.mjs' -import { RemoteHandlerAbstract } from '@xen-orchestra/fs' +import { asyncEach } from '@vates/async-each' +import { unlinkTolerant } from './_unlinkTolerant.mjs' const COMPRESSED_MAGIC_NUMBERS: Buffer[] = [ // https://tools.ietf.org/html/rfc1952.html#page-5 @@ -80,6 +82,29 @@ export async function isValidXva( const noop = (): void => {} +/** + * Deletes a full VM backup's metadata json and, when provided, its xva and checksum. + * `xva` is the path as stored in the metadata, i.e. relative to the metadata file. + * Shared with the legacy @xen-orchestra/backups RemoteAdapter. + */ +export async function deleteFullVmBackups( + handler: RemoteHandlerAbstract, + backups: Array<{ metadataPath: string; xva?: string }> +): Promise { + await asyncEach( + backups, + async ({ metadataPath, xva }) => { + await unlinkTolerant(handler, metadataPath) + if (xva !== undefined) { + const xvaPath = resolveFromFile(metadataPath, xva) + await unlinkTolerant(handler, xvaPath) + await unlinkTolerant(handler, `${xvaPath}.checksum`) + } + }, + { concurrency: DEFAULT_REMOVE_CONCURRENCY, stopOnError: false } + ) +} + export class VmFullBackupArchive implements VmBackupInterface { handler: RemoteHandlerAbstract rootPath: string @@ -158,12 +183,10 @@ export class VmFullBackupArchive implements VmBackupInterface { files: this.getAssociatedFiles({ prefix: false }), }) } else { - for (const file of removedFiles) { - try { - await this.handler.unlink(file) - } catch (error) { - this.opts.logWarn(`Issue removing ${file}`, { error }) - } + try { + await deleteFullVmBackups(this.handler, [{ metadataPath: this.metadataPath, xva: this.metadata.xva }]) + } catch (error) { + this.opts.logWarn(`Issue removing backup files`, { error, metadataPath: this.metadataPath }) } } } @@ -173,7 +196,7 @@ export class VmFullBackupArchive implements VmBackupInterface { } getAssociatedFiles({ prefix = false }): Array { - let validFiles = [this.metadataPath, this.xvaPath, `${this.xvaPath}.checksum`] + const validFiles = [this.metadataPath, this.xvaPath, `${this.xvaPath}.checksum`] return prefix ? validFiles : validFiles.map(file => basename(file)) } } diff --git a/@xen-orchestra/backup-archive/src/VmIncrementalBackupArchive.integ.mjs b/@xen-orchestra/backup-archive/src/VmIncrementalBackupArchive.integ.mjs index e69d7f740b..69bb0b741e 100644 --- a/@xen-orchestra/backup-archive/src/VmIncrementalBackupArchive.integ.mjs +++ b/@xen-orchestra/backup-archive/src/VmIncrementalBackupArchive.integ.mjs @@ -6,10 +6,13 @@ import fs from 'fs-extra' import * as uuid from 'uuid' import { getHandler } from '@xen-orchestra/fs' import { pFromCallback } from 'promise-toolbox' -// eslint-disable-next-line n/no-missing-import +// `dist/` is built by the `test-integration` script, which starts by removing it: these imports +// cannot be resolved when linting a working tree that has not been built yet +/* eslint-disable n/no-missing-import */ import { VmBackupDirectory } from '../dist/VmBackupDirectory.mjs' import { RemoteVhdDisk } from '../dist/disks/RemoteVhdDisk.mjs' import { MergeRemoteDisk } from '../dist/disks/MergeRemoteDisk.mjs' +/* eslint-enable n/no-missing-import */ import { VHDFOOTER, VHDHEADER } from './tests.fixtures.mjs' import { VhdFile, Constants, VhdDirectory, VhdAbstract } from 'vhd-lib' import { dirname, basename } from 'node:path' @@ -20,6 +23,7 @@ import { rimraf } from 'rimraf' const { beforeEach, afterEach, describe } = test const gunzip = promisify(zlib.gunzip) +const gzip = promisify(zlib.gzip) let tempDir, handler, jobId, vdiId, basePath, relativePath, vdiId2, basePath2, relativePath2 const rootPath = 'xo-vm-backups/VMUUID/' @@ -205,8 +209,8 @@ test('it remove backup metadata referencing a missing vhd in delta backup', asyn assert.equal(matched.length, 2) // all vhds (orphan and child ) should have been deleted assert.ok( - (await handler.list(rootPath)).includes('cache.json.gz'), - 'cache.json.gz should be regenerated after metadata deletion' + !(await handler.list(rootPath)).includes('cache.json.gz'), + 'cache.json.gz should not be created when none existed before the clean' ) }) @@ -364,7 +368,6 @@ test('it merges delta of non destroyed chain', async () => { logged.push(message) } await VmBackupDirectory.cleanVm(handler, rootPath, { remove: true, logInfo, logWarn: logInfo }) - assert.equal(logged[0], `unexpected number of entries in backup cache`) logged = [] await VmBackupDirectory.cleanVm(handler, rootPath, { remove: true, merge: true, logInfo, logWarn: () => {} }) @@ -726,10 +729,10 @@ describe('tests multiple combination ', { concurrency: 1 }, () => { const metadata = JSON.parse(await handler.readFile(`${rootPath}/metadata.json`)) // size should be the size of children + grand children + clean after the merge assert.deepEqual(metadata.size, 6501888) - // cache.json.gz must be regenerated when metadata changes after a merge + // no cache.json.gz existed before this run, so a merge must not create one assert.ok( - (await handler.list(rootPath)).includes('cache.json.gz'), - 'cache.json.gz should be regenerated after a merge' + !(await handler.list(rootPath)).includes('cache.json.gz'), + 'cache.json.gz should not be created when none existed before the merge' ) // broken vhd, non referenced, abandoned should be deleted ( alias and data) @@ -833,7 +836,7 @@ test('it preserves VDI directory when handler.list() throws during lineage init' assert.ok(remaining.includes('snapshot.vhd'), 'VHD must survive when handler.list() throws during lineage init') }) -test('it regenerates the cache when its entry count is out of sync, even with nothing to merge/remove', async () => { +test('it does not create nor warn about the cache when none existed, even with a count mismatch', async () => { // a referenced disk + its metadata, but no cache.json.gz on disk await generateVhd(`${basePath}/diskA1.vhd`) await handler.writeFile( @@ -842,6 +845,78 @@ test('it regenerates the cache when its entry count is out of sync, even with no ) await assert.rejects(handler.readFile(`${rootPath}/cache.json.gz`), { code: 'ENOENT' }) + const logged = [] + // remove/merge both false: cacheNeedsRegen stays false and no merge happens, + // so only a pre-existing cache's count mismatch could trigger regeneration + await VmBackupDirectory.cleanVm(handler, rootPath, { + remove: false, + merge: false, + logInfo: () => {}, + logWarn: message => logged.push(message), + }) + + assert.ok(!logged.includes('unexpected number of entries in backup cache'), 'no cache to be mismatched, no warning') + await assert.rejects(handler.readFile(`${rootPath}/cache.json.gz`), { code: 'ENOENT' }) +}) + +test('it regenerates a pre-existing cache after a merge', async () => { + await handler.writeFile( + `${rootPath}/metadata.json`, + JSON.stringify({ + mode: 'delta', + size: 12000, // a size too small, the merge will fix it + vhds: [`${relativePath}/grandchild.vhd`, `${relativePath}/child.vhd`], + }) + ) + + // one orphan, which is a full vhd, no parent + const orphan = await generateVhd(`${basePath}/orphan.vhd`) + // a child to the orphan, orphan will be merged into it + const child = await generateVhd(`${basePath}/child.vhd`, { + header: { + parentUnicodeName: 'orphan.vhd', + parentUuid: orphan.footer.uuid, + }, + blocks: [0, 1], + }) + await generateVhd(`${basePath}/grandchild.vhd`, { + header: { + parentUnicodeName: 'child.vhd', + parentUuid: child.footer.uuid, + }, + }) + + // a cache with the expected number of entries: it is not out of sync, only the merge can trigger the regeneration + await handler.writeFile( + `${rootPath}/cache.json.gz`, + await gzip(JSON.stringify({ [`/${rootPath}metadata.json`]: { size: 12000 } })) + ) + + await VmBackupDirectory.cleanVm(handler, rootPath, { + remove: true, + merge: true, + logInfo: () => {}, + logWarn: () => {}, + }) + + const metadata = JSON.parse(await handler.readFile(`${rootPath}/metadata.json`)) + // size of child + grandchild after orphan has been merged into child + assert.equal(metadata.size, 4299776, 'metadata.size should be updated after merge') + + const cache = JSON.parse((await gunzip(await handler.readFile(`${rootPath}/cache.json.gz`))).toString()) + assert.equal(Object.keys(cache).length, 1) + assert.equal(Object.values(cache)[0].size, metadata.size, 'cache entry should carry the post-merge size') +}) + +test('it regenerates a pre-existing cache when its entry count is out of sync, even with nothing to merge/remove', async () => { + // a referenced disk + its metadata, and a pre-existing cache with the wrong entry count + await generateVhd(`${basePath}/diskA1.vhd`) + await handler.writeFile( + `${rootPath}/metadata.json`, + JSON.stringify({ mode: 'delta', vhds: [`${relativePath}/diskA1.vhd`] }) + ) + await handler.writeFile(`${rootPath}/cache.json.gz`, await gzip(JSON.stringify({}))) + const logged = [] // remove/merge both false: cacheNeedsRegen stays false and no merge happens, // so only the cache-count mismatch can trigger the regeneration @@ -857,3 +932,53 @@ test('it regenerates the cache when its entry count is out of sync, even with no const cache = JSON.parse((await gunzip(await handler.readFile(`${rootPath}/cache.json.gz`))).toString()) assert.equal(Object.keys(cache).length, 1, 'cache should be regenerated to match the archives on disk') }) + +test('it regenerates a pre-existing cache after a metadata removal', async () => { + // two backups, each on its own VDI directory, so removing one does not touch the other's disks + await generateVhd(`${basePath}/diskA1.vhd`) + await handler.writeFile( + `${rootPath}/metadata1.json`, + JSON.stringify({ mode: 'delta', vhds: [`${relativePath}/diskA1.vhd`] }) + ) + // metadata2 references a vhd missing from disk: clean() removes it + await handler.writeFile( + `${rootPath}/metadata2.json`, + JSON.stringify({ mode: 'delta', vhds: [`${relativePath2}/deleted.vhd`] }) + ) + + // a cache with the expected number of entries: only the removal can trigger the regeneration + await handler.writeFile( + `${rootPath}/cache.json.gz`, + await gzip( + JSON.stringify({ + [`/${rootPath}metadata1.json`]: { stale: true }, + [`/${rootPath}metadata2.json`]: { stale: true }, + }) + ) + ) + + const logged = [] + await VmBackupDirectory.cleanVm(handler, rootPath, { + remove: true, + merge: false, + logInfo: () => {}, + logWarn: message => logged.push(message), + }) + + const rootFiles = await handler.list(rootPath) + assert.ok(rootFiles.includes('metadata1.json'), 'the complete backup must survive') + assert.ok(!rootFiles.includes('metadata2.json'), 'the backup with a missing vhd must be removed') + assert.ok(rootFiles.includes('cache.json.gz'), 'an existing cache.json.gz must not be dropped by a remove') + assert.ok( + !logged.includes('unexpected number of entries in backup cache'), + 'the cache matched the archives before the clean: the removal alone must trigger the regeneration' + ) + + const cache = JSON.parse((await gunzip(await handler.readFile(`${rootPath}/cache.json.gz`))).toString()) + assert.deepEqual( + Object.keys(cache), + [`/${rootPath}metadata1.json`], + 'the removed backup must be gone from the regenerated cache' + ) + assert.equal(cache[`/${rootPath}metadata1.json`].stale, undefined, 'the entry must come from the archive on disk') +}) diff --git a/@xen-orchestra/backup-archive/src/VmIncrementalBackupArchive.mts b/@xen-orchestra/backup-archive/src/VmIncrementalBackupArchive.mts index c115db4cd9..029014cc87 100644 --- a/@xen-orchestra/backup-archive/src/VmIncrementalBackupArchive.mts +++ b/@xen-orchestra/backup-archive/src/VmIncrementalBackupArchive.mts @@ -5,10 +5,27 @@ import { ResolvedBackupCleanOptions, VmBackupInterface, PartialBackupMetadata, + DEFAULT_REMOVE_CONCURRENCY, } from './VmBackup.types.mjs' import { RemoteHandlerAbstract } from '@xen-orchestra/fs' import { basename, dirname, normalize } from '@xen-orchestra/fs/path' import { RemoteDiskLineage } from './RemoteDiskLineage.mjs' +import { asyncEach } from '@vates/async-each' + +/** + * Deletes a delta VM backup's metadata json. Unused VHDs are detected and removed + * separately by VmBackupDirectory's clean(). Shared with the legacy @xen-orchestra/backups + * RemoteAdapter. + */ +export async function deleteDeltaVmBackups( + handler: RemoteHandlerAbstract, + backups: Array<{ metadataPath: string }> +): Promise { + await asyncEach(backups, ({ metadataPath }) => handler.unlink(metadataPath), { + concurrency: DEFAULT_REMOVE_CONCURRENCY, + stopOnError: false, + }) +} export class VmIncrementalBackupArchive implements VmBackupInterface { handler: RemoteHandlerAbstract diff --git a/@xen-orchestra/backup-archive/src/_unlinkTolerant.mts b/@xen-orchestra/backup-archive/src/_unlinkTolerant.mts new file mode 100644 index 0000000000..4e98e1024a --- /dev/null +++ b/@xen-orchestra/backup-archive/src/_unlinkTolerant.mts @@ -0,0 +1,20 @@ +import { RemoteHandlerAbstract } from '@xen-orchestra/fs' + +/** + * Deletes a file, ignoring the given error codes. + * By default only a missing file is tolerated. + */ +export async function unlinkTolerant( + handler: RemoteHandlerAbstract, + path: string, + ignoredCodes: string[] = ['ENOENT'] +): Promise { + try { + await handler.unlink(path) + } catch (error) { + if (!ignoredCodes.includes(error?.code)) { + error.path ??= path + throw error + } + } +} diff --git a/@xen-orchestra/backup-archive/src/index.mts b/@xen-orchestra/backup-archive/src/index.mts index f43b526339..818ec03b5d 100644 --- a/@xen-orchestra/backup-archive/src/index.mts +++ b/@xen-orchestra/backup-archive/src/index.mts @@ -1 +1,5 @@ export { VmBackupDirectory } from './VmBackupDirectory.mjs' +export { deleteFullVmBackups } from './VmFullBackupArchive.mjs' +export { deleteDeltaVmBackups } from './VmIncrementalBackupArchive.mjs' +export { deleteMetadataBackup } from './MetadataBackup.mjs' +export { readBackupCache, writeBackupCache } from './BackupCache.mjs' diff --git a/@xen-orchestra/backup-archive/tsconfig.json b/@xen-orchestra/backup-archive/tsconfig.json index 6d845fb955..76f6da8921 100644 --- a/@xen-orchestra/backup-archive/tsconfig.json +++ b/@xen-orchestra/backup-archive/tsconfig.json @@ -14,5 +14,6 @@ "types": ["node"] // Only use types from Node.js and packages that are explicitly imported }, "include": ["src/**/*"], - "exclude": ["node_modules", "dist"] + // *.integ.mjs deliberately imports from the built dist/ output to test it + "exclude": ["node_modules", "dist", "**/*.integ.mjs"] } diff --git a/@xen-orchestra/backups/RemoteAdapter.mjs b/@xen-orchestra/backups/RemoteAdapter.mjs index 82b45080f5..517c50fb30 100644 --- a/@xen-orchestra/backups/RemoteAdapter.mjs +++ b/@xen-orchestra/backups/RemoteAdapter.mjs @@ -6,14 +6,19 @@ import { decorateMethodsWith } from '@vates/decorate-with' import { dirname, join, resolve } from 'node:path' import { synchronized } from 'decorator-synchronized' import Disposable from 'promise-toolbox/Disposable' -import fromCallback from 'promise-toolbox/fromCallback' import groupBy from 'lodash/groupBy.js' import pickBy from 'lodash/pickBy.js' import reduce from 'lodash/reduce.js' -import zlib from 'zlib' import { BACKUP_DIR } from './_getVmBackupDir.mjs' -import { VmBackupDirectory } from '@xen-orchestra/backup-archive' +import { + VmBackupDirectory, + deleteFullVmBackups as deleteFullVmBackupFiles, + deleteDeltaVmBackups as deleteDeltaVmBackupFiles, + deleteMetadataBackup as deleteMetadataBackupFiles, + readBackupCache, + writeBackupCache, +} from '@xen-orchestra/backup-archive' import { fileRestoreDecorators, fileRestoreMethods } from './_fileRestore.mjs' import { formatFilenameDate } from './_filenameDate.mjs' import { isMetadataFile } from './_backupType.mjs' @@ -36,8 +41,6 @@ export const compareTimestamp = (a, b) => a.timestamp - b.timestamp const noop = Function.prototype -const resolveRelativeFromFile = (file, path) => resolve('/', dirname(file), path).slice(1) - const createSafeReaddir = (handler, methodName) => (path, options) => handler.list(path, options).catch(error => { if (error?.code !== 'ENOENT') { @@ -100,24 +103,17 @@ export class RemoteAdapter { } async deleteDeltaVmBackups(backups) { - const handler = this._handler - // this will delete the json, unused VHDs will be detected by `cleanVm` - await asyncMapSettled(backups, ({ _filename }) => handler.unlink(_filename)) + await deleteDeltaVmBackupFiles( + this._handler, + backups.map(({ _filename }) => ({ metadataPath: _filename })) + ) await this.#removeVmBackupsFromCache(backups) } async deleteMetadataBackup(backupId) { - const uuidReg = '\\w{8}(-\\w{4}){3}-\\w{12}' - const metadataDirReg = 'xo-(config|pool-metadata)-backups' - const timestampReg = '\\d{8}T\\d{6}Z' - const regexp = new RegExp(`^${metadataDirReg}/${uuidReg}(/${uuidReg})?/${timestampReg}`) - if (!regexp.test(backupId)) { - throw new Error(`The id (${backupId}) not correspond to a metadata folder`) - } - - await this._handler.rmtree(backupId) + await deleteMetadataBackupFiles(this._handler, backupId) } async deleteOldMetadataBackups(dir, retention) { @@ -129,24 +125,10 @@ export class RemoteAdapter { } async deleteFullVmBackups(backups) { - const handler = this._handler - await asyncMapSettled(backups, ({ _filename, xva }) => - Promise.all([ - handler.unlink(_filename).catch(error => { - warn('error while removing full vm backup metadata', { error, filename: _filename }) - if (error.code !== 'ENOENT') throw error - }), - handler.unlink(resolveRelativeFromFile(_filename, xva)).catch(error => { - warn('error while removing full vm backup file', { error, filename: _filename }) - if (error.code !== 'ENOENT') throw error - }), - handler.unlink(resolveRelativeFromFile(_filename, `${xva}.checksum`)).catch(error => { - // checksum can be missing , it's not an issue - if (error.code !== 'ENOENT') throw error - }), - ]) + await deleteFullVmBackupFiles( + this._handler, + backups.map(({ _filename, xva }) => ({ metadataPath: _filename, xva })) ) - await this.#removeVmBackupsFromCache(backups) } @@ -298,13 +280,7 @@ export class RemoteAdapter { } async _readCache(path) { - try { - return JSON.parse(await fromCallback(zlib.gunzip, await this.handler.readFile(path))) - } catch (error) { - if (error.code !== 'ENOENT') { - warn('#readCache', { error, path }) - } - } + return readBackupCache(this.handler, path, warn) } _updateCache = synchronized.withKey()(this._updateCache) @@ -324,11 +300,7 @@ export class RemoteAdapter { } async _writeCache(path, data) { - try { - await this.handler.writeFile(path, await fromCallback(zlib.gzip, JSON.stringify(data)), { flags: 'w' }) - } catch (error) { - warn('#writeCache', { error, path }) - } + return writeBackupCache(this.handler, path, data, warn) } async #getCacheableDataListVmBackups(dir) { diff --git a/@xen-orchestra/backups/_cleanVm.integ.mjs b/@xen-orchestra/backups/_cleanVm.integ.mjs index 5332ab78cf..bfc00f7e17 100644 --- a/@xen-orchestra/backups/_cleanVm.integ.mjs +++ b/@xen-orchestra/backups/_cleanVm.integ.mjs @@ -230,7 +230,6 @@ test('it merges delta of non destroyed chain', async () => { logged.push(message) } await adapter.cleanVm(rootPath, { remove: true, logInfo, logWarn: logInfo, lock: false }) - assert.equal(logged[0], `unexpected number of entries in backup cache`) logged = [] const result = await adapter.cleanVm(rootPath, { remove: true, merge: true, logInfo, logWarn: () => {}, lock: false }) diff --git a/CHANGELOG.unreleased.md b/CHANGELOG.unreleased.md index cf0e196236..85de8fafab 100644 --- a/CHANGELOG.unreleased.md +++ b/CHANGELOG.unreleased.md @@ -21,6 +21,7 @@ - [Web-core] Fix "console offline" illustration sparks color (PR [#10309](https://github.com/vatesfr/xen-orchestra/pull/10309)) - [Web-core] Fix 404 illustration color (PR [#10325](https://github.com/vatesfr/xen-orchestra/pull/10325)) +- [Backup-archive] No longer create a `cache.json.gz` file on immutable/S3 remote during cleanup, which could not be deleted afterwards and stayed billed forever (PR [#10243](https://github.com/vatesfr/xen-orchestra/pull/10243)) ### Packages to release @@ -38,6 +39,8 @@ +- @xen-orchestra/backup-archive patch +- @xen-orchestra/backups patch - @xen-orchestra/web minor - @xen-orchestra/web-core minor