Skip to content

Commit bd1916f

Browse files
authored
Make inspect error handling for missing resources consistent (apple#1564)
- Closes apple#1539. - All `inspect` commands now consistently return exit 1 with a clear error when a requested resource is missing.
1 parent 5b83b4a commit bd1916f

8 files changed

Lines changed: 77 additions & 45 deletions

File tree

Sources/ContainerCommands/Container/ContainerInspect.swift

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@
1717
import ArgumentParser
1818
import ContainerAPIClient
1919
import ContainerResource
20+
import ContainerizationError
2021
import Foundation
21-
import SwiftProtobuf
2222

2323
extension Application {
2424
public struct ContainerInspect: AsyncLoggableCommand {
@@ -36,12 +36,21 @@ extension Application {
3636

3737
public func run() async throws {
3838
let client = ContainerClient()
39+
let uniqueIds = Set(containerIds)
3940
let containers = try await client.list().filter {
40-
containerIds.contains($0.id)
41-
}.map {
42-
PrintableContainer($0)
41+
uniqueIds.contains($0.id)
4342
}
44-
try Output.emit(Output.renderJSON(containers))
43+
44+
if containers.count != uniqueIds.count {
45+
let found = Set(containers.map { $0.id })
46+
let missing = uniqueIds.subtracting(found).sorted()
47+
throw ContainerizationError(
48+
.notFound,
49+
message: "container not found: \(missing.joined(separator: ", "))"
50+
)
51+
}
52+
53+
try Output.emit(Output.renderJSON(containers.map { PrintableContainer($0) }))
4554
}
4655
}
4756
}

Sources/ContainerCommands/Image/ImageInspect.swift

Lines changed: 13 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,10 @@
1616

1717
import ArgumentParser
1818
import ContainerAPIClient
19-
import ContainerLog
2019
import ContainerPersistence
21-
import ContainerPlugin
2220
import ContainerResource
2321
import ContainerizationError
2422
import Foundation
25-
import Logging
26-
import SwiftProtobuf
2723

2824
extension Application {
2925
public struct ImageInspect: AsyncLoggableCommand {
@@ -39,19 +35,22 @@ extension Application {
3935

4036
public init() {}
4137

42-
struct InspectError: Error {
43-
let succeeded: [String]
44-
let failed: [(String, Error)]
45-
}
46-
4738
public func run() async throws {
4839
let containerSystemConfig: ContainerSystemConfig = try await ConfigurationLoader.load()
49-
var printable: [ImageDetail] = []
50-
var succeededImages: [String] = []
51-
var allErrors: [(String, Error)] = []
40+
let uniqueNames = Set(images)
41+
let result = try await ClientImage.get(
42+
names: Array(uniqueNames), containerSystemConfig: containerSystemConfig
43+
)
5244

53-
let result = try await ClientImage.get(names: images, containerSystemConfig: containerSystemConfig)
45+
if !result.error.isEmpty {
46+
let missing = result.error.sorted()
47+
throw ContainerizationError(
48+
.notFound,
49+
message: "image not found: \(missing.joined(separator: ", "))"
50+
)
51+
}
5452

53+
var printable: [ImageDetail] = []
5554
for image in result.images {
5655
guard
5756
!Utility.isInfraImage(
@@ -61,29 +60,9 @@ extension Application {
6160
)
6261
else { continue }
6362
printable.append(try await image.details())
64-
succeededImages.append(image.reference)
6563
}
6664

67-
for missing in result.error {
68-
allErrors.append((missing, ContainerizationError(.notFound, message: "Image not found")))
69-
}
70-
71-
if !printable.isEmpty {
72-
try Output.emit(Output.renderJSON(printable))
73-
}
74-
75-
if !allErrors.isEmpty {
76-
for (name, error) in allErrors {
77-
log.error(
78-
"image inspect failed",
79-
metadata: [
80-
"name": "\(name)",
81-
"error": "\(error.localizedDescription)",
82-
])
83-
}
84-
85-
throw InspectError(succeeded: succeededImages, failed: allErrors)
86-
}
65+
try Output.emit(Output.renderJSON(printable))
8766
}
8867
}
8968
}

Sources/ContainerCommands/Network/NetworkInspect.swift

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
import ArgumentParser
1818
import ContainerAPIClient
19+
import ContainerizationError
1920
import Foundation
2021

2122
extension Application {
@@ -34,7 +35,18 @@ extension Application {
3435

3536
public func run() async throws {
3637
let networkClient = NetworkClient()
37-
let items = try await networkClient.list().filter { networks.contains($0.id) }
38+
let uniqueNames = Set(networks)
39+
let items = try await networkClient.list().filter { uniqueNames.contains($0.id) }
40+
41+
if items.count != uniqueNames.count {
42+
let found = Set(items.map { $0.id })
43+
let missing = uniqueNames.subtracting(found).sorted()
44+
throw ContainerizationError(
45+
.notFound,
46+
message: "network not found: \(missing.joined(separator: ", "))"
47+
)
48+
}
49+
3850
try Output.emit(Output.renderJSON(items))
3951
}
4052
}

Sources/ContainerCommands/Volume/VolumeInspect.swift

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import ArgumentParser
1818
import ContainerAPIClient
1919
import ContainerResource
20+
import ContainerizationError
2021
import Foundation
2122

2223
extension Application.VolumeCommand {
@@ -35,11 +36,16 @@ extension Application.VolumeCommand {
3536
public init() {}
3637

3738
public func run() async throws {
38-
var volumes: [Volume] = []
39-
40-
for name in names {
41-
let volume = try await ClientVolume.inspect(name)
42-
volumes.append(volume)
39+
let uniqueNames = Set(names)
40+
let volumes = try await ClientVolume.list().filter { uniqueNames.contains($0.id) }
41+
42+
if volumes.count != uniqueNames.count {
43+
let found = Set(volumes.map { $0.id })
44+
let missing = uniqueNames.subtracting(found).sorted()
45+
throw ContainerizationError(
46+
.notFound,
47+
message: "volume not found: \(missing.joined(separator: ", "))"
48+
)
4349
}
4450

4551
let options = JSONOptions(

Tests/CLITests/Subcommands/Containers/TestCLIRemove.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,4 +132,10 @@ class TestCLIRemove: CLITest {
132132
let lines = output.split(separator: "\n").filter { $0.contains(name) }
133133
#expect(lines.count == 1, "Expected container to be deleted exactly once, got \(lines.count) lines")
134134
}
135+
136+
@Test func testInspectMissingContainerFails() throws {
137+
let (_, _, error, status) = try run(arguments: ["inspect", "definitely-missing-container"])
138+
#expect(status != 0, "Expected non-zero exit for missing container")
139+
#expect(error.contains("container not found"))
140+
}
135141
}

Tests/CLITests/Subcommands/Images/TestCLIImagesCommand.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -593,4 +593,10 @@ class TestCLIImagesCommand: CLITest {
593593
try FileManager.default.removeItem(atPath: tarPath)
594594
try FileManager.default.moveItem(at: tempModifiedTar, to: URL(fileURLWithPath: tarPath))
595595
}
596+
597+
@Test func testInspectMissingImageFails() throws {
598+
let (_, _, error, status) = try run(arguments: ["image", "inspect", "definitely-missing-image:latest"])
599+
#expect(status != 0, "Expected non-zero exit for missing image")
600+
#expect(error.contains("image not found"))
601+
}
596602
}

Tests/CLITests/Subcommands/Networks/TestCLINetwork.swift

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,4 +313,10 @@ class TestCLINetwork: CLITest {
313313
}
314314
#expect(json.contains { ($0["id"] as? String) == name }, "JSON should contain the created network")
315315
}
316+
317+
@Test func testInspectMissingNetworkFails() throws {
318+
let (_, _, error, status) = try run(arguments: ["network", "inspect", "definitely-missing-network"])
319+
#expect(status != 0, "Expected non-zero exit for missing network")
320+
#expect(error.contains("network not found"))
321+
}
316322
}

Tests/CLITests/Subcommands/Volumes/TestCLIVolumes.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -466,6 +466,14 @@ class TestCLIVolumes: CLITest {
466466
#expect(error.contains("conflict"))
467467
}
468468

469+
// MARK: - Inspect validation tests
470+
471+
@Test func testVolumeInspectMissingFails() throws {
472+
let (_, _, error, status) = try run(arguments: ["volume", "inspect", "definitely-missing-volume"])
473+
#expect(status != 0, "Expected non-zero exit for missing volume")
474+
#expect(error.contains("volume not found"))
475+
}
476+
469477
// MARK: - Journal option tests
470478

471479
@Test func testVolumeCreateWithJournalOrdered() throws {

0 commit comments

Comments
 (0)