diff --git a/Sources/ContainerClient/Utility.swift b/Sources/ContainerClient/Utility.swift index f11a4d08..e5ece4cd 100644 --- a/Sources/ContainerClient/Utility.swift +++ b/Sources/ContainerClient/Utility.swift @@ -160,35 +160,7 @@ public struct Utility { case .filesystem(let fs): resolvedMounts.append(fs) case .volume(let parsed): - let volume: Volume - - if parsed.isAnonymous { - // Anonymous volume so try to create it, inspect if already exists - do { - volume = try await ClientVolume.create( - name: parsed.name, - driver: "local", - driverOpts: [:], - labels: [Volume.anonymousLabel: ""] - ) - } catch let error as VolumeError { - guard case .volumeAlreadyExists = error else { - throw error - } - // Volume already exists, just inspect it - volume = try await ClientVolume.inspect(parsed.name) - } catch let error as ContainerizationError { - // Handle XPC-wrapped volumeAlreadyExists error - guard error.message.contains("already exists") else { - throw error - } - volume = try await ClientVolume.inspect(parsed.name) - } - } else { - // Named volume so it must already exist - volume = try await ClientVolume.inspect(parsed.name) - } - + let volume = try await getOrCreateVolume(parsed: parsed) let volumeMount = Filesystem.volume( name: parsed.name, format: volume.format, @@ -305,4 +277,36 @@ public struct Utility { } return result } + + /// Gets an existing volume or creates it if it doesn't exist. + /// Shows a warning for named volumes when auto-creating. + private static func getOrCreateVolume(parsed: ParsedVolume) async throws -> Volume { + let labels = parsed.isAnonymous ? [Volume.anonymousLabel: ""] : [:] + + let volume: Volume + do { + volume = try await ClientVolume.create( + name: parsed.name, + driver: "local", + driverOpts: [:], + labels: labels + ) + } catch let error as VolumeError { + guard case .volumeAlreadyExists = error else { + throw error + } + // Volume already exists, just inspect it + volume = try await ClientVolume.inspect(parsed.name) + } catch let error as ContainerizationError { + // Handle XPC-wrapped volumeAlreadyExists error + guard error.message.contains("already exists") else { + throw error + } + volume = try await ClientVolume.inspect(parsed.name) + } + + // TODO: Warn user if named volume was auto-created + + return volume + } } diff --git a/Tests/CLITests/Subcommands/Volumes/TestCLIAnonymousVolumes.swift b/Tests/CLITests/Subcommands/Volumes/TestCLIAnonymousVolumes.swift index b5df2aa3..fd21af6a 100644 --- a/Tests/CLITests/Subcommands/Volumes/TestCLIAnonymousVolumes.swift +++ b/Tests/CLITests/Subcommands/Volumes/TestCLIAnonymousVolumes.swift @@ -475,35 +475,4 @@ class TestCLIAnonymousVolumes: CLITest { let afterCount = try getAnonymousVolumeNames().count #expect(afterCount == beforeCount + 1, "anonymous volume should persist") } - - @Test func testAnonymousVolumeErrorOnNonExistentNamedVolume() throws { - let testName = getTestName() - let containerName = "\(testName)_c1" - let nonExistentVolume = "\(testName)_nonexistent" - - defer { - doRemoveIfExists(name: containerName, force: true) - } - - // Try to run with non-existent named volume - let (_, error, status) = try run(arguments: [ - "run", - "--name", - containerName, - "-v", "\(nonExistentVolume):/data", - alpine, - "echo", "test", - ]) - - // Should fail with volume not found error - #expect(status != 0, "should fail when named volume doesn't exist") - #expect( - error.contains("not found") || error.contains("nonexistent"), - "error should mention volume not found") - - // No anonymous volume should be created - let volumes = try getAnonymousVolumeNames() - let hasMatchingVolume = volumes.contains { $0.contains(testName) } - #expect(!hasMatchingVolume, "no anonymous volume should be created for non-existent named volume") - } } diff --git a/Tests/CLITests/Subcommands/Volumes/TestCLIVolumes.swift b/Tests/CLITests/Subcommands/Volumes/TestCLIVolumes.swift index e72e3f64..7746dad0 100644 --- a/Tests/CLITests/Subcommands/Volumes/TestCLIVolumes.swift +++ b/Tests/CLITests/Subcommands/Volumes/TestCLIVolumes.swift @@ -251,4 +251,76 @@ class TestCLIVolumes: CLITest { // Delete volume try doVolumeDelete(name: volumeName) } + + @Test func testImplicitNamedVolumeCreation() throws { + let testName = getTestName() + let containerName = "\(testName)_c1" + let volumeName = "\(testName)_autovolume" + + defer { + doRemoveIfExists(name: containerName, force: true) + doVolumeDeleteIfExists(name: volumeName) + } + + // Verify volume doesn't exist yet + let (listOutput, _, _) = try run(arguments: ["volume", "list", "--quiet"]) + let volumeExistsBefore = listOutput.contains(volumeName) + #expect(!volumeExistsBefore, "volume should not exist initially") + + // Run container with non-existent named volume - should auto-create + let (output, _, status) = try run(arguments: [ + "run", + "--name", + containerName, + "-v", "\(volumeName):/data", + alpine, + "echo", "test", + ]) + + // Should succeed and create volume automatically + #expect(status == 0, "should succeed and auto-create named volume") + #expect(output.contains("test"), "container should run successfully") + + // Volume should now exist + let (listOutputAfter, _, _) = try run(arguments: ["volume", "list", "--quiet"]) + let volumeExistsAfter = listOutputAfter.contains(volumeName) + #expect(volumeExistsAfter, "volume should be created") + } + + @Test func testImplicitNamedVolumeReuse() throws { + let testName = getTestName() + let containerName1 = "\(testName)_c1" + let containerName2 = "\(testName)_c2" + let volumeName = "\(testName)_sharedvolume" + + defer { + doRemoveIfExists(name: containerName1, force: true) + doRemoveIfExists(name: containerName2, force: true) + doVolumeDeleteIfExists(name: volumeName) + } + + // First container - should auto-create volume + let (_, _, status1) = try run(arguments: [ + "run", + "--name", + containerName1, + "-v", "\(volumeName):/data", + alpine, + "sh", "-c", "echo 'first' > /data/test.txt", + ]) + + #expect(status1 == 0, "first container should succeed") + + // Second container - should reuse existing volume + let (_, _, status2) = try run(arguments: [ + "run", + "--name", + containerName2, + "-v", "\(volumeName):/data", + alpine, + "cat", "/data/test.txt", + ]) + + #expect(status2 == 0, "second container should succeed") + } } diff --git a/docs/command-reference.md b/docs/command-reference.md index 16bbb654..f0f66dce 100644 --- a/docs/command-reference.md +++ b/docs/command-reference.md @@ -575,7 +575,7 @@ Only global flags are available for debugging, version, and help. ## Volume Management -Manage persistent volumes for containers. Volumes can be explicitly created with `volume create` or implicitly created using anonymous volume syntax (`-v /path` without a source name). +Manage persistent volumes for containers. Volumes can be explicitly created with `volume create` or implicitly created when referenced in container commands (e.g., `-v myvolume:/path` or `-v /path` for anonymous volumes). ### `container volume create`