diff --git a/create-a-container/bin/create-container.js b/create-a-container/bin/create-container.js index 335b73a9..c5f33a14 100755 --- a/create-a-container/bin/create-container.js +++ b/create-a-container/bin/create-container.js @@ -35,6 +35,7 @@ const { parseArgs } = require(path.join(__dirname, '..', 'utils', 'cli')); const { isDockerImage, parseDockerRef, getImageDigest } = require(path.join(__dirname, '..', 'utils', 'docker-registry')); const { manageDnsRecords } = require(path.join(__dirname, '..', 'utils', 'cloudflare-dns')); const { createVirtualMachine, withNetbox } = require(path.join(__dirname, '..', 'utils', 'netbox')); +const { withVmidRetry } = require(path.join(__dirname, '..', 'utils', 'vmid')); const { resolveVolumesRoot, deriveVolumeHostPaths, @@ -357,14 +358,14 @@ async function main() { console.log('Preparing volumes...'); const preparedVolumes = await prepareVolumes(client, node, container); - // Allocate the provider ID right before creating to minimize race condition window. - // Proxmox requires us to allocate a VMID first; Docker returns its real container - // ID after create. + // Proxmox requires us to allocate a VMID first; it is generated pseudo-randomly + // (time + random) so concurrent jobs don't collide. Docker returns its real + // container ID after create. let vmid = null; if (isDockerNode) { console.log('Docker node selected; Docker will allocate the container ID during create.'); } else { - console.log('Allocating VMID from Proxmox...'); + console.log('Allocating VMID...'); vmid = await client.nextId(); console.log(`Allocated VMID: ${vmid}`); } @@ -412,8 +413,8 @@ async function main() { // Create container from the pulled image (Proxmox adds .tar to the filename) console.log(`Creating container from ${filename}.tar...`); const ostemplate = `${templateStorage}:vztmpl/${filename}.tar`; - const createUpid = await client.createLxc(node.name, { - vmid, + const createOptions = (id) => ({ + vmid: id, hostname: container.hostname, ostemplate, description: `Created from Docker image ${container.template}`, @@ -432,6 +433,12 @@ async function main() { // prepareVolumes above). Setting mpN here would 400 against a // not-yet-created directory (issue #421). }); + let createUpid; + if (isDockerNode) { + createUpid = await client.createLxc(node.name, createOptions(vmid)); + } else { + ({ vmid, result: createUpid } = await withVmidRetry(vmid, (id) => client.createLxc(node.name, createOptions(id)))); + } console.log(`Create task started: ${createUpid}`); if (isDockerNode) { @@ -466,12 +473,18 @@ async function main() { // Clone the template console.log(`Cloning template ${templateVmid} to VMID ${vmid}...`); - const cloneUpid = await client.cloneLxc(node.name, templateVmid, vmid, { + const cloneOptions = { hostname: container.hostname, description: `Cloned from template ${container.template}`, full: 1, storage: rootfsStorage - }); + }; + let cloneUpid; + if (isDockerNode) { + cloneUpid = await client.cloneLxc(node.name, templateVmid, vmid, cloneOptions); + } else { + ({ vmid, result: cloneUpid } = await withVmidRetry(vmid, (id) => client.cloneLxc(node.name, templateVmid, id, cloneOptions))); + } console.log(`Clone task started: ${cloneUpid}`); // Wait for clone to complete diff --git a/create-a-container/utils/__tests__/vmid.test.js b/create-a-container/utils/__tests__/vmid.test.js new file mode 100644 index 00000000..7a33a0dc --- /dev/null +++ b/create-a-container/utils/__tests__/vmid.test.js @@ -0,0 +1,67 @@ +const { generateVmid, isVmidConflict, withVmidRetry, VMID_MIN, VMID_MAX } = require('../vmid'); + +describe('generateVmid', () => { + it('stays within the Proxmox VMID range', () => { + for (const now of [0, 999, Date.now(), 9998999, 9999000, Number.MAX_SAFE_INTEGER]) { + for (let i = 0; i < 100; i++) { + const vmid = generateVmid(now); + expect(Number.isInteger(vmid)).toBe(true); + expect(vmid).toBeGreaterThanOrEqual(VMID_MIN); + expect(vmid).toBeLessThanOrEqual(VMID_MAX); + } + } + }); + + it('randomizes IDs allocated in the same second', () => { + const now = Date.now(); + const ids = new Set(Array.from({ length: 1000 }, () => generateVmid(now))); + // ~5 birthday collisions expected out of a 100000-wide random space + expect(ids.size).toBeGreaterThan(980); + }); + + it('never collides across different seconds', () => { + const now = Date.now(); + const a = Array.from({ length: 50 }, () => generateVmid(now)); + const b = Array.from({ length: 50 }, () => generateVmid(now + 1000)); + expect(a.some((id) => b.includes(id))).toBe(false); + }); +}); + +describe('isVmidConflict', () => { + it('detects Proxmox "already exists" errors', () => { + const err = new Error('Request failed with status code 500'); + err.response = { statusText: "CT 123 already exists on node 'pve1'", data: {} }; + expect(isVmidConflict(err)).toBe(true); + }); + + it('ignores unrelated errors', () => { + expect(isVmidConflict(new Error('connection refused'))).toBe(false); + }); +}); + +describe('withVmidRetry', () => { + const conflict = () => Object.assign(new Error('CT already exists'), {}); + beforeEach(() => jest.spyOn(console, 'warn').mockImplementation(() => {})); + afterEach(() => jest.restoreAllMocks()); + + it('retries with a new VMID on conflict', async () => { + const fn = jest.fn() + .mockRejectedValueOnce(conflict()) + .mockResolvedValueOnce('UPID:ok'); + const out = await withVmidRetry(100, fn, { generate: () => 200 }); + expect(out).toEqual({ vmid: 200, result: 'UPID:ok' }); + expect(fn.mock.calls.map((c) => c[0])).toEqual([100, 200]); + }); + + it('does not retry unrelated errors', async () => { + const fn = jest.fn().mockRejectedValue(new Error('boom')); + await expect(withVmidRetry(100, fn)).rejects.toThrow('boom'); + expect(fn).toHaveBeenCalledTimes(1); + }); + + it('gives up after maxAttempts', async () => { + const fn = jest.fn().mockRejectedValue(conflict()); + await expect(withVmidRetry(100, fn, { maxAttempts: 3 })).rejects.toThrow('already exists'); + expect(fn).toHaveBeenCalledTimes(3); + }); +}); diff --git a/create-a-container/utils/proxmox-api.js b/create-a-container/utils/proxmox-api.js index 8225f5af..adf4bad3 100644 --- a/create-a-container/utils/proxmox-api.js +++ b/create-a-container/utils/proxmox-api.js @@ -1,5 +1,6 @@ const axios = require('axios'); const { URL } = require('url'); +const { generateVmid } = require('./vmid'); /** * @@ -253,11 +254,12 @@ class ProxmoxApi { } /** - * @returns {Promise} - The next available VMID + * Allocate a VMID locally instead of via /cluster/nextid, which returns the + * same ID to concurrent callers and causes create conflicts. + * @returns {Promise} - A pseudo-random VMID */ async nextId() { - const response = await axios.get(`${this.baseUrl}/api2/json/cluster/nextid`, this.options); - return response.data.data; + return generateVmid(); } /** diff --git a/create-a-container/utils/vmid.js b/create-a-container/utils/vmid.js new file mode 100644 index 00000000..4698d1f2 --- /dev/null +++ b/create-a-container/utils/vmid.js @@ -0,0 +1,64 @@ +const crypto = require('crypto'); + +// Proxmox accepts VMIDs in the range 100..999999999. +const VMID_MIN = 100; +const VMID_MAX = 999999999; + +// UUIDv7-style layout squeezed into the VMID range: a coarse timestamp in the +// high digits and random bits in the low digits. IDs allocated in different +// seconds never collide within a time cycle, and concurrent allocations in the +// same second only collide with probability 1/RANDOM_SPACE. This replaces +// Proxmox's /cluster/nextid, which hands the same ID to concurrent callers. +const RANDOM_SPACE = 100000; +const TIME_SLOTS = Math.floor((VMID_MAX - VMID_MIN + 1) / RANDOM_SPACE); + +/** + * Generate a pseudo-random VMID that is safe to allocate without coordination. + * @param {number} [now] - Epoch milliseconds (injectable for tests) + * @returns {number} + */ +function generateVmid(now = Date.now()) { + const timeSlot = Math.floor(now / 1000) % TIME_SLOTS; + return VMID_MIN + timeSlot * RANDOM_SPACE + crypto.randomInt(RANDOM_SPACE); +} + +/** + * True if an error from a Proxmox create/clone call means the VMID is taken + * (e.g. "CT 123 already exists on node 'pve1'"). + * @param {Error} err + * @returns {boolean} + */ +function isVmidConflict(err) { + const msg = [ + err?.response?.data?.message, + err?.response?.statusText, + err?.message, + ].filter(Boolean).join(' '); + return /already exists/i.test(msg); +} + +/** + * Run `fn(vmid)` and, if it fails because the VMID is already in use, retry + * with a freshly generated VMID up to `maxAttempts` total attempts. + * @template T + * @param {number} vmid - Initial VMID to try + * @param {(vmid: number) => Promise} fn + * @param {object} [opts] + * @param {number} [opts.maxAttempts=5] + * @param {() => number} [opts.generate=generateVmid] + * @returns {Promise<{ vmid: number, result: T }>} + */ +async function withVmidRetry(vmid, fn, { maxAttempts = 5, generate = generateVmid } = {}) { + for (let attempt = 1; ; attempt++) { + try { + return { vmid, result: await fn(vmid) }; + } catch (err) { + if (attempt >= maxAttempts || !isVmidConflict(err)) throw err; + const next = generate(); + console.warn(`VMID ${vmid} already in use; retrying with ${next} (attempt ${attempt + 1}/${maxAttempts})`); + vmid = next; + } + } +} + +module.exports = { generateVmid, isVmidConflict, withVmidRetry, VMID_MIN, VMID_MAX };