From 1826d210a061b06fafeeebbb02317779c3fc86c6 Mon Sep 17 00:00:00 2001 From: Brian Cain Date: Tue, 11 Feb 2020 16:00:54 -0800 Subject: [PATCH] Recover from cloning and resizin disk failures This commit adds some recovery if Vagrant fails to reattach or clone a vmdk disk that needs to be resized. It first moves the original disk (after cloning) to a backup file. If something fails, it will reattach the original disk to the guest and continue to raise the exception. --- .../virtualbox/cap/configure_disks.rb | 55 ++++++++++++++----- .../virtualbox/cap/configure_disks_test.rb | 31 +++++++++++ 2 files changed, 73 insertions(+), 13 deletions(-) diff --git a/plugins/providers/virtualbox/cap/configure_disks.rb b/plugins/providers/virtualbox/cap/configure_disks.rb index 41604dc05..092645006 100644 --- a/plugins/providers/virtualbox/cap/configure_disks.rb +++ b/plugins/providers/virtualbox/cap/configure_disks.rb @@ -1,4 +1,5 @@ require "log4r" +require "fileutils" require "vagrant/util/numeric" require "vagrant/util/experimental" @@ -190,34 +191,62 @@ module VagrantPlugins if defined_disk["Storage format"] == "VMDK" LOGGER.warn("Disk type VMDK cannot be resized in VirtualBox. Vagrant will convert disk to VDI format to resize first, and then convert resized disk back to VMDK format") + # grab disk to be resized port and device number disk_info = machine.provider.driver.get_port_and_device(defined_disk["UUID"]) + # original disk information in case anything goes wrong during clone/resize + original_disk = defined_disk # clone disk to vdi formatted disk vdi_disk_file = machine.provider.driver.vmdk_to_vdi(defined_disk["Location"]) # resize vdi machine.provider.driver.resize_disk(vdi_disk_file, disk_config.size.to_i) - # remove and close original volume - machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) - machine.provider.driver.close_medium(defined_disk["UUID"]) + begin + # Danger Zone - # clone back to original vmdk format and attach resized disk - vmdk_disk_file = machine.provider.driver.vdi_to_vmdk(vdi_disk_file) - machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], vmdk_disk_file, "hdd") + # remove and close original volume + machine.provider.driver.remove_disk(disk_info[:port], disk_info[:device]) + # Create a backup of the original disk if something goes wrong + LOGGER.warn("Making a backup of the original disk at #{defined_disk["Location"]}") + FileUtils.mv(defined_disk["Location"], "#{defined_disk["Location"]}.backup") - # close cloned volume format + # we have to close here, otherwise we can't re-clone after + # resizing the vdi disk + machine.provider.driver.close_medium(defined_disk["UUID"]) + + # clone back to original vmdk format and attach resized disk + vmdk_disk_file = machine.provider.driver.vdi_to_vmdk(vdi_disk_file) + machine.provider.driver.attach_disk(disk_info[:port], disk_info[:device], vmdk_disk_file, "hdd") + rescue Exception => e + LOGGER.warn("Vagrant encountered an error while trying to resize a disk. Vagrant will now attempt to reattach and preserve the original disk...") + # TODO: Actually write some good recovery steps for this scenario + machine.ui.error("Vagrant has encountered an exception while trying to resize a disk. It will now attempt to reattach the original disk, as to prevent any data loss.") + machine.ui.error("The original disk is located at : #{original_disk["Location"]}") + # move backup to original name + FileUtils.mv("#{original_disk["Location"]}.backup", + original_disk["Location"], force: true) + # Attach disk + machine.provider.driver. + attach_disk(disk_info[:port], disk_info[:device], original_disk["Location"], "hdd") + + # Remove cloned disk if still hanging around + if vdi_disk_file + machine.provider.driver.close_medium(vdi_disk_file) + end + + raise e + ensure + # Remove backup disk file if all goes well + FileUtils.remove("#{original_disk["Location"]}.backup", force: true) + end + + # Remove cloned resized volume format machine.provider.driver.close_medium(vdi_disk_file) # Get new updated disk UUID for vagrant disk_meta file new_disk_info = machine.provider.driver.list_hdds.select { |h| h["Location"] == defined_disk["Location"] }.first defined_disk = new_disk_info - - # TODO: If any of the above steps fail, display a useful error message - # telling the user how to recover - # - # Vagrant could also have a "rescue" here where in the case of failure, it simply - # reattaches the original disk else machine.provider.driver.resize_disk(defined_disk["Location"], disk_config.size.to_i) end diff --git a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb index 943d446ef..b9c0d74ba 100644 --- a/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb +++ b/test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb @@ -280,6 +280,37 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do let(:vmdk_disk_file) { "/home/vagrant/VirtualBox VMs/ubuntu-18.04-amd64-disk001.vmdk" } it "converts the disk to vdi, resizes it, and converts back to vmdk" do + expect(FileUtils).to receive(:mv).and_return(true) + + expect(driver).to receive(:get_port_and_device).with("12345"). + and_return(attach_info) + + expect(driver).to receive(:vmdk_to_vdi).with(all_disks[0]["Location"]). + and_return(vdi_disk_file) + + expect(driver).to receive(:resize_disk).with(vdi_disk_file, disk_config.size.to_i). + and_return(true) + + expect(driver).to receive(:remove_disk).with(attach_info[:port], attach_info[:device]). + and_return(true) + expect(driver).to receive(:close_medium).with("12345") + + expect(driver).to receive(:vdi_to_vmdk).with(vdi_disk_file). + and_return(vmdk_disk_file) + + expect(driver).to receive(:attach_disk). + with(attach_info[:port], attach_info[:device], vmdk_disk_file, "hdd").and_return(true) + expect(driver).to receive(:close_medium).with(vdi_disk_file).and_return(true) + + expect(driver).to receive(:list_hdds).and_return(all_disks) + + subject.resize_disk(machine, disk_config, all_disks[0]) + end + + it "reattaches original disk if something goes wrong" do + # fix this to properly "mv" file + expect(FileUtils).to receive(:mv).and_return(true) + expect(driver).to receive(:get_port_and_device).with("12345"). and_return(attach_info)