fix(docker): reject failed image pulls instead of treating them as success
Every Docker pull went through `followProgress(pullStream, resolve)`, passing the Promise's resolve as dockerode's onFinished(err, output) callback — so the error argument was ignored and a failed pull (dropped/metered connection, bad manifest, registry error, disk full mid-pull) resolved as if it had succeeded. The code then tried to create/start a container from a missing or partial image, surfacing a confusing downstream error rather than the real cause. (#790) Add a DockerService.pullImage() helper that rejects when followProgress reports an error, and route all five pull sites through it: - service install - AMD ROCm image pull - service update - force-reinstall / recreate (forcePull) - sysbench benchmark image pull Closes #790 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
36068c645e
commit
98d235679e
|
|
@ -614,8 +614,7 @@ export class BenchmarkService {
|
||||||
await this.dockerService.docker.getImage(SYSBENCH_IMAGE).inspect()
|
await this.dockerService.docker.getImage(SYSBENCH_IMAGE).inspect()
|
||||||
} catch {
|
} catch {
|
||||||
this._updateStatus('starting', `Pulling sysbench image...`)
|
this._updateStatus('starting', `Pulling sysbench image...`)
|
||||||
const pullStream = await this.dockerService.docker.pull(SYSBENCH_IMAGE)
|
await this.dockerService.pullImage(SYSBENCH_IMAGE)
|
||||||
await new Promise((resolve) => this.dockerService.docker.modem.followProgress(pullStream, resolve))
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -58,6 +58,27 @@ export class DockerService {
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Pull a Docker image and resolve only when the pull genuinely completes.
|
||||||
|
*
|
||||||
|
* dockerode's `followProgress(stream, onFinished)` reports failures via the
|
||||||
|
* first argument of onFinished. Every call site used to pass the Promise's
|
||||||
|
* `resolve` directly as that callback, so a failed pull (dropped/metered
|
||||||
|
* connection, bad manifest, registry error, disk full mid-pull) resolved as
|
||||||
|
* if it had succeeded — and the code then tried to create/start a container
|
||||||
|
* from a missing or partial image, surfacing a confusing downstream error.
|
||||||
|
* Rejecting on that error here lets callers fail fast with the real cause (#790).
|
||||||
|
*/
|
||||||
|
async pullImage(imageName: string): Promise<void> {
|
||||||
|
const pullStream = await this.docker.pull(imageName)
|
||||||
|
await new Promise<void>((resolve, reject) => {
|
||||||
|
this.docker.modem.followProgress(pullStream, (error: Error | null) => {
|
||||||
|
if (error) reject(error)
|
||||||
|
else resolve()
|
||||||
|
})
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
async affectContainer(
|
async affectContainer(
|
||||||
serviceName: string,
|
serviceName: string,
|
||||||
action: 'start' | 'stop' | 'restart'
|
action: 'start' | 'stop' | 'restart'
|
||||||
|
|
@ -632,13 +653,12 @@ export class DockerService {
|
||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
// Start pulling the Docker image and wait for it to complete
|
// Start pulling the Docker image and wait for it to complete
|
||||||
const pullStream = await this.docker.pull(service.container_image)
|
|
||||||
this._broadcast(
|
this._broadcast(
|
||||||
service.service_name,
|
service.service_name,
|
||||||
'pulling',
|
'pulling',
|
||||||
`Pulling Docker image ${service.container_image}...`
|
`Pulling Docker image ${service.container_image}...`
|
||||||
)
|
)
|
||||||
await new Promise((res) => this.docker.modem.followProgress(pullStream, res))
|
await this.pullImage(service.container_image)
|
||||||
}
|
}
|
||||||
|
|
||||||
if (service.service_name === SERVICE_NAMES.KIWIX) {
|
if (service.service_name === SERVICE_NAMES.KIWIX) {
|
||||||
|
|
@ -728,8 +748,7 @@ export class DockerService {
|
||||||
'pulling',
|
'pulling',
|
||||||
`Pulling Docker image ${finalImage}...`
|
`Pulling Docker image ${finalImage}...`
|
||||||
)
|
)
|
||||||
const rocmPullStream = await this.docker.pull(finalImage)
|
await this.pullImage(finalImage)
|
||||||
await new Promise((res) => this.docker.modem.followProgress(rocmPullStream, res))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const amdDevices = await this._discoverAMDDevices()
|
const amdDevices = await this._discoverAMDDevices()
|
||||||
|
|
@ -1523,8 +1542,7 @@ export class DockerService {
|
||||||
|
|
||||||
// Step 1: Pull new image (runtimeImage diverges from newImage for AMD, see above)
|
// Step 1: Pull new image (runtimeImage diverges from newImage for AMD, see above)
|
||||||
this._broadcast(serviceName, 'update-pulling', `Pulling image ${runtimeImage}...`)
|
this._broadcast(serviceName, 'update-pulling', `Pulling image ${runtimeImage}...`)
|
||||||
const pullStream = await this.docker.pull(runtimeImage)
|
await this.pullImage(runtimeImage)
|
||||||
await new Promise((res) => this.docker.modem.followProgress(pullStream, res))
|
|
||||||
|
|
||||||
// Step 2: Find and stop existing container
|
// Step 2: Find and stop existing container
|
||||||
this._broadcast(serviceName, 'update-stopping', `Stopping current container...`)
|
this._broadcast(serviceName, 'update-stopping', `Stopping current container...`)
|
||||||
|
|
@ -1996,8 +2014,7 @@ export class DockerService {
|
||||||
|
|
||||||
// Pull the image if it's missing locally, or always when forcePull (e.g. :latest updates).
|
// Pull the image if it's missing locally, or always when forcePull (e.g. :latest updates).
|
||||||
if (opts.forcePull || !(await this._checkImageExists(service.container_image))) {
|
if (opts.forcePull || !(await this._checkImageExists(service.container_image))) {
|
||||||
const pullStream = await this.docker.pull(service.container_image)
|
await this.pullImage(service.container_image)
|
||||||
await new Promise((res) => this.docker.modem.followProgress(pullStream, res))
|
|
||||||
}
|
}
|
||||||
|
|
||||||
const newContainer = await this.docker.createContainer({
|
const newContainer = await this.docker.createContainer({
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue