Skip to content

Commit d1d7635

Browse files
authored
Fix BuilderStart race, parallelize container build tests. (#2002)
- Closes #2001. - Handle "container exists" error gracefully instead of failing, when trying to start the buildkit container. - Move build tests to parallel suites, while the builder lifecycle tests remain serial. Parallel builds don't use the fixture lock that deletes and restarts the builder and runs a build block in isolation.
1 parent 78e2cb4 commit d1d7635

13 files changed

Lines changed: 1469 additions & 1595 deletions

Sources/ContainerCommands/Builder/BuilderStart.swift

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -292,11 +292,17 @@ extension Application {
292292
.setDescription("Starting BuildKit container")
293293
])
294294

295-
try await client.create(
296-
configuration: config,
297-
options: .default,
298-
kernel: kernel
299-
)
295+
do {
296+
try await client.create(
297+
configuration: config,
298+
options: .default,
299+
kernel: kernel
300+
)
301+
} catch let error as ContainerizationError where error.code == .exists {
302+
// A concurrent `container build` invocation already created the builder
303+
// while we were fetching the image/kernel above. `bootstrap` below is
304+
// idempotent, so just proceed against the container the winner created.
305+
}
300306

301307
try await startBuildKit(client: client, id: Builder.builderContainerId, progressUpdate, taskManager)
302308
log.debug("starting BuildKit and BuildKit-shim")

Sources/ContainerTestSupport/BuildFixture.swift

Lines changed: 0 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -94,42 +94,6 @@ extension ContainerFixture {
9494
}
9595
throw CommandError.executionFailed("timed out waiting for container-builder-shim on buildkit")
9696
}
97-
98-
/// Deletes any existing builder, starts a fresh one, runs `body`, then deletes the builder.
99-
///
100-
/// Each build test gets an isolated builder to avoid inter-test contamination.
101-
/// Acquires a process-wide lock so only one test holds the buildkit singleton at a time,
102-
/// regardless of how many suites run concurrently in the global pass.
103-
public func withBuilder(
104-
cpus: Int64 = 2,
105-
memoryInGBs: Int64 = 2,
106-
_ body: @Sendable (ContainerFixture) async throws -> Void
107-
) async throws {
108-
try await withoutActuallyEscaping(body) { escapingBody in
109-
try await Self.builderLock.withLock { _ in
110-
_ = try? self.run(["builder", "delete", "--force"])
111-
try self.builderStart(cpus: cpus, memoryInGBs: memoryInGBs)
112-
defer { _ = try? self.run(["builder", "delete", "--force"]) }
113-
try await self.waitForBuilderRunning()
114-
try await escapingBody(self)
115-
}
116-
}
117-
}
118-
119-
/// Acquires the process-wide builder lock without starting a builder.
120-
///
121-
/// Use this in tests that manually manage the builder lifecycle (e.g. lifecycle
122-
/// tests that call ``builderStart()``/``builderStop()`` directly) so they
123-
/// serialise correctly with tests that use ``withBuilder(_:)``.
124-
public func withBuilderLock<T: Sendable>(_ body: @Sendable () async throws -> T) async throws -> T {
125-
try await withoutActuallyEscaping(body) { escapingBody in
126-
try await Self.builderLock.withLock { _ in
127-
try await escapingBody()
128-
}
129-
}
130-
}
131-
132-
private static let builderLock = AsyncLock()
13397
}
13498

13599
// MARK: - Build context helpers

Sources/Services/ContainerAPIService/Client/ContainerClient.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ public struct ContainerClient: Sendable {
7171
}
7272

7373
try await xpcSend(message: request)
74+
} catch let error as ContainerizationError {
75+
throw error
7476
} catch {
7577
throw ContainerizationError(
7678
.internalError,

0 commit comments

Comments
 (0)