Atomic tart {create,clone,pull} operations using rename(2) (#109)

* tart clone: always re-generate MAC-address

This is not really an issue for non-enterprise users[1].

[1]: https://github.com/cirruslabs/tart/issues/20#issuecomment-1136944455

* Atomic tart {create,clone,pull} operations using rename(2)

* Print a nicer error message when attempting to double-run a VM

* tart clone: bring back the old MAC-address generation logic

* Ensure VMDirectory.temporary() will be deleted on failure

Sources/tart/Commands/Clone.swift

@@ -11,24 +11,34 @@ struct Clone: AsyncParsableCommand {
   @Argument(help: "new VM name")
   var newName: String
 
+  func validate() throws {
+    if newName.contains("/") {
+      throw ValidationError("<new-name> should be a local name")
+    }
+  }
+
   func run() async throws {
     do {
-      if let remoteName = try? RemoteName(sourceName) {
-        if !VMStorageOCI().exists(remoteName) {
-          // Pull the VM in case it's OCI-based and doesn't exist locally yet
-          let registry = try Registry(host: remoteName.host, namespace: remoteName.namespace)
-          try await VMStorageOCI().pull(remoteName, registry: registry)
-        }
-        let remoteVM = try VMStorageHelper.open(sourceName)
+      let ociStorage = VMStorageOCI()
+      let localStorage = VMStorageLocal()
 
-        let remoteConfig = try VMConfig.init(fromURL: remoteVM.configURL)
-        let needToGenerateNewMAC = try localVMExistsWith(macAddress: remoteConfig.macAddress.string)
-
-        try remoteVM.clone(to: VMStorageLocal().create(newName), generateMAC: needToGenerateNewMAC)
-      } else {
-        try VMStorageHelper.open(sourceName).clone(to: VMStorageLocal().create(newName), generateMAC: true)
+      if let remoteName = try? RemoteName(sourceName), !ociStorage.exists(remoteName) {
+        // Pull the VM in case it's OCI-based and doesn't exist locally yet
+        let registry = try Registry(host: remoteName.host, namespace: remoteName.namespace)
+        try await ociStorage.pull(remoteName, registry: registry)
       }
 
+      let sourceVM = try VMStorageHelper.open(sourceName)
+      let generateMAC = try localStorage.hasVMsWithMACAddress(macAddress: sourceVM.macAddress())
+
+      let tmpVMDir = try VMDirectory.temporary()
+      try await withTaskCancellationHandler(operation: {
+        try sourceVM.clone(to: tmpVMDir, generateMAC: generateMAC)
+        try localStorage.move(newName, from: tmpVMDir)
+      }, onCancel: {
+        try? FileManager.default.removeItem(at: tmpVMDir.baseURL)
+      })
+
       Foundation.exit(0)
     } catch {
       print(error)
@@ -36,15 +46,16 @@ struct Clone: AsyncParsableCommand {
       Foundation.exit(1)
     }
   }
+}
 
-  private func localVMExistsWith(macAddress: String) throws -> Bool {
-    var needToGenerateNewMAC = false
-    for (_, localDir) in try VMStorageLocal().list() {
-      let localConfig = try VMConfig.init(fromURL: localDir.configURL)
-      if localConfig.macAddress.string == macAddress {
-        needToGenerateNewMAC = true
-      }
-    }
-    return needToGenerateNewMAC
+fileprivate extension VMDirectory {
+  func macAddress() throws -> String {
+    try VMConfig(fromURL: configURL).macAddress.string
+  }
+}
+
+fileprivate extension VMStorageLocal {
+  func hasVMsWithMACAddress(macAddress: String) throws -> Bool {
+    try list().contains { try $1.macAddress() == macAddress }
   }
 }

Sources/tart/Commands/Create.swift

@@ -23,13 +23,18 @@ struct Create: AsyncParsableCommand {
 
   func run() async throws {
     do {
-      let vmDir = try VMStorageLocal().create(name)
+      let tmpVMDir = try VMDirectory.temporary()
+      try await withTaskCancellationHandler(operation: {
+        if fromIPSW! == "latest" {
+          _ = try await VM(vmDir: tmpVMDir, ipswURL: nil, diskSizeGB: diskSize)
+        } else {
+          _ = try await VM(vmDir: tmpVMDir, ipswURL: URL(fileURLWithPath: fromIPSW!), diskSizeGB: diskSize)
+        }
 
-      if fromIPSW! == "latest" {
-        _ = try await VM(vmDir: vmDir, ipswURL: nil, diskSizeGB: diskSize)
-      } else {
-        _ = try await VM(vmDir: vmDir, ipswURL: URL(fileURLWithPath: fromIPSW!), diskSizeGB: diskSize)
-      }
+        try VMStorageLocal().move(name, from: tmpVMDir)
+      }, onCancel: {
+        try? FileManager.default.removeItem(at: tmpVMDir.baseURL)
+      })
 
       Foundation.exit(0)
     } catch {

Sources/tart/Commands/Run.swift

@@ -60,7 +60,11 @@ struct Run: AsyncParsableCommand {
 
           Foundation.exit(0)
         } catch {
-          print(error)
+          if error.localizedDescription.contains("Failed to lock auxiliary storage.") {
+            print("Virtual machine \"\(name)\" is already running!")
+          } else {
+            print(error)
+          }
 
           Foundation.exit(1)
         }

Sources/tart/Root.swift

@@ -20,11 +20,15 @@ struct Root: AsyncParsableCommand {
     ])
 
   public static func main() async throws {
-    // Handle cancellation by Ctrl+C
+    // Ensure the default SIGINT handled is disabled,
+    // otherwise there's a race between two handlers
+    signal(SIGINT, SIG_IGN);
+    // Handle cancellation by Ctrl+C ourselves
     let task = withUnsafeCurrentTask { $0 }!
     let sigintSrc = DispatchSource.makeSignalSource(signal: SIGINT)
     sigintSrc.setEventHandler {
       task.cancel()
+      Darwin.exit(1)
     }
     sigintSrc.activate()
 

Sources/tart/VMDirectory.swift

@@ -24,6 +24,13 @@ struct VMDirectory {
     baseURL.lastPathComponent
   }
 
+  static func temporary() throws -> VMDirectory {
+    let tmpDir = FileManager.default.temporaryDirectory.appendingPathComponent(UUID().uuidString)
+    try FileManager.default.createDirectory(at: tmpDir, withIntermediateDirectories: false)
+
+    return VMDirectory(baseURL: tmpDir)
+  }
+
   var initialized: Bool {
     FileManager.default.fileExists(atPath: configURL.path) &&
       FileManager.default.fileExists(atPath: diskURL.path) &&

Sources/tart/VMStorageLocal.swift

@@ -27,6 +27,10 @@ class VMStorageLocal {
     return vmDir
   }
 
+  func move(_ name: String, from: VMDirectory) throws {
+    _ = try FileManager.default.replaceItemAt(vmURL(name), withItemAt: from.baseURL)
+  }
+
   func delete(_ name: String) throws {
     try FileManager.default.removeItem(at: vmURL(name))
   }

Sources/tart/VMStorageOCI.swift

@@ -27,6 +27,17 @@ class VMStorageOCI {
     return vmDir
   }
 
+  func move(_ name: RemoteName, from: VMDirectory) throws{
+    let targetURL = vmURL(name)
+
+    // Pre-create intermediate directories (e.g. creates ~/.tart/cache/OCIs/github.com/org/repo/
+    // for github.com/org/repo:latest)
+    try FileManager.default.createDirectory(at: targetURL.deletingLastPathComponent(),
+            withIntermediateDirectories: true)
+
+    _ = try FileManager.default.replaceItemAt(targetURL, withItemAt: from.baseURL)
+  }
+
   func delete(_ name: RemoteName) throws {
     try FileManager.default.removeItem(at: vmURL(name))
   }
@@ -66,19 +77,17 @@ class VMStorageOCI {
 
     let (manifest, _) = try await registry.pullManifest(reference: name.reference.value)
 
-    if let cacheToVMDir = try cache(name: name, digest: manifest.digest()) {
-      try await cacheToVMDir.pullFromRegistry(registry: registry, manifest: manifest)
-    }
-  }
-
-  func cache(name: RemoteName, digest: String) throws -> VMDirectory? {
-    var result: VMDirectory? = nil
-
-    var digestName = name
-    digestName.reference = Reference(digest: digest)
+    var digestName = RemoteName(host: name.host, namespace: name.namespace,
+            reference: Reference(digest: try manifest.digest()))
 
     if !exists(digestName) {
-      result = try create(digestName)
+      let tmpVMDir = try VMDirectory.temporary()
+      try await withTaskCancellationHandler(operation: {
+        try await tmpVMDir.pullFromRegistry(registry: registry, manifest: manifest)
+        try move(digestName, from: tmpVMDir)
+      }, onCancel: {
+        try? FileManager.default.removeItem(at: tmpVMDir.baseURL)
+      })
     } else {
       defaultLogger.appendNewLine("\(digestName) image is already cached! creating a symlink...")
     }
@@ -87,8 +96,6 @@ class VMStorageOCI {
       // Overwrite the old symbolic link
       try link(from: digestName, to: name)
     }
-
-    return result
   }
 
   func link(from: RemoteName, to: RemoteName) throws {