diff --git a/Makefile b/Makefile index 0e7dea0e..d1ca97b0 100644 --- a/Makefile +++ b/Makefile @@ -174,6 +174,7 @@ integration: init-block $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLINetwork || exit_code=1 ; \ $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLIRunLifecycle || exit_code=1 ; \ $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLIExecCommand || exit_code=1 ; \ + $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLIStopKillErrors || exit_code=1 ; \ $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLICreateCommand || exit_code=1 ; \ $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLIRunCommand || exit_code=1 ; \ $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLIImagesCommand || exit_code=1 ; \ diff --git a/Sources/ContainerCommands/Container/ContainerKill.swift b/Sources/ContainerCommands/Container/ContainerKill.swift index c6bc4079..e2aac839 100644 --- a/Sources/ContainerCommands/Container/ContainerKill.swift +++ b/Sources/ContainerCommands/Container/ContainerKill.swift @@ -40,6 +40,11 @@ extension Application { @Argument(help: "Container IDs") var containerIds: [String] = [] + struct KillError: Error { + let succeeded: [String] + let failed: [(String, Error)] + } + public func validate() throws { if containerIds.count == 0 && !all { throw ContainerizationError(.invalidArgument, message: "no containers specified and --all not supplied") @@ -50,31 +55,67 @@ extension Application { } public mutating func run() async throws { - let set = Set(containerIds) + var containers = [ClientContainer]() + var allErrors: [String] = [] - var containers = try await ClientContainer.list().filter { c in - c.status == .running - } - if !self.all { - containers = containers.filter { c in - set.contains(c.id) + let allContainers = try await ClientContainer.list() + + if self.all { + containers = allContainers.filter { c in + c.status == .running + } + } else { + let containerMap = Dictionary(uniqueKeysWithValues: allContainers.map { ($0.id, $0) }) + + for id in containerIds { + if let container = containerMap[id] { + containers.append(container) + } else { + allErrors.append("Error: No such container: \(id)") + } } } let signalNumber = try Signals.parseSignal(signal) - var failed: [String] = [] + do { + try await Self.killContainers(containers: containers, signal: signalNumber) + for container in containers { + print(container.id) + } + } catch let error as KillError { + for id in error.succeeded { + print(id) + } + + for (_, err) in error.failed { + allErrors.append("Error from APIServer: \(err.localizedDescription)") + } + } + if !allErrors.isEmpty { + var stderr = StandardError() + for error in allErrors { + print(error, to: &stderr) + } + throw ExitCode(1) + } + } + + static func killContainers(containers: [ClientContainer], signal: Int32) async throws { + var succeeded: [String] = [] + var failed: [(String, Error)] = [] + for container in containers { do { - try await container.kill(signalNumber) - print(container.id) + try await container.kill(signal) + succeeded.append(container.id) } catch { - log.error("failed to kill container \(container.id): \(error)") - failed.append(container.id) + failed.append((container.id, error)) } } - if failed.count > 0 { - throw ContainerizationError(.internalError, message: "kill failed for one or more containers \(failed.joined(separator: ","))") + + if !failed.isEmpty { + throw KillError(succeeded: succeeded, failed: failed) } } } diff --git a/Sources/ContainerCommands/Container/ContainerStop.swift b/Sources/ContainerCommands/Container/ContainerStop.swift index 09654656..3d245b95 100644 --- a/Sources/ContainerCommands/Container/ContainerStop.swift +++ b/Sources/ContainerCommands/Container/ContainerStop.swift @@ -43,24 +43,42 @@ extension Application { @Argument(help: "Container IDs") var containerIds: [String] = [] + package struct StopError: Error { + let succeeded: [String] + let failed: [(String, Error)] + } + public func validate() throws { if containerIds.count == 0 && !all { - throw ContainerizationError(.invalidArgument, message: "no containers specified and --all not supplied") + throw ContainerizationError( + .invalidArgument, + message: "no containers specified and --all not supplied" + ) } if containerIds.count > 0 && all { throw ContainerizationError( - .invalidArgument, message: "explicitly supplied container IDs conflict with the --all flag") + .invalidArgument, + message: "explicitly supplied container IDs conflict with the --all flag" + ) } } public mutating func run() async throws { - let set = Set(containerIds) var containers = [ClientContainer]() + var allErrors: [String] = [] + if self.all { containers = try await ClientContainer.list() } else { - containers = try await ClientContainer.list().filter { c in - set.contains(c.id) + let allContainers = try await ClientContainer.list() + let containerMap = Dictionary(uniqueKeysWithValues: allContainers.map { ($0.id, $0) }) + + for id in containerIds { + if let container = containerMap[id] { + containers.append(container) + } else { + allErrors.append("Error: No such container: \(id)") + } } } @@ -68,40 +86,58 @@ extension Application { timeoutInSeconds: self.time, signal: try Signals.parseSignal(self.signal) ) - let failed = try await Self.stopContainers(containers: containers, stopOptions: opts) - if failed.count > 0 { - throw ContainerizationError( - .internalError, - message: "stop failed for one or more containers \(failed.joined(separator: ","))" - ) + + do { + try await Self.stopContainers(containers: containers, stopOptions: opts) + for container in containers { + print(container.id) + } + } catch let error as ContainerStop.StopError { + for id in error.succeeded { + print(id) + } + + for (_, err) in error.failed { + allErrors.append("Error from APIServer: \(err)") + } + } + + if !allErrors.isEmpty { + var stderr = StandardError() + for err in allErrors { + print(err, to: &stderr) + } + throw ExitCode(1) } } - static func stopContainers(containers: [ClientContainer], stopOptions: ContainerStopOptions) async throws -> [String] { - var failed: [String] = [] - try await withThrowingTaskGroup(of: ClientContainer?.self) { group in + static func stopContainers(containers: [ClientContainer], stopOptions: ContainerStopOptions) async throws { + var succeeded: [String] = [] + var failed: [(String, Error)] = [] + try await withThrowingTaskGroup(of: (String, Error?).self) { group in for container in containers { group.addTask { do { try await container.stop(opts: stopOptions) - print(container.id) - return nil + return (container.id, nil) } catch { - log.error("failed to stop container \(container.id): \(error)") - return container + return (container.id, error) } } } - for try await ctr in group { - guard let ctr else { - continue + for try await (id, error) in group { + if let error = error { + failed.append((id, error)) + } else { + succeeded.append(id) } - failed.append(ctr.id) } } - return failed + if !failed.isEmpty { + throw StopError(succeeded: succeeded, failed: failed) + } } } } diff --git a/Sources/ContainerCommands/Container/ProcessUtils.swift b/Sources/ContainerCommands/Container/ProcessUtils.swift index a2cb981d..ca4ed466 100644 --- a/Sources/ContainerCommands/Container/ProcessUtils.swift +++ b/Sources/ContainerCommands/Container/ProcessUtils.swift @@ -20,6 +20,14 @@ import ContainerizationError import ContainerizationOS import Foundation +struct StandardError: TextOutputStream, Sendable { + private static let handle = FileHandle.standardError + + public func write(_ string: String) { + Self.handle.write(Data(string.utf8)) + } +} + extension Application { static func ensureRunning(container: ClientContainer) throws { if container.status != .running { diff --git a/Sources/ContainerCommands/System/SystemStop.swift b/Sources/ContainerCommands/System/SystemStop.swift index a6617c72..fd3e3a88 100644 --- a/Sources/ContainerCommands/System/SystemStop.swift +++ b/Sources/ContainerCommands/System/SystemStop.swift @@ -65,9 +65,13 @@ extension Application { let containers = try await ClientContainer.list() let signal = try Signals.parseSignal("SIGTERM") let opts = ContainerStopOptions(timeoutInSeconds: Self.stopTimeoutSeconds, signal: signal) - let failed = try await ContainerStop.stopContainers(containers: containers, stopOptions: opts) - if !failed.isEmpty { - log.warning("some containers could not be stopped gracefully", metadata: ["ids": "\(failed)"]) + + do { + try await ContainerStop.stopContainers(containers: containers, stopOptions: opts) + } catch let error as ContainerStop.StopError { + for (id, error) in error.failed { + log.warning("container \(id) failed to stop: \(error)") + } } } catch { log.warning("failed to stop all containers", metadata: ["error": "\(error)"]) diff --git a/Tests/CLITests/Subcommands/Containers/TestCLIStopKillErrors.swift b/Tests/CLITests/Subcommands/Containers/TestCLIStopKillErrors.swift new file mode 100644 index 00000000..05dd18e9 --- /dev/null +++ b/Tests/CLITests/Subcommands/Containers/TestCLIStopKillErrors.swift @@ -0,0 +1,122 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2025 Apple Inc. and the container project authors. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +//===----------------------------------------------------------------------===// + +import Foundation +import Testing + +class TestCLIStopKillErrors: CLITest { + @Test func testStopNonExistentContainer() throws { + let nonExistentName = "nonexistent-container-\(UUID().uuidString)" + + let (_, error, status) = try run(arguments: [ + "stop", + nonExistentName, + ]) + + #expect(status == 1) + #expect(error.contains(nonExistentName)) + } + + @Test func testStopMultipleNonExistentContainers() throws { + let nonExistent1 = "nonexistent-1-\(UUID().uuidString)" + let nonExistent2 = "nonexistent-2-\(UUID().uuidString)" + + let (_, error, status) = try run(arguments: [ + "stop", + nonExistent1, + nonExistent2, + ]) + + #expect(status == 1) + #expect(error.contains(nonExistent1)) + #expect(error.contains(nonExistent2)) + } + + @Test func testKillNonExistentContainer() throws { + let nonExistentName = "nonexistent-container-\(UUID().uuidString)" + + let (_, error, status) = try run(arguments: [ + "kill", + nonExistentName, + ]) + + #expect(status == 1) + #expect(error.contains(nonExistentName)) + } + + @Test func testKillMultipleNonExistentContainers() throws { + let nonExistent1 = "nonexistent-1-\(UUID().uuidString)" + let nonExistent2 = "nonexistent-2-\(UUID().uuidString)" + + let (_, error, status) = try run(arguments: [ + "kill", + nonExistent1, + nonExistent2, + ]) + + #expect(status == 1) + #expect(error.contains(nonExistent1)) + #expect(error.contains(nonExistent2)) + } + + @Test func testStopMixedExistentAndNonExistent() throws { + let existentName = "test-stop-mixed-\(UUID().uuidString)" + let nonExistentName = "nonexistent-\(UUID().uuidString)" + + try doCreate(name: existentName) + try doStart(name: existentName) + try waitForContainerRunning(existentName) + + defer { + try? doStop(name: existentName) + try? doRemove(name: existentName) + } + + let (output, error, status) = try run(arguments: [ + "stop", + existentName, + nonExistentName, + ]) + + #expect(status == 1) + #expect(output.contains(existentName)) + #expect(error.contains(nonExistentName)) + } + + @Test func testKillMixedExistentAndNonExistent() throws { + let existentName = "test-kill-mixed-\(UUID().uuidString)" + let nonExistentName = "nonexistent-\(UUID().uuidString)" + + try doCreate(name: existentName) + try doStart(name: existentName) + try waitForContainerRunning(existentName) + + defer { + try? doStop(name: existentName) + try? doRemove(name: existentName) + } + + let (output, error, status) = try run(arguments: [ + "kill", + existentName, + nonExistentName, + ]) + + #expect(status == 1) + #expect(output.contains(existentName)) + #expect(error.contains(nonExistentName)) + } +}