Skip to content

Commit

Permalink
Remove tmp image if the download operation fails
Browse files Browse the repository at this point in the history
If the download operation fails for any reason, subsequent attempts
result in a `File exists @ rb_sysopen - /tmp/ubuntu-jammy.img.tmp
(Errno::EEXIST)` error. In case of failure, the temporary image must be
removed.

Fixes #911
  • Loading branch information
enescakir committed Jan 24, 2024
1 parent 4c29c74 commit 85c88bf
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 0 deletions.
3 changes: 3 additions & 0 deletions rhizome/host/lib/vm_setup.rb
Expand Up @@ -451,6 +451,9 @@ def download_boot_image(boot_image, custom_url: nil)
else
r "curl -f -L10 -o #{temp_path.shellescape} #{download.shellescape}"
end
rescue CommandFail
rm_if_exists(temp_path)
raise
end

# Images are presumed to be atomically renamed into the path,
Expand Down
13 changes: 13 additions & 0 deletions rhizome/host/spec/vm_setup_spec.rb
Expand Up @@ -63,6 +63,19 @@ def key_wrapping_secrets
vs.download_boot_image("github-ubuntu-2204", custom_url: "https://images.blob.core.windows.net/images/ubuntu2204.vhd?sp=r&st=2023-09-05T22:44:05Z&se=2023-10-07T06:44:05")
end

it "cleans up tmp image if the downloading fails" do
expect(File).to receive(:exist?).with("/var/storage/images/ubuntu-jammy.raw").and_return(false)
expect(File).to receive(:open) do |path, *_args|
expect(path).to eq("/tmp/ubuntu-jammy.img.tmp")
end.and_yield
expect(FileUtils).to receive(:mkdir_p).with("/var/storage/images/")
expect(Arch).to receive(:render).and_return("amd64").at_least(:once)
expect(vs).to receive(:r).with("curl -f -L10 -o /tmp/ubuntu-jammy.img.tmp https://cloud-images.ubuntu.com/releases/jammy/release-20231010/ubuntu-22.04-server-cloudimg-amd64.img").and_raise(CommandFail.new("Couldn't download image", "", "404 Not Found"))
expect(FileUtils).to receive(:rm_r).with("/tmp/ubuntu-jammy.img.tmp")

expect { vs.download_boot_image("ubuntu-jammy") }.to raise_error CommandFail
end

it "can use an image that's already downloaded" do
expect(File).to receive(:exist?).with("/var/storage/images/almalinux-9.1.raw").and_return(true)
vs.download_boot_image("almalinux-9.1")
Expand Down

0 comments on commit 85c88bf

Please sign in to comment.