From 63329ef3637847e60f291f83f1876876bebb6131 Mon Sep 17 00:00:00 2001 From: Nikolay Edigaryev Date: Wed, 1 Jun 2022 18:02:31 +0300 Subject: [PATCH] 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 | 55 ++++++++++++++++++------------ Sources/tart/Commands/Create.swift | 17 +++++---- Sources/tart/Commands/Run.swift | 6 +++- Sources/tart/Root.swift | 6 +++- Sources/tart/VMDirectory.swift | 7 ++++ Sources/tart/VMStorageLocal.swift | 4 +++ Sources/tart/VMStorageOCI.swift | 33 +++++++++++------- 7 files changed, 85 insertions(+), 43 deletions(-) diff --git a/Sources/tart/Commands/Clone.swift b/Sources/tart/Commands/Clone.swift index 39e45f7..02d965c 100644 --- a/Sources/tart/Commands/Clone.swift +++ b/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(" 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 } } } diff --git a/Sources/tart/Commands/Create.swift b/Sources/tart/Commands/Create.swift index fed204c..a274df9 100644 --- a/Sources/tart/Commands/Create.swift +++ b/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 { diff --git a/Sources/tart/Commands/Run.swift b/Sources/tart/Commands/Run.swift index f0c13d7..7785065 100644 --- a/Sources/tart/Commands/Run.swift +++ b/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) } diff --git a/Sources/tart/Root.swift b/Sources/tart/Root.swift index b04e2a9..2e25576 100644 --- a/Sources/tart/Root.swift +++ b/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() diff --git a/Sources/tart/VMDirectory.swift b/Sources/tart/VMDirectory.swift index 492eda3..46e3563 100644 --- a/Sources/tart/VMDirectory.swift +++ b/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) && diff --git a/Sources/tart/VMStorageLocal.swift b/Sources/tart/VMStorageLocal.swift index 94cf271..8da2c0d 100644 --- a/Sources/tart/VMStorageLocal.swift +++ b/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)) } diff --git a/Sources/tart/VMStorageOCI.swift b/Sources/tart/VMStorageOCI.swift index 5770aef..208e3f6 100644 --- a/Sources/tart/VMStorageOCI.swift +++ b/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 {