From 7ef99ccfe686b42e39929645ec49667c3babc9b3 Mon Sep 17 00:00:00 2001 From: superuserjr <80784472+turbodaemon@users.noreply.github.com> Date: Mon, 9 Mar 2026 20:17:03 -0400 Subject: [PATCH 01/34] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(compose):?= =?UTF-8?q?=20replace=20CLI=20shelling=20with=20Docker=20Engine=20API?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rewrite validateComposeConfiguration to parse YAML in-process instead of spawning docker compose config - Rewrite updateContainerWithCompose to use Docker Engine API (pullImage + stopAndRemoveContainer + recreateContainer) instead of docker compose pull/up - Delegate executeSelfUpdate to parent orchestrator for health gate and rollback safety - Log deprecation for composeFileOnce batch mode (unavailable without CLI) - Remove dead code: updateComposeServicesWithCompose, runComposeCommand, executeCommand, getContainerRunningState, execFile import, and COMPOSE_COMMAND_* constants - Remove 18 dead tests and clean up spy references to removed methods --- .../dockercompose/Dockercompose.test.ts | 728 ++++++------------ .../providers/dockercompose/Dockercompose.ts | 407 ++-------- 2 files changed, 294 insertions(+), 841 deletions(-) diff --git a/app/triggers/providers/dockercompose/Dockercompose.test.ts b/app/triggers/providers/dockercompose/Dockercompose.test.ts index fe79d24c..fbcdb16b 100644 --- a/app/triggers/providers/dockercompose/Dockercompose.test.ts +++ b/app/triggers/providers/dockercompose/Dockercompose.test.ts @@ -1,4 +1,3 @@ -import { execFile } from 'node:child_process'; import { EventEmitter } from 'node:events'; import { watch } from 'node:fs'; import fs from 'node:fs/promises'; @@ -64,10 +63,6 @@ vi.mock('../../../store/container.js', () => ({ vi.mock('../../hooks/HookRunner.js', () => ({ runHook: vi.fn() })); vi.mock('../docker/HealthMonitor.js', () => ({ startHealthMonitor: vi.fn() })); -vi.mock('node:child_process', () => ({ - execFile: vi.fn(), -})); - vi.mock('node:fs', async (importOriginal) => { const actual = await importOriginal(); return { @@ -209,6 +204,37 @@ function makeExecMocks({ return { startStream, mockExec, recreatedContainer }; } +function makeDockerContainerHandle({ + running = true, + image = 'nginx:1.0.0', + id = 'container-id', + name = 'nginx', + autoRemove = false, +} = {}) { + return { + id, + inspect: vi.fn().mockResolvedValue({ + Id: id, + Name: `/${name}`, + Config: { + Image: image, + Env: [], + Labels: {}, + }, + HostConfig: { + AutoRemove: autoRemove, + }, + NetworkSettings: { + Networks: {}, + }, + State: { Running: running }, + }), + stop: vi.fn().mockResolvedValue(undefined), + remove: vi.fn().mockResolvedValue(undefined), + wait: vi.fn().mockResolvedValue(undefined), + }; +} + /** * Set up the common spies used by processComposeFile tests that exercise * the write / trigger / hooks path. @@ -290,11 +316,23 @@ describe('Dockercompose Trigger', () => { mockDockerApi = { modem: { socketPath: '/var/run/docker.sock', - }, - getContainer: vi.fn().mockReturnValue({ - inspect: vi.fn().mockResolvedValue({ - State: { Running: true }, + followProgress: vi.fn((_stream, onDone, onProgress) => { + onProgress?.({ + status: 'Pulling fs layer', + id: 'layer-1', + progressDetail: { current: 1, total: 1 }, + }); + onDone?.(null, [{ status: 'Pull complete', id: 'layer-1' }]); }), + }, + pull: vi.fn().mockResolvedValue({}), + createContainer: vi.fn().mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + inspect: vi.fn().mockResolvedValue({ State: { Running: true } }), + }), + getContainer: vi.fn().mockReturnValue(makeDockerContainerHandle()), + getNetwork: vi.fn().mockReturnValue({ + connect: vi.fn().mockResolvedValue(undefined), }), }; @@ -314,11 +352,6 @@ describe('Dockercompose Trigger', () => { }, }, }); - - execFile.mockImplementation((_command, _args, _options, callback) => { - callback(null, '', ''); - return {}; - }); }); // ----------------------------------------------------------------------- @@ -1198,74 +1231,62 @@ describe('Dockercompose Trigger', () => { // compose command execution // ----------------------------------------------------------------------- - test('updateContainerWithCompose should skip compose commands in dry-run mode', async () => { + test('updateContainerWithCompose should skip Docker API calls in dry-run mode', async () => { trigger.configuration.dryrun = true; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - const container = { name: 'nginx', watcher: 'local' }; + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const container = makeContainer({ name: 'nginx' }); await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container); - expect(runComposeCommandSpy).not.toHaveBeenCalled(); + expect(pullImageSpy).not.toHaveBeenCalled(); expect(mockLog.child).toHaveBeenCalledWith({ container: 'nginx' }); expect(mockLog.info).toHaveBeenCalledWith(expect.stringContaining('dry-run mode is enabled')); }); - test('updateContainerWithCompose should run pull then up for the target service', async () => { + test('updateContainerWithCompose should pull and recreate the target service via Docker API', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - mockDockerApi.getContainer.mockReturnValueOnce({ - inspect: vi.fn().mockResolvedValue({ - State: { Running: true }, - }), - }); - const container = { name: 'nginx', watcher: 'local' }; + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const stopContainerSpy = vi.spyOn(trigger, 'stopContainer').mockResolvedValue(); + const removeContainerSpy = vi.spyOn(trigger, 'removeContainer').mockResolvedValue(); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); + const startContainerSpy = vi.spyOn(trigger, 'startContainer').mockResolvedValue(); + const container = makeContainer({ name: 'nginx' }); await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx'], - mockLog, - ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 2, - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', 'nginx'], - mockLog, - ); + expect(pullImageSpy).toHaveBeenCalledTimes(1); + expect(stopContainerSpy).toHaveBeenCalledTimes(1); + expect(removeContainerSpy).toHaveBeenCalledTimes(1); + expect(createContainerSpy).toHaveBeenCalledTimes(1); + expect(startContainerSpy).toHaveBeenCalledTimes(1); }); test('updateContainerWithCompose should preserve stopped runtime state', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - mockDockerApi.getContainer.mockReturnValueOnce({ - inspect: vi.fn().mockResolvedValue({ - State: { Running: false }, + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const startContainerSpy = vi.spyOn(trigger, 'startContainer').mockResolvedValue(); + mockDockerApi.getContainer.mockReturnValueOnce( + makeDockerContainerHandle({ + running: false, }), - }); - const container = { name: 'nginx', watcher: 'local' }; + ); + const container = makeContainer({ name: 'nginx' }); await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx'], - mockLog, - ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 2, - '/opt/drydock/test/stack.yml', - ['up', '--no-start', '--no-deps', 'nginx'], - mockLog, - ); + expect(pullImageSpy).toHaveBeenCalledTimes(1); + expect(startContainerSpy).not.toHaveBeenCalled(); }); - test('updateContainerWithCompose should add force-recreate argument when requested', async () => { + test('updateContainerWithCompose should skip pull when requested and still recreate', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - const container = { name: 'nginx', watcher: 'local' }; + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); + const container = makeContainer({ name: 'nginx' }); await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container, { shouldStart: true, @@ -1273,18 +1294,14 @@ describe('Dockercompose Trigger', () => { forceRecreate: true, }); - expect(runComposeCommandSpy).toHaveBeenCalledTimes(1); - expect(runComposeCommandSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', '--force-recreate', 'nginx'], - mockLog, - ); + expect(pullImageSpy).not.toHaveBeenCalled(); + expect(createContainerSpy).toHaveBeenCalledTimes(1); }); - test('updateContainerWithCompose should pass compose file chain to compose commands', async () => { + test('updateContainerWithCompose should ignore compose file chain and use Docker API path', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - const container = { name: 'nginx', watcher: 'local' }; + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const container = makeContainer({ name: 'nginx' }); const composeFiles = ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml']; await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container, { @@ -1293,11 +1310,18 @@ describe('Dockercompose Trigger', () => { composeFiles, }); - expect(runComposeCommandSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', 'nginx'], - mockLog, - composeFiles, + expect(pullImageSpy).not.toHaveBeenCalled(); + }); + + test('updateContainerWithCompose should throw when current container cannot be resolved', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'getCurrentContainer').mockResolvedValue(undefined); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow( + 'Unable to refresh compose service nginx from /opt/drydock/test/stack.yml because container nginx no longer exists', ); }); @@ -1393,7 +1417,7 @@ describe('Dockercompose Trigger', () => { expect(resolveContextSpy).toHaveBeenCalledWith(container, 'nginx:1.0.0'); }); - test('recreateContainer integration should update compose image and run force-recreate without pull', async () => { + test('recreateContainer integration should update compose image and recreate via Docker API without pull', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ name: 'nginx', @@ -1408,7 +1432,10 @@ describe('Dockercompose Trigger', () => { vi.spyOn(trigger, 'getComposeFileAsObject').mockResolvedValue( makeCompose({ nginx: { image: 'nginx:1.1.0' } }), ); - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); await trigger.recreateContainer( mockDockerApi, @@ -1425,15 +1452,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', expect.stringContaining('nginx:1.0.0'), ); - expect(runComposeCommandSpy).toHaveBeenCalledTimes(1); - expect(runComposeCommandSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', '--force-recreate', 'nginx'], - mockLog, - ); + expect(pullImageSpy).not.toHaveBeenCalled(); + expect(createContainerSpy).toHaveBeenCalledTimes(1); }); - test('executeSelfUpdate should run compose-native self-update strategy', async () => { + test('executeSelfUpdate should delegate to parent self-update transition with hydrated runtime context', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ name: 'drydock', @@ -1448,8 +1471,25 @@ describe('Dockercompose Trigger', () => { service: 'drydock', serviceDefinition: {}, }; + const currentContainer = makeDockerContainerHandle(); + const currentContainerSpec = { + Id: 'current-id', + Name: '/drydock', + State: { Running: true }, + HostConfig: { + Binds: ['/var/run/docker.sock:/var/run/docker.sock'], + }, + }; - const insertBackupSpy = vi.spyOn(trigger, 'insertContainerImageBackup'); + const getCurrentContainerSpy = vi + .spyOn(trigger, 'getCurrentContainer') + .mockResolvedValue(currentContainer); + const inspectContainerSpy = vi + .spyOn(trigger, 'inspectContainer') + .mockResolvedValue(currentContainerSpec as any); + const orchestratorExecuteSpy = vi + .spyOn(trigger.selfUpdateOrchestrator, 'execute') + .mockResolvedValue(true); const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose').mockResolvedValue(); const hooksSpy = vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); @@ -1469,13 +1509,19 @@ describe('Dockercompose Trigger', () => { ); expect(updated).toBe(true); - expect(insertBackupSpy).toHaveBeenCalledTimes(1); - expect(composeUpdateSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - 'drydock', + expect(getCurrentContainerSpy).toHaveBeenCalledWith(mockDockerApi, container); + expect(inspectContainerSpy).toHaveBeenCalledWith(currentContainer, mockLog); + expect(orchestratorExecuteSpy).toHaveBeenCalledWith( + expect.objectContaining({ + currentContainer, + currentContainerSpec, + }), container, + mockLog, + undefined, ); - expect(hooksSpy).toHaveBeenCalledWith(container, 'drydock', {}); + expect(composeUpdateSpy).not.toHaveBeenCalled(); + expect(hooksSpy).not.toHaveBeenCalled(); }); test('performContainerUpdate should throw when compose context is missing', async () => { @@ -1517,6 +1563,12 @@ describe('Dockercompose Trigger', () => { }; const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose').mockResolvedValue(); const hooksSpy = vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + const getCurrentContainerSpy = vi + .spyOn(trigger, 'getCurrentContainer') + .mockResolvedValue(makeDockerContainerHandle()); + const orchestratorExecuteSpy = vi + .spyOn(trigger.selfUpdateOrchestrator, 'execute') + .mockResolvedValue(true); const updated = await trigger.executeSelfUpdate( { @@ -1538,77 +1590,13 @@ describe('Dockercompose Trigger', () => { expect(updated).toBe(false); expect(composeUpdateSpy).not.toHaveBeenCalled(); expect(hooksSpy).not.toHaveBeenCalled(); + expect(getCurrentContainerSpy).not.toHaveBeenCalled(); + expect(orchestratorExecuteSpy).not.toHaveBeenCalled(); expect(mockLog.info).toHaveBeenCalledWith( 'Do not replace the existing container because dry-run mode is enabled', ); }); - test('runComposeCommand should use docker compose when available', async () => { - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer); - - expect(execFile).toHaveBeenCalledWith( - 'docker', - ['compose', '-f', composeFilePath, 'pull', 'nginx'], - expect.objectContaining({ - cwd: path.dirname(composeFilePath), - }), - expect.any(Function), - ); - expect(logContainer.warn).not.toHaveBeenCalled(); - }); - - test('runComposeCommand should include ordered compose file chain when provided', async () => { - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - const composeOverridePath = path.resolve(process.cwd(), 'test', 'stack.override.yml'); - - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer, [ - composeFilePath, - composeOverridePath, - ]); - - expect(execFile).toHaveBeenCalledWith( - 'docker', - ['compose', '-f', composeFilePath, '-f', composeOverridePath, 'pull', 'nginx'], - expect.objectContaining({ - cwd: path.dirname(composeFilePath), - }), - expect.any(Function), - ); - }); - - test('runComposeCommand should not forward non-allowlisted parent environment variables', async () => { - const secretKey = 'DRYDOCK_TEST_COMPOSE_SECRET'; - const originalSecret = process.env[secretKey]; - const originalPath = process.env.PATH; - process.env[secretKey] = 'top-secret'; - process.env.PATH = '/tmp/drydock-compose-path'; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - try { - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer); - - const commandOptions = execFile.mock.calls[0][2]; - expect(commandOptions.env.PATH).toBe('/tmp/drydock-compose-path'); - expect(commandOptions.env[secretKey]).toBeUndefined(); - } finally { - if (originalSecret === undefined) { - delete process.env[secretKey]; - } else { - process.env[secretKey] = originalSecret; - } - if (originalPath === undefined) { - delete process.env.PATH; - } else { - process.env.PATH = originalPath; - } - } - }); - test('resolveComposeFilePath should allow absolute compose files while blocking relative traversal when boundary is enforced', () => { const composeFilePathOutsideWorkingDirectory = path.resolve( process.cwd(), @@ -1637,187 +1625,6 @@ describe('Dockercompose Trigger', () => { ).not.toThrow(); }); - test('runComposeCommand should fall back to docker-compose when docker compose plugin is missing', async () => { - execFile - .mockImplementationOnce((_command, _args, _options, callback) => { - const error = new Error('compose plugin missing'); - error.stderr = "docker: 'compose' is not a docker command."; - callback(error, '', error.stderr); - return {}; - }) - .mockImplementationOnce((_command, _args, _options, callback) => { - callback(null, '', ''); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer); - - expect(execFile).toHaveBeenNthCalledWith( - 1, - 'docker', - ['compose', '-f', composeFilePath, 'pull', 'nginx'], - expect.objectContaining({ cwd: path.dirname(composeFilePath) }), - expect.any(Function), - ); - expect(execFile).toHaveBeenNthCalledWith( - 2, - 'docker-compose', - ['-f', composeFilePath, 'pull', 'nginx'], - expect.objectContaining({ cwd: path.dirname(composeFilePath) }), - expect.any(Function), - ); - expect(logContainer.warn).toHaveBeenCalledWith( - expect.stringContaining('trying docker-compose'), - ); - }); - - test('runComposeCommand should accept compose files outside the working directory', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(null, '', ''); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), '..', 'external', 'compose.yaml'); - - await trigger.runComposeCommand(composeFilePath, ['up', '-d'], logContainer); - - expect(execFile).toHaveBeenCalledWith( - 'docker', - ['compose', '-f', composeFilePath, 'up', '-d'], - expect.objectContaining({ cwd: path.dirname(composeFilePath) }), - expect.any(Function), - ); - }); - - test('runComposeCommand should throw when compose command fails', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(new Error('boom'), '', 'boom'); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await expect( - trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer), - ).rejects.toThrow(`Error when running docker compose pull nginx for ${composeFilePath} (boom)`); - - expect(execFile).toHaveBeenCalledTimes(1); - }); - - test('runComposeCommand should handle failures without stderr payload', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(new Error('boom-without-stderr'), '', undefined); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await expect( - trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer), - ).rejects.toThrow( - `Error when running docker compose pull nginx for ${composeFilePath} (boom-without-stderr)`, - ); - }); - - test('runComposeCommand should log stdout and stderr output', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(null, 'pulled image\n', 'minor warning\n'); - return {}; - }); - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const composeFilePath = path.resolve(process.cwd(), 'test', 'stack.yml'); - - await trigger.runComposeCommand(composeFilePath, ['pull', 'nginx'], logContainer); - - expect(logContainer.debug).toHaveBeenCalledWith( - expect.stringContaining('docker compose pull nginx stdout:\npulled image'), - ); - expect(logContainer.debug).toHaveBeenCalledWith( - expect.stringContaining('docker compose pull nginx stderr:\nminor warning'), - ); - }); - - test('runComposeCommand should resolve relative compose file paths outside working directory', async () => { - execFile.mockImplementationOnce((_command, _args, _options, callback) => { - callback(null, '', ''); - return {}; - }); - - const logContainer = { debug: vi.fn(), warn: vi.fn() }; - const expectedPath = path.resolve(process.cwd(), '../outside/stack.yml'); - - await trigger.runComposeCommand('../outside/stack.yml', ['pull', 'nginx'], logContainer); - - expect(execFile).toHaveBeenCalledWith( - 'docker', - ['compose', '-f', expectedPath, 'pull', 'nginx'], - expect.objectContaining({ cwd: path.dirname(expectedPath) }), - expect.any(Function), - ); - }); - - test('getContainerRunningState should assume running when inspect fails', async () => { - const logContainer = { warn: vi.fn() }; - mockDockerApi.getContainer.mockReturnValueOnce({ - inspect: vi.fn().mockRejectedValue(new Error('inspect failed')), - }); - - const running = await trigger.getContainerRunningState( - { - name: 'nginx', - watcher: 'local', - }, - logContainer, - ); - - expect(running).toBe(true); - expect(logContainer.warn).toHaveBeenCalledWith( - 'Unable to inspect running state for nginx; assuming running (inspect failed)', - ); - }); - - test('getContainerRunningState should assume running when watcher dockerApi is unavailable', async () => { - const logContainer = { warn: vi.fn() }; - - const running = await trigger.getContainerRunningState( - { - name: 'nginx', - watcher: 'missing', - }, - logContainer, - ); - - expect(running).toBe(true); - }); - - test('getContainerRunningState should assume running when watcher dockerApi is not an object', async () => { - const logContainer = { warn: vi.fn() }; - getState.mockReturnValue({ - registry: getState().registry, - watcher: { - 'docker.local': { - dockerApi: 'invalid', - }, - }, - }); - - const running = await trigger.getContainerRunningState( - { - name: 'nginx', - watcher: 'local', - }, - logContainer, - ); - - expect(running).toBe(true); - }); - test('resolveComposeServiceContext should throw when no compose file is configured', async () => { trigger.configuration.file = undefined; @@ -2582,43 +2389,17 @@ describe('Dockercompose Trigger', () => { expect(writeSpy).not.toHaveBeenCalled(); }); - test('validateComposeConfiguration should fall back to docker-compose when docker compose plugin is missing', async () => { - const executeCommandSpy = vi - .spyOn(trigger, 'executeCommand') - .mockRejectedValueOnce( - Object.assign(new Error('compose plugin missing'), { - stderr: "docker: 'compose' is not a docker command.", - }), - ) - .mockResolvedValueOnce({ stdout: '', stderr: '' }); - + test('validateComposeConfiguration should validate compose text in-process without shell commands', async () => { await trigger.validateComposeConfiguration( '/opt/drydock/test/compose.yml', 'services:\n nginx:\n image: nginx:1.1.0\n', ); - - expect(executeCommandSpy).toHaveBeenNthCalledWith( - 1, - 'docker', - ['compose', '-f', expect.stringContaining('.compose.yml.validate-'), 'config', '--quiet'], - expect.objectContaining({ - cwd: '/opt/drydock/test', - }), - ); - expect(executeCommandSpy).toHaveBeenNthCalledWith( - 2, - 'docker-compose', - ['-f', expect.stringContaining('.compose.yml.validate-'), 'config', '--quiet'], - expect.objectContaining({ - cwd: '/opt/drydock/test', - }), - ); }); test('validateComposeConfiguration should validate updated file against full compose file chain', async () => { - const executeCommandSpy = vi - .spyOn(trigger, 'executeCommand') - .mockResolvedValueOnce({ stdout: '', stderr: '' }); + const getComposeFileAsObjectSpy = vi + .spyOn(trigger, 'getComposeFileAsObject') + .mockResolvedValue(makeCompose({ base: { image: 'busybox:1.0.0' } })); await trigger.validateComposeConfiguration( '/opt/drydock/test/stack.override.yml', @@ -2628,21 +2409,7 @@ describe('Dockercompose Trigger', () => { }, ); - expect(executeCommandSpy).toHaveBeenCalledWith( - 'docker', - [ - 'compose', - '-f', - '/opt/drydock/test/stack.yml', - '-f', - expect.stringContaining('.stack.override.yml.validate-'), - 'config', - '--quiet', - ], - expect.objectContaining({ - cwd: '/opt/drydock/test', - }), - ); + expect(getComposeFileAsObjectSpy).toHaveBeenCalledWith('/opt/drydock/test/stack.yml'); }); test('updateComposeServiceImageInText should throw when compose document has parse errors', () => { @@ -3583,7 +3350,7 @@ describe('Dockercompose Trigger', () => { expect(parseDocumentSpy).toHaveBeenCalledTimes(1); }); - test('processComposeFile should batch compose pull/up by file when composeFileOnce mode is enabled', async () => { + test('processComposeFile should skip compose-file-once batching in Docker Engine API mode', async () => { trigger.configuration.dryrun = false; trigger.configuration.prune = false; trigger.configuration.composeFileOnce = true; @@ -3618,7 +3385,9 @@ describe('Dockercompose Trigger', () => { ), ); vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); + const runContainerUpdateLifecycleSpy = vi + .spyOn(trigger, 'runContainerUpdateLifecycle') + .mockResolvedValue(); vi.spyOn(trigger, 'maybeScanAndGateUpdate').mockResolvedValue(); vi.spyOn(trigger, 'runPreUpdateHook').mockResolvedValue(); vi.spyOn(trigger, 'runPostUpdateHook').mockResolvedValue(); @@ -3630,18 +3399,9 @@ describe('Dockercompose Trigger', () => { redisContainer, ]); - expect(runComposeCommandSpy).toHaveBeenCalledTimes(2); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx', 'redis'], - expect.anything(), - ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 2, - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', 'nginx', 'redis'], - expect.anything(), + expect(runContainerUpdateLifecycleSpy).toHaveBeenCalledTimes(2); + expect(mockLog.info).toHaveBeenCalledWith( + expect.stringContaining('can be re-implemented via Docker Engine API later'), ); }); @@ -4464,91 +4224,15 @@ describe('Dockercompose Trigger', () => { expect(composeFile).toBe('/opt/drydock/test/stack.yml'); }); - test('validateComposeConfiguration should throw a compose-command error for non-fallback failures', async () => { - vi.spyOn(trigger, 'executeCommand').mockRejectedValueOnce(new Error('invalid compose file')); - + test('validateComposeConfiguration should throw when the updated compose text is invalid YAML', async () => { await expect( trigger.validateComposeConfiguration( '/opt/drydock/test/compose.yml', - 'services:\n nginx:\n image: nginx:broken\n', + 'services:\n nginx: [\n', ), ).rejects.toThrow('Error when validating compose configuration'); }); - test('updateComposeServicesWithCompose should return immediately when no services are provided', async () => { - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - - await trigger.updateComposeServicesWithCompose('/opt/drydock/test/stack.yml', []); - - expect(runComposeCommandSpy).not.toHaveBeenCalled(); - }); - - test('updateComposeServicesWithCompose should no-op in dry-run mode', async () => { - trigger.configuration.dryrun = true; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - - await trigger.updateComposeServicesWithCompose('/opt/drydock/test/stack.yml', ['nginx']); - - expect(runComposeCommandSpy).not.toHaveBeenCalled(); - expect(mockLog.info).toHaveBeenCalledWith(expect.stringContaining('dry-run mode is enabled')); - }); - - test('updateComposeServicesWithCompose should run chain-aware pull/up for started and stopped services', async () => { - trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - const composeFiles = ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml']; - - await trigger.updateComposeServicesWithCompose( - '/opt/drydock/test/stack.yml', - ['nginx', 'redis'], - { - composeFiles, - serviceRunningStates: new Map([ - ['nginx', true], - ['redis', false], - ]), - }, - ); - - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx', 'redis'], - expect.anything(), - composeFiles, - ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 2, - '/opt/drydock/test/stack.yml', - ['up', '-d', '--no-deps', 'nginx'], - expect.anything(), - composeFiles, - ); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 3, - '/opt/drydock/test/stack.yml', - ['up', '--no-start', '--no-deps', 'redis'], - expect.anything(), - composeFiles, - ); - }); - - test('updateComposeServicesWithCompose should keep stopped services stopped without compose-file chain', async () => { - trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - - await trigger.updateComposeServicesWithCompose('/opt/drydock/test/stack.yml', ['nginx'], { - serviceRunningStates: new Map([['nginx', false]]), - }); - - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 2, - '/opt/drydock/test/stack.yml', - ['up', '--no-start', '--no-deps', 'nginx'], - expect.anything(), - ); - }); - test('mutateComposeFile should validate compose chain when multiple compose files are provided', async () => { vi.spyOn(trigger, 'getComposeFile').mockResolvedValue( Buffer.from('services:\n nginx:\n image: nginx:1.0.0\n'), @@ -4606,16 +4290,52 @@ describe('Dockercompose Trigger', () => { ); }); - test('executeSelfUpdate should pass compose chain to compose refresh when multiple files are present', async () => { + test('performContainerUpdate should skip per-service refresh when compose-file-once is already applied', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'nginx', + }); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + const hooksSpy = vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + + const updated = await trigger.performContainerUpdate({} as any, container as any, mockLog, { + composeFile: '/opt/drydock/test/stack.yml', + service: 'nginx', + serviceDefinition: {}, + composeFileOnceApplied: true, + } as any); + + expect(updated).toBe(true); + expect(updateContainerWithComposeSpy).not.toHaveBeenCalled(); + expect(hooksSpy).toHaveBeenCalledWith(container, 'nginx', {}); + expect(mockLog.info).toHaveBeenCalledWith( + expect.stringContaining('Skip per-service compose refresh for nginx'), + ); + }); + + test('executeSelfUpdate should forward operation id to parent self-update transition', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ name: 'drydock', imageName: 'codeswhat/drydock', }); + const currentContainer = makeDockerContainerHandle(); + const currentContainerSpec = { + Id: 'current-id', + Name: '/drydock', + State: { Running: true }, + HostConfig: { + Binds: ['/var/run/docker.sock:/var/run/docker.sock'], + }, + }; + vi.spyOn(trigger, 'getCurrentContainer').mockResolvedValue(currentContainer); + vi.spyOn(trigger, 'inspectContainer').mockResolvedValue(currentContainerSpec as any); + const executeSpy = vi.spyOn(trigger.selfUpdateOrchestrator, 'execute').mockResolvedValue(true); const updateContainerWithComposeSpy = vi .spyOn(trigger, 'updateContainerWithCompose') .mockResolvedValue(); - vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); const updated = await trigger.executeSelfUpdate( { @@ -4628,7 +4348,7 @@ describe('Dockercompose Trigger', () => { }, container, mockLog, - undefined, + 'op-self-update-123', { composeFile: '/opt/drydock/test/stack.override.yml', composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], @@ -4638,17 +4358,19 @@ describe('Dockercompose Trigger', () => { ); expect(updated).toBe(true); - expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.override.yml', - 'drydock', + expect(executeSpy).toHaveBeenCalledWith( + expect.objectContaining({ + currentContainer, + currentContainerSpec, + }), container, - { - composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], - }, + mockLog, + 'op-self-update-123', ); + expect(updateContainerWithComposeSpy).not.toHaveBeenCalled(); }); - test('processComposeFile should fetch running state once per service in compose-file-once mode', async () => { + test('processComposeFile should bypass running-state batching in compose-file-once mode for Docker Engine API runtime', async () => { trigger.configuration.dryrun = false; trigger.configuration.composeFileOnce = true; const firstContainer = makeContainer({ @@ -4669,16 +4391,16 @@ describe('Dockercompose Trigger', () => { Buffer.from(['services:', ' nginx:', ' image: nginx:1.0.0', ''].join('\n')), ); vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); - const runningStateSpy = vi.spyOn(trigger, 'getContainerRunningState').mockResolvedValue(true); - vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); - vi.spyOn(trigger, 'runContainerUpdateLifecycle').mockResolvedValue(); + const runContainerUpdateLifecycleSpy = vi + .spyOn(trigger, 'runContainerUpdateLifecycle') + .mockResolvedValue(); await trigger.processComposeFile('/opt/drydock/test/stack.yml', [ firstContainer, secondContainer, ]); - expect(runningStateSpy).toHaveBeenCalledTimes(1); + expect(runContainerUpdateLifecycleSpy).toHaveBeenCalledTimes(2); }); test('preview should passthrough base preview errors without compose metadata', async () => { @@ -4758,9 +4480,9 @@ describe('Dockercompose Trigger', () => { } }); - test('updateContainerWithCompose should pass compose file chain to pull command', async () => { + test('updateContainerWithCompose should use Docker API pull regardless of compose file chain', async () => { trigger.configuration.dryrun = false; - const runComposeCommandSpy = vi.spyOn(trigger, 'runComposeCommand').mockResolvedValue(); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); const composeFiles = ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml']; const container = makeContainer({ name: 'nginx', @@ -4772,13 +4494,7 @@ describe('Dockercompose Trigger', () => { skipPull: false, }); - expect(runComposeCommandSpy).toHaveBeenNthCalledWith( - 1, - '/opt/drydock/test/stack.yml', - ['pull', 'nginx'], - expect.anything(), - composeFiles, - ); + expect(pullImageSpy).toHaveBeenCalledTimes(1); }); test('recreateContainer should include compose file chain when compose service is defined in overrides', async () => { @@ -4834,9 +4550,9 @@ describe('Dockercompose Trigger', () => { }); test('validateComposeConfiguration should append target compose file when compose chain omits it', async () => { - const executeCommandSpy = vi - .spyOn(trigger, 'executeCommand') - .mockResolvedValueOnce({ stdout: '', stderr: '' }); + const getComposeFileAsObjectSpy = vi + .spyOn(trigger, 'getComposeFileAsObject') + .mockResolvedValue(makeCompose({ base: { image: 'busybox:1.0.0' } })); await trigger.validateComposeConfiguration( '/opt/drydock/test/stack.override.yml', @@ -4846,20 +4562,6 @@ describe('Dockercompose Trigger', () => { }, ); - expect(executeCommandSpy).toHaveBeenCalledWith( - 'docker', - [ - 'compose', - '-f', - '/opt/drydock/test/stack.yml', - '-f', - expect.stringContaining('.stack.override.yml.validate-'), - 'config', - '--quiet', - ], - expect.objectContaining({ - cwd: '/opt/drydock/test', - }), - ); + expect(getComposeFileAsObjectSpy).toHaveBeenCalledWith('/opt/drydock/test/stack.yml'); }); }); diff --git a/app/triggers/providers/dockercompose/Dockercompose.ts b/app/triggers/providers/dockercompose/Dockercompose.ts index 95959772..6c19f6b4 100644 --- a/app/triggers/providers/dockercompose/Dockercompose.ts +++ b/app/triggers/providers/dockercompose/Dockercompose.ts @@ -1,18 +1,14 @@ -import { execFile } from 'node:child_process'; import { constants as fsConstants } from 'node:fs'; import fs from 'node:fs/promises'; import path from 'node:path'; import yaml, { type Pair, type ParsedNode } from 'yaml'; import type { ContainerImage } from '../../../model/container.js'; import { getState } from '../../../registry/index.js'; -import { buildComposeCommandEnvironment } from '../../../runtime/child-process-env.js'; import { resolveConfiguredPath, resolveConfiguredPathWithinBase } from '../../../runtime/paths.js'; import { sleep } from '../../../util/sleep.js'; import Docker from '../docker/Docker.js'; import ComposeFileLockManager from './ComposeFileLockManager.js'; -const COMPOSE_COMMAND_TIMEOUT_MS = 60_000; -const COMPOSE_COMMAND_MAX_BUFFER_BYTES = 10 * 1024 * 1024; const YAML_MAX_ALIAS_COUNT = 10_000; const COMPOSE_RENAME_MAX_RETRIES = 5; const COMPOSE_RENAME_RETRY_MS = 200; @@ -1084,148 +1080,25 @@ class Dockercompose extends Docker { const effectiveComposeFileChain = composeFileChain.includes(composeFilePath) ? composeFileChain : [...composeFileChain, composeFilePath]; - - const composeDirectory = path.dirname(composeFilePath); - const composeFileName = path.basename(composeFilePath); - const validationFilePath = path.join( - composeDirectory, - `.${composeFileName}.validate-${process.pid}-${Date.now()}-${Math.random().toString(16).slice(2)}`, - ); - - await fs.writeFile(validationFilePath, composeFileText); - const validationArguments = [ - ...effectiveComposeFileChain.flatMap((composeFile) => [ - '-f', - composeFile === composeFilePath ? validationFilePath : composeFile, - ]), - 'config', - '--quiet', - ]; - const commandsToTry = [ - { - command: 'docker', - args: ['compose', ...validationArguments], - label: 'docker compose', - }, - { - command: 'docker-compose', - args: validationArguments, - label: 'docker-compose', - }, - ]; - try { - for (const composeCommand of commandsToTry) { - try { - await this.executeCommand(composeCommand.command, composeCommand.args, { - cwd: composeDirectory, - env: buildComposeCommandEnvironment(), - }); - return; - } catch (e) { - const stderr = `${e?.stderr || ''}`; - const dockerComposePluginMissing = - composeCommand.command === 'docker' && - /docker: ['"]?compose['"]? is not a docker command/i.test(stderr); - const executableMissing = e?.code === 'ENOENT'; - - if ( - composeCommand.command === 'docker' && - (dockerComposePluginMissing || executableMissing) - ) { - this.log.warn( - `Cannot use docker compose for compose validation on ${composeFilePath} (${e.message}); trying docker-compose`, - ); - continue; - } - - throw new Error( - `Error when validating compose configuration for ${composeFilePath} using ${composeCommand.label} (${e.message})`, + const composeByFile = new Map(); + for (const composeFile of effectiveComposeFileChain) { + if (composeFile === composeFilePath) { + composeByFile.set( + composeFile, + yaml.parse(composeFileText, { + maxAliasCount: YAML_MAX_ALIAS_COUNT, + }), ); + continue; } + composeByFile.set(composeFile, await this.getComposeFileAsObject(composeFile)); } - } finally { - await this.cleanupComposeTemporaryFile(validationFilePath); - } - } - - async updateComposeServicesWithCompose(composeFile, services, options = {}) { - if (services.length === 0) { - return; - } - - const { composeFiles = [composeFile], serviceRunningStates = new Map() } = - options as { - composeFiles?: string[]; - serviceRunningStates?: Map; - }; - const composeFileChain = this.normalizeComposeFileChain(composeFile, composeFiles); - const runWithComposeFileChain = composeFileChain.length > 1; - - const logContainer = this.log.child({ - composeFile, - services, - }); - - if (this.configuration.dryrun) { - logContainer.info( - `Do not refresh compose services ${services.join(', ')} from ${composeFile} because dry-run mode is enabled`, + await this.getComposeFileChainAsObject(effectiveComposeFileChain, composeByFile); + } catch (e) { + throw new Error( + `Error when validating compose configuration for ${composeFilePath} (${e.message})`, ); - return; - } - - if (runWithComposeFileChain) { - await this.runComposeCommand( - composeFile, - ['pull', ...services], - logContainer, - composeFileChain, - ); - } else { - await this.runComposeCommand(composeFile, ['pull', ...services], logContainer); - } - - const servicesToStart: string[] = []; - const servicesToKeepStopped: string[] = []; - for (const service of services) { - if (serviceRunningStates.get(service) === false) { - servicesToKeepStopped.push(service); - } else { - servicesToStart.push(service); - } - } - - if (servicesToStart.length > 0) { - if (runWithComposeFileChain) { - await this.runComposeCommand( - composeFile, - ['up', '-d', '--no-deps', ...servicesToStart], - logContainer, - composeFileChain, - ); - } else { - await this.runComposeCommand( - composeFile, - ['up', '-d', '--no-deps', ...servicesToStart], - logContainer, - ); - } - } - if (servicesToKeepStopped.length > 0) { - if (runWithComposeFileChain) { - await this.runComposeCommand( - composeFile, - ['up', '--no-start', '--no-deps', ...servicesToKeepStopped], - logContainer, - composeFileChain, - ); - } else { - await this.runComposeCommand( - composeFile, - ['up', '--no-start', '--no-deps', ...servicesToKeepStopped], - logContainer, - ); - } } } @@ -1257,8 +1130,8 @@ class Dockercompose extends Docker { } /** - * Override: compose doesn't need to inspect the existing container - * (compose CLI handles the container lifecycle). Lighter context. + * Override: keep trigger context lightweight. Runtime container state is + * resolved on demand inside updateContainerWithCompose(). */ async createTriggerContext(container, logContainer, _composeContext) { const watcher = this.getWatcher(container); @@ -1277,7 +1150,8 @@ class Dockercompose extends Docker { } /** - * Override: use compose CLI for pull/recreate instead of Docker API. + * Override: apply compose-specific hooks while performing runtime refresh + * through the Docker Engine API. */ async performContainerUpdate(_context, container, _logContainer, composeCtx) { if (!composeCtx) { @@ -1329,10 +1203,11 @@ class Dockercompose extends Docker { } /** - * Self-update for compose-managed Drydock service. This must stay in compose - * lifecycle instead of Docker API recreate to preserve compose ownership. + * Self-update for compose-managed Drydock service. Delegate to the parent + * self-update transition so the helper container can enforce startup/health + * gates and rollback before retiring the old process. */ - async executeSelfUpdate(context, container, logContainer, _operationId, composeCtx) { + async executeSelfUpdate(context, container, logContainer, operationId, composeCtx) { if (!composeCtx) { throw new Error(`Missing compose context for self-update container ${container.name}`); } @@ -1342,20 +1217,16 @@ class Dockercompose extends Docker { return false; } - this.insertContainerImageBackup(context, container); - if (Array.isArray(composeCtx.composeFiles) && composeCtx.composeFiles.length > 1) { - await this.updateContainerWithCompose(composeCtx.composeFile, composeCtx.service, container, { - composeFiles: composeCtx.composeFiles, - }); - } else { - await this.updateContainerWithCompose(composeCtx.composeFile, composeCtx.service, container); - } - await this.runServicePostStartHooks( - container, - composeCtx.service, - composeCtx.serviceDefinition, - ); - return true; + const currentContainer = await this.getCurrentContainer(context.dockerApi, container); + const currentContainerSpec = await this.inspectContainer(currentContainer, logContainer); + + const selfUpdateContext = { + ...context, + currentContainer, + currentContainerSpec, + }; + + return super.executeSelfUpdate(selfUpdateContext, container, logContainer, operationId); } /** @@ -1572,34 +1443,15 @@ class Dockercompose extends Docker { } } - let composeFileOnceHandledServices = new Set(); + const composeFileOnceHandledServices = new Set(); if ( this.configuration.composeFileOnce === true && this.configuration.dryrun !== true && mappingsNeedingRuntimeUpdate.length > 1 ) { - const serviceRunningStates = new Map(); - const servicesToRefresh: string[] = []; - for (const { container, service } of mappingsNeedingRuntimeUpdate) { - if (serviceRunningStates.has(service)) { - continue; - } - const runningStateLogger = this.log.child({ - container: container.name, - }); - serviceRunningStates.set( - service, - await this.getContainerRunningState(container, runningStateLogger), - ); - servicesToRefresh.push(service); - } - await this.updateComposeServicesWithCompose(composeFile, servicesToRefresh, { - composeFiles: composeFileChain, - serviceRunningStates, - }); - composeFileOnceHandledServices = new Set(servicesToRefresh); + // TODO: Re-introduce compose-file-once batching with Docker Engine API primitives. this.log.info( - `Compose-file-once mode refreshed ${servicesToRefresh.length} service(s) for ${composeFileChainSummary}`, + `Compose-file-once mode is unavailable in Docker Engine API runtime mode; refreshing containers individually for ${composeFileChainSummary}. This can be re-implemented via Docker Engine API later.`, ); } @@ -1617,116 +1469,6 @@ class Dockercompose extends Docker { } } - async executeCommand(command, args, options = {}) { - return new Promise((resolve, reject) => { - execFile( - command, - args, - { - ...options, - timeout: COMPOSE_COMMAND_TIMEOUT_MS, - maxBuffer: COMPOSE_COMMAND_MAX_BUFFER_BYTES, - }, - (error, stdout, stderr) => { - if (error) { - error.stdout = stdout; - error.stderr = stderr; - reject(error); - return; - } - resolve({ - stdout: stdout || '', - stderr: stderr || '', - }); - }, - ); - }); - } - - async runComposeCommand(composeFile, composeArgs, logContainer, composeFiles = [composeFile]) { - const composeFileChain = this.normalizeComposeFileChain(composeFile, composeFiles); - const composeFilePaths = composeFileChain.map((composeFilePathToResolve) => - this.resolveComposeFilePath(composeFilePathToResolve), - ); - const composeFileArgs = composeFilePaths.flatMap((composeFilePath) => ['-f', composeFilePath]); - const composeWorkingDirectory = path.dirname(composeFilePaths[0]); - const composeFilePathSummary = composeFilePaths.join(', '); - const commandsToTry = [ - { - command: 'docker', - args: ['compose', ...composeFileArgs, ...composeArgs], - label: 'docker compose', - }, - { - command: 'docker-compose', - args: [...composeFileArgs, ...composeArgs], - label: 'docker-compose', - }, - ]; - - for (const composeCommand of commandsToTry) { - try { - const { stdout, stderr } = (await this.executeCommand( - composeCommand.command, - composeCommand.args, - { - cwd: composeWorkingDirectory, - env: buildComposeCommandEnvironment(), - }, - )) as { stdout: string; stderr: string }; - if (stdout.trim()) { - logContainer.debug( - `${composeCommand.label} ${composeArgs.join(' ')} stdout:\n${stdout.trim()}`, - ); - } - if (stderr.trim()) { - logContainer.debug( - `${composeCommand.label} ${composeArgs.join(' ')} stderr:\n${stderr.trim()}`, - ); - } - return; - } catch (e) { - const stderr = `${e?.stderr || ''}`; - const dockerComposePluginMissing = - composeCommand.command === 'docker' && - /docker: ['"]?compose['"]? is not a docker command/i.test(stderr); - const executableMissing = e?.code === 'ENOENT'; - - if ( - composeCommand.command === 'docker' && - (dockerComposePluginMissing || executableMissing) - ) { - logContainer.warn( - `Cannot use docker compose for ${composeFilePathSummary} (${e.message}); trying docker-compose`, - ); - continue; - } - - throw new Error( - `Error when running ${composeCommand.label} ${composeArgs.join(' ')} for ${composeFilePathSummary} (${e.message})`, - ); - } - } - } - - async getContainerRunningState(container, logContainer) { - try { - const watcher = this.getWatcher(container); - const dockerApi = getDockerApiFromWatcher(watcher); - if (!dockerApi) { - return true; - } - const containerToInspect = dockerApi.getContainer(container.name); - const containerState = await containerToInspect.inspect(); - return containerState?.State?.Running !== false; - } catch (e) { - logContainer.warn( - `Unable to inspect running state for ${container.name}; assuming running (${e.message})`, - ); - return true; - } - } - async resolveComposeServiceContext(container, currentImage) { const composeFiles = await this.resolveComposeFilesForContainer(container); if (composeFiles.length === 0) { @@ -1828,11 +1570,6 @@ class Dockercompose extends Docker { forceRecreate?: boolean; composeFiles?: string[]; }; - const composeFileChain = this.normalizeComposeFileChain( - composeFile, - (options as { composeFiles?: string[] })?.composeFiles, - ); - const runWithComposeFileChain = composeFileChain.length > 1; if (this.configuration.dryrun) { logContainer.info( @@ -1841,43 +1578,57 @@ class Dockercompose extends Docker { return; } + const watcher = this.getWatcher(container); + const { dockerApi } = watcher; + const registry = this.resolveRegistryManager(container, logContainer, { + allowAnonymousFallback: true, + }); + const auth = await registry.getAuthPull(); + const newImage = this.getNewImageFullName(registry, container); + const currentContainer = await this.getCurrentContainer(dockerApi, container); + if (!currentContainer) { + throw new Error( + `Unable to refresh compose service ${service} from ${composeFile} because container ${container.name} no longer exists`, + ); + } + const currentContainerSpec = await this.inspectContainer(currentContainer, logContainer); const serviceShouldStart = - shouldStart !== undefined - ? shouldStart - : await this.getContainerRunningState(container, logContainer); + shouldStart !== undefined ? shouldStart : currentContainerSpec?.State?.Running !== false; - logContainer.info(`Refresh compose service ${service} from ${composeFile}`); + logContainer.info( + `Refresh compose service ${service} from ${composeFile} using Docker Engine API`, + ); if (!skipPull) { - if (runWithComposeFileChain) { - await this.runComposeCommand( - composeFile, - ['pull', service], - logContainer, - composeFileChain, - ); - } else { - await this.runComposeCommand(composeFile, ['pull', service], logContainer); - } + await this.pullImage(dockerApi, auth, newImage, logContainer); } else { - logContainer.debug(`Skip compose pull for ${service} from ${composeFile}`); + logContainer.debug(`Skip image pull for ${service} from ${composeFile}`); } - - const upArgs = ['up']; - if (serviceShouldStart) { - upArgs.push('-d'); - } else { - upArgs.push('--no-start'); - } - upArgs.push('--no-deps'); if (forceRecreate) { - upArgs.push('--force-recreate'); - } - upArgs.push(service); - if (runWithComposeFileChain) { - await this.runComposeCommand(composeFile, upArgs, logContainer, composeFileChain); - } else { - await this.runComposeCommand(composeFile, upArgs, logContainer); + logContainer.debug( + `Force recreate requested for ${service}; Docker Engine API path always recreates containers`, + ); } + + const recreationContainerSpec = { + ...currentContainerSpec, + State: { + ...currentContainerSpec?.State, + Running: serviceShouldStart, + }, + }; + await super.stopAndRemoveContainer( + currentContainer, + currentContainerSpec, + container, + logContainer, + ); + await super.recreateContainer( + dockerApi, + recreationContainerSpec, + newImage, + container, + logContainer, + ); } async stopAndRemoveContainer(_currentContainer, _currentContainerSpec, container, logContainer) { From 1a2164c746759eac3658beb4d8c551d4d09ce1f7 Mon Sep 17 00:00:00 2001 From: superuserjr <80784472+turbodaemon@users.noreply.github.com> Date: Mon, 9 Mar 2026 20:17:16 -0400 Subject: [PATCH 02/34] =?UTF-8?q?=F0=9F=90=9B=20fix(api):=20include=20dock?= =?UTF-8?q?ercompose=20in=20manual=20update=20trigger=20search?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The updateContainer endpoint only searched for type 'docker' triggers, missing dockercompose triggers entirely. Add triggerTypes option to match the preview endpoint behavior. --- app/api/container-actions.test.ts | 28 ++++++++++++++++++++++++++++ app/api/container-actions.ts | 4 +++- 2 files changed, 31 insertions(+), 1 deletion(-) diff --git a/app/api/container-actions.test.ts b/app/api/container-actions.test.ts index 034ac37d..42c805b5 100644 --- a/app/api/container-actions.test.ts +++ b/app/api/container-actions.test.ts @@ -460,6 +460,34 @@ describe('Container Actions Router', () => { ); }); + test('should update container successfully with a dockercompose trigger', async () => { + const container = { + id: 'c1', + name: 'nginx', + image: { name: 'nginx' }, + updateAvailable: true, + }; + const updatedContainer = { ...container, image: { name: 'nginx:latest' } }; + mockGetContainer.mockReturnValueOnce(container).mockReturnValueOnce(updatedContainer); + const mockTriggerFn = vi.fn().mockResolvedValue(undefined); + const trigger = { type: 'dockercompose', trigger: mockTriggerFn }; + mockGetState.mockReturnValue({ trigger: { 'dockercompose.default': trigger } }); + + const handler = getHandler('post', '/:id/update'); + const req = createMockRequest({ params: { id: 'c1' } }); + const res = createMockResponse(); + await handler(req, res); + + expect(mockTriggerFn).toHaveBeenCalledWith(container); + expect(res.status).toHaveBeenCalledWith(200); + expect(res.json).toHaveBeenCalledWith( + expect.objectContaining({ + message: 'Container updated successfully', + result: updatedContainer, + }), + ); + }); + test('should return 404 when container not found', async () => { mockGetContainer.mockReturnValue(undefined); diff --git a/app/api/container-actions.ts b/app/api/container-actions.ts index a1863d09..7bb8778c 100644 --- a/app/api/container-actions.ts +++ b/app/api/container-actions.ts @@ -163,7 +163,9 @@ async function updateContainer(req: Request, res: Response) { return; } - const trigger = findDockerTriggerForContainer(registry.getState().trigger, container); + const trigger = findDockerTriggerForContainer(registry.getState().trigger, container, { + triggerTypes: ['docker', 'dockercompose'], + }); if (!trigger) { sendErrorResponse(res, 404, NO_DOCKER_TRIGGER_FOUND_ERROR); return; From f139817b78b8ef68e2be7b62ab372e5d9cff2141 Mon Sep 17 00:00:00 2001 From: s-b-e-n-s-o-n <80784472+s-b-e-n-s-o-n@users.noreply.github.com> Date: Mon, 9 Mar 2026 17:51:21 -0400 Subject: [PATCH 03/34] =?UTF-8?q?=F0=9F=90=9B=20fix(ci):=20retry=20qlty=20?= =?UTF-8?q?on=20timeout=20and=20increase=20limit=20to=208=20minutes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit retry_on was set to "error" which only retries on non-zero exit codes, not on timeouts. Changed to "any" so the 5-minute timeout actually triggers a retry. Also bumped timeout from 5 to 8 minutes since qlty regularly takes longer in CI. --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 88bebdf1..27c8cac0 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -78,9 +78,9 @@ jobs: - name: Qlty check (all plugins) uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 # v3.0.2 with: - timeout_minutes: 5 + timeout_minutes: 8 max_attempts: 3 - retry_on: error + retry_on: any command: qlty check --all --no-progress test: From 3945d2c11d968c9c9f0b8f8dd79b44e61e540188 Mon Sep 17 00:00:00 2001 From: superuserjr <80784472+turbodaemon@users.noreply.github.com> Date: Mon, 9 Mar 2026 20:19:01 -0400 Subject: [PATCH 04/34] =?UTF-8?q?=F0=9F=93=9D=20docs(changelog):=20add=20c?= =?UTF-8?q?ompose=20Engine=20API=20and=20trigger=20search=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d60db55c..5f36b5e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -91,6 +91,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **Compose trigger uses Docker Engine API** — Compose-managed container updates now use the Docker Engine API directly (pull, stop, recreate) instead of shelling out to `docker compose` / `docker-compose` CLI. Eliminates `spawn docker ENOENT` errors in environments without Docker CLI binaries installed. +- **Compose self-update delegates to parent orchestrator** — Self-update for compose-managed Drydock containers now uses the parent Docker trigger's helper-container transition with health gates and rollback, instead of direct stop/recreate. - **Self-update controller testable entrypoint** — Extracted process-level entry logic to a separate entrypoint module, making the controller independently testable without triggering process-exit side effects. - **Dockercompose YAML patching simplified** — Removed redundant type guards and dead-code branches from compose file patching helpers, reducing code paths and improving maintainability. - **Dashboard fetches recent-status from backend** — Dashboard now fetches pre-computed container statuses from `/api/containers/recent-status` instead of scanning the raw audit log client-side. @@ -118,6 +120,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Manual update button works with compose triggers** — The update container endpoint now searches for both `docker` and `dockercompose` trigger types, matching the existing preview endpoint behavior. Previously, users with only a compose trigger saw "No docker trigger found for this container". +- **CI: qlty retry on timeout** — Changed `retry_on` from `error` to `any` so qlty timeouts trigger retries. Increased timeout from 5 to 8 minutes. - **Compose trigger rejects compose files mounted outside app directory** — Removed overly strict working-directory boundary enforcement from `runComposeCommand` that rejected compose files bind-mounted outside `/home/node/app`, breaking documented mount patterns like `/drydock/docker-compose.yml`. Compose file paths are operator-configured and already validated during resolution. - **Compose trigger uses host paths instead of container paths** — Docker label auto-detection (`com.docker.compose.project.config_files`) now remaps host-side paths to container-internal paths using Drydock's own bind mount information. Previously, host paths like `/mnt/volume1/docker/stacks/monitoring/compose.yaml` were used directly inside the container where they don't exist, causing "does not exist" errors even when the file was properly mounted. - **Compose trigger logs spurious warnings for unrelated containers** — When multiple compose triggers are configured, each trigger now silently skips containers whose resolved compose files don't match the trigger's configured `FILE` path, eliminating noisy cross-stack "does not exist" warnings. From 25413d52d7a6c820d8e37e90ec1d64ba48d978cc Mon Sep 17 00:00:00 2001 From: superuserjr <80784472+turbodaemon@users.noreply.github.com> Date: Mon, 9 Mar 2026 21:20:22 -0400 Subject: [PATCH 05/34] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20refactor(compose):?= =?UTF-8?q?=20extract=20shared=20runtime=20refresh=20and=20re-enable=20com?= =?UTF-8?q?poseFileOnce?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract refreshComposeServiceWithDockerApi() as shared private helper called by both updateContainerWithCompose() and recreateContainer(), eliminating the recursive call chain - Add ensureComposeRuntimeState() guard that throws when inspect data is missing State.Running - Re-enable composeFileOnce batch mode: first container per service gets a full runtime refresh, subsequent containers for the same service skip the refresh via composeFileOnceApplied flag - Add error-path tests for pullImage, stopAndRemoveContainer, recreateContainer failures, and malformed inspect data --- .../dockercompose/Dockercompose.test.ts | 486 +++++++++++++++++- .../providers/dockercompose/Dockercompose.ts | 214 ++++++-- 2 files changed, 640 insertions(+), 60 deletions(-) diff --git a/app/triggers/providers/dockercompose/Dockercompose.test.ts b/app/triggers/providers/dockercompose/Dockercompose.test.ts index fbcdb16b..e11d2042 100644 --- a/app/triggers/providers/dockercompose/Dockercompose.test.ts +++ b/app/triggers/providers/dockercompose/Dockercompose.test.ts @@ -477,6 +477,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/portainer.yml', 'portainer', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -509,11 +514,21 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', tagContainer, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); expect(composeUpdateSpy).toHaveBeenCalledWith( '/opt/drydock/test/stack.yml', 'redis', digestContainer, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -542,6 +557,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'redis', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -571,6 +591,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'redis', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -598,6 +623,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -657,6 +687,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'filebrowser', container, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -1078,6 +1113,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', container1, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -1115,6 +1155,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', containerInProject, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -1224,6 +1269,11 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', 'nginx', container1, + expect.objectContaining({ + runtimeContext: expect.objectContaining({ + dockerApi: mockDockerApi, + }), + }), ); }); @@ -1267,7 +1317,7 @@ describe('Dockercompose Trigger', () => { trigger.configuration.dryrun = false; const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); const startContainerSpy = vi.spyOn(trigger, 'startContainer').mockResolvedValue(); - mockDockerApi.getContainer.mockReturnValueOnce( + vi.spyOn(trigger, 'getCurrentContainer').mockResolvedValue( makeDockerContainerHandle({ running: false, }), @@ -1313,6 +1363,32 @@ describe('Dockercompose Trigger', () => { expect(pullImageSpy).not.toHaveBeenCalled(); }); + test('updateContainerWithCompose should reuse runtime context without resolving registry manager', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const resolveRegistryManagerSpy = vi.spyOn(trigger, 'resolveRegistryManager'); + const getWatcherSpy = vi.spyOn(trigger, 'getWatcher'); + const runtimeContext = { + dockerApi: mockDockerApi, + auth: { from: 'context' }, + newImage: 'nginx:9.9.9', + }; + + await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container, { + runtimeContext, + }); + + expect(resolveRegistryManagerSpy).not.toHaveBeenCalled(); + expect(getWatcherSpy).not.toHaveBeenCalled(); + expect(pullImageSpy).toHaveBeenCalledWith( + runtimeContext.dockerApi, + runtimeContext.auth, + runtimeContext.newImage, + expect.anything(), + ); + }); + test('updateContainerWithCompose should throw when current container cannot be resolved', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ name: 'nginx' }); @@ -1325,6 +1401,66 @@ describe('Dockercompose Trigger', () => { ); }); + test('updateContainerWithCompose should surface pullImage failures and stop before recreation', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'pullImage').mockRejectedValue(new Error('pull failed')); + const stopContainerSpy = vi.spyOn(trigger, 'stopContainer').mockResolvedValue(); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow('pull failed'); + + expect(stopContainerSpy).not.toHaveBeenCalled(); + expect(createContainerSpy).not.toHaveBeenCalled(); + }); + + test('updateContainerWithCompose should surface stopAndRemoveContainer failures and skip recreation', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + vi.spyOn(trigger, 'stopContainer').mockRejectedValue(new Error('stop failed')); + const createContainerSpy = vi.spyOn(trigger, 'createContainer').mockResolvedValue({ + start: vi.fn().mockResolvedValue(undefined), + } as any); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow('stop failed'); + + expect(createContainerSpy).not.toHaveBeenCalled(); + }); + + test('updateContainerWithCompose should surface recreateContainer failures', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + vi.spyOn(trigger, 'stopContainer').mockResolvedValue(); + vi.spyOn(trigger, 'removeContainer').mockResolvedValue(); + vi.spyOn(trigger, 'createContainer').mockRejectedValue(new Error('create failed')); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow('create failed'); + }); + + test('updateContainerWithCompose should throw when inspectContainer returns malformed runtime state', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + vi.spyOn(trigger, 'inspectContainer').mockResolvedValue({ + Config: { Image: 'nginx:1.0.0' }, + } as any); + + await expect( + trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container), + ).rejects.toThrow( + 'Unable to refresh compose service nginx from /opt/drydock/test/stack.yml because Docker inspection data is missing runtime state', + ); + }); + test('stopAndRemoveContainer should be a no-op with compose lifecycle log', async () => { await trigger.stopAndRemoveContainer({}, {}, { name: 'nginx' }, mockLog); @@ -1333,7 +1469,7 @@ describe('Dockercompose Trigger', () => { ); }); - test('recreateContainer should rewrite compose service image and recreate via compose lifecycle', async () => { + test('recreateContainer should rewrite compose service image without routing through updateContainerWithCompose', async () => { const container = makeContainer({ name: 'nginx', labels: { @@ -1350,7 +1486,7 @@ describe('Dockercompose Trigger', () => { ].join('\n'); vi.spyOn(trigger, 'getComposeFile').mockResolvedValue(Buffer.from(composeFileContent)); const writeComposeFileSpy = vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); - const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose').mockResolvedValue(); + const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose'); vi.spyOn(trigger, 'getComposeFileAsObject').mockResolvedValue( makeCompose({ nginx: { image: 'nginx:1.1.0' } }), ); @@ -1374,16 +1510,7 @@ describe('Dockercompose Trigger', () => { '/opt/drydock/test/stack.yml', expect.stringContaining('# existing comment'), ); - expect(composeUpdateSpy).toHaveBeenCalledWith( - '/opt/drydock/test/stack.yml', - 'nginx', - container, - { - shouldStart: false, - skipPull: true, - forceRecreate: true, - }, - ); + expect(composeUpdateSpy).not.toHaveBeenCalled(); }); test('recreateContainer should fallback to registry-derived image when current spec image is missing', async () => { @@ -1524,6 +1651,135 @@ describe('Dockercompose Trigger', () => { expect(hooksSpy).not.toHaveBeenCalled(); }); + test('executeSelfUpdate should reuse current container and inspection from context when available', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'drydock', + imageName: 'codeswhat/drydock', + labels: { + 'dd.compose.file': '/opt/drydock/test/stack.yml', + 'com.docker.compose.service': 'drydock', + }, + }); + const composeContext = { + composeFile: '/opt/drydock/test/stack.yml', + service: 'drydock', + serviceDefinition: {}, + }; + const currentContainer = makeDockerContainerHandle({ id: 'context-container-id' }); + const currentContainerSpec = { + Id: 'context-id', + Name: '/drydock', + State: { Running: true }, + HostConfig: { + Binds: ['/var/run/docker.sock:/var/run/docker.sock'], + }, + }; + + const getCurrentContainerSpy = vi + .spyOn(trigger, 'getCurrentContainer') + .mockResolvedValue(makeDockerContainerHandle({ id: 'fetched-id' })); + const inspectContainerSpy = vi.spyOn(trigger, 'inspectContainer').mockResolvedValue({ + Id: 'fetched-id', + State: { Running: true }, + } as any); + const orchestratorExecuteSpy = vi + .spyOn(trigger.selfUpdateOrchestrator, 'execute') + .mockResolvedValue(true); + + const updated = await trigger.executeSelfUpdate( + { + dockerApi: mockDockerApi, + registry: getState().registry.hub, + auth: {}, + newImage: 'codeswhat/drydock:1.1.0', + currentContainer, + currentContainerSpec, + }, + container, + mockLog, + 'op-self-update-context', + composeContext, + ); + + expect(updated).toBe(true); + expect(getCurrentContainerSpy).not.toHaveBeenCalled(); + expect(inspectContainerSpy).not.toHaveBeenCalled(); + expect(orchestratorExecuteSpy).toHaveBeenCalledWith( + expect.objectContaining({ + currentContainer, + currentContainerSpec, + }), + container, + mockLog, + 'op-self-update-context', + ); + }); + + test('executeSelfUpdate should inspect context current container when inspection is missing', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'drydock', + imageName: 'codeswhat/drydock', + labels: { + 'dd.compose.file': '/opt/drydock/test/stack.yml', + 'com.docker.compose.service': 'drydock', + }, + }); + const composeContext = { + composeFile: '/opt/drydock/test/stack.yml', + service: 'drydock', + serviceDefinition: {}, + }; + const currentContainer = makeDockerContainerHandle({ id: 'context-container-id' }); + const currentContainerSpec = { + Id: 'context-inspected-id', + Name: '/drydock', + State: { Running: true }, + HostConfig: { + Binds: ['/var/run/docker.sock:/var/run/docker.sock'], + }, + }; + + const getCurrentContainerSpy = vi + .spyOn(trigger, 'getCurrentContainer') + .mockResolvedValue(makeDockerContainerHandle({ id: 'fetched-id' })); + const inspectContainerSpy = vi + .spyOn(trigger, 'inspectContainer') + .mockResolvedValue(currentContainerSpec as any); + const orchestratorExecuteSpy = vi + .spyOn(trigger.selfUpdateOrchestrator, 'execute') + .mockResolvedValue(true); + + const updated = await trigger.executeSelfUpdate( + { + dockerApi: mockDockerApi, + registry: getState().registry.hub, + auth: {}, + newImage: 'codeswhat/drydock:1.1.0', + currentContainer, + currentContainerSpec: null, + }, + container, + mockLog, + undefined, + composeContext, + ); + + expect(updated).toBe(true); + expect(getCurrentContainerSpy).not.toHaveBeenCalled(); + expect(inspectContainerSpy).toHaveBeenCalledWith(currentContainer, mockLog); + expect(orchestratorExecuteSpy).toHaveBeenCalledWith( + expect.objectContaining({ + currentContainer, + currentContainerSpec, + }), + container, + mockLog, + undefined, + ); + }); + test('performContainerUpdate should throw when compose context is missing', async () => { await expect( trigger.performContainerUpdate( @@ -3350,7 +3606,7 @@ describe('Dockercompose Trigger', () => { expect(parseDocumentSpy).toHaveBeenCalledTimes(1); }); - test('processComposeFile should skip compose-file-once batching in Docker Engine API mode', async () => { + test('processComposeFile should refresh each distinct service in compose-file-once mode', async () => { trigger.configuration.dryrun = false; trigger.configuration.prune = false; trigger.configuration.composeFileOnce = true; @@ -3400,8 +3656,91 @@ describe('Dockercompose Trigger', () => { ]); expect(runContainerUpdateLifecycleSpy).toHaveBeenCalledTimes(2); - expect(mockLog.info).toHaveBeenCalledWith( - expect.stringContaining('can be re-implemented via Docker Engine API later'), + expect(runContainerUpdateLifecycleSpy).toHaveBeenNthCalledWith( + 1, + nginxContainer, + expect.objectContaining({ + service: 'nginx', + composeFileOnceApplied: false, + }), + ); + expect(runContainerUpdateLifecycleSpy).toHaveBeenNthCalledWith( + 2, + redisContainer, + expect.objectContaining({ + service: 'redis', + composeFileOnceApplied: false, + }), + ); + }); + + test('processComposeFile should pre-pull distinct services and skip per-service pull in compose-file-once mode', async () => { + trigger.configuration.dryrun = false; + trigger.configuration.prune = false; + trigger.configuration.composeFileOnce = true; + + const nginxContainer = makeContainer({ + labels: { 'com.docker.compose.service': 'nginx' }, + }); + const redisContainer = makeContainer({ + name: 'redis', + imageName: 'redis', + tagValue: '7.0.0', + remoteValue: '7.1.0', + labels: { 'com.docker.compose.service': 'redis' }, + }); + + vi.spyOn(trigger, 'getComposeFileAsObject').mockResolvedValue( + makeCompose({ + nginx: { image: 'nginx:1.0.0' }, + redis: { image: 'redis:7.0.0' }, + }), + ); + vi.spyOn(trigger, 'getComposeFile').mockResolvedValue( + Buffer.from( + [ + 'services:', + ' nginx:', + ' image: nginx:1.0.0', + ' redis:', + ' image: redis:7.0.0', + '', + ].join('\n'), + ), + ); + vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + vi.spyOn(trigger, 'maybeScanAndGateUpdate').mockResolvedValue(); + vi.spyOn(trigger, 'runPreUpdateHook').mockResolvedValue(); + vi.spyOn(trigger, 'runPostUpdateHook').mockResolvedValue(); + vi.spyOn(trigger, 'cleanupOldImages').mockResolvedValue(); + vi.spyOn(trigger, 'maybeStartAutoRollbackMonitor').mockResolvedValue(); + + await trigger.processComposeFile('/opt/drydock/test/stack.yml', [ + nginxContainer, + redisContainer, + ]); + + expect(pullImageSpy).toHaveBeenCalledTimes(2); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.yml', + 'nginx', + nginxContainer, + expect.objectContaining({ + skipPull: true, + }), + ); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.yml', + 'redis', + redisContainer, + expect.objectContaining({ + skipPull: true, + }), ); }); @@ -4290,6 +4629,47 @@ describe('Dockercompose Trigger', () => { ); }); + test('performContainerUpdate should pass runtime context to per-service update when available', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'nginx', + }); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + const runtimeContext = { + dockerApi: mockDockerApi, + auth: { from: 'context' }, + newImage: 'nginx:9.9.9', + registry: getState().registry.hub, + }; + + const updated = await trigger.performContainerUpdate( + runtimeContext as any, + container as any, + mockLog, + { + composeFile: '/opt/drydock/test/stack.override.yml', + composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], + service: 'nginx', + serviceDefinition: {}, + composeFileOnceApplied: false, + } as any, + ); + + expect(updated).toBe(true); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.override.yml', + 'nginx', + container, + { + composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], + runtimeContext, + }, + ); + }); + test('performContainerUpdate should skip per-service refresh when compose-file-once is already applied', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ @@ -4370,7 +4750,7 @@ describe('Dockercompose Trigger', () => { expect(updateContainerWithComposeSpy).not.toHaveBeenCalled(); }); - test('processComposeFile should bypass running-state batching in compose-file-once mode for Docker Engine API runtime', async () => { + test('processComposeFile should mark repeated compose services as already refreshed in compose-file-once mode', async () => { trigger.configuration.dryrun = false; trigger.configuration.composeFileOnce = true; const firstContainer = makeContainer({ @@ -4401,6 +4781,72 @@ describe('Dockercompose Trigger', () => { ]); expect(runContainerUpdateLifecycleSpy).toHaveBeenCalledTimes(2); + expect(runContainerUpdateLifecycleSpy).toHaveBeenNthCalledWith( + 1, + firstContainer, + expect.objectContaining({ + service: 'nginx', + composeFileOnceApplied: false, + }), + ); + expect(runContainerUpdateLifecycleSpy).toHaveBeenNthCalledWith( + 2, + secondContainer, + expect.objectContaining({ + service: 'nginx', + composeFileOnceApplied: true, + }), + ); + }); + + test('processComposeFile should pre-pull once for repeated compose services in compose-file-once mode', async () => { + trigger.configuration.dryrun = false; + trigger.configuration.prune = false; + trigger.configuration.composeFileOnce = true; + const firstContainer = makeContainer({ + name: 'nginx-a', + labels: { 'com.docker.compose.service': 'nginx' }, + }); + const secondContainer = makeContainer({ + name: 'nginx-b', + labels: { 'com.docker.compose.service': 'nginx' }, + }); + + vi.spyOn(trigger, 'getComposeFileAsObject').mockResolvedValue( + makeCompose({ + nginx: { image: 'nginx:1.0.0' }, + }), + ); + vi.spyOn(trigger, 'getComposeFile').mockResolvedValue( + Buffer.from(['services:', ' nginx:', ' image: nginx:1.0.0', ''].join('\n')), + ); + vi.spyOn(trigger, 'writeComposeFile').mockResolvedValue(); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + vi.spyOn(trigger, 'maybeScanAndGateUpdate').mockResolvedValue(); + vi.spyOn(trigger, 'runPreUpdateHook').mockResolvedValue(); + vi.spyOn(trigger, 'runPostUpdateHook').mockResolvedValue(); + vi.spyOn(trigger, 'cleanupOldImages').mockResolvedValue(); + vi.spyOn(trigger, 'maybeStartAutoRollbackMonitor').mockResolvedValue(); + + await trigger.processComposeFile('/opt/drydock/test/stack.yml', [ + firstContainer, + secondContainer, + ]); + + expect(pullImageSpy).toHaveBeenCalledTimes(1); + expect(updateContainerWithComposeSpy).toHaveBeenCalledTimes(1); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.yml', + 'nginx', + firstContainer, + expect.objectContaining({ + skipPull: true, + }), + ); }); test('preview should passthrough base preview errors without compose metadata', async () => { @@ -4511,7 +4957,9 @@ describe('Dockercompose Trigger', () => { service: 'nginx', } as any); vi.spyOn(trigger, 'mutateComposeFile').mockResolvedValue(true); - const composeUpdateSpy = vi.spyOn(trigger, 'updateContainerWithCompose').mockResolvedValue(); + const refreshComposeServiceSpy = vi + .spyOn(trigger as any, 'refreshComposeServiceWithDockerApi') + .mockResolvedValue(); await trigger.recreateContainer( mockDockerApi, @@ -4524,7 +4972,7 @@ describe('Dockercompose Trigger', () => { mockLog, ); - expect(composeUpdateSpy).toHaveBeenCalledWith( + expect(refreshComposeServiceSpy).toHaveBeenCalledWith( '/opt/drydock/test/stack.override.yml', 'nginx', container, diff --git a/app/triggers/providers/dockercompose/Dockercompose.ts b/app/triggers/providers/dockercompose/Dockercompose.ts index 6c19f6b4..e32ca2cb 100644 --- a/app/triggers/providers/dockercompose/Dockercompose.ts +++ b/app/triggers/providers/dockercompose/Dockercompose.ts @@ -88,6 +88,19 @@ type RegistryImageContainerReference = { }; }; +type ComposeRuntimeRefreshOptions = { + shouldStart?: boolean; + skipPull?: boolean; + forceRecreate?: boolean; + composeFiles?: string[]; + runtimeContext?: { + dockerApi?: unknown; + auth?: unknown; + newImage?: string; + registry?: unknown; + }; +}; + function getDockerApiFromWatcher(watcher: unknown): DockerApiLike | undefined { if (!watcher || typeof watcher !== 'object') { return undefined; @@ -1130,10 +1143,28 @@ class Dockercompose extends Docker { } /** - * Override: keep trigger context lightweight. Runtime container state is - * resolved on demand inside updateContainerWithCompose(). + * Override: provide shared runtime dependencies once per lifecycle run. + * Runtime container state is still resolved on demand per service refresh. */ - async createTriggerContext(container, logContainer, _composeContext) { + async createTriggerContext(container, logContainer, composeContext) { + const runtimeContext = (composeContext as { runtimeContext?: unknown } | undefined) + ?.runtimeContext as ComposeRuntimeRefreshOptions['runtimeContext'] | undefined; + if ( + runtimeContext?.dockerApi && + runtimeContext?.registry && + runtimeContext?.auth !== undefined && + runtimeContext?.newImage + ) { + return { + dockerApi: runtimeContext.dockerApi, + registry: runtimeContext.registry, + auth: runtimeContext.auth, + newImage: runtimeContext.newImage, + currentContainer: null, + currentContainerSpec: null, + }; + } + const watcher = this.getWatcher(container); const { dockerApi } = watcher; const registry = getState().registry[container.image.registry.name]; @@ -1153,10 +1184,26 @@ class Dockercompose extends Docker { * Override: apply compose-specific hooks while performing runtime refresh * through the Docker Engine API. */ - async performContainerUpdate(_context, container, _logContainer, composeCtx) { + async performContainerUpdate(context, container, _logContainer, composeCtx) { if (!composeCtx) { throw new Error(`Missing compose context for container ${container.name}`); } + const composeRuntimeContext = (composeCtx as { runtimeContext?: unknown })?.runtimeContext as + | ComposeRuntimeRefreshOptions['runtimeContext'] + | undefined; + const runtimeContext = { + dockerApi: context?.dockerApi, + auth: context?.auth, + newImage: context?.newImage, + registry: context?.registry, + ...(composeRuntimeContext || {}), + }; + const hasRuntimeContext = + runtimeContext.dockerApi !== undefined || + runtimeContext.auth !== undefined || + runtimeContext.newImage !== undefined || + runtimeContext.registry !== undefined; + if (composeCtx.composeFileOnceApplied === true) { const logContainer = this.log.child({ container: container.name, @@ -1172,6 +1219,8 @@ class Dockercompose extends Docker { container, { composeFiles: composeCtx.composeFiles, + ...(composeCtx.skipPull === true ? { skipPull: true } : {}), + ...(hasRuntimeContext ? { runtimeContext } : {}), }, ); } else { @@ -1179,6 +1228,10 @@ class Dockercompose extends Docker { composeCtx.composeFile, composeCtx.service, container, + { + ...(composeCtx.skipPull === true ? { skipPull: true } : {}), + ...(hasRuntimeContext ? { runtimeContext } : {}), + }, ); } } @@ -1217,8 +1270,11 @@ class Dockercompose extends Docker { return false; } - const currentContainer = await this.getCurrentContainer(context.dockerApi, container); - const currentContainerSpec = await this.inspectContainer(currentContainer, logContainer); + const currentContainer = + context?.currentContainer ?? (await this.getCurrentContainer(context.dockerApi, container)); + const currentContainerSpec = + context?.currentContainerSpec ?? + (await this.inspectContainer(currentContainer, logContainer)); const selfUpdateContext = { ...context, @@ -1444,28 +1500,68 @@ class Dockercompose extends Docker { } const composeFileOnceHandledServices = new Set(); - if ( - this.configuration.composeFileOnce === true && - this.configuration.dryrun !== true && - mappingsNeedingRuntimeUpdate.length > 1 - ) { - // TODO: Re-introduce compose-file-once batching with Docker Engine API primitives. - this.log.info( - `Compose-file-once mode is unavailable in Docker Engine API runtime mode; refreshing containers individually for ${composeFileChainSummary}. This can be re-implemented via Docker Engine API later.`, + const composeFileOnceEnabled = + this.configuration.composeFileOnce === true && this.configuration.dryrun !== true; + const composeFileOnceRuntimeContextByService = new Map< + string, + NonNullable + >(); + if (composeFileOnceEnabled) { + const firstContainerByService = new Map< + string, + (typeof mappingsNeedingRuntimeUpdate)[number] + >(); + for (const mapping of mappingsNeedingRuntimeUpdate) { + if (!firstContainerByService.has(mapping.service)) { + firstContainerByService.set(mapping.service, mapping); + } + } + await Promise.all( + [...firstContainerByService.entries()].map(async ([service, mapping]) => { + const runtimeContainer = mapping.container; + const logContainer = this.log.child({ + container: runtimeContainer.name, + }); + const watcher = this.getWatcher(runtimeContainer); + const { dockerApi } = watcher; + const registry = this.resolveRegistryManager(runtimeContainer, logContainer, { + allowAnonymousFallback: true, + }); + const auth = await registry.getAuthPull(); + const newImage = this.getNewImageFullName(registry, runtimeContainer); + composeFileOnceRuntimeContextByService.set(service, { + dockerApi, + registry, + auth, + newImage, + }); + await this.pullImage(dockerApi, auth, newImage, logContainer); + }), ); } // Refresh all containers requiring a runtime update via the shared // lifecycle orchestrator (security gate, hooks, prune/backup, events). for (const { container, service } of mappingsNeedingRuntimeUpdate) { + const composeFileOnceApplied = + composeFileOnceEnabled && composeFileOnceHandledServices.has(service); + const composeFileOnceRuntimeContext = composeFileOnceRuntimeContextByService.get(service); const composeContext = { composeFile, composeFiles: composeFileChain, service, serviceDefinition: compose.services[service], - composeFileOnceApplied: composeFileOnceHandledServices.has(service), + composeFileOnceApplied, + skipPull: + composeFileOnceEnabled && + composeFileOnceApplied !== true && + composeFileOnceRuntimeContext !== undefined, + runtimeContext: composeFileOnceRuntimeContext, }; await this.runContainerUpdateLifecycle(container, composeContext); + if (composeFileOnceEnabled && !composeFileOnceApplied) { + composeFileOnceHandledServices.add(service); + } } } @@ -1556,20 +1652,33 @@ class Dockercompose extends Docker { } async updateContainerWithCompose(composeFile, service, container, options = {}) { + await this.refreshComposeServiceWithDockerApi(composeFile, service, container, options); + } + + private ensureComposeRuntimeState(currentContainerSpec, composeFile, service): void { + if (typeof currentContainerSpec?.State?.Running !== 'boolean') { + throw new Error( + `Unable to refresh compose service ${service} from ${composeFile} because Docker inspection data is missing runtime state`, + ); + } + } + + /** + * Refresh one compose-managed service by using the Docker Engine API + * directly. Shared by updateContainerWithCompose() and recreateContainer() + * to keep the runtime recreation path explicit and non-recursive. + */ + private async refreshComposeServiceWithDockerApi( + composeFile, + service, + container, + options: ComposeRuntimeRefreshOptions = {}, + ) { const logContainer = this.log.child({ container: container.name, }); - const { - shouldStart = undefined, - skipPull = false, - forceRecreate = false, - } = options as { - shouldStart?: boolean; - skipPull?: boolean; - forceRecreate?: boolean; - composeFiles?: string[]; - }; + const { shouldStart = undefined, skipPull = false, forceRecreate = false } = options; if (this.configuration.dryrun) { logContainer.info( @@ -1578,13 +1687,24 @@ class Dockercompose extends Docker { return; } - const watcher = this.getWatcher(container); - const { dockerApi } = watcher; - const registry = this.resolveRegistryManager(container, logContainer, { - allowAnonymousFallback: true, - }); - const auth = await registry.getAuthPull(); - const newImage = this.getNewImageFullName(registry, container); + const runtimeContext = options.runtimeContext || {}; + const dockerApi = runtimeContext.dockerApi || this.getWatcher(container)?.dockerApi; + let auth = runtimeContext.auth; + let newImage = runtimeContext.newImage; + + if (!newImage || (!skipPull && auth === undefined)) { + const registry = + runtimeContext.registry || + this.resolveRegistryManager(container, logContainer, { + allowAnonymousFallback: true, + }); + if (!newImage) { + newImage = this.getNewImageFullName(registry, container); + } + if (!skipPull && auth === undefined) { + auth = await registry.getAuthPull(); + } + } const currentContainer = await this.getCurrentContainer(dockerApi, container); if (!currentContainer) { throw new Error( @@ -1592,8 +1712,9 @@ class Dockercompose extends Docker { ); } const currentContainerSpec = await this.inspectContainer(currentContainer, logContainer); + this.ensureComposeRuntimeState(currentContainerSpec, composeFile, service); const serviceShouldStart = - shouldStart !== undefined ? shouldStart : currentContainerSpec?.State?.Running !== false; + shouldStart !== undefined ? shouldStart : currentContainerSpec.State.Running; logContainer.info( `Refresh compose service ${service} from ${composeFile} using Docker Engine API`, @@ -1612,10 +1733,12 @@ class Dockercompose extends Docker { const recreationContainerSpec = { ...currentContainerSpec, State: { - ...currentContainerSpec?.State, + ...currentContainerSpec.State, Running: serviceShouldStart, }, }; + // Intentionally bypass Dockercompose.stopAndRemoveContainer() no-op: this + // internal Engine API refresh path must perform the real stop/remove. await super.stopAndRemoveContainer( currentContainer, currentContainerSpec, @@ -1631,6 +1754,15 @@ class Dockercompose extends Docker { ); } + /** + * No-op for generic callers that invoke stop/remove and recreate as two + * separate steps (for example health-monitor rollback paths). In compose + * mode, recreateContainer() owns the full mutation + runtime refresh and + * would otherwise duplicate stop/remove work. + * + * When a compose refresh must actually stop/remove, we bypass this override + * via super.stopAndRemoveContainer() in refreshComposeServiceWithDockerApi(). + */ async stopAndRemoveContainer(_currentContainer, _currentContainerSpec, container, logContainer) { logContainer.info( `Skip direct stop/remove for compose-managed container ${container.name}; using compose lifecycle`, @@ -1671,17 +1803,17 @@ class Dockercompose extends Docker { shouldStart: currentContainerSpec?.State?.Running === true, skipPull: true, forceRecreate: true, - } as { - shouldStart: boolean; - skipPull: boolean; - forceRecreate: boolean; - composeFiles?: string[]; - }; + } as ComposeRuntimeRefreshOptions; if (composeFiles.length > 1) { composeUpdateOptions.composeFiles = composeFiles; } - await this.updateContainerWithCompose(composeFile, service, container, composeUpdateOptions); + await this.refreshComposeServiceWithDockerApi( + composeFile, + service, + container, + composeUpdateOptions, + ); } async runServicePostStartHooks(container, serviceKey, service) { From ee066ae1564a823a714d4c8e164688391c47fe14 Mon Sep 17 00:00:00 2001 From: superuserjr <80784472+turbodaemon@users.noreply.github.com> Date: Mon, 9 Mar 2026 21:20:29 -0400 Subject: [PATCH 06/34] =?UTF-8?q?=F0=9F=93=9D=20docs(compose):=20replace?= =?UTF-8?q?=20CLI=20references=20with=20Engine=20API=20and=20add=20compose?= =?UTF-8?q?=20trigger=20support=20to=20actions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- content/docs/current/configuration/actions/index.mdx | 2 +- .../configuration/triggers/docker-compose/index.mdx | 10 ++++++---- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/content/docs/current/configuration/actions/index.mdx b/content/docs/current/configuration/actions/index.mdx index ffb29aad..d6dd7000 100644 --- a/content/docs/current/configuration/actions/index.mdx +++ b/content/docs/current/configuration/actions/index.mdx @@ -33,7 +33,7 @@ Container actions can be individually toggled with environment variables: ## Update action -The update action triggers the Docker trigger for the selected container. It will: +The update action triggers the Docker or Docker Compose trigger for the selected container. It will: 1. Pull the latest image 2. Backup the current image diff --git a/content/docs/current/configuration/triggers/docker-compose/index.mdx b/content/docs/current/configuration/triggers/docker-compose/index.mdx index d942db95..34e65e11 100644 --- a/content/docs/current/configuration/triggers/docker-compose/index.mdx +++ b/content/docs/current/configuration/triggers/docker-compose/index.mdx @@ -56,9 +56,11 @@ Most Docker Compose users do not need to configure the file path at all. As long Do not forget to mount the docker-compose.yml file in the drydock container. +The Docker Compose trigger uses the Docker Engine API directly for all runtime operations (pull, stop, recreate). The `docker compose` / `docker-compose` CLI binary is **not required** inside the drydock container. + ## Tag vs digest updates -For **tag-based updates**, the trigger patches the compose file with the new tag and then recreates the container. For **digest-only updates** (same tag, new digest), the compose file is left unchanged and only the container runtime is refreshed via `docker compose pull` + `docker compose up -d`. +For **tag-based updates**, the trigger patches the compose file with the new tag and then recreates the container. For **digest-only updates** (same tag, new digest), the compose file is left unchanged and only the container runtime is refreshed (pull new image, stop/remove old container, recreate with new image). When `DIGESTPINNING=true`, compose mutations use digest references (`image@sha256:...`) when a digest is available. @@ -72,7 +74,7 @@ When a tag-based update requires a compose file change, Drydock patches Compose If a service uses flow-style mapping without an `image` key (for example `nginx: { restart: always }`), Drydock will not auto-insert `image`. Convert that service to block-style YAML first. -Before persisting a compose mutation, Drydock validates the candidate file with `docker compose config --quiet` (and falls back to `docker-compose config --quiet` when needed). Invalid candidate configs are not written. +Before persisting a compose mutation, Drydock validates the candidate file by parsing the full compose chain in-process. Invalid candidate configs are not written. ## Reconciliation behavior @@ -84,7 +86,7 @@ Before lifecycle execution, Drydock checks the running container image against t ## Compose-file-once mode -When `COMPOSEFILEONCE=true` and multiple services in the same compose stack/file require runtime refresh, Drydock executes one grouped compose pull/up pass for that stack and skips repeated per-service compose pull/up calls in that batch. +When `COMPOSEFILEONCE=true` and multiple services in the same compose stack/file require runtime refresh, Drydock performs one runtime refresh per unique service and skips repeated refreshes for subsequent containers sharing the same service in that batch. ## Multi-file compose chains @@ -94,7 +96,7 @@ During an update: 1. Drydock searches every file in the chain to find which one declares the target service's `image` key. 2. The last file in the chain that contains the service definition and is writable is selected as the update target. This matches Docker Compose override semantics where later files take precedence. -3. All files in the chain are passed to `docker compose -f file1 -f file2 config --quiet` for validation before and after any mutation. +3. All files in the chain are parsed in-process for validation before and after any mutation. The compose file chain for each container is visible in the container detail view of the UI. From 9d22888457cfebebbbe1a507a8738a35cf1e6ba9 Mon Sep 17 00:00:00 2001 From: superuserjr <80784472+turbodaemon@users.noreply.github.com> Date: Mon, 9 Mar 2026 21:20:34 -0400 Subject: [PATCH 07/34] =?UTF-8?q?=F0=9F=93=9D=20docs(oidc):=20mark=20DD=5F?= =?UTF-8?q?PUBLIC=5FURL=20as=20required=20and=20add=20to=20all=20examples?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../configuration/authentications/oidc/index.mdx | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/content/docs/current/configuration/authentications/oidc/index.mdx b/content/docs/current/configuration/authentications/oidc/index.mdx index 24858dea..0d94f690 100644 --- a/content/docs/current/configuration/authentications/oidc/index.mdx +++ b/content/docs/current/configuration/authentications/oidc/index.mdx @@ -20,11 +20,11 @@ The `oidc` authentication lets you protect drydock access using the [Openid Conn | `DD_AUTH_OIDC_{auth_name}_REDIRECT` | ⚪ | Skip internal login page & automatically redirect to the OIDC provider | `true`, `false` | `false` | | `DD_AUTH_OIDC_{auth_name}_LOGOUTURL` | ⚪ | Explicit IdP logout URL fallback when discovery metadata has no `end_session_endpoint` | URL | empty | | `DD_AUTH_OIDC_{auth_name}_TIMEOUT` | ⚪ | Timeout (in ms) when calling the OIDC provider | Minimum is 500 | `5000` | +| `DD_PUBLIC_URL` | 🔴 | Public URL of your drydock instance (used to build the OIDC callback URL) | URL | | -The callback URL (to configure in the IDP is built as `${drydock_public_url}/auth/oidc/${auth_name}/cb` +The callback URL (to configure in the IDP) is built as `${DD_PUBLIC_URL}/auth/oidc/${auth_name}/cb` -drydock tries its best to determine the public address to forge redirections on its own. -If it fails (irregular reverse proxy configuration...), you can enforce the value using the env var `DD_PUBLIC_URL` +`DD_PUBLIC_URL` is **required** when OIDC is configured. Without it, the OIDC provider will fail to register at startup and the login page will show "No authentication methods configured". Check startup logs for the exact error. If your IdP is on a different domain and callback fails with `OIDC session is missing or expired`, check `DD_SERVER_COOKIE_SAMESITE` (server config). Default is `lax` for OIDC compatibility; `strict` can break cross-site callbacks. Use `none` only for explicit cross-site/embed cases over HTTPS. @@ -88,6 +88,7 @@ services: image: codeswhat/drydock ... environment: + - DD_PUBLIC_URL=https:// - DD_AUTH_OIDC_AUTHELIA_CLIENTID=my-drydock-client-id - DD_AUTH_OIDC_AUTHELIA_CLIENTSECRET=this-is-a-very-secure-secret - DD_AUTH_OIDC_AUTHELIA_DISCOVERY=https:///.well-known/openid-configuration @@ -97,6 +98,7 @@ services: ```bash docker run \ + -e DD_PUBLIC_URL="https://" \ -e DD_AUTH_OIDC_AUTHELIA_CLIENTID="my-drydock-client-id" \ -e DD_AUTH_OIDC_AUTHELIA_CLIENTSECRET="this-is-a-very-secure-secret" \ -e DD_AUTH_OIDC_AUTHELIA_DISCOVERY="https:///.well-known/openid-configuration" \ @@ -129,6 +131,7 @@ services: image: codeswhat/drydock ... environment: + - DD_PUBLIC_URL=https:// - DD_AUTH_OIDC_AUTH0_CLIENTID= - DD_AUTH_OIDC_AUTH0_CLIENTSECRET= - DD_AUTH_OIDC_AUTH0_DISCOVERY=https:///.well-known/openid-configuration @@ -138,6 +141,7 @@ services: ```bash docker run \ + -e DD_PUBLIC_URL="https://" \ -e DD_AUTH_OIDC_AUTH0_CLIENTID="" \ -e DD_AUTH_OIDC_AUTH0_CLIENTSECRET="" \ -e DD_AUTH_OIDC_AUTH0_DISCOVERY="https:///.well-known/openid-configuration" \ @@ -182,6 +186,7 @@ services: image: codeswhat/drydock ... environment: + - DD_PUBLIC_URL=https:// - DD_AUTH_OIDC_AUTHENTIK_CLIENTID= - DD_AUTH_OIDC_AUTHENTIK_CLIENTSECRET= - DD_AUTH_OIDC_AUTHENTIK_DISCOVERY=/application/o//.well-known/openid-configuration @@ -192,6 +197,7 @@ services: ```bash docker run \ + -e DD_PUBLIC_URL="https://" \ -e DD_AUTH_OIDC_AUTHENTIK_CLIENTID="" \ -e DD_AUTH_OIDC_AUTHENTIK_CLIENTSECRET="" \ -e DD_AUTH_OIDC_AUTHENTIK_DISCOVERY="/application/o//.well-known/openid-configuration" \ @@ -217,6 +223,7 @@ services: image: codeswhat/drydock ... environment: + - DD_PUBLIC_URL=https:// - DD_AUTH_OIDC_DEX_CLIENTID= - DD_AUTH_OIDC_DEX_CLIENTSECRET= - DD_AUTH_OIDC_DEX_DISCOVERY=https:///dex/.well-known/openid-configuration @@ -228,6 +235,7 @@ services: ```bash docker run \ + -e DD_PUBLIC_URL="https://" \ -e DD_AUTH_OIDC_DEX_CLIENTID="" \ -e DD_AUTH_OIDC_DEX_CLIENTSECRET="" \ -e DD_AUTH_OIDC_DEX_DISCOVERY="https:///dex/.well-known/openid-configuration" \ From 63a0db0734ee34fa5dd63eb62053cdcb7c52d2d7 Mon Sep 17 00:00:00 2001 From: superuserjr <80784472+turbodaemon@users.noreply.github.com> Date: Mon, 9 Mar 2026 21:27:03 -0400 Subject: [PATCH 08/34] =?UTF-8?q?=E2=9C=85=20test(compose):=20add=20runtim?= =?UTF-8?q?e=20context=20passthrough=20and=20multi-file=20skipPull=20cover?= =?UTF-8?q?age?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../dockercompose/Dockercompose.test.ts | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) diff --git a/app/triggers/providers/dockercompose/Dockercompose.test.ts b/app/triggers/providers/dockercompose/Dockercompose.test.ts index e11d2042..4ccf34eb 100644 --- a/app/triggers/providers/dockercompose/Dockercompose.test.ts +++ b/app/triggers/providers/dockercompose/Dockercompose.test.ts @@ -1389,6 +1389,36 @@ describe('Dockercompose Trigger', () => { ); }); + test('updateContainerWithCompose should fetch auth when runtime context provides newImage without auth', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ name: 'nginx' }); + const pullImageSpy = vi.spyOn(trigger, 'pullImage').mockResolvedValue(); + const resolveRegistryManagerSpy = vi.spyOn(trigger, 'resolveRegistryManager'); + const getNewImageFullNameSpy = vi.spyOn(trigger, 'getNewImageFullName'); + const registryGetAuthPull = vi.fn().mockResolvedValue({ from: 'registry-auth' }); + const runtimeContext = { + dockerApi: mockDockerApi, + newImage: 'nginx:9.9.9', + registry: { + getAuthPull: registryGetAuthPull, + }, + }; + + await trigger.updateContainerWithCompose('/opt/drydock/test/stack.yml', 'nginx', container, { + runtimeContext, + }); + + expect(resolveRegistryManagerSpy).not.toHaveBeenCalled(); + expect(getNewImageFullNameSpy).not.toHaveBeenCalled(); + expect(registryGetAuthPull).toHaveBeenCalledTimes(1); + expect(pullImageSpy).toHaveBeenCalledWith( + runtimeContext.dockerApi, + { from: 'registry-auth' }, + runtimeContext.newImage, + expect.anything(), + ); + }); + test('updateContainerWithCompose should throw when current container cannot be resolved', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ name: 'nginx' }); @@ -4670,6 +4700,63 @@ describe('Dockercompose Trigger', () => { ); }); + test('performContainerUpdate should pass skipPull in multi-file compose context', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'nginx', + }); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + + const updated = await trigger.performContainerUpdate({} as any, container as any, mockLog, { + composeFile: '/opt/drydock/test/stack.override.yml', + composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], + service: 'nginx', + serviceDefinition: {}, + composeFileOnceApplied: false, + skipPull: true, + } as any); + + expect(updated).toBe(true); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.override.yml', + 'nginx', + container, + { + composeFiles: ['/opt/drydock/test/stack.yml', '/opt/drydock/test/stack.override.yml'], + skipPull: true, + }, + ); + }); + + test('performContainerUpdate should avoid passing runtime context when none is available in single-file path', async () => { + trigger.configuration.dryrun = false; + const container = makeContainer({ + name: 'nginx', + }); + const updateContainerWithComposeSpy = vi + .spyOn(trigger, 'updateContainerWithCompose') + .mockResolvedValue(); + vi.spyOn(trigger, 'runServicePostStartHooks').mockResolvedValue(); + + const updated = await trigger.performContainerUpdate({} as any, container as any, mockLog, { + composeFile: '/opt/drydock/test/stack.yml', + service: 'nginx', + serviceDefinition: {}, + composeFileOnceApplied: false, + } as any); + + expect(updated).toBe(true); + expect(updateContainerWithComposeSpy).toHaveBeenCalledWith( + '/opt/drydock/test/stack.yml', + 'nginx', + container, + {}, + ); + }); + test('performContainerUpdate should skip per-service refresh when compose-file-once is already applied', async () => { trigger.configuration.dryrun = false; const container = makeContainer({ From 989cc230ae550c5c781ac4b9f217549d4b0f19ab Mon Sep 17 00:00:00 2001 From: superuserjr <80784472+turbodaemon@users.noreply.github.com> Date: Mon, 9 Mar 2026 21:27:34 -0400 Subject: [PATCH 09/34] =?UTF-8?q?=F0=9F=93=9D=20docs(changelog):=20add=20c?= =?UTF-8?q?ompose=20refactor,=20composeFileOnce,=20and=20docs=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f36b5e1..6900f92c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -93,6 +93,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Compose trigger uses Docker Engine API** — Compose-managed container updates now use the Docker Engine API directly (pull, stop, recreate) instead of shelling out to `docker compose` / `docker-compose` CLI. Eliminates `spawn docker ENOENT` errors in environments without Docker CLI binaries installed. - **Compose self-update delegates to parent orchestrator** — Self-update for compose-managed Drydock containers now uses the parent Docker trigger's helper-container transition with health gates and rollback, instead of direct stop/recreate. +- **Compose runtime refresh extracted** — Shared `refreshComposeServiceWithDockerApi()` helper eliminates the recursive `recreateContainer` → `updateContainerWithCompose` call chain. Both code paths now converge on the same explicit, non-recursive method. +- **Compose-file-once batch mode re-enabled** — `COMPOSEFILEONCE=true` now works with the Docker Engine API runtime. First container per service gets a full runtime refresh; subsequent containers sharing the same service skip the refresh. - **Self-update controller testable entrypoint** — Extracted process-level entry logic to a separate entrypoint module, making the controller independently testable without triggering process-exit side effects. - **Dockercompose YAML patching simplified** — Removed redundant type guards and dead-code branches from compose file patching helpers, reducing code paths and improving maintainability. - **Dashboard fetches recent-status from backend** — Dashboard now fetches pre-computed container statuses from `/api/containers/recent-status` instead of scanning the raw audit log client-side. @@ -122,6 +124,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **Manual update button works with compose triggers** — The update container endpoint now searches for both `docker` and `dockercompose` trigger types, matching the existing preview endpoint behavior. Previously, users with only a compose trigger saw "No docker trigger found for this container". - **CI: qlty retry on timeout** — Changed `retry_on` from `error` to `any` so qlty timeouts trigger retries. Increased timeout from 5 to 8 minutes. +- **OIDC docs clarified `DD_PUBLIC_URL` requirement** — OIDC documentation now explicitly marks `DD_PUBLIC_URL` as required and includes it in all provider example configurations (Authelia, Auth0, Authentik, Dex). Without this variable, the OIDC provider fails to register at startup. +- **Compose trigger docs updated for Engine API** — Removed stale references to `docker compose config --quiet` CLI validation and `docker compose pull`/`up` commands. Docs now reflect in-process YAML validation and Docker Engine API runtime. Added callout that Docker Compose CLI is not required. +- **Container actions docs updated for compose triggers** — Update action documentation now mentions both Docker and Docker Compose trigger support. - **Compose trigger rejects compose files mounted outside app directory** — Removed overly strict working-directory boundary enforcement from `runComposeCommand` that rejected compose files bind-mounted outside `/home/node/app`, breaking documented mount patterns like `/drydock/docker-compose.yml`. Compose file paths are operator-configured and already validated during resolution. - **Compose trigger uses host paths instead of container paths** — Docker label auto-detection (`com.docker.compose.project.config_files`) now remaps host-side paths to container-internal paths using Drydock's own bind mount information. Previously, host paths like `/mnt/volume1/docker/stacks/monitoring/compose.yaml` were used directly inside the container where they don't exist, causing "does not exist" errors even when the file was properly mounted. - **Compose trigger logs spurious warnings for unrelated containers** — When multiple compose triggers are configured, each trigger now silently skips containers whose resolved compose files don't match the trigger's configured `FILE` path, eliminating noisy cross-stack "does not exist" warnings. From c8c48a0fa834e5929a82ad951bbbe744602a436e Mon Sep 17 00:00:00 2001 From: superuserjr <80784472+turbodaemon@users.noreply.github.com> Date: Mon, 9 Mar 2026 21:48:33 -0400 Subject: [PATCH 10/34] =?UTF-8?q?=F0=9F=90=9B=20fix(ui):=20show=20loading?= =?UTF-8?q?=20state=20in=20confirm=20dialog=20during=20async=20actions?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Confirm dialog now awaits the async accept callback before closing, showing a spinner and disabling all dismiss interactions while the action runs. Previously the dialog closed immediately on confirm, making the UI appear unresponsive during container actions. --- CHANGELOG.md | 1 + ui/src/components/ConfirmDialog.vue | 16 ++- ui/src/composables/useConfirmDialog.ts | 36 +++++- ui/tests/components/ConfirmDialog.spec.ts | 27 ++++ ui/tests/composables/useConfirmDialog.spec.ts | 119 +++++++++++++++++- 5 files changed, 187 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6900f92c..e060cc4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -122,6 +122,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Confirm dialog shows loading state during async actions** — Confirm dialog now awaits the async accept callback before closing, showing a spinner on the accept button and disabling all dismiss interactions (buttons, Escape, Enter, backdrop click) while the action runs. Previously, the dialog closed immediately on confirm, causing the UI to appear unresponsive until the action completed in the background. - **Manual update button works with compose triggers** — The update container endpoint now searches for both `docker` and `dockercompose` trigger types, matching the existing preview endpoint behavior. Previously, users with only a compose trigger saw "No docker trigger found for this container". - **CI: qlty retry on timeout** — Changed `retry_on` from `error` to `any` so qlty timeouts trigger retries. Increased timeout from 5 to 8 minutes. - **OIDC docs clarified `DD_PUBLIC_URL` requirement** — OIDC documentation now explicitly marks `DD_PUBLIC_URL` as required and includes it in all provider example configurations (Authelia, Auth0, Authentik, Dex). Without this variable, the OIDC provider fails to register at startup. diff --git a/ui/src/components/ConfirmDialog.vue b/ui/src/components/ConfirmDialog.vue index 417806d0..d5aeeca1 100644 --- a/ui/src/components/ConfirmDialog.vue +++ b/ui/src/components/ConfirmDialog.vue @@ -1,8 +1,9 @@