From 7523caa16b9a433b7978a7b5c822e8edd7ced235 Mon Sep 17 00:00:00 2001 From: Vitor Hugo Date: Mon, 30 Mar 2026 16:29:50 -0300 Subject: [PATCH] build: extend signal handling scope to cover the unpack phase (#1358) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 --- Sources/ContainerCommands/BuildCommand.swift | 133 +++++++++--------- .../Container/ContainerStats.swift | 42 ++++-- 2 files changed, 97 insertions(+), 78 deletions(-) diff --git a/Sources/ContainerCommands/BuildCommand.swift b/Sources/ContainerCommands/BuildCommand.swift index b8f633b9..d5ed36ed 100644 --- a/Sources/ContainerCommands/BuildCommand.swift +++ b/Sources/ContainerCommands/BuildCommand.swift @@ -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)"]) } diff --git a/Sources/ContainerCommands/Container/ContainerStats.swift b/Sources/ContainerCommands/Container/ContainerStats.swift index f424689c..f2090dd1 100644 --- a/Sources/ContainerCommands/Container/ContainerStats.swift +++ b/Sources/ContainerCommands/Container/ContainerStats.swift @@ -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)