Fix container remove timeouts and make force-remove reliable.
Release rolling / release (push) Successful in 9m13s
Release rolling / release (push) Successful in 9m13s
Lifecycle RPCs now use a 120s operation timeout, remove cleans up stats/terminal/log streams then SIGKILLs before force-remove, and the UI awaits the request instead of a fragile 30s wait race.
This commit is contained in:
@@ -7,9 +7,72 @@ import * as validation from '../utils/validation.js'
|
||||
import { Pushes } from '../../shared/protocol.js'
|
||||
import { peers } from '../core/peer-registry.js'
|
||||
import { getHistory } from '../services/stats-history.js'
|
||||
import { destroyStatsForContainer } from '../services/stats.js'
|
||||
import { validateCreateOptions } from '../utils/engine-capabilities.js'
|
||||
import logger from '../utils/logger.js'
|
||||
|
||||
/**
|
||||
* True when Docker reports the container is already gone.
|
||||
* @param {unknown} err
|
||||
*/
|
||||
function isNoSuchContainer(err) {
|
||||
const status = err?.statusCode || err?.status
|
||||
if (status === 404) return true
|
||||
return /no such container/i.test(String(err?.message || err || ''))
|
||||
}
|
||||
|
||||
/**
|
||||
* Release attachments that can delay Docker force-remove, then delete.
|
||||
* @param {string} id
|
||||
* @param {import('../rpc/session.js').PeerSession} session
|
||||
* @param {{ force?: boolean, v?: boolean, removeVolumes?: boolean, link?: boolean }} args
|
||||
*/
|
||||
async function forceRemoveContainer(id, session, args = {}) {
|
||||
session._cleanupLogsForContainer?.(id)
|
||||
session._cleanupTerminalsForContainer?.(id)
|
||||
try {
|
||||
destroyStatsForContainer(id)
|
||||
} catch {
|
||||
// ignore
|
||||
}
|
||||
|
||||
const container = docker.getContainer(id)
|
||||
const force = args.force !== false
|
||||
const removeOpts = {
|
||||
force,
|
||||
v: Boolean(args.v || args.removeVolumes),
|
||||
}
|
||||
if (args.link) removeOpts.link = true
|
||||
|
||||
// SIGKILL first so remove does not wait on a stuck PID / long stop period
|
||||
if (force) {
|
||||
try {
|
||||
await container.kill({ signal: 'SIGKILL' })
|
||||
} catch (err) {
|
||||
// not running / already dead — fine
|
||||
if (!isNoSuchContainer(err) && !/is not running|already stopped/i.test(String(err?.message || ''))) {
|
||||
logger.debug('pre-remove kill skipped', { id: id.slice(0, 12), error: err?.message })
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
await container.remove(removeOpts)
|
||||
} catch (err) {
|
||||
if (isNoSuchContainer(err)) return
|
||||
// Concurrent remove in progress — wait briefly, then treat as gone if inspect 404s
|
||||
if (/already in progress|removal of container/i.test(String(err?.message || ''))) {
|
||||
await new Promise((r) => setTimeout(r, 750))
|
||||
try {
|
||||
await container.inspect()
|
||||
} catch (inspectErr) {
|
||||
if (isNoSuchContainer(inspectErr)) return
|
||||
}
|
||||
}
|
||||
throw err
|
||||
}
|
||||
}
|
||||
|
||||
export function registerContainerHandlers(session) {
|
||||
session.respond('validateCreateOptions', async (args) => {
|
||||
const result = await validateCreateOptions(args?.options || args || {}, {
|
||||
@@ -169,12 +232,7 @@ export function registerContainerHandlers(session) {
|
||||
|
||||
session.respond('removeContainer', async (args) => {
|
||||
const id = args.id
|
||||
session._cleanupLogsForContainer?.(id)
|
||||
await docker.getContainer(id).remove({
|
||||
force: args.force !== false,
|
||||
v: Boolean(args.v || args.removeVolumes),
|
||||
link: Boolean(args.link),
|
||||
})
|
||||
await forceRemoveContainer(id, session, args)
|
||||
return { success: true, message: `Container ${id} removed` }
|
||||
})
|
||||
|
||||
@@ -541,7 +599,7 @@ export function registerContainerHandlers(session) {
|
||||
else await container.kill()
|
||||
break
|
||||
case 'remove':
|
||||
await container.remove({ force: true })
|
||||
await forceRemoveContainer(containerId, session, { force: true })
|
||||
break
|
||||
}
|
||||
results.push({ id: containerId, success: true })
|
||||
@@ -742,8 +800,6 @@ async function recreateContainer(args, session) {
|
||||
const wasRunning = Boolean(inspect.State?.Running)
|
||||
const shouldStart = args.start !== false && (args.start === true || wasRunning)
|
||||
|
||||
session._cleanupLogsForContainer?.(id)
|
||||
|
||||
try {
|
||||
if (wasRunning) {
|
||||
await container.stop({ t: Number(args.timeout) >= 0 ? Number(args.timeout) : 10 })
|
||||
@@ -752,9 +808,10 @@ async function recreateContainer(args, session) {
|
||||
// already stopped
|
||||
}
|
||||
|
||||
await container.remove({
|
||||
await forceRemoveContainer(id, session, {
|
||||
force: true,
|
||||
v: Boolean(args.removeVolumes || args.v),
|
||||
removeVolumes: Boolean(args.removeVolumes || args.v),
|
||||
})
|
||||
|
||||
const createOpts = createOptsFromInspect(inspect, name)
|
||||
|
||||
@@ -524,13 +524,7 @@ export function registerTerminalHandlers(session) {
|
||||
}
|
||||
}
|
||||
if (args.containerId) {
|
||||
let killed = 0
|
||||
for (const [id, entry] of [...sessions.entries()]) {
|
||||
if (entry.containerId === args.containerId) {
|
||||
endOne(sessions, id)
|
||||
killed += 1
|
||||
}
|
||||
}
|
||||
const killed = cleanupTerminalsForContainer(session, args.containerId)
|
||||
if (killed) {
|
||||
return { success: true, message: `Killed ${killed} terminal(s) for ${args.containerId}` }
|
||||
}
|
||||
@@ -543,6 +537,48 @@ export function registerTerminalHandlers(session) {
|
||||
}
|
||||
return { success: false, message: 'No terminal session found' }
|
||||
})
|
||||
|
||||
/** Used by removeContainer / recreate before Docker delete. */
|
||||
session._cleanupTerminalsForContainer = (containerId) => {
|
||||
cleanupTerminalsForContainer(session, containerId)
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* @param {import('../rpc/session.js').PeerSession} session
|
||||
* @param {string} containerId
|
||||
* @returns {number} sessions ended
|
||||
*/
|
||||
function cleanupTerminalsForContainer(session, containerId) {
|
||||
if (!containerId) return 0
|
||||
const sessions = getSessions(session)
|
||||
const needle = String(containerId)
|
||||
let killed = 0
|
||||
for (const [id, entry] of [...sessions.entries()]) {
|
||||
const cid = String(entry?.containerId || '')
|
||||
if (
|
||||
cid === needle ||
|
||||
(needle.length >= 12 && cid.startsWith(needle)) ||
|
||||
(cid.length >= 12 && needle.startsWith(cid.slice(0, 12)))
|
||||
) {
|
||||
endOne(sessions, id)
|
||||
killed += 1
|
||||
}
|
||||
}
|
||||
const legacy = session.state.get('terminal')
|
||||
if (legacy) {
|
||||
const cid = String(legacy.containerId || '')
|
||||
if (
|
||||
cid === needle ||
|
||||
(needle.length >= 12 && cid.startsWith(needle)) ||
|
||||
(cid.length >= 12 && needle.startsWith(cid.slice(0, 12)))
|
||||
) {
|
||||
endOne(sessions, legacy.sessionId || 'default')
|
||||
session.state.delete('terminal')
|
||||
killed += 1
|
||||
}
|
||||
}
|
||||
return killed
|
||||
}
|
||||
|
||||
function endOne(sessions, sessionId) {
|
||||
|
||||
@@ -266,6 +266,26 @@ function destroyStatsEntry(id) {
|
||||
statsCache.delete(id)
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop live stats stream for a container before stop/remove so Docker is not
|
||||
* held open by NDJSON stats attachments (can delay force-remove).
|
||||
* @param {string} id
|
||||
*/
|
||||
export function destroyStatsForContainer(id) {
|
||||
if (!id) return
|
||||
const needle = String(id)
|
||||
destroyStatsEntry(needle)
|
||||
for (const key of Object.keys(containerStats)) {
|
||||
if (key === needle) continue
|
||||
if (
|
||||
(needle.length >= 12 && key.startsWith(needle)) ||
|
||||
(key.length >= 12 && needle.startsWith(key.slice(0, Math.min(12, key.length))))
|
||||
) {
|
||||
destroyStatsEntry(key)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async function collectContainerStats() {
|
||||
// Running only for streams; we still list all so stopped ids are cleaned up
|
||||
const running = await docker.listContainers({ all: false })
|
||||
|
||||
Reference in New Issue
Block a user