From 623ab5111ba98db9ac7b13187627a10c99d1590d Mon Sep 17 00:00:00 2001 From: rajaryan-singh Date: Fri, 15 May 2026 15:22:32 -0700 Subject: [PATCH] Make inspect error handling for missing resources consistent --- .../Container/ContainerInspect.swift | 19 ++++++-- .../Image/ImageInspect.swift | 47 +++++-------------- .../Network/NetworkInspect.swift | 14 +++++- .../Volume/VolumeInspect.swift | 16 +++++-- .../Containers/TestCLIRemove.swift | 6 +++ .../Images/TestCLIImagesCommand.swift | 6 +++ .../Subcommands/Networks/TestCLINetwork.swift | 6 +++ .../Subcommands/Volumes/TestCLIVolumes.swift | 8 ++++ 8 files changed, 77 insertions(+), 45 deletions(-) diff --git a/Sources/ContainerCommands/Container/ContainerInspect.swift b/Sources/ContainerCommands/Container/ContainerInspect.swift index a0e937af7..1dcf29008 100644 --- a/Sources/ContainerCommands/Container/ContainerInspect.swift +++ b/Sources/ContainerCommands/Container/ContainerInspect.swift @@ -17,8 +17,8 @@ import ArgumentParser import ContainerAPIClient import ContainerResource +import ContainerizationError import Foundation -import SwiftProtobuf extension Application { public struct ContainerInspect: AsyncLoggableCommand { @@ -36,12 +36,21 @@ extension Application { public func run() async throws { let client = ContainerClient() + let uniqueIds = Set(containerIds) let containers = try await client.list().filter { - containerIds.contains($0.id) - }.map { - PrintableContainer($0) + uniqueIds.contains($0.id) } - try Output.emit(Output.renderJSON(containers)) + + if containers.count != uniqueIds.count { + let found = Set(containers.map { $0.id }) + let missing = uniqueIds.subtracting(found).sorted() + throw ContainerizationError( + .notFound, + message: "container not found: \(missing.joined(separator: ", "))" + ) + } + + try Output.emit(Output.renderJSON(containers.map { PrintableContainer($0) })) } } } diff --git a/Sources/ContainerCommands/Image/ImageInspect.swift b/Sources/ContainerCommands/Image/ImageInspect.swift index 35bb11197..bcb3055b0 100644 --- a/Sources/ContainerCommands/Image/ImageInspect.swift +++ b/Sources/ContainerCommands/Image/ImageInspect.swift @@ -16,14 +16,10 @@ import ArgumentParser import ContainerAPIClient -import ContainerLog import ContainerPersistence -import ContainerPlugin import ContainerResource import ContainerizationError import Foundation -import Logging -import SwiftProtobuf extension Application { public struct ImageInspect: AsyncLoggableCommand { @@ -39,19 +35,22 @@ extension Application { public init() {} - struct InspectError: Error { - let succeeded: [String] - let failed: [(String, Error)] - } - public func run() async throws { let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load() - var printable: [ImageDetail] = [] - var succeededImages: [String] = [] - var allErrors: [(String, Error)] = [] + let uniqueNames = Set(images) + let result = try await ClientImage.get( + names: Array(uniqueNames), containerSystemConfig: containerSystemConfig + ) - let result = try await ClientImage.get(names: images, containerSystemConfig: containerSystemConfig) + if !result.error.isEmpty { + let missing = result.error.sorted() + throw ContainerizationError( + .notFound, + message: "image not found: \(missing.joined(separator: ", "))" + ) + } + var printable: [ImageDetail] = [] for image in result.images { guard !Utility.isInfraImage( @@ -61,29 +60,9 @@ extension Application { ) else { continue } printable.append(try await image.details()) - succeededImages.append(image.reference) } - for missing in result.error { - allErrors.append((missing, ContainerizationError(.notFound, message: "Image not found"))) - } - - if !printable.isEmpty { - try Output.emit(Output.renderJSON(printable)) - } - - if !allErrors.isEmpty { - for (name, error) in allErrors { - log.error( - "image inspect failed", - metadata: [ - "name": "\(name)", - "error": "\(error.localizedDescription)", - ]) - } - - throw InspectError(succeeded: succeededImages, failed: allErrors) - } + try Output.emit(Output.renderJSON(printable)) } } } diff --git a/Sources/ContainerCommands/Network/NetworkInspect.swift b/Sources/ContainerCommands/Network/NetworkInspect.swift index 4d8239ca8..ef7c5823b 100644 --- a/Sources/ContainerCommands/Network/NetworkInspect.swift +++ b/Sources/ContainerCommands/Network/NetworkInspect.swift @@ -16,6 +16,7 @@ import ArgumentParser import ContainerAPIClient +import ContainerizationError import Foundation extension Application { @@ -34,7 +35,18 @@ extension Application { public func run() async throws { let networkClient = NetworkClient() - let items = try await networkClient.list().filter { networks.contains($0.id) } + let uniqueNames = Set(networks) + let items = try await networkClient.list().filter { uniqueNames.contains($0.id) } + + if items.count != uniqueNames.count { + let found = Set(items.map { $0.id }) + let missing = uniqueNames.subtracting(found).sorted() + throw ContainerizationError( + .notFound, + message: "network not found: \(missing.joined(separator: ", "))" + ) + } + try Output.emit(Output.renderJSON(items)) } } diff --git a/Sources/ContainerCommands/Volume/VolumeInspect.swift b/Sources/ContainerCommands/Volume/VolumeInspect.swift index 533954004..8110a1d3b 100644 --- a/Sources/ContainerCommands/Volume/VolumeInspect.swift +++ b/Sources/ContainerCommands/Volume/VolumeInspect.swift @@ -17,6 +17,7 @@ import ArgumentParser import ContainerAPIClient import ContainerResource +import ContainerizationError import Foundation extension Application.VolumeCommand { @@ -35,11 +36,16 @@ extension Application.VolumeCommand { public init() {} public func run() async throws { - var volumes: [Volume] = [] - - for name in names { - let volume = try await ClientVolume.inspect(name) - volumes.append(volume) + let uniqueNames = Set(names) + let volumes = try await ClientVolume.list().filter { uniqueNames.contains($0.id) } + + if volumes.count != uniqueNames.count { + let found = Set(volumes.map { $0.id }) + let missing = uniqueNames.subtracting(found).sorted() + throw ContainerizationError( + .notFound, + message: "volume not found: \(missing.joined(separator: ", "))" + ) } let options = JSONOptions( diff --git a/Tests/CLITests/Subcommands/Containers/TestCLIRemove.swift b/Tests/CLITests/Subcommands/Containers/TestCLIRemove.swift index 321621a26..640c2c9d9 100644 --- a/Tests/CLITests/Subcommands/Containers/TestCLIRemove.swift +++ b/Tests/CLITests/Subcommands/Containers/TestCLIRemove.swift @@ -132,4 +132,10 @@ class TestCLIRemove: CLITest { let lines = output.split(separator: "\n").filter { $0.contains(name) } #expect(lines.count == 1, "Expected container to be deleted exactly once, got \(lines.count) lines") } + + @Test func testInspectMissingContainerFails() throws { + let (_, _, error, status) = try run(arguments: ["inspect", "definitely-missing-container"]) + #expect(status != 0, "Expected non-zero exit for missing container") + #expect(error.contains("container not found")) + } } diff --git a/Tests/CLITests/Subcommands/Images/TestCLIImagesCommand.swift b/Tests/CLITests/Subcommands/Images/TestCLIImagesCommand.swift index 003d308fa..e3d2aa5b3 100644 --- a/Tests/CLITests/Subcommands/Images/TestCLIImagesCommand.swift +++ b/Tests/CLITests/Subcommands/Images/TestCLIImagesCommand.swift @@ -593,4 +593,10 @@ class TestCLIImagesCommand: CLITest { try FileManager.default.removeItem(atPath: tarPath) try FileManager.default.moveItem(at: tempModifiedTar, to: URL(fileURLWithPath: tarPath)) } + + @Test func testInspectMissingImageFails() throws { + let (_, _, error, status) = try run(arguments: ["image", "inspect", "definitely-missing-image:latest"]) + #expect(status != 0, "Expected non-zero exit for missing image") + #expect(error.contains("image not found")) + } } diff --git a/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift b/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift index b20cd1eaa..cdef26d95 100644 --- a/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift +++ b/Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift @@ -313,4 +313,10 @@ class TestCLINetwork: CLITest { } #expect(json.contains { ($0["id"] as? String) == name }, "JSON should contain the created network") } + + @Test func testInspectMissingNetworkFails() throws { + let (_, _, error, status) = try run(arguments: ["network", "inspect", "definitely-missing-network"]) + #expect(status != 0, "Expected non-zero exit for missing network") + #expect(error.contains("network not found")) + } } diff --git a/Tests/CLITests/Subcommands/Volumes/TestCLIVolumes.swift b/Tests/CLITests/Subcommands/Volumes/TestCLIVolumes.swift index 002213092..4376f4ea6 100644 --- a/Tests/CLITests/Subcommands/Volumes/TestCLIVolumes.swift +++ b/Tests/CLITests/Subcommands/Volumes/TestCLIVolumes.swift @@ -466,6 +466,14 @@ class TestCLIVolumes: CLITest { #expect(error.contains("conflict")) } + // MARK: - Inspect validation tests + + @Test func testVolumeInspectMissingFails() throws { + let (_, _, error, status) = try run(arguments: ["volume", "inspect", "definitely-missing-volume"]) + #expect(status != 0, "Expected non-zero exit for missing volume") + #expect(error.contains("volume not found")) + } + // MARK: - Journal option tests @Test func testVolumeCreateWithJournalOrdered() throws {