diff --git a/.github/docker/docker-compose.yaml b/.github/docker/docker-compose.yaml index 8880f765f2..a0a586d864 100644 --- a/.github/docker/docker-compose.yaml +++ b/.github/docker/docker-compose.yaml @@ -126,7 +126,7 @@ services: depends_on: - redis metadata-standalone: - image: ghcr.io/scality/metadata:8.25.0-standalone + image: ghcr.io/scality/metadata:9.17.0-standalone profiles: ['metadata-standalone'] network_mode: 'host' volumes: diff --git a/package.json b/package.json index 5fb4b68aea..ca81b36578 100644 --- a/package.json +++ b/package.json @@ -35,7 +35,7 @@ "@opentelemetry/instrumentation-ioredis": "~0.64.0", "@opentelemetry/instrumentation-mongodb": "~0.69.0", "@smithy/node-http-handler": "^3.0.0", - "arsenal": "git+https://github.com/scality/arsenal#8.5.15", + "arsenal": "git+https://github.com/scality/arsenal#8.5.17", "async": "2.6.4", "aws-crt": "^1.24.0", "bucketclient": "scality/bucketclient#8.2.7", diff --git a/tests/functional/backbeat/listLifecyclePHDKeys.js b/tests/functional/backbeat/listLifecyclePHDKeys.js new file mode 100644 index 0000000000..1e096708b7 --- /dev/null +++ b/tests/functional/backbeat/listLifecyclePHDKeys.js @@ -0,0 +1,495 @@ +const assert = require("assert"); +const async = require("async"); +const crypto = require("crypto"); +const { + CreateBucketCommand, + DeleteBucketCommand, + DeleteObjectCommand, + HeadObjectCommand, + PutBucketVersioningCommand, + PutObjectCommand, +} = require("@aws-sdk/client-s3"); +const BucketUtility = require("../aws-node-sdk/lib/utility/bucket-util"); +const { + removeAllVersions, +} = require("../aws-node-sdk/lib/utility/versioning-util"); +const { makeBackbeatRequest } = require("./utils"); +const { promisify } = require("util"); + +/** + * Integration coverage for lifecycle listings over PHD master keys (ARSN-620). + * + * Metadata writes a PHD master when you delete the current version of an object + * by version id. A run of PHD masters longer than + * max-scanned-lifecycle-listing-entries is a "desert". A desert used to truncate + * the orphan and noncurrent lifecycle listings with no resume marker, so backbeat + * requeued the same listing from scratch forever. + * + * The listing algorithm itself (handlePHDMaster: marker placement, candidate + * flushing, surviving-version protection, exact cap landing on a PHD) is + * exhaustively unit-tested in Arsenal against both v0 and v1, deterministically: + * https://github.com/scality/Arsenal/pull/2685. This suite does not re-derive + * those cases. Its only job is to prove that a real running metadata backend, + * which actually writes PHD masters and races a repair timer, does not get + * lifecycle listings stuck behind one -- through the real backbeat route, at the + * pinned Arsenal version. + * + * Timing contract + * --------------- + * These tests race PHD repair, because a single-process metadata backend does + * run it. Metadata starts a 15s repair timer for each key when you delete that + * key's version (arsenal VersioningRequestProcessor.processVersionSpecificDelete). + * assertDesertWasScanned() requires a resume marker on a desert key, proving the + * scan cap ran out inside the desert before repair could shrink it. Seeding takes + * about 200ms against a 15s deadline, so the margin is wide; if a run ever misses + * it, the failure message says so instead of blaming the wrong thing. + * + * Backends that cannot build a desert + * ----------------------------------- + * v1 buckets: PHD masters exist only in v0, so there is nothing to test. + * mongo: MongoClientInterface.deleteOrRepairPHD removes a zero-version PHD master + * before its own DELETE request answers, so the S3 API cannot seed a desert. + * bucketd and file keep the master and repair it 15s later, which is why seeding + * works there. Skipping mongo loses no coverage: handlePHDMaster only reads the + * v0 key stream, the same on every backend. + */ +const isV1 = process.env.DEFAULT_BUCKET_KEY_FORMAT === "v1"; +const isMongo = process.env.S3METADATA === "mongodb"; +const describePHD = isV1 || isMongo ? describe.skip : describe; + +const bucketUtil = new BucketUtility("default", {}); +const s3 = bucketUtil.s3; + +const removeAllVersionsPromise = promisify(removeAllVersions); + +const DESERT_SIZE = 12; +const DESERT_PREFIX = "phd-"; +const SEED_CONCURRENCY = 8; +const SCAN_CAP = "5"; +// Hard page guard. A markerless listing that keeps restarting fails fast, and does not hang. +const MAX_PAGES = 20; +// Metadata repair delay, per key. See "Timing contract" above. +const PHD_REPAIR_WINDOW_MS = 15000; + +let credentials = null; + +async function getCredentials() { + const creds = await s3.config.credentials(); + return { + accessKey: creds.accessKeyId, + secretKey: creds.secretAccessKey, + }; +} + +function uniqueBucket(prefix) { + return `${prefix}-${crypto.randomBytes(4).toString("hex")}`; +} + +function desertKey(n) { + return `${DESERT_PREFIX}${`00${n}`.slice(-3)}`; +} + +function putObject(bucket, key, cb) { + return s3 + .send(new PutObjectCommand({ Bucket: bucket, Key: key, Body: "123" })) + .then((data) => cb(null, data.VersionId)) + .catch(cb); +} + +function deleteVersion(bucket, key, versionId, cb) { + return s3 + .send( + new DeleteObjectCommand({ + Bucket: bucket, + Key: key, + VersionId: versionId, + }), + ) + .then(() => cb()) + .catch(cb); +} + +/** + * Creates a dangling PHD master. Put one version, then delete that exact version. + * Metadata replaces the master with { isPHD: true } and starts its 15s repair. + * Until the repair runs, or a GET or HEAD triggers it, the key is a zero-version + * PHD master. These keys are the desert material for the tests below. + */ +function createDanglingPHD(bucket, key, cb) { + return putObject(bucket, key, (err, versionId) => + err ? cb(err) : deleteVersion(bucket, key, versionId, cb), + ); +} + +/** + * Seeds DESERT_SIZE dangling PHD masters. Returns the time seeding started. That + * time bounds the earliest repair deadline, because each key's own delete happens + * later. The bound is therefore safe. + */ +function seedDesert(bucket, cb) { + const seededAt = Date.now(); + return async.timesLimit( + DESERT_SIZE, + SEED_CONCURRENCY, + (n, next) => createDanglingPHD(bucket, desertKey(n), next), + (err) => cb(err, seededAt), + ); +} + +/** + * HEADs every desert key. A GET or HEAD on a PHD master triggers the metadata + * repair, which deletes a zero-version master. Every key returns 404. The code + * ignores errors on purpose. This clears the desert at once, instead of waiting + * for the repair timers. It also matters more than tidiness, because PHD masters + * outlive their bucket. + */ +function repairDesert(bucket, cb) { + return async.timesLimit( + DESERT_SIZE, + SEED_CONCURRENCY, + (n, next) => + s3 + .send(new HeadObjectCommand({ Bucket: bucket, Key: desertKey(n) })) + .then(() => next()) + .catch(() => next()), + cb, + ); +} + +function createOrphanDeleteMarker(bucket, key, cb) { + return putObject(bucket, key, (err, versionId) => { + if (err) { + return cb(err); + } + return s3 + .send(new DeleteObjectCommand({ Bucket: bucket, Key: key })) + .then(() => deleteVersion(bucket, key, versionId, cb)) + .catch(cb); + }); +} + +function createVersionedBucket(bucket, cb) { + return s3 + .send(new CreateBucketCommand({ Bucket: bucket })) + .then(() => + s3.send( + new PutBucketVersioningCommand({ + Bucket: bucket, + VersioningConfiguration: { Status: "Enabled" }, + }), + ), + ) + .then(() => cb()) + .catch(cb); +} + +function cleanupBucket(bucket, cb) { + return async.series( + [ + (next) => repairDesert(bucket, next), + (next) => + removeAllVersionsPromise({ Bucket: bucket }) + .then(() => next()) + .catch(next), + (next) => + s3 + .send(new DeleteBucketCommand({ Bucket: bucket })) + .then(() => next()) + .catch(next), + ], + cb, + ); +} + +/** + * Reads every page of a lifecycle listing. Feeds each returned marker back into + * the next request. Checks the core invariant on every page: a truncated page + * must return a resume marker, and that marker must move forward. This is the + * regression itself -- before the fix, a desert truncated with no marker at all. + */ +function listAllPages(params, cb) { + const { bucket, listType, scanCap } = params; + const pages = []; + let keyMarker; + let versionIdMarker; + let done = false; + + return async.whilst( + () => !done, + (next) => { + const queryObj = { + "list-type": listType, + "max-scanned-lifecycle-listing-entries": scanCap, + }; + if (keyMarker !== undefined) { + if (listType === "orphan") { + queryObj.marker = keyMarker; + } else { + queryObj["key-marker"] = keyMarker; + if (versionIdMarker !== undefined) { + queryObj["version-id-marker"] = versionIdMarker; + } + } + } + return makeBackbeatRequest( + { + method: "GET", + bucket, + queryObj, + authCredentials: credentials, + }, + (err, response) => { + if (err) { + return next(err); + } + if (response.statusCode !== 200) { + return next( + new Error( + `${listType} listing returned ${response.statusCode}: ` + + `${String(response.body).slice(0, 200)}`, + ), + ); + } + const data = JSON.parse(response.body); + pages.push(data); + + if (pages.length > MAX_PAGES) { + return next( + new Error( + `listing did not terminate within ${MAX_PAGES} pages: ` + + "markerless truncation restarts it from scratch", + ), + ); + } + + if (!data.IsTruncated) { + done = true; + return next(); + } + + // Report invariant violations through the callback, do not throw them. An + // assertion thrown in this HTTP callback becomes an uncaught exception, and + // mocha can blame another test for it. + const nextKeyMarker = + listType === "orphan" ? data.NextMarker : data.NextKeyMarker; + // The core invariant. A truncated listing must return a resume marker, + if (!nextKeyMarker) { + return next( + new Error( + `truncated ${listType} listing page ${pages.length} returned no marker ` + + `(request marker: ${keyMarker || ""})`, + ), + ); + } + // and that marker must move forward on every page. + if (keyMarker !== undefined) { + if (nextKeyMarker < keyMarker) { + return next( + new Error( + `marker went backwards: ${keyMarker} -> ${nextKeyMarker}`, + ), + ); + } + const prevTuple = `${keyMarker}\0${versionIdMarker || ""}`; + const newTuple = `${nextKeyMarker}\0${data.NextVersionIdMarker || ""}`; + if (newTuple === prevTuple) { + return next( + new Error( + "marker did not advance on truncated " + + `${listType} page ${pages.length}: ${nextKeyMarker}`, + ), + ); + } + } + keyMarker = nextKeyMarker; + versionIdMarker = data.NextVersionIdMarker; + return next(); + }, + ); + }, + (err) => (err ? cb(err) : cb(null, pages)), + ); +} + +/** Seeds a fresh desert, then reads every page of a capped listing across it. */ +function seedThenList(bucket, listType, scanCap, cb) { + return seedDesert(bucket, (err, seededAt) => { + if (err) { + return cb(err); + } + return listAllPages({ bucket, listType, scanCap }, (err, pages) => + err ? cb(err) : cb(null, pages, seededAt), + ); + }); +} + +/** + * Proves the listing paged through the desert, so the assertions that follow mean + * something. + * + * It requires at least one resume marker on a desert key. "Page 0 was truncated" + * does not prove that. Any bucket that holds more than scanCap entries of its own + * truncates page 0, with or without a PHD master, so that check also passes on a + * repaired desert. Only a scan cap that runs out inside the desert puts a marker + * on a desert key, and that is the condition under test. + */ +function assertDesertWasScanned(pages, seededAt, label) { + const markers = pages + .map((page) => page.NextMarker || page.NextKeyMarker) + .filter(Boolean); + if (markers.some((marker) => marker.startsWith(DESERT_PREFIX))) { + return; + } + const elapsed = Date.now() - seededAt; + const cause = + elapsed >= PHD_REPAIR_WINDOW_MS + ? `seeding+listing took ${elapsed}ms, over the ${PHD_REPAIR_WINDOW_MS}ms metadata repair ` + + "window: the desert was repaired before the listing ran (slow runner, not a code bug)" + : `only ${elapsed}ms elapsed, well inside the ${PHD_REPAIR_WINDOW_MS}ms repair window: the ` + + "desert was never created, so this backend did not write PHD masters (v0 buckets only)"; + assert.fail( + `${label}: no resume marker landed inside the desert ` + + `(markers: ${JSON.stringify(markers)}) -- ${cause}`, + ); +} + +/** + * Reports the outcome of a test through done(). Mocha treats an assertion thrown + * inside an HTTP callback as an uncaught exception, and can blame the wrong test + * for it. done() keeps each failure on the test that caused it. + */ +function finish(done, err, assertions) { + if (err) { + return done(err); + } + try { + assertions(); + } catch (assertionErr) { + return done(assertionErr); + } + return done(); +} + +function contentsKeys(pages) { + return pages.reduce( + (acc, page) => acc.concat((page.Contents || []).map((entry) => entry.Key)), + [], + ); +} + +function contentsEntries(pages) { + return pages.reduce((acc, page) => acc.concat(page.Contents || []), []); +} + +describePHD("listLifecycle over a dangling-PHD desert", () => { + before((done) => { + getCredentials() + .then((creds) => { + credentials = creds; + done(); + }) + .catch(done); + }); + + // One layout, read two ways, exercising every behaviorally-important case in a + // single crawl: an orphan DM held as a candidate right before the desert, the + // desert itself, a PHD master with surviving versions just past it, and more + // noncurrent versions and an orphan DM past that. Both listing types must cross + // the desert -- advancing the resume marker across several truncated pages, + // never getting stuck -- to reach what lies beyond it. + describe("crossing the desert", () => { + const bucket = uniqueBucket("lc-phd-desert"); + const aaaNcVersionIds = []; + const survivorVersionIds = []; + const zzzNcVersionIds = []; + + before((done) => + async.series( + [ + (next) => createVersionedBucket(bucket, next), + // Held candidate: proven orphan only once the desert's first PHD key is + // scanned. A fix that only advanced the marker past it, without emitting + // it, would strand this delete marker forever. + (next) => createOrphanDeleteMarker(bucket, "aaa-dm", next), + (next) => + async.timesSeries( + 2, + (n, cb) => + putObject(bucket, "aaa-nc", (err, versionId) => { + aaaNcVersionIds.push(versionId); + cb(err); + }), + next, + ), + // A PHD master that still has versions, sitting right past the desert. + // Deleting the current version by id leaves 2 versions under it; the + // newest is the version repair promotes back to master, and NCVE must + // never treat it as expirable -- that would be live-data loss. + (next) => + async.timesSeries( + 3, + (n, cb) => + putObject(bucket, "www-survivors", (err, versionId) => { + survivorVersionIds.push(versionId); + cb(err); + }), + next, + ), + (next) => + deleteVersion( + bucket, + "www-survivors", + survivorVersionIds[2], + next, + ), + (next) => + async.timesSeries( + 2, + (n, cb) => + putObject(bucket, "zzz-nc", (err, versionId) => { + zzzNcVersionIds.push(versionId); + cb(err); + }), + next, + ), + (next) => createOrphanDeleteMarker(bucket, "zzz-dm", next), + ], + done, + ), + ); + + after((done) => cleanupBucket(bucket, done)); + + it("should list both orphan delete markers across the desert", (done) => + seedThenList(bucket, "orphan", SCAN_CAP, (err, pages, seededAt) => + finish(done, err, () => { + assertDesertWasScanned(pages, seededAt, "orphan"); + assert.deepStrictEqual(contentsKeys(pages), ["aaa-dm", "zzz-dm"]); + }), + )); + + it("should list noncurrent versions on both sides of the desert and protect the PHD survivor", (done) => + seedThenList(bucket, "noncurrent", SCAN_CAP, (err, pages, seededAt) => + finish(done, err, () => { + assertDesertWasScanned(pages, seededAt, "noncurrent"); + const listed = contentsEntries(pages); + assert.deepStrictEqual( + listed.map((entry) => `${entry.Key}:${entry.VersionId}`).sort(), + [ + `aaa-nc:${aaaNcVersionIds[0]}`, + `www-survivors:${survivorVersionIds[0]}`, + `zzz-nc:${zzzNcVersionIds[0]}`, + ], + ); + const survivorVersions = listed + .filter((entry) => entry.Key === "www-survivors") + .map((entry) => entry.VersionId); + assert( + !survivorVersions.includes(survivorVersionIds[1]), + "newest surviving version under the PHD master listed as noncurrent: " + + "NCVE would expire live data", + ); + }), + )); + }); +}); diff --git a/yarn.lock b/yarn.lock index 854e838c90..c762dbc9fa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5838,9 +5838,9 @@ arraybuffer.prototype.slice@^1.0.4: optionalDependencies: ioctl "^2.0.2" -"arsenal@git+https://github.com/scality/arsenal#8.5.15": - version "8.5.15" - resolved "git+https://github.com/scality/arsenal#0bbe970dd72b2e235c47910474883af9b9c13eb1" +"arsenal@git+https://github.com/scality/arsenal#8.5.17": + version "8.5.17" + resolved "git+https://github.com/scality/arsenal#ecb7d87e367ebc92a2273739eab9efac7c0b4c65" dependencies: "@aws-sdk/client-kms" "^3.975.0" "@aws-sdk/client-s3" "^3.975.0"