fix(system): roll back service update when the new container fails to start
The service update path stopped and renamed the old container aside before the new one was confirmed running, but only wired up rollback if createContainer threw or the 5s health check failed. A throw from newContainer.start() itself (bad device/GPU config, host port already bound, image incompatibility) bubbled straight to the outer catch, which returned a generic 400 and never restored the old container, leaving the service down. Retrying then wedged: the failed new container still held the service name, so the next attempt's rename to `<name>_old` collided with the leftover from the first attempt and threw the same error every time. - Wrap newContainer.start() so a start failure removes the half-created container and rolls back to the previous one. - Clear any stale `<name>_old` before renaming so retries can't collide. - Dedupe the three rollback sites into a single rollbackToOld() helper (also removes a non-null assertion in the create-failure path that could itself throw when no `_old` existed). Refs #949 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
dfc284c34d
commit
25000b9869
|
|
@ -1547,8 +1547,36 @@ export class DockerService {
|
||||||
|
|
||||||
// Step 3: Rename old container as safety net
|
// Step 3: Rename old container as safety net
|
||||||
const oldName = `${serviceName}_old`
|
const oldName = `${serviceName}_old`
|
||||||
|
|
||||||
|
// Clear any stale rollback container left behind by a previously failed update.
|
||||||
|
// Otherwise the rename below collides with the existing `<name>_old` and throws,
|
||||||
|
// which wedges every subsequent retry on the same error.
|
||||||
|
const staleOld = (await this.docker.listContainers({ all: true })).find((c) =>
|
||||||
|
c.Names.includes(`/${oldName}`)
|
||||||
|
)
|
||||||
|
if (staleOld) {
|
||||||
|
try {
|
||||||
|
await this.docker.getContainer(staleOld.Id).remove({ force: true })
|
||||||
|
} catch {
|
||||||
|
// Best effort — if it can't be removed the rename below will surface the error.
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
await oldContainer.rename({ name: oldName })
|
await oldContainer.rename({ name: oldName })
|
||||||
|
|
||||||
|
// Restore the previous container after a failed update: rename the renamed-aside
|
||||||
|
// old container back into place and start it, so a failure anywhere between here
|
||||||
|
// and the health check never leaves the service down.
|
||||||
|
const rollbackToOld = async () => {
|
||||||
|
const containers = await this.docker.listContainers({ all: true })
|
||||||
|
const oldRef = containers.find((c) => c.Names.includes(`/${oldName}`))
|
||||||
|
if (oldRef) {
|
||||||
|
const rollbackContainer = this.docker.getContainer(oldRef.Id)
|
||||||
|
await rollbackContainer.rename({ name: serviceName }).catch(() => {})
|
||||||
|
await rollbackContainer.start().catch(() => {})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Step 4: Create new container with inspected config + new image
|
// Step 4: Create new container with inspected config + new image
|
||||||
this._broadcast(serviceName, 'update-creating', `Creating updated container...`)
|
this._broadcast(serviceName, 'update-creating', `Creating updated container...`)
|
||||||
|
|
||||||
|
|
@ -1604,16 +1632,35 @@ export class DockerService {
|
||||||
} catch (createError: any) {
|
} catch (createError: any) {
|
||||||
// Rollback: rename old container back
|
// Rollback: rename old container back
|
||||||
this._broadcast(serviceName, 'update-rollback', `Failed to create new container: ${createError.message}. Rolling back...`)
|
this._broadcast(serviceName, 'update-rollback', `Failed to create new container: ${createError.message}. Rolling back...`)
|
||||||
const rollbackContainer = this.docker.getContainer((await this.docker.listContainers({ all: true })).find((c) => c.Names.includes(`/${oldName}`))!.Id)
|
await rollbackToOld()
|
||||||
await rollbackContainer.rename({ name: serviceName })
|
|
||||||
await rollbackContainer.start()
|
|
||||||
this.activeInstallations.delete(serviceName)
|
this.activeInstallations.delete(serviceName)
|
||||||
return { success: false, message: `Failed to create updated container: ${createError.message}` }
|
return { success: false, message: `Failed to create updated container: ${createError.message}` }
|
||||||
}
|
}
|
||||||
|
|
||||||
// Step 5: Start new container
|
// Step 5: Start new container. If the start itself throws (bad device/GPU config,
|
||||||
|
// a host port already bound, image incompatibility), roll back to the previous
|
||||||
|
// container instead of leaving the service stopped with no replacement running.
|
||||||
this._broadcast(serviceName, 'update-starting', `Starting updated container...`)
|
this._broadcast(serviceName, 'update-starting', `Starting updated container...`)
|
||||||
await newContainer.start()
|
try {
|
||||||
|
await newContainer.start()
|
||||||
|
} catch (startError: any) {
|
||||||
|
this._broadcast(
|
||||||
|
serviceName,
|
||||||
|
'update-rollback',
|
||||||
|
`Updated container failed to start: ${startError.message}. Rolling back to previous version...`
|
||||||
|
)
|
||||||
|
try {
|
||||||
|
await newContainer.remove({ force: true })
|
||||||
|
} catch {
|
||||||
|
// Best effort — leave the half-created container for manual cleanup if needed.
|
||||||
|
}
|
||||||
|
await rollbackToOld()
|
||||||
|
this.activeInstallations.delete(serviceName)
|
||||||
|
return {
|
||||||
|
success: false,
|
||||||
|
message: `Update failed: new container did not start (${startError.message}). Rolled back to previous version.`,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Step 6: Health check — verify container stays running for 5 seconds
|
// Step 6: Health check — verify container stays running for 5 seconds
|
||||||
await new Promise((resolve) => setTimeout(resolve, 5000))
|
await new Promise((resolve) => setTimeout(resolve, 5000))
|
||||||
|
|
@ -1659,14 +1706,7 @@ export class DockerService {
|
||||||
// Best effort cleanup
|
// Best effort cleanup
|
||||||
}
|
}
|
||||||
|
|
||||||
// Restore old container
|
await rollbackToOld()
|
||||||
const oldContainers = await this.docker.listContainers({ all: true })
|
|
||||||
const oldRef = oldContainers.find((c) => c.Names.includes(`/${oldName}`))
|
|
||||||
if (oldRef) {
|
|
||||||
const rollbackContainer = this.docker.getContainer(oldRef.Id)
|
|
||||||
await rollbackContainer.rename({ name: serviceName })
|
|
||||||
await rollbackContainer.start()
|
|
||||||
}
|
|
||||||
|
|
||||||
this.activeInstallations.delete(serviceName)
|
this.activeInstallations.delete(serviceName)
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue