Fix listing VMs deleted during metadata lookup (#1354)

* Fix listing VMs deleted during metadata lookup

* Isolate list deletion regressions by storage source

Sources/tart/Commands/List.swift

@@ -38,31 +38,47 @@ struct List: AsyncParsableCommand {
     var infos: [VMInfo] = []
 
     if source == nil || source == "local" {
-      infos += sortedInfos(try VMStorageLocal().list().map { (name, vmDir) in
-        try VMInfo(
-          Source: "local",
-          Name: name,
-          // ASIF capacity lookup can fail while a running VM holds the disk open.
-          Disk: HumanReadableByteCount(try? vmDir.diskSizeBytes()) { $0 / 1000 / 1000 / 1000 },
-          Size: HumanReadableByteCount(try vmDir.allocatedSizeBytes()) { $0 / 1000 / 1000 / 1000 },
-          Accessed: formatAccessDate(try vmDir.accessDate()),
-          Running: vmDir.running(),
-          State: vmDir.state().rawValue
-        )
+      infos += sortedInfos(try VMStorageLocal().list().compactMap { (name, vmDir) -> VMInfo? in
+        do {
+          return try VMInfo(
+            Source: "local",
+            Name: name,
+            // ASIF capacity lookup can fail while a running VM holds the disk open.
+            Disk: HumanReadableByteCount(try? vmDir.diskSizeBytes()) { $0 / 1000 / 1000 / 1000 },
+            Size: HumanReadableByteCount(try vmDir.allocatedSizeBytes()) { $0 / 1000 / 1000 / 1000 },
+            Accessed: formatAccessDate(try vmDir.accessDate()),
+            Running: vmDir.running(),
+            State: vmDir.state().rawValue
+          )
+        } catch {
+          // A VM can disappear after storage enumeration when another process deletes it.
+          if error.isFileNotFound() {
+            return nil
+          }
+          throw error
+        }
       })
     }
 
     if source == nil || source == "oci" {
-      infos += sortedInfos(try VMStorageOCI().list().map { (name, vmDir, _) in
-        try VMInfo(
-          Source: "OCI",
-          Name: name,
-          Disk: HumanReadableByteCount(try? vmDir.diskSizeBytes()) { $0 / 1000 / 1000 / 1000 },
-          Size: HumanReadableByteCount(try vmDir.allocatedSizeBytes()) { $0 / 1000 / 1000 / 1000 },
-          Accessed: formatAccessDate(try vmDir.accessDate()),
-          Running: vmDir.running(),
-          State: vmDir.state().rawValue
-        )
+      infos += sortedInfos(try VMStorageOCI().list().compactMap { (name, vmDir, _) -> VMInfo? in
+        do {
+          return try VMInfo(
+            Source: "OCI",
+            Name: name,
+            Disk: HumanReadableByteCount(try? vmDir.diskSizeBytes()) { $0 / 1000 / 1000 / 1000 },
+            Size: HumanReadableByteCount(try vmDir.allocatedSizeBytes()) { $0 / 1000 / 1000 / 1000 },
+            Accessed: formatAccessDate(try vmDir.accessDate()),
+            Running: vmDir.running(),
+            State: vmDir.state().rawValue
+          )
+        } catch {
+          // A VM can disappear after storage enumeration when another process deletes it.
+          if error.isFileNotFound() {
+            return nil
+          }
+          throw error
+        }
       })
     }
 

Tests/TartTests/CommandBehaviorTests.swift

@@ -55,6 +55,45 @@ final class CommandBehaviorTests: XCTestCase {
     }
   }
 
+  func testListSkipsLocalVMsDeletedDuringMetadataLookup() async throws {
+    try await assertListSurvivesDeletion(sourceArguments: ["--source", "local"])
+  }
+
+  func testListSkipsOCIVMsDeletedDuringMetadataLookup() async throws {
+    try await assertListSurvivesDeletion(sourceArguments: ["--source", "oci"])
+  }
+
+  func testListSkipsVMsDeletedDuringMetadataLookupAcrossSources() async throws {
+    try await assertListSurvivesDeletion(sourceArguments: [])
+  }
+
+  func testListPreservesMetadataPermissionErrors() async throws {
+    for source in ["local", "oci"] {
+      try await withTemporaryTartHome {
+        let vmDir = try source == "local" ? VMStorageLocal().create("inaccessible") :
+          VMStorageOCI().create(RemoteName("example.com/org/inaccessible:latest"))
+        var vmConfig = config()
+        vmConfig.diskFormat = .asif
+        try vmConfig.save(toURL: vmDir.configURL)
+        XCTAssertTrue(FileManager.default.createFile(atPath: vmDir.nvramURL.path, contents: Data()))
+        XCTAssertTrue(FileManager.default.createFile(atPath: vmDir.diskURL.path, contents: Data()))
+        defer {
+          try? FileManager.default.setAttributes([.posixPermissions: 0o755], ofItemAtPath: vmDir.baseURL.path)
+        }
+
+        let previousPath = try installUnavailableDiskutil(beforeFailure: "/bin/chmod 000 \"${4%/*}\"")
+        defer { restoreEnvironment("PATH", to: previousPath) }
+        do {
+          _ = try await commandOutput(List.self, ["--source", source, "--format", "json"])
+          XCTFail("list should report metadata permission errors")
+        } catch {
+          XCTAssertEqual((error as NSError).domain, NSCocoaErrorDomain)
+          XCTAssertEqual((error as NSError).code, NSFileReadNoPermissionError)
+        }
+      }
+    }
+  }
+
   func testGetSurvivesUnavailableDiskCapacity() async throws {
     try await withTemporaryTartHome {
       let previousPath = try installUnavailableDiskutil()
@@ -269,6 +308,46 @@ final class CommandBehaviorTests: XCTestCase {
     }
   }
 
+  private func assertListSurvivesDeletion(sourceArguments: [String]) async throws {
+    let digest = "sha256:" + String(repeating: "a", count: 64)
+    for outputArguments in [["--format", "json"], [], ["--quiet"]] {
+      try await withTemporaryTartHome {
+        // diskutil runs after enumeration, making the deletion race deterministic.
+        let previousPath = try installUnavailableDiskutil(beforeFailure: "/bin/rm -rf \"${4%/*}\"")
+        defer { restoreEnvironment("PATH", to: previousPath) }
+
+        let local = try VMStorageLocal()
+        let oci = try VMStorageOCI()
+        for (name, diskFormat) in [("disappearing", DiskImageFormat.asif), ("healthy", .raw)] {
+          let remoteName = try RemoteName("example.com/org/\(name)@\(digest)")
+          for vmDir in [try local.create(name), try oci.create(remoteName)] {
+            var vmConfig = config()
+            vmConfig.diskFormat = diskFormat
+            try vmConfig.save(toURL: vmDir.configURL)
+            XCTAssertTrue(FileManager.default.createFile(atPath: vmDir.nvramURL.path, contents: Data()))
+            XCTAssertTrue(FileManager.default.createFile(atPath: vmDir.diskURL.path, contents: Data()))
+          }
+        }
+
+        let arguments = sourceArguments + outputArguments
+        let output = try await commandOutput(List.self, arguments)
+        let expectedNames = sourceArguments.isEmpty ? ["healthy", "example.com/org/healthy@\(digest)"] :
+          [sourceArguments.last == "local" ? "healthy" : "example.com/org/healthy@\(digest)"]
+        if outputArguments == ["--format", "json"] {
+          let rows = try XCTUnwrap(JSONSerialization.jsonObject(with: Data(output.utf8)) as? [[String: Any]])
+          XCTAssertEqual(rows.compactMap { $0["Name"] as? String }, expectedNames)
+        } else if outputArguments == ["--quiet"] {
+          XCTAssertEqual(output.split(separator: "\n").map(String.init), expectedNames)
+        } else {
+          for name in expectedNames {
+            XCTAssertTrue(output.contains(name))
+          }
+          XCTAssertFalse(output.contains("disappearing"))
+        }
+      }
+    }
+  }
+
   private func config() -> VMConfig {
     VMConfig(
       platform: Linux(),
@@ -293,11 +372,12 @@ final class CommandBehaviorTests: XCTestCase {
     )
   }
 
-  private func installUnavailableDiskutil() throws -> String? {
+  private func installUnavailableDiskutil(beforeFailure: String = "") throws -> String? {
     let binDirectory = try temporaryDirectory()
     let diskutilURL = binDirectory.appendingPathComponent("diskutil")
     let script = """
     #!/bin/sh
+    \(beforeFailure)
     echo 'Resource temporarily unavailable' >&2
     exit 1
     """