Allow only one concurrent image download with flock

Right now, we use the "File::CREAT | File::EXCL" flags to open a
temporary file for image downloads, which should stop multiple downloads
happening at once. These flags only let one process create the file, and
if the file is already there, it fails. However, if a download gets
interrupted, the file sticks around and the next download attempt fails.
It's tricky to figure out if the file is incomplete, so we have to
delete it manually. Using flock to lock a lock file is a more effective
way to stop multiple downloads idempotently at the same time.

rhizome/host/lib/vm_setup.rb

@@ -489,23 +489,24 @@ def download_boot_image(boot_image, force: false, custom_url: nil, ca_path: nil)
       fail "Unsupported boot_image format: #{image_ext}"
     end
 
-    # Use of File::EXCL provokes a crash rather than a race
-    # condition if two VMs are lazily getting their images at the
-    # same time.
-    download_path = File.join(vp.image_root, boot_image + image_ext + ".tmp")
-    ca_arg = ca_path ? " --cacert #{ca_path.shellescape}" : ""
-    File.open(download_path, File::RDWR | File::CREAT | File::EXCL, 0o644) do
-      r "curl -f -L10 -o #{download_path.shellescape} #{download.shellescape}#{ca_arg}"
-    end
+    File.open(File.join(vp.image_root, boot_image + ".lock"), File::RDWR | File::CREAT) do |lock|
+      fail "Another vm is downloading #{boot_image}" unless lock.flock(File::LOCK_EX | File::LOCK_NB)
+
+      download_path = File.join(vp.image_root, boot_image + image_ext + ".tmp")
+      ca_arg = ca_path ? " --cacert #{ca_path.shellescape}" : ""
+      File.open(download_path, File::RDWR | File::CREAT, 0o644) do
+        r "curl -f -L10 -o #{download_path.shellescape} #{download.shellescape}#{ca_arg}"
+      end
 
-    temp_path = File.join(vp.image_root, boot_image + ".raw.tmp")
-    if initial_format != "raw"
-      # Images are presumed to be atomically renamed into the path,
-      # i.e. no partial images will be passed to qemu-image.
-      r "qemu-img convert -p -f #{initial_format.shellescape} -O raw #{download_path.shellescape} #{temp_path.shellescape}"
-      rm_if_exists(download_path)
+      temp_path = File.join(vp.image_root, boot_image + ".raw.tmp")
+      if initial_format != "raw"
+        # Images are presumed to be atomically renamed into the path,
+        # i.e. no partial images will be passed to qemu-image.
+        r "qemu-img convert -p -f #{initial_format.shellescape} -O raw #{download_path.shellescape} #{temp_path.shellescape}"
+        rm_if_exists(download_path)
+      end
+      File.rename(temp_path, image_path)
     end
-    File.rename(temp_path, image_path)
   end
 
   # Unnecessary if host has this set before creating the netns, but

rhizome/host/spec/vm_setup_spec.rb

@@ -56,6 +56,7 @@ def key_wrapping_secrets
   describe "#download_boot_image" do
     it "can download an image" do
       expect(File).to receive(:exist?).with("/var/storage/images/ubuntu-jammy.raw").and_return(false)
+      expect(File).to receive(:open).with("/var/storage/images/ubuntu-jammy.lock", File::RDWR | File::CREAT).and_yield(instance_double(File, flock: true))
       expect(File).to receive(:open) do |path, *_args|
         expect(path).to eq("/var/storage/images/ubuntu-jammy.img.tmp")
       end.and_yield
@@ -71,6 +72,7 @@ def key_wrapping_secrets
 
     it "can download vhd image with custom URL that has query params using curl" do
       expect(File).to receive(:exist?).with("/var/storage/images/github-ubuntu-2204.raw").and_return(false)
+      expect(File).to receive(:open).with("/var/storage/images/github-ubuntu-2204.lock", File::RDWR | File::CREAT).and_yield(instance_double(File, flock: true))
       expect(File).to receive(:open) do |path, *_args|
         expect(path).to eq("/var/storage/images/github-ubuntu-2204.vhd.tmp")
       end.and_yield
@@ -85,6 +87,7 @@ def key_wrapping_secrets
 
     it "does not convert image if it's in raw format already" do
       expect(File).to receive(:exist?).with("/var/storage/images/github-ubuntu-2204.raw").and_return(false)
+      expect(File).to receive(:open).with("/var/storage/images/github-ubuntu-2204.lock", File::RDWR | File::CREAT).and_yield(instance_double(File, flock: true))
       expect(File).to receive(:open) do |path, *_args|
         expect(path).to eq("/var/storage/images/github-ubuntu-2204.raw.tmp")
       end.and_yield
@@ -97,6 +100,7 @@ def key_wrapping_secrets
 
     it "can download the image with force even if it exists" do
       expect(File).to receive(:exist?).with("/var/storage/images/ubuntu-jammy.raw").and_return(true)
+      expect(File).to receive(:open).with("/var/storage/images/ubuntu-jammy.lock", File::RDWR | File::CREAT).and_yield(instance_double(File, flock: true))
       expect(File).to receive(:open) do |path, *_args|
         expect(path).to eq("/var/storage/images/ubuntu-jammy.img.tmp")
       end.and_yield
@@ -125,6 +129,15 @@ def key_wrapping_secrets
       expect(FileUtils).to receive(:mkdir_p).with("/var/storage/images/")
       expect { vs.download_boot_image("github-ubuntu-2204", custom_url: "https://example.com/ubuntu.iso") }.to raise_error RuntimeError, "Unsupported boot_image format: .iso"
     end
+
+    it "fails if another vm is already downloading the image" do
+      expect(File).to receive(:exist?).with("/var/storage/images/ubuntu-jammy.raw").and_return(false)
+      expect(FileUtils).to receive(:mkdir_p).with("/var/storage/images/")
+      expect(Arch).to receive(:render).and_return("amd64").at_least(:once)
+      expect(File).to receive(:open).with("/var/storage/images/ubuntu-jammy.lock", File::RDWR | File::CREAT).and_yield(instance_double(File, flock: false))
+
+      expect { vs.download_boot_image("ubuntu-jammy") }.to raise_error RuntimeError, "Another vm is downloading ubuntu-jammy"
+    end
   end
 
   describe "#purge_storage" do