tart exec: use 64 KiB Tart Guest Agent receive windows (#1372)

* 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
+      infos += sortedInfos(try VMStorageLocal().list().compactMap { (name, vmDir) -> VMInfo? in
-        try VMInfo(
+        do {
-          Source: "local",
+          return try VMInfo(
-          Name: name,
+            Source: "local",
-          // ASIF capacity lookup can fail while a running VM holds the disk open.
+            Name: name,
-          Disk: HumanReadableByteCount(try? vmDir.diskSizeBytes()) { $0 / 1000 / 1000 / 1000 },
+            // ASIF capacity lookup can fail while a running VM holds the disk open.
-          Size: HumanReadableByteCount(try vmDir.allocatedSizeBytes()) { $0 / 1000 / 1000 / 1000 },
+            Disk: HumanReadableByteCount(try? vmDir.diskSizeBytes()) { $0 / 1000 / 1000 / 1000 },
-          Accessed: formatAccessDate(try vmDir.accessDate()),
+            Size: HumanReadableByteCount(try vmDir.allocatedSizeBytes()) { $0 / 1000 / 1000 / 1000 },
-          Running: vmDir.running(),
+            Accessed: formatAccessDate(try vmDir.accessDate()),
-          State: vmDir.state().rawValue
+            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
+      infos += sortedInfos(try VMStorageOCI().list().compactMap { (name, vmDir, _) -> VMInfo? in
-        try VMInfo(
+        do {
-          Source: "OCI",
+          return try VMInfo(
-          Name: name,
+            Source: "OCI",
-          Disk: HumanReadableByteCount(try? vmDir.diskSizeBytes()) { $0 / 1000 / 1000 / 1000 },
+            Name: name,
-          Size: HumanReadableByteCount(try vmDir.allocatedSizeBytes()) { $0 / 1000 / 1000 / 1000 },
+            Disk: HumanReadableByteCount(try? vmDir.diskSizeBytes()) { $0 / 1000 / 1000 / 1000 },
-          Accessed: formatAccessDate(try vmDir.accessDate()),
+            Size: HumanReadableByteCount(try vmDir.allocatedSizeBytes()) { $0 / 1000 / 1000 / 1000 },
-          Running: vmDir.running(),
+            Accessed: formatAccessDate(try vmDir.accessDate()),
-          State: vmDir.state().rawValue
+            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
+        }
       })
     }
 

Sources/tart/GuestAgentChannel.swift

@@ -14,8 +14,11 @@ func withGuestAgentChannel<T>(
   let channel = try GRPCChannelPool.with(
     target: .unixDomainSocket(socketPath),
     transportSecurity: .plaintext,
-    eventLoopGroup: .singletonMultiThreadedEventLoopGroup,
+    eventLoopGroup: .singletonMultiThreadedEventLoopGroup
-  )
+  ) { configuration in
+    // Limit guest output in flight to work around VirtIO socket device implementation data loss
+    configuration.http2.targetWindowSize = 64 * 1024
+  }
 
   do {
     let result = try await body(channel)

Sources/tart/MACAddressResolver/ARPCache.swift

@@ -41,9 +41,9 @@ struct ARPCacheInternalError: Error, CustomStringConvertible {
 struct ARPCache {
   let arpCommandOutput: Data
 
-  init() throws {
+  init(arpCommandURL: URL = URL(fileURLWithPath: "/usr/sbin/arp")) throws {
     let process = Process.init()
-    process.executableURL = URL.init(fileURLWithPath: "/usr/sbin/arp")
+    process.executableURL = arpCommandURL
     process.arguments = ["-an"]
 
     let pipe = Pipe()
@@ -53,9 +53,7 @@ struct ARPCache {
 
     try process.run()
 
-    guard let arpCommandOutput = try pipe.fileHandleForReading.readToEnd() else {
+    let arpCommandOutput = try pipe.fileHandleForReading.readToEnd() ?? Data()
-      throw ARPCommandYieldedInvalidOutputError(explanation: "empty output")
-    }
 
     process.waitUntilExit()
 
@@ -69,9 +67,12 @@ struct ARPCache {
   }
 
   func ResolveMACAddress(macAddress: MACAddress) throws -> IPv4Address? {
-    let lines = String(decoding: arpCommandOutput, as: UTF8.self)
+    let output = String(decoding: arpCommandOutput, as: UTF8.self)
       .trimmingCharacters(in: .whitespacesAndNewlines)
-      .components(separatedBy: "\n")
+    if output.isEmpty {
+      return nil
+    }
+    let lines = output.components(separatedBy: "\n")
 
     // Based on https://opensource.apple.com/source/network_cmds/network_cmds-606.40.2/arp.tproj/arp.c.auto.html
     let regex = try NSRegularExpression(pattern: #"^.* \((?<ip>.*)\) at (?<mac>.*) on (?<interface>.*) .*$"#)

Sources/tart/OCI/Manifest.swift

@@ -226,11 +226,26 @@ struct OCIConfig: Codable {
   var architecture: Architecture = .arm64
   var os: OS = .darwin
   var config: ConfigContainer?
+  // Required by the OCI image spec and enforced by registries that validate
+  // image configs (e.g. Quay). Optional so that configs pushed by older Tart
+  // versions, which lack it, can still be decoded.
+  var rootfs: RootFS? = RootFS()
 
   struct ConfigContainer: Codable {
     var Labels: [String: String]?
   }
 
+  struct RootFS: Codable {
+    var type: String = "layers"
+    // Tart layers are not filesystem tarballs, so there are no DiffIDs to list
+    var diffIDs: [String] = []
+
+    enum CodingKeys: String, CodingKey {
+      case type
+      case diffIDs = "diff_ids"
+    }
+  }
+
   func toJSON() throws -> Data {
     try Config.jsonEncoder().encode(self)
   }

Tests/TartTests/ARPCacheTests.swift

@@ -0,0 +1,71 @@
+import XCTest
+import Foundation
+import Network
+@testable import tart
+
+final class ARPCacheTests: XCTestCase {
+  private let mac = MACAddress(fromString: "00:11:22:33:44:55")!
+
+  private func cache(_ output: String, exitCode: Int = 0) throws -> ARPCache {
+    let directory = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    try FileManager.default.createDirectory(at: directory, withIntermediateDirectories: true)
+    defer { try? FileManager.default.removeItem(at: directory) }
+    let command = directory.appendingPathComponent("arp-fixture")
+    let quoted = output.replacingOccurrences(of: "'", with: "'\"'\"'")
+    try "#!/bin/sh\nprintf '%s' '\(quoted)'\nexit \(exitCode)\n".write(to: command, atomically: true, encoding: .utf8)
+    try FileManager.default.setAttributes([.posixPermissions: 0o700], ofItemAtPath: command.path)
+    return try ARPCache(arpCommandURL: command)
+  }
+
+  func testEmptySuccessfulOutputHasNoAddress() throws {
+    XCTAssertNil(try cache("").ResolveMACAddress(macAddress: mac))
+  }
+
+  func testWhitespaceSuccessfulOutputHasNoAddress() throws {
+    for output in ["\n", " \t\r\n", "\n\n"] {
+      XCTAssertNil(try cache(output).ResolveMACAddress(macAddress: mac))
+    }
+  }
+
+  func testEmptyOutputPreservesCommandFailure() throws {
+    XCTAssertThrowsError(try cache("", exitCode: 7)) { error in
+      XCTAssertEqual((error as? ARPCommandFailedError)?.terminationStatus, 7)
+    }
+  }
+
+  func testNonemptyOutputPreservesCommandFailure() throws {
+    XCTAssertThrowsError(try cache("synthetic failure", exitCode: 9)) { error in
+      XCTAssertEqual((error as? ARPCommandFailedError)?.terminationStatus, 9)
+    }
+  }
+
+  func testValidEntryResolvesAddress() throws {
+    let value = try cache("? (192.0.2.8) at 0:11:22:33:44:55 on en0 ifscope [ethernet]\n")
+    XCTAssertEqual(try value.ResolveMACAddress(macAddress: mac), IPv4Address("192.0.2.8"))
+  }
+
+  func testIncompleteAndOtherEntriesDoNotMatch() throws {
+    let value = try cache("""
+    ? (192.0.2.9) at (incomplete) on en0 ifscope [ethernet]
+    ? (192.0.2.10) at aa:bb:cc:dd:ee:ff on en0 ifscope [ethernet]
+    """)
+    XCTAssertNil(try value.ResolveMACAddress(macAddress: mac))
+  }
+
+  func testMalformedNonemptyOutputStillFails() throws {
+    XCTAssertThrowsError(try cache("synthetic invalid entry").ResolveMACAddress(macAddress: mac)) {
+      XCTAssertTrue($0 is ARPCommandYieldedInvalidOutputError)
+    }
+  }
+
+  func testInvalidAddressesStillFail() throws {
+    for line in [
+      "? (not-an-ip) at 0:11:22:33:44:55 on en0 ifscope [ethernet]",
+      "? (192.0.2.8) at not-a-mac on en0 ifscope [ethernet]",
+    ] {
+      XCTAssertThrowsError(try cache(line).ResolveMACAddress(macAddress: mac)) {
+        XCTAssertTrue($0 is ARPCommandYieldedInvalidOutputError)
+      }
+    }
+  }
+}

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
     """

Tests/TartTests/OCIManifestTests.swift

@@ -154,6 +154,38 @@ final class OCIManifestTests: XCTestCase {
     XCTAssertNil(manifest.diskBlockCount())
   }
 
+  func testOCIConfigIncludesRootFS() throws {
+    let config = OCIConfig(architecture: .arm64, os: .darwin, config: .init(Labels: [diskFormatLabel: "raw"]))
+    let json = try JSONSerialization.jsonObject(with: config.toJSON()) as! [String: Any]
+
+    let rootfs = try XCTUnwrap(json["rootfs"] as? [String: Any])
+    XCTAssertEqual(rootfs["type"] as? String, "layers")
+    XCTAssertEqual(rootfs["diff_ids"] as? [String], [])
+
+    XCTAssertEqual(
+      String(decoding: try config.toJSON(), as: UTF8.self),
+      #"{"architecture":"arm64","config":{"Labels":{"org.cirruslabs.tart.disk.format":"raw"}},"os":"darwin","rootfs":{"diff_ids":[],"type":"layers"}}"#
+    )
+  }
+
+  func testOCIConfigWithoutRootFSDecodes() throws {
+    // OCI config as pushed by Tart 2.40.1 and earlier
+    let json = #"{"architecture":"arm64","config":{"Labels":{"org.cirruslabs.tart.disk.format":"raw"}},"os":"linux"}"#
+    let config = try Config.jsonDecoder().decode(OCIConfig.self, from: Data(json.utf8))
+
+    XCTAssertEqual(config.architecture, .arm64)
+    XCTAssertEqual(config.os, .linux)
+    XCTAssertEqual(config.config?.Labels, [diskFormatLabel: "raw"])
+    XCTAssertNil(config.rootfs)
+  }
+
+  func testOCIConfigRootFSRoundTrips() throws {
+    let config = try Config.jsonDecoder().decode(OCIConfig.self, from: OCIConfig().toJSON())
+
+    XCTAssertEqual(config.rootfs?.type, "layers")
+    XCTAssertEqual(config.rootfs?.diffIDs, [])
+  }
+
   private func assertManifestError(_ expected: OCIManifestValidationError, diskDescriptors: [OCIManifestLayer]) {
     XCTAssertThrowsError(try manifest(diskDescriptors: diskDescriptors).tartDiskRepresentation()) { error in
       XCTAssertEqual(error as? OCIManifestValidationError, expected)