From e4960c38a9db4970e0d59bcbe722e0f7a8f1a53e Mon Sep 17 00:00:00 2001 From: zaineel <127276018+zaineel@users.noreply.github.com> Date: Thu, 18 Dec 2025 14:19:15 -0600 Subject: [PATCH 1/2] Handle stop/kill when containers are missing --- Package.swift | 8 +++ .../Container/ContainerKill.swift | 17 +++--- .../Container/ContainerStop.swift | 38 ++++++++---- .../ContainerStopValidationTests.swift | 61 +++++++++++++++++++ 4 files changed, 103 insertions(+), 21 deletions(-) create mode 100644 Tests/ContainerCommandsTests/ContainerStopValidationTests.swift diff --git a/Package.swift b/Package.swift index 16f023b8..e92014d4 100644 --- a/Package.swift +++ b/Package.swift @@ -347,6 +347,14 @@ let package = Package( name: "SocketForwarderTests", dependencies: ["SocketForwarder"] ), + .testTarget( + name: "ContainerCommandsTests", + dependencies: [ + "ContainerCommands", + "ContainerClient", + .product(name: "ContainerizationError", package: "containerization"), + ] + ), .testTarget( name: "CLITests", dependencies: [ diff --git a/Sources/ContainerCommands/Container/ContainerKill.swift b/Sources/ContainerCommands/Container/ContainerKill.swift index c6bc4079..8e9d2fc5 100644 --- a/Sources/ContainerCommands/Container/ContainerKill.swift +++ b/Sources/ContainerCommands/Container/ContainerKill.swift @@ -50,21 +50,18 @@ extension Application { } public mutating func run() async throws { - let set = Set(containerIds) + let allContainers = try await ClientContainer.list() - 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 containersToSignal = try self.all + ? allContainers.filter { $0.status == .running } + : ContainerStop.containers(matching: containerIds, in: allContainers) + + let runningContainers = containersToSignal.filter { $0.status == .running } let signalNumber = try Signals.parseSignal(signal) var failed: [String] = [] - for container in containers { + for container in runningContainers { do { try await container.kill(signalNumber) print(container.id) diff --git a/Sources/ContainerCommands/Container/ContainerStop.swift b/Sources/ContainerCommands/Container/ContainerStop.swift index 09654656..9f8db65f 100644 --- a/Sources/ContainerCommands/Container/ContainerStop.swift +++ b/Sources/ContainerCommands/Container/ContainerStop.swift @@ -20,6 +20,13 @@ import ContainerizationError import ContainerizationOS import Foundation +package protocol ContainerIdentifiable { + var id: String { get } + var status: RuntimeStatus { get } +} + +extension ClientContainer: ContainerIdentifiable {} + extension Application { public struct ContainerStop: AsyncParsableCommand { public init() {} @@ -44,25 +51,20 @@ extension Application { var containerIds: [String] = [] public func validate() throws { - if containerIds.count == 0 && !all { + if containerIds.isEmpty && !all { throw ContainerizationError(.invalidArgument, message: "no containers specified and --all not supplied") } - if containerIds.count > 0 && all { + if !containerIds.isEmpty && all { throw ContainerizationError( .invalidArgument, message: "explicitly supplied container IDs conflict with the --all flag") } } public mutating func run() async throws { - let set = Set(containerIds) - var containers = [ClientContainer]() - 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 containers = try self.all + ? allContainers + : Self.containers(matching: containerIds, in: allContainers) let opts = ContainerStopOptions( timeoutInSeconds: self.time, @@ -103,5 +105,19 @@ extension Application { return failed } + + static func containers( + matching containerIds: [String], + in allContainers: [C] + ) throws -> [C] { + var matched: [C] = [] + for containerId in containerIds { + guard let container = allContainers.first(where: { $0.id == containerId || $0.id.starts(with: containerId) }) else { + throw ContainerizationError(.notFound, message: "no such container: \(containerId)") + } + matched.append(container) + } + return matched + } } } diff --git a/Tests/ContainerCommandsTests/ContainerStopValidationTests.swift b/Tests/ContainerCommandsTests/ContainerStopValidationTests.swift new file mode 100644 index 00000000..45d5e666 --- /dev/null +++ b/Tests/ContainerCommandsTests/ContainerStopValidationTests.swift @@ -0,0 +1,61 @@ +//===----------------------------------------------------------------------===// +// 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 ContainerClient +import ContainerCommands +import ContainerizationError +import Testing + +@testable import ContainerCommands + +struct ContainerStopValidationTests { + struct StubContainer: ContainerIdentifiable { + let id: String + let status: RuntimeStatus + } + + @Test + func resolvesExactIds() throws { + let containers = [StubContainer(id: "abc123", status: .running)] + let matched = try Application.ContainerStop.containers(matching: ["abc123"], in: containers) + #expect(matched.count == 1) + #expect(matched.first?.id == "abc123") + } + + @Test + func resolvesIdPrefixes() throws { + let containers = [ + StubContainer(id: "abcdef", status: .running), + StubContainer(id: "123456", status: .running), + ] + let matched = try Application.ContainerStop.containers(matching: ["abc"], in: containers) + #expect(matched.count == 1) + #expect(matched.first?.id == "abcdef") + } + + @Test + func throwsForMissingContainers() throws { + let containers = [StubContainer(id: "abcdef", status: .running)] + #expect { + _ = try Application.ContainerStop.containers(matching: ["missing"], in: containers) + } throws: { error in + guard let error = error as? ContainerizationError else { + return false + } + return error.kind == .notFound + } + } +} From 14afdfce6b4da4fe38c5e62e1d8b3442ac4f5905 Mon Sep 17 00:00:00 2001 From: zaineel <127276018+zaineel@users.noreply.github.com> Date: Thu, 18 Dec 2025 14:31:03 -0600 Subject: [PATCH 2/2] Fix test dependency and run stop validation tests --- Package.swift | 2 +- Tests/ContainerCommandsTests/ContainerStopValidationTests.swift | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Package.swift b/Package.swift index e92014d4..3e7f7d01 100644 --- a/Package.swift +++ b/Package.swift @@ -352,7 +352,7 @@ let package = Package( dependencies: [ "ContainerCommands", "ContainerClient", - .product(name: "ContainerizationError", package: "containerization"), + .product(name: "Containerization", package: "containerization"), ] ), .testTarget( diff --git a/Tests/ContainerCommandsTests/ContainerStopValidationTests.swift b/Tests/ContainerCommandsTests/ContainerStopValidationTests.swift index 45d5e666..d39f1bf7 100644 --- a/Tests/ContainerCommandsTests/ContainerStopValidationTests.swift +++ b/Tests/ContainerCommandsTests/ContainerStopValidationTests.swift @@ -55,7 +55,7 @@ struct ContainerStopValidationTests { guard let error = error as? ContainerizationError else { return false } - return error.kind == .notFound + return error.code == .notFound } } }