From 31c866aa0bb4a9477e27edef8ab33071028cf30d Mon Sep 17 00:00:00 2001 From: Danny Canter Date: Wed, 25 Mar 2026 15:57:34 -0700 Subject: [PATCH] CLI: stop and delete should error on not found containers (#878) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Same as we'd done with kill. ``` ➜ container git:(stop-kill-throw-error) ✗ ./bin/container stop foo bar baz Warning! Running debug build. Performance may be degraded. Error: internalError: "failed to stop container" (cause: "notFound: "container with ID baz not found"") internalError: "failed to stop container" (cause: "notFound: "container with ID bar not found"") internalError: "failed to stop container" (cause: "notFound: "container with ID foo not found"") ➜ container git:(stop-kill-throw-error) ✗ ./bin/container kill foo bar baz Warning! Running debug build. Performance may be degraded. Error: internalError: "failed to kill container" (cause: "notFound: "container with ID foo not found"") internalError: "failed to kill container" (cause: "notFound: "container with ID bar not found"") internalError: "failed to kill container" (cause: "notFound: "container with ID baz not found"") ➜ container git:(stop-kill-throw-error) ✗ ./bin/container delete foo bar baz Warning! Running debug build. Performance may be degraded. Error: internalError: "failed to delete container" (cause: "notFound: "container with ID foo not found"") internalError: "failed to delete container" (cause: "notFound: "container with ID baz not found"") internalError: "failed to delete container" (cause: "notFound: "container with ID bar not found"") ``` --- Makefile | 1 + .../Container/ContainerDelete.swift | 42 +++++-------------- .../Container/ContainerStop.swift | 16 ++++--- .../ContainerCommands/System/SystemStop.swift | 2 +- .../Containers/TestCLINotFound.swift | 37 ++++++++++++++++ 5 files changed, 57 insertions(+), 41 deletions(-) create mode 100644 Tests/CLITests/Subcommands/Containers/TestCLINotFound.swift diff --git a/Makefile b/Makefile index 704dbf97..5295247a 100644 --- a/Makefile +++ b/Makefile @@ -207,6 +207,7 @@ integration: init-block $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLIVolumes || exit_code=1 ; \ $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLIKernelSet || exit_code=1 ; \ $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLIAnonymousVolumes || exit_code=1 ; \ + $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLINotFound || exit_code=1 ; \ $(SWIFT) test -c $(BUILD_CONFIGURATION) $(SWIFT_CONFIGURATION) --filter TestCLINoParallelCases || exit_code=1 ; \ echo Ensuring apiserver stopped after the CLI integration tests ; \ scripts/ensure-container-stopped.sh ; \ diff --git a/Sources/ContainerCommands/Container/ContainerDelete.swift b/Sources/ContainerCommands/Container/ContainerDelete.swift index 04d3842f..c75e48e1 100644 --- a/Sources/ContainerCommands/Container/ContainerDelete.swift +++ b/Sources/ContainerCommands/Container/ContainerDelete.swift @@ -54,49 +54,29 @@ extension Application { } public mutating func run() async throws { - let set = Set(containerIds) let client = ContainerClient() - var containers = [ContainerSnapshot]() + let force = self.force + let containers: [String] if all { - containers = try await client.list() - } else { - let ctrs = try await client.list() - containers = ctrs.filter { c in - set.contains(c.id) - } - // If one of the containers requested isn't present, let's throw. We don't need to do - // this for --all as --all should be perfectly usable with no containers to remove; otherwise, - // it'd be quite clunky. - if containers.count != set.count { - let missing = set.filter { id in - !containers.contains { c in - c.id == id - } + containers = try await client.list().compactMap { c in + // Skip running containers when using --all without --force + if c.status == .running && !force { + return nil } - throw ContainerizationError( - .notFound, - message: "failed to delete one or more containers: \(missing)" - ) + return c.id } + } else { + containers = containerIds } var errors: [any Error] = [] - let force = self.force - let all = self.all try await withThrowingTaskGroup(of: (any Error)?.self) { group in for container in containers { group.addTask { do { - if container.status == .running && !force { - guard all else { - throw ContainerizationError(.invalidState, message: "container is running") - } - return nil // Skip running container when using --all - } - - try await client.delete(id: container.id, force: force) - print(container.id) + try await client.delete(id: container, force: force) + print(container) return nil } catch { return error diff --git a/Sources/ContainerCommands/Container/ContainerStop.swift b/Sources/ContainerCommands/Container/ContainerStop.swift index fd79a784..e42434c3 100644 --- a/Sources/ContainerCommands/Container/ContainerStop.swift +++ b/Sources/ContainerCommands/Container/ContainerStop.swift @@ -56,15 +56,13 @@ extension Application { } public mutating func run() async throws { - let set = Set(containerIds) let client = ContainerClient() - var containers = [ContainerSnapshot]() + + let containers: [String] if self.all { - containers = try await client.list() + containers = try await client.list().map { $0.id } } else { - containers = try await client.list().filter { c in - set.contains(c.id) - } + containers = containerIds } let opts = ContainerStopOptions( @@ -78,14 +76,14 @@ extension Application { ) } - static func stopContainers(client: ContainerClient, containers: [ContainerSnapshot], stopOptions: ContainerStopOptions) async throws { + static func stopContainers(client: ContainerClient, containers: [String], stopOptions: ContainerStopOptions) async throws { var errors: [any Error] = [] await withTaskGroup(of: (any Error)?.self) { group in for container in containers { group.addTask { do { - try await client.stop(id: container.id, opts: stopOptions) - print(container.id) + try await client.stop(id: container, opts: stopOptions) + print(container) return nil } catch { return error diff --git a/Sources/ContainerCommands/System/SystemStop.swift b/Sources/ContainerCommands/System/SystemStop.swift index 73cdc28a..031f3132 100644 --- a/Sources/ContainerCommands/System/SystemStop.swift +++ b/Sources/ContainerCommands/System/SystemStop.swift @@ -64,7 +64,7 @@ extension Application { let client = ContainerClient() log.info("stopping containers", metadata: ["stopTimeoutSeconds": "\(Self.stopTimeoutSeconds)"]) do { - let containers = try await client.list() + let containers = try await client.list().map { $0.id } let signal = try Signals.parseSignal("SIGTERM") let opts = ContainerStopOptions(timeoutInSeconds: Self.stopTimeoutSeconds, signal: signal) try await ContainerStop.stopContainers( diff --git a/Tests/CLITests/Subcommands/Containers/TestCLINotFound.swift b/Tests/CLITests/Subcommands/Containers/TestCLINotFound.swift new file mode 100644 index 00000000..feba22d4 --- /dev/null +++ b/Tests/CLITests/Subcommands/Containers/TestCLINotFound.swift @@ -0,0 +1,37 @@ +//===----------------------------------------------------------------------===// +// Copyright © 2026 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 + +/// Tests that stop, kill, and delete return errors for non-existent containers. +class TestCLINotFound: CLITest { + + @Test func testStopNonExistentContainer() throws { + let (_, _, _, status) = try run(arguments: ["stop", "does-not-exist"]) + #expect(status != 0, "stop should fail for a non-existent container") + } + + @Test func testKillNonExistentContainer() throws { + let (_, _, _, status) = try run(arguments: ["kill", "does-not-exist"]) + #expect(status != 0, "kill should fail for a non-existent container") + } + + @Test func testDeleteNonExistentContainer() throws { + let (_, _, _, status) = try run(arguments: ["delete", "does-not-exist"]) + #expect(status != 0, "delete should fail for a non-existent container") + } +}