CLI: stop and kill should error on not found containers

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 ; \

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<String>(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)
             }
         }
     }

Sources/ContainerCommands/Container/ContainerStop.swift

@@ -43,65 +43,101 @@ 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<String>(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)")
+                    }
                 }
             }
 
             let opts = ContainerStopOptions(
                 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)
+            }
         }
     }
 }

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 {

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)"])

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))
+    }
+}