Skip to content

Commit ef8edc7

Browse files
committed
clean up anonymous volumes when --rm containers exit
1 parent e1e016b commit ef8edc7

File tree

5 files changed

+135
-60
lines changed

5 files changed

+135
-60
lines changed

Sources/ContainerClient/Core/Volume.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,10 +62,18 @@ extension Volume {
6262
/// Reserved label key for marking anonymous volumes
6363
public static let anonymousLabel = "com.apple.container.resource.anonymous"
6464

65+
/// Reserved label key for storing the container ID that created this volume
66+
public static let createdByLabel = "com.apple.container.resource.created-by"
67+
6568
/// Whether this is an anonymous volume (detected via label)
6669
public var isAnonymous: Bool {
6770
labels[Self.anonymousLabel] != nil
6871
}
72+
73+
/// The container ID that created this anonymous volume (if applicable)
74+
public var createdByContainerID: String? {
75+
labels[Self.createdByLabel]
76+
}
6977
}
7078

7179
/// Error types for volume operations.

Sources/ContainerClient/Utility.swift

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ public struct Utility {
168168
case .filesystem(let fs):
169169
resolvedMounts.append(fs)
170170
case .volume(let parsed):
171-
let volume = try await getOrCreateVolume(parsed: parsed)
171+
let volume = try await getOrCreateVolume(parsed: parsed, containerID: id)
172172
let volumeMount = Filesystem.volume(
173173
name: parsed.name,
174174
format: volume.format,
@@ -315,8 +315,11 @@ public struct Utility {
315315

316316
/// Gets an existing volume or creates it if it doesn't exist.
317317
/// Shows a warning for named volumes when auto-creating.
318-
private static func getOrCreateVolume(parsed: ParsedVolume) async throws -> Volume {
319-
let labels = parsed.isAnonymous ? [Volume.anonymousLabel: ""] : [:]
318+
private static func getOrCreateVolume(parsed: ParsedVolume, containerID: String) async throws -> Volume {
319+
var labels = parsed.isAnonymous ? [Volume.anonymousLabel: ""] : [:]
320+
if parsed.isAnonymous {
321+
labels[Volume.createdByLabel] = containerID
322+
}
320323

321324
let volume: Volume
322325
do {

Sources/Services/ContainerAPIService/Containers/ContainersService.swift

Lines changed: 60 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -434,12 +434,17 @@ public actor ContainersService {
434434
}
435435

436436
private func handleContainerExit(id: String, code: ExitStatus? = nil) async throws {
437-
try await self.lock.withLock { [self] context in
437+
let shouldCleanupVolumes = try await self.lock.withLock { [self] context in
438438
try await handleContainerExit(id: id, code: code, context: context)
439439
}
440+
441+
// Lock released, safe for XPC calls
442+
if shouldCleanupVolumes {
443+
await self.cleanupAnonymousVolumes(forContainer: id)
444+
}
440445
}
441446

442-
private func handleContainerExit(id: String, code: ExitStatus?, context: AsyncLock.Context) async throws {
447+
private func handleContainerExit(id: String, code: ExitStatus?, context: AsyncLock.Context) async throws -> Bool {
443448
if let code {
444449
self.log.info("Handling container \(id) exit. Code \(code)")
445450
}
@@ -448,11 +453,11 @@ public actor ContainersService {
448453
do {
449454
state = try self.getContainerState(id: id, context: context)
450455
if state.snapshot.status == .stopped {
451-
return
456+
return false
452457
}
453458
} catch {
454459
// Was auto removed by the background thread, nothing for us to do.
455-
return
460+
return false
456461
}
457462

458463
await self.exitMonitor.stopTracking(id: id)
@@ -475,7 +480,9 @@ public actor ContainersService {
475480
let options = try getContainerCreationOptions(id: id)
476481
if options.autoRemove {
477482
try await self.cleanup(id: id, context: context)
483+
return true
478484
}
485+
return false
479486
}
480487

481488
private static func fullLaunchdServiceLabel(runtimeName: String, instanceId: String) -> String {
@@ -511,6 +518,55 @@ public actor ContainersService {
511518
try await self._cleanup(id: id)
512519
}
513520

521+
/// Clean up anonymous volumes created by a container
522+
private func cleanupAnonymousVolumes(forContainer id: String) async {
523+
do {
524+
let allVolumes = try await ClientVolume.list()
525+
let anonymousVolumes = allVolumes.filter { $0.isAnonymous && $0.createdByContainerID == id }
526+
527+
guard !anonymousVolumes.isEmpty else { return }
528+
529+
await withTaskGroup(of: (String, Error?).self) { group in
530+
for volume in anonymousVolumes {
531+
group.addTask {
532+
do {
533+
try await ClientVolume.delete(name: volume.name)
534+
return (volume.name, nil)
535+
} catch {
536+
return (volume.name, error)
537+
}
538+
}
539+
}
540+
541+
for await (volumeName, error) in group {
542+
if let error = error {
543+
self.log.warning(
544+
"Failed to delete anonymous volume",
545+
metadata: [
546+
"volume": "\(volumeName)",
547+
"container": "\(id)",
548+
"error": "\(error)",
549+
])
550+
} else {
551+
self.log.info(
552+
"Deleted anonymous volume",
553+
metadata: [
554+
"volume": "\(volumeName)",
555+
"container": "\(id)",
556+
])
557+
}
558+
}
559+
}
560+
} catch {
561+
self.log.error(
562+
"Failed to query anonymous volumes for cleanup",
563+
metadata: [
564+
"container": "\(id)",
565+
"error": "\(error)",
566+
])
567+
}
568+
}
569+
514570
private func getContainerCreationOptions(id: String) throws -> ContainerCreateOptions {
515571
let path = self.containerRoot.appendingPathComponent(id)
516572
let bundle = ContainerClient.Bundle(path: path)

Tests/CLITests/Subcommands/Volumes/TestCLIAnonymousVolumes.swift

Lines changed: 60 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -99,13 +99,13 @@ class TestCLIAnonymousVolumes: CLITest {
9999
let (_, _, _) = (try? run(arguments: args)) ?? ("", "", 1)
100100
}
101101

102-
@Test func testAnonymousVolumeCreationAndPersistence() async throws {
102+
@Test func testAnonymousVolumeCreationAndAutoCleanup() async throws {
103103
let testName = getTestName()
104104
let containerName = "\(testName)_c1"
105105

106106
defer {
107107
doRemoveIfExists(name: containerName, force: true)
108-
// Clean up anonymous volumes
108+
// Clean up any remaining anonymous volumes
109109
if let volumes = try? getAnonymousVolumeNames() {
110110
volumes.forEach { doVolumeDeleteIfExists(name: $0) }
111111
}
@@ -129,7 +129,7 @@ class TestCLIAnonymousVolumes: CLITest {
129129

130130
#expect(status == 0, "container run should succeed")
131131

132-
// Give time for container removal to complete
132+
// Give time for cleanup to complete
133133
try await Task.sleep(for: .seconds(1))
134134

135135
// Verify container was removed
@@ -138,9 +138,9 @@ class TestCLIAnonymousVolumes: CLITest {
138138
.filter { $0.contains(containerName) }
139139
#expect(containers.isEmpty, "container should be removed with --rm")
140140

141-
// Verify anonymous volume persists (no auto-cleanup)
141+
// Verify anonymous volume was auto-cleaned up
142142
let afterCount = try getAnonymousVolumeNames().count
143-
#expect(afterCount == beforeCount + 1, "anonymous volume should persist even with --rm")
143+
#expect(afterCount == beforeCount, "anonymous volume should be auto-cleaned up with --rm")
144144
}
145145

146146
@Test func testAnonymousVolumePersistenceWithoutRm() throws {
@@ -191,43 +191,6 @@ class TestCLIAnonymousVolumes: CLITest {
191191
doVolumeDeleteIfExists(name: volumeID)
192192
}
193193

194-
@Test func testMultipleAnonymousVolumes() async throws {
195-
let testName = getTestName()
196-
let containerName = "\(testName)_c1"
197-
198-
defer {
199-
doRemoveIfExists(name: containerName, force: true)
200-
// Clean up anonymous volumes
201-
if let volumes = try? getAnonymousVolumeNames() {
202-
volumes.forEach { doVolumeDeleteIfExists(name: $0) }
203-
}
204-
}
205-
206-
let beforeCount = try getAnonymousVolumeNames().count
207-
208-
// Run with multiple anonymous volumes
209-
let (_, _, status) = try run(arguments: [
210-
"run",
211-
"--rm",
212-
"--name",
213-
containerName,
214-
"-v", "/data1",
215-
"-v", "/data2",
216-
"-v", "/data3",
217-
alpine,
218-
"sh", "-c", "ls -d /data*",
219-
])
220-
221-
#expect(status == 0, "container run should succeed")
222-
223-
// Give time for container removal
224-
try await Task.sleep(for: .seconds(1))
225-
226-
// All 3 volumes should persist (no auto-cleanup)
227-
let afterCount = try getAnonymousVolumeNames().count
228-
#expect(afterCount == beforeCount + 3, "all 3 anonymous volumes should persist")
229-
}
230-
231194
@Test func testAnonymousMountSyntax() async throws {
232195
let testName = getTestName()
233196
let containerName = "\(testName)_c1"
@@ -255,12 +218,12 @@ class TestCLIAnonymousVolumes: CLITest {
255218

256219
#expect(status == 0, "container run with --mount should succeed")
257220

258-
// Give time for container removal
259-
try await Task.sleep(for: .seconds(1))
221+
// Give time for container removal and volume cleanup
222+
try await Task.sleep(for: .seconds(2))
260223

261-
// Anonymous volume should persist (no auto-cleanup)
224+
// Anonymous volume should be auto-cleaned up with --rm
262225
let afterCount = try getAnonymousVolumeNames().count
263-
#expect(afterCount == beforeCount + 1, "anonymous volume should persist")
226+
#expect(afterCount == beforeCount, "anonymous volume should be auto-cleaned up with --rm")
264227
}
265228

266229
@Test func testAnonymousVolumeUUIDFormat() throws {
@@ -394,15 +357,15 @@ class TestCLIAnonymousVolumes: CLITest {
394357

395358
#expect(status == 0, "container run should succeed")
396359

397-
// Give time for container removal
398-
try await Task.sleep(for: .seconds(1))
360+
// Give time for container removal and volume cleanup
361+
try await Task.sleep(for: .seconds(2))
399362

400363
// Named volume should still exist
401364
let namedExists = try volumeExists(name: namedVolumeName)
402365
#expect(namedExists, "named volume should persist")
403366

404367
let afterAnonCount = try getAnonymousVolumeNames().count
405-
#expect(afterAnonCount == beforeAnonCount + 1, "anonymous volume should persist")
368+
#expect(afterAnonCount == beforeAnonCount, "anonymous volume should be auto-cleaned up with --rm")
406369
}
407370

408371
@Test func testAnonymousVolumeManualDeletion() throws {
@@ -463,8 +426,8 @@ class TestCLIAnonymousVolumes: CLITest {
463426

464427
#expect(status == 0, "detached container run should succeed")
465428

466-
// Wait for container to exit
467-
try await Task.sleep(for: .seconds(3))
429+
// Wait for container to exit and volume cleanup
430+
try await Task.sleep(for: .seconds(4))
468431

469432
// Container should be removed
470433
let (lsOutput, _, _) = try run(arguments: ["ls", "-a"])
@@ -473,6 +436,51 @@ class TestCLIAnonymousVolumes: CLITest {
473436
#expect(containers.isEmpty, "container should be auto-removed")
474437

475438
let afterCount = try getAnonymousVolumeNames().count
476-
#expect(afterCount == beforeCount + 1, "anonymous volume should persist")
439+
#expect(afterCount == beforeCount, "anonymous volume should be auto-cleaned up with --rm")
440+
}
441+
442+
@Test func testMultipleAnonymousVolumesAutoCleanupWithRm() async throws {
443+
let testName = getTestName()
444+
let containerName = "\(testName)_c1"
445+
446+
defer {
447+
doRemoveIfExists(name: containerName, force: true)
448+
// Clean up any remaining anonymous volumes
449+
if let volumes = try? getAnonymousVolumeNames() {
450+
volumes.forEach { doVolumeDeleteIfExists(name: $0) }
451+
}
452+
}
453+
454+
let beforeCount = try getAnonymousVolumeNames().count
455+
456+
// Run container with --rm and 5 anonymous volumes
457+
let (_, _, status) = try run(arguments: [
458+
"run",
459+
"--rm",
460+
"--name",
461+
containerName,
462+
"-v", "/data1",
463+
"-v", "/data2",
464+
"-v", "/data3",
465+
"-v", "/data4",
466+
"-v", "/data5",
467+
alpine,
468+
"sh", "-c", "echo 'test' > /data1/file.txt && cat /data1/file.txt",
469+
])
470+
471+
#expect(status == 0, "container run should succeed")
472+
473+
// Wait for cleanup to complete
474+
try await Task.sleep(for: .seconds(2))
475+
476+
// Verify container was removed
477+
let (lsOutput, _, _) = try run(arguments: ["ls", "-a"])
478+
let containers = lsOutput.components(separatedBy: .newlines)
479+
.filter { $0.contains(containerName) }
480+
#expect(containers.isEmpty, "container should be removed with --rm")
481+
482+
// Verify all 5 anon volumes were cleaned up
483+
let afterCount = try getAnonymousVolumeNames().count
484+
#expect(afterCount == beforeCount, "all 5 anonymous volumes should be auto-cleaned up with --rm")
477485
}
478486
}

docs/command-reference.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -711,7 +711,7 @@ container run -v $VOL:/data alpine
711711
container volume rm $VOL
712712
```
713713

714-
**Note**: Unlike Docker, anonymous volumes do NOT auto-cleanup with `--rm`. Manual deletion is required.
714+
**Note**: Anonymous volumes auto-cleanup when containers exit with `--rm` flag. Without `--rm`, they persist and require manual deletion.
715715

716716
### `container volume delete (rm)`
717717

0 commit comments

Comments
 (0)