build: extend signal handling scope to cover the unpack phase (#1358)

## Type of Change
- [x] Bug fix
- [ ] New feature
- [ ] Breaking change
- [ ] Documentation update

## Motivation and Context
Fixes #1354.

When `container build` completes the build phase, it enters an unpack
phase that runs outside the `withThrowingTaskGroup` containing the
signal handler. As a result, pressing Ctrl+C during unpacking has no
effect — the SIGINT is never caught and the process keeps running until
the unpack finishes.

Fix this by moving the unpack phase inside the existing build task, so
both build and unpack run within the same task group that hosts the
`AsyncSignalHandler`. The `Task.checkCancellation()` calls already
present in the unpack loop will now fire correctly when a signal is
received.

## Testing
- [x] Tested locally
- [ ] Added/updated tests
- [ ] Added/updated docs
This commit is contained in:
Vitor Hugo
2026-03-30 12:29:50 -07:00
committed by GitHub
parent 4a08bb0daa
commit 7523caa16b
2 changed files with 97 additions and 78 deletions
+67 -66
View File
@@ -350,7 +350,8 @@ extension Application {
}
return results
}()
group.addTask { [terminal, buildArg, secretsData, contextDir, hiddenDockerDir, label, noCache, target, quiet, cacheIn, cacheOut, pull] in
group.addTask {
[terminal, buildArg, secretsData, contextDir, hiddenDockerDir, label, noCache, target, quiet, cacheIn, cacheOut, pull, exports, imageNames, tempURL, log] in
let config = Builder.BuildConfig(
buildID: buildID,
contentStore: RemoteContentStoreClient(),
@@ -374,75 +375,75 @@ extension Application {
progress.finish()
try await builder.build(config)
let unpackProgressConfig = try ProgressConfig(
description: "Unpacking built image",
itemsName: "entries",
showTasks: exports.count > 1,
totalTasks: exports.count
)
let unpackProgress = ProgressBar(config: unpackProgressConfig)
defer {
unpackProgress.finish()
}
unpackProgress.start()
var finalMessage = "Successfully built \(imageNames.joined(separator: ", "))"
let taskManager = ProgressTaskCoordinator()
// Currently, only a single export can be specified.
for exp in exports {
unpackProgress.add(tasks: 1)
let unpackTask = await taskManager.startTask()
switch exp.type {
case "oci":
try Task.checkCancellation()
guard let dest = exp.destination else {
throw ContainerizationError(.invalidArgument, message: "dest is required \(exp.rawValue)")
}
let result = try await ClientImage.load(from: dest.absolutePath(), force: false)
guard result.rejectedMembers.isEmpty else {
log.error("archive contains invalid members", metadata: ["paths": "\(result.rejectedMembers)"])
throw ContainerizationError(.internalError, message: "failed to load archive")
}
for image in result.images {
try Task.checkCancellation()
try await image.unpack(platform: nil, progressUpdate: ProgressTaskCoordinator.handler(for: unpackTask, from: unpackProgress.handler))
// Tag the unpacked image with all requested tags
for tagName in imageNames {
try Task.checkCancellation()
_ = try await image.tag(new: tagName)
}
}
case "tar":
guard let dest = exp.destination else {
throw ContainerizationError(.invalidArgument, message: "dest is required \(exp.rawValue)")
}
let tarURL = tempURL.appendingPathComponent("out.tar")
try FileManager.default.moveItem(at: tarURL, to: dest)
finalMessage = "Successfully exported to \(dest.absolutePath())"
case "local":
guard let dest = exp.destination else {
throw ContainerizationError(.invalidArgument, message: "dest is required \(exp.rawValue)")
}
let localDir = tempURL.appendingPathComponent("local")
guard FileManager.default.fileExists(atPath: localDir.path) else {
throw ContainerizationError(.invalidArgument, message: "expected local output not found")
}
try FileManager.default.copyItem(at: localDir, to: dest)
finalMessage = "Successfully exported to \(dest.absolutePath())"
default:
throw ContainerizationError(.invalidArgument, message: "invalid exporter \(exp.rawValue)")
}
}
await taskManager.finish()
unpackProgress.finish()
print(finalMessage)
}
try await group.next()
}
let unpackProgressConfig = try ProgressConfig(
description: "Unpacking built image",
itemsName: "entries",
showTasks: exports.count > 1,
totalTasks: exports.count
)
let unpackProgress = ProgressBar(config: unpackProgressConfig)
defer {
unpackProgress.finish()
}
unpackProgress.start()
var finalMessage = "Successfully built \(imageNames.joined(separator: ", "))"
let taskManager = ProgressTaskCoordinator()
// Currently, only a single export can be specified.
for exp in exports {
unpackProgress.add(tasks: 1)
let unpackTask = await taskManager.startTask()
switch exp.type {
case "oci":
try Task.checkCancellation()
guard let dest = exp.destination else {
throw ContainerizationError(.invalidArgument, message: "dest is required \(exp.rawValue)")
}
let result = try await ClientImage.load(from: dest.absolutePath(), force: false)
guard result.rejectedMembers.isEmpty else {
log.error("archive contains invalid members", metadata: ["paths": "\(result.rejectedMembers)"])
throw ContainerizationError(.internalError, message: "failed to load archive")
}
for image in result.images {
try Task.checkCancellation()
try await image.unpack(platform: nil, progressUpdate: ProgressTaskCoordinator.handler(for: unpackTask, from: unpackProgress.handler))
// Tag the unpacked image with all requested tags
for tagName in imageNames {
try Task.checkCancellation()
_ = try await image.tag(new: tagName)
}
}
case "tar":
guard let dest = exp.destination else {
throw ContainerizationError(.invalidArgument, message: "dest is required \(exp.rawValue)")
}
let tarURL = tempURL.appendingPathComponent("out.tar")
try FileManager.default.moveItem(at: tarURL, to: dest)
finalMessage = "Successfully exported to \(dest.absolutePath())"
case "local":
guard let dest = exp.destination else {
throw ContainerizationError(.invalidArgument, message: "dest is required \(exp.rawValue)")
}
let localDir = tempURL.appendingPathComponent("local")
guard FileManager.default.fileExists(atPath: localDir.path) else {
throw ContainerizationError(.invalidArgument, message: "expected local output not found")
}
try FileManager.default.copyItem(at: localDir, to: dest)
finalMessage = "Successfully exported to \(dest.absolutePath())"
default:
throw ContainerizationError(.invalidArgument, message: "invalid exporter \(exp.rawValue)")
}
}
await taskManager.finish()
unpackProgress.finish()
print(finalMessage)
} catch {
throw NSError(domain: "Build", code: 1, userInfo: [NSLocalizedDescriptionKey: "\(error)"])
}
@@ -19,6 +19,7 @@ import ContainerAPIClient
import ContainerResource
import ContainerizationError
import ContainerizationExtras
import ContainerizationOS
import Foundation
extension Application {
@@ -57,7 +58,24 @@ extension Application {
fflush(stdout)
}
try await runStreaming()
let containerIds = containers
try await withThrowingTaskGroup(of: Void.self) { group in
defer { group.cancelAll() }
group.addTask {
let handler = AsyncSignalHandler.create(notify: [SIGINT, SIGTERM])
for await _ in handler.signals {
throw CancellationError()
}
}
group.addTask { [containerIds] in
try await Self.runStreaming(containerIds: containerIds)
}
do {
try await group.next()
} catch is CancellationError {
// Normal exit on signal, defer will restore the terminal
}
}
}
}
@@ -82,7 +100,7 @@ extension Application {
}
}
let statsData = try await collectStats(client: client, for: containersToShow)
let statsData = try await Self.collectStats(client: client, for: containersToShow)
if format == .json {
let jsonStats = statsData.map { $0.stats2 }
@@ -91,16 +109,16 @@ extension Application {
return
}
printStatsTable(statsData)
Self.printStatsTable(statsData)
}
private func runStreaming() async throws {
private static func runStreaming(containerIds: [String]) async throws {
let client = ContainerClient()
// If containers were specified, validate they all exist upfront
if !containers.isEmpty {
let specifiedContainers = try await client.list(filters: ContainerListFilters(ids: containers))
for containerId in containers {
if !containerIds.isEmpty {
let specifiedContainers = try await client.list(filters: ContainerListFilters(ids: containerIds))
for containerId in containerIds {
guard specifiedContainers.contains(where: { $0.id == containerId }) else {
throw ContainerizationError(
.notFound,
@@ -117,10 +135,10 @@ extension Application {
while true {
do {
let containersToShow: [ContainerSnapshot]
if containers.isEmpty {
if containerIds.isEmpty {
containersToShow = try await client.list(filters: ContainerListFilters(status: .running))
} else {
containersToShow = try await client.list(filters: ContainerListFilters(ids: containers))
containersToShow = try await client.list(filters: ContainerListFilters(ids: containerIds))
}
let statsData = try await collectStats(client: client, for: containersToShow)
@@ -146,7 +164,7 @@ extension Application {
let stats2: ContainerResource.ContainerStats
}
private func collectStats(client: ContainerClient, for containers: [ContainerSnapshot]) async throws -> [StatsSnapshot] {
private static func collectStats(client: ContainerClient, for containers: [ContainerSnapshot]) async throws -> [StatsSnapshot] {
var snapshots: [StatsSnapshot] = []
// First sample
@@ -218,7 +236,7 @@ extension Application {
}
}
private func printStatsTable(_ statsData: [StatsSnapshot]) {
private static func printStatsTable(_ statsData: [StatsSnapshot]) {
let headerRow = ["Container ID", "Cpu %", "Memory Usage", "Net Rx/Tx", "Block I/O", "Pids"]
let notAvailable = "--"
var rows = [headerRow]
@@ -263,7 +281,7 @@ extension Application {
print(formatter.format())
}
private func clearScreen() {
private static func clearScreen() {
// Move cursor to home position and clear from cursor to end of screen
print("\u{001B}[H\u{001B}[J", terminator: "")
fflush(stdout)