Merge 4c135c251f into 54ee30a449

admin/app/services/docker_service.ts

@@ -5,6 +5,7 @@ import { inject } from '@adonisjs/core'
 import transmit from '@adonisjs/transmit/services/main'
 import { doResumableDownloadWithRetry } from '../utils/downloads.js'
 import { pickAmdHsaOverride } from '../utils/amd_hsa_override.js'
+import { joinInFlight } from '../utils/in_flight.js'
 import { join } from 'path'
 import os from 'node:os'
 import env from '#start/env'
@@ -35,6 +36,9 @@ const AMD_GFX_MARKER_PATH = '/app/storage/.nomad-amd-gfx'
 export class DockerService {
   public docker: Docker
   private activeInstallations: Set<string> = new Set()
+  // Static, not per-instance: controllers get a fresh DockerService per request, and
+  // the installs that race (#1439) arrive on separate requests.
+  private static installsInFlight = new Map<string, Promise<void>>()
   public static NOMAD_NETWORK = 'project-nomad_default'
   public static ADMIN_CONTAINER_NAME = 'nomad_admin'
   // The admin's own storage mount destination inside its container (compose maps
@@ -627,6 +631,17 @@ export class DockerService {
   async _createContainer(
     service: Service & { dependencies?: Service[] },
     containerConfig: any
+  ): Promise<void> {
+    // A service that is already installing (its own install, or another service's
+    // dependency install) is joined rather than started again (#1439).
+    return joinInFlight(DockerService.installsInFlight, service.service_name, () =>
+      this._installContainer(service, containerConfig)
+    )
+  }
+
+  private async _installContainer(
+    service: Service & { dependencies?: Service[] },
+    containerConfig: any
   ): Promise<void> {
     try {
       this._broadcast(service.service_name, 'initializing', '')
@@ -652,11 +667,19 @@ export class DockerService {
         )
         for (const dependency of dependencies) {
           if (!dependency.installed) {
-            this._broadcast(
-              service.service_name,
-              'dependency-not-installed',
-              `Dependency service ${dependency.service_name} is not installed. Installing it first...`
-            )
+            if (DockerService.installsInFlight.has(dependency.service_name)) {
+              this._broadcast(
+                service.service_name,
+                'dependency-installing',
+                `Dependency service ${dependency.service_name} is already being installed. Waiting for it to finish...`
+              )
+            } else {
+              this._broadcast(
+                service.service_name,
+                'dependency-not-installed',
+                `Dependency service ${dependency.service_name} is not installed. Installing it first...`
+              )
+            }
             await this._createContainer(
               dependency,
               this._parseContainerConfig(dependency.container_config)

admin/app/utils/in_flight.ts

@@ -0,0 +1,29 @@
+/**
+ * Shares one run of an async task between every caller that asks for the same
+ * key while it is running.
+ *
+ * Used by `DockerService._createContainer` so that two installs of the same
+ * service can't run side by side (#1439). Installing Ollama and Qdrant together
+ * from the setup wizard starts Qdrant's own install, then Ollama's dependency
+ * check, which sees Qdrant not yet installed and starts a second one. The second
+ * hits Docker's 409 on the container name, and its failure cleanup removes the
+ * container the first install created.
+ *
+ * The run is registered synchronously, before `start` yields, so a caller that
+ * arrives on the next tick already finds it. It is removed once it settles,
+ * whether it resolved or rejected, so a failed install can be retried.
+ */
+export function joinInFlight<T>(
+  inFlight: Map<string, Promise<T>>,
+  key: string,
+  start: () => Promise<T>
+): Promise<T> {
+  const running = inFlight.get(key)
+  if (running) return running
+
+  const run = start().finally(() => {
+    if (inFlight.get(key) === run) inFlight.delete(key)
+  })
+  inFlight.set(key, run)
+  return run
+}

admin/tests/unit/in_flight.spec.ts

@@ -0,0 +1,79 @@
+import * as assert from 'node:assert/strict'
+import { test } from 'node:test'
+
+import { joinInFlight } from '../../app/utils/in_flight.js'
+
+function deferred() {
+  let resolve!: () => void
+  let reject!: (err: Error) => void
+  const promise = new Promise<void>((res, rej) => {
+    resolve = res
+    reject = rej
+  })
+  return { promise, resolve, reject }
+}
+
+test('a second install of a service that is still installing joins the first (#1439)', async () => {
+  const inFlight = new Map<string, Promise<void>>()
+  const qdrant = deferred()
+  let starts = 0
+  const install = () => {
+    starts++
+    return qdrant.promise
+  }
+
+  // Qdrant's own install, then Ollama's dependency check for Qdrant, on the next tick.
+  const own = joinInFlight(inFlight, 'nomad_qdrant', install)
+  await Promise.resolve()
+  const fromDependency = joinInFlight(inFlight, 'nomad_qdrant', install)
+
+  assert.equal(starts, 1)
+  assert.equal(fromDependency, own)
+
+  qdrant.resolve()
+  await fromDependency
+  assert.equal(inFlight.has('nomad_qdrant'), false)
+})
+
+test('a joined install that fails rejects every caller once, and can be retried', async () => {
+  const inFlight = new Map<string, Promise<void>>()
+  const first = deferred()
+  let starts = 0
+
+  const a = joinInFlight(inFlight, 'nomad_qdrant', () => {
+    starts++
+    return first.promise
+  })
+  const b = joinInFlight(inFlight, 'nomad_qdrant', () => {
+    starts++
+    return Promise.resolve()
+  })
+  first.reject(new Error('pull failed'))
+
+  await assert.rejects(a, /pull failed/)
+  await assert.rejects(b, /pull failed/)
+  assert.equal(starts, 1)
+  assert.equal(inFlight.has('nomad_qdrant'), false)
+
+  await joinInFlight(inFlight, 'nomad_qdrant', () => {
+    starts++
+    return Promise.resolve()
+  })
+  assert.equal(starts, 2)
+})
+
+test('installs of different services run independently', async () => {
+  const inFlight = new Map<string, Promise<void>>()
+  const qdrant = deferred()
+  let ollamaStarted = false
+
+  const q = joinInFlight(inFlight, 'nomad_qdrant', () => qdrant.promise)
+  await joinInFlight(inFlight, 'nomad_ollama', async () => {
+    ollamaStarted = true
+  })
+
+  assert.equal(ollamaStarted, true)
+  assert.equal(inFlight.has('nomad_qdrant'), true)
+  qdrant.resolve()
+  await q
+})