beckermj/vaguerent
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Fix stale controller state

Browse Source 
This was causing multiple dvds to be attached to the same port/device.
This commit is contained in:
Jeff Bonhag 2020-06-24 16:52:55 -04:00
parent 3c0021aac3
commit 724687a601
No known key found for this signature in database
GPG Key ID: 32966F3FB5AC1129
6 changed files with 95 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
plugins/providers/virtualbox/cap/configure_disks.rb
View File

@ -63,7 +63,7 @@ module VagrantPlugins
raise Vagrant::Errors::VirtualBoxDisksControllerNotFound, storage_bus: "IDE"
end
if disks_defined.size > dvd_controller.limit
if dvds_defined.size > dvd_controller.limit
raise Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit,
limit: dvd_controller.limit,
name: dvd_controller.name
@ -77,13 +77,14 @@ module VagrantPlugins
defined_disks.each do |disk|
if disk.type == :disk
disk_data = handle_configure_disk(machine, disk, current_disks, disk_controller)
disk_data = handle_configure_disk(machine, disk, current_disks, disk_controller.name)
configured_disks[:disk] << disk_data unless disk_data.empty?
elsif disk.type == :floppy
# TODO: Write me
machine.ui.info(I18n.t("vagrant.cap.configure_disks.floppy_not_supported", name: disk.name))
elsif disk.type == :dvd
dvd_data = handle_configure_dvd(machine, disk, dvd_controller)
# refresh controller state here
dvd_data = handle_configure_dvd(machine, disk, dvd_controller.name)
configured_disks[:dvd] << dvd_data unless dvd_data.empty?
end
end
@ -124,10 +125,11 @@ module VagrantPlugins
# @param [Vagrant::Machine] machine - the current machine
# @param [Config::Disk] disk - the current disk to configure
# @param [Array] all_disks - A list of all currently defined disks in VirtualBox
# @param [VagrantPlugins::ProviderVirtualBox::Model::StorageController] controller -
# the storage controller to use
# @param [String] controller_name - the name of the storage controller to use
# @return [Hash] - disk_metadata
def self.handle_configure_disk(machine, disk, all_disks, controller)
def self.handle_configure_disk(machine, disk, all_disks, controller_name)
controller = machine.provider.driver.get_storage_controller(controller_name)
disk_metadata = {}
# Grab the existing configured disk, if it exists
@ -171,10 +173,11 @@ module VagrantPlugins
#
# @param [Vagrant::Machine] machine - the current machine
# @param [Config::Disk] dvd - the current disk to configure
# @param [VagrantPlugins::ProviderVirtualBox::Model::StorageController] controller -
# the storage controller to use
# @param [String] controller_name - the name of the storage controller to use
# @return [Hash] - dvd_metadata
def self.handle_configure_dvd(machine, dvd, controller)
def self.handle_configure_dvd(machine, dvd, controller_name)
controller = machine.provider.driver.get_storage_controller(controller_name)
disk_info = get_next_port(machine, controller)
port = disk_info[:port]
device = disk_info[:device]
@ -182,8 +185,7 @@ module VagrantPlugins
machine.provider.driver.attach_disk(controller.name, port, device, "dvddrive", dvd.file)
# Refresh the controller information
storage_controllers = machine.provider.driver.read_storage_controllers
controller = storage_controllers.detect { |c| c.name == controller.name }
controller = machine.provider.driver.get_storage_controller(controller.name)
attachment = controller.attachments.detect { |a| a[:port] == port && a[:device] == device }
if attachment

2
plugins/providers/virtualbox/driver/meta.rb
View File

@ -116,8 +116,8 @@ module VagrantPlugins
:execute_command,
:export,
:forward_ports,
:get_controller,
:get_port_and_device,
:get_storage_controller,
:halt,
:import,
:list_snapshots,

13
plugins/providers/virtualbox/driver/version_5_0.rb
View File

@ -955,6 +955,19 @@ module VagrantPlugins
end
end
# Gets the storage controller matching the specified name. Raises an
# error if the storage controller can't be found.
#
# @param [String] name - storage controller name
# @return [VagrantPlugins::ProviderVirtualBox::Model::StorageController]
def get_storage_controller(name)
controller = read_storage_controllers.detect { |c| c.name == name }
if !controller
raise Vagrant::Errors::VirtualBoxDisksControllerNotFound, name: name
end
controller
end
protected
def valid_ip_address?(ip)

7
templates/locales/en.yml
View File

@ -1679,10 +1679,9 @@ en:
Vagrant. VirtualBox 4.2.16+ fixes this problem. Please upgrade
VirtualBox.
virtualbox_disks_controller_not_found: |-
The current disk configuration requires a controller with storage bus
'%{storage_bus}', but no such controller was found. Please add the
proper controller to your guest using provider-specific
customizations.
Vagrant was not able to find a storage controller called '%{name}'.
If you have changed or removed any storage controllers, please restore
them to their prior configuration.
virtualbox_disks_defined_exceed_limit: |-
VirtualBox only allows up to %{limit} disks to be attached to the
storage controller '%{name}'. Please remove some disks from your disk

52
test/unit/plugins/providers/virtualbox/cap/configure_disks_test.rb
View File

@ -69,6 +69,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
allow(Vagrant::Util::Experimental).to receive(:feature_enabled?).and_return(true)
allow(controller).to receive(:attachments).and_return(attachments)
allow(driver).to receive(:read_storage_controllers).and_return([controller])
allow(driver).to receive(:get_storage_controller).with(controller.name).and_return(controller)
end
describe "#configure_disks" do
@ -80,7 +81,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
it "configures disks and returns the disks defined" do
expect(subject).to receive(:handle_configure_disk).with(machine, anything, [], controller).
expect(subject).to receive(:handle_configure_disk).with(machine, anything, [], controller.name).
exactly(4).and_return(dsk_data)
subject.configure_disks(machine, defined_disks)
end
@ -88,7 +89,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
it "configures dvd and returns the disks defined" do
defined_disks = [ dvd ]
expect(subject).to receive(:handle_configure_dvd).with(machine, dvd, controller).
expect(subject).to receive(:handle_configure_dvd).with(machine, dvd, controller.name).
and_return({})
subject.configure_disks(machine, defined_disks)
end
@ -113,16 +114,17 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
end
end
# hashicorp/bionic64
context "with more than one storage controller" do
let(:controller1) { double("controller1", storage_bus: "IDE", limit: 4) }
let(:controller2) { double("controller2", storage_bus: "SATA", limit: 30) }
let(:controller1) { double("controller1", name: "IDE Controller", storage_bus: "IDE", limit: 4) }
let(:controller2) { double("controller2", name: "SATA Controller", storage_bus: "SATA", limit: 30) }
before do
allow(driver).to receive(:read_storage_controllers).and_return([controller1, controller2])
end
it "attaches disks to the SATA controller" do
expect(subject).to receive(:handle_configure_disk).with(machine, anything, [], controller2).
expect(subject).to receive(:handle_configure_disk).with(machine, anything, [], controller2.name).
exactly(4).and_return(dsk_data)
subject.configure_disks(machine, defined_disks)
end
@ -130,10 +132,36 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
it "attaches dvds to the IDE controller" do
defined_disks = [ dvd ]
expect(subject).to receive(:handle_configure_dvd).with(machine, dvd, controller1).
expect(subject).to receive(:handle_configure_dvd).with(machine, dvd, controller1.name).
and_return({})
subject.configure_disks(machine, defined_disks)
end
it "raises an error if there are more than 4 dvds configured" do
defined_disks = [
double("dvd", name: "installer1", type: :dvd, file: "installer.iso"),
double("dvd", name: "installer2", type: :dvd, file: "installer.iso"),
double("dvd", name: "installer3", type: :dvd, file: "installer.iso"),
double("dvd", name: "installer4", type: :dvd, file: "installer.iso"),
double("dvd", name: "installer5", type: :dvd, file: "installer.iso")
]
expect { subject.configure_disks(machine, defined_disks) }.
to raise_error(Vagrant::Errors::VirtualBoxDisksDefinedExceedLimit)
end
it "attaches multiple dvds" do
defined_disks = [
double("dvd", name: "installer1", type: :dvd, file: "installer.iso"),
double("dvd", name: "installer2", type: :dvd, file: "installer.iso"),
double("dvd", name: "installer3", type: :dvd, file: "installer.iso"),
double("dvd", name: "installer4", type: :dvd, file: "installer.iso"),
]
expect(subject).to receive(:handle_configure_dvd).exactly(4).times.and_return({})
subject.configure_disks(machine, defined_disks)
end
end
end
@ -178,14 +206,14 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
expect(subject).to receive(:create_disk).with(machine, defined_disks[1], controller)
.and_return(disk_meta)
subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller)
subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name)
end
it "includes disk attachment info in metadata" do
expect(subject).to receive(:create_disk).with(machine, defined_disks[1], controller)
.and_return(disk_meta)
disk_metadata = subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller)
disk_metadata = subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name)
expect(disk_metadata).to have_key(:controller)
expect(disk_metadata).to have_key(:port)
expect(disk_metadata).to have_key(:device)
@ -222,7 +250,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
expect(subject).to receive(:resize_disk).
with(machine, defined_disks[1], all_disks[1], controller).and_return({})
subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller)
subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name)
end
end
@ -264,7 +292,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
"hdd",
all_disks[1]["Location"])
subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller)
subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name)
end
it "does nothing if all disks are properly configured" do
@ -277,7 +305,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
expect(driver).to receive(:get_port_and_device).with("67890").
and_return(disk_info)
subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller)
subject.handle_configure_disk(machine, defined_disks[1], all_disks, controller.name)
end
end
end
@ -469,7 +497,7 @@ describe VagrantPlugins::ProviderVirtualBox::Cap::ConfigureDisks do
it "includes disk attachment info in metadata" do
expect(driver).to receive(:attach_disk).with(controller.name, "0", "0", "dvddrive", "/tmp/untitled.iso")
dvd_metadata = subject.handle_configure_dvd(machine, dvd_config, controller)
dvd_metadata = subject.handle_configure_dvd(machine, dvd_config, controller.name)
expect(dvd_metadata[:uuid]).to eq("12345")
expect(dvd_metadata).to have_key(:controller)
expect(dvd_metadata).to have_key(:port)

25
test/unit/plugins/providers/virtualbox/driver/version_5_0_test.rb
View File

@ -138,4 +138,29 @@ OUTPUT
expect(storage_controllers.first.attachments).to include(port: "1", device: "0", uuid: "67890")
end
end
describe "#get_storage_controller" do
let(:storage_controllers) { [double("controller", name: "IDE Controller")] }
before do
allow(subject).to receive(:read_storage_controllers).and_return(storage_controllers)
end
it "refreshes the storage controller state" do
expect(subject).to receive(:read_storage_controllers)
subject.get_storage_controller("IDE Controller")
end
it "returns the storage controller matching the specified name" do
controller = subject.get_storage_controller("IDE Controller")
expect(controller.name).to eq("IDE Controller")
end
it "raises an exception if the storage controller can't be found" do
expect { subject.get_storage_controller("SCSI Controller") }.
to raise_error(Vagrant::Errors::VirtualBoxDisksControllerNotFound)
end
end
end