Return early from set_boot_order if not needed (#1578)

Switch to calling the returning the next middleware in the chain as soon
as possible in the set boot order action. Makes the overall remaining
logic tidier.

Include basic tests to check existing behaviour.
This commit is contained in:
Darragh Bailey 2022-09-03 16:11:11 +01:00 committed by GitHub
parent 8f868b9b88
commit 606b86df67
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 264 additions and 42 deletions

View File

@ -16,6 +16,12 @@ module VagrantPlugins
end
def call(env)
# Only execute specific boot ordering if this is defined
# in the Vagrant file
unless @boot_order.count >= 1
return @app.call(env)
end
# Get domain first
begin
domain = env[:machine].provider
@ -30,53 +36,49 @@ module VagrantPlugins
error_message: e.message
end
# Only execute specific boot ordering if this is defined
# in the Vagrant file
if @boot_order.count >= 1
# If a domain is initially defined with no box or disk or
# with an explicit boot order, Libvirt adds <boot dev="foo">
# This conflicts with an explicit boot_order configuration,
# so we need to remove it from the domain xml and feed it back.
# Also see https://bugzilla.redhat.com/show_bug.cgi?id=1248514
# as to why we have to do this after all devices have been defined.
xml = Nokogiri::XML(domain.xml_desc)
xml.search('/domain/os/boot').each(&:remove)
# If a domain is initially defined with no box or disk or
# with an explicit boot order, Libvirt adds <boot dev="foo">
# This conflicts with an explicit boot_order configuration,
# so we need to remove it from the domain xml and feed it back.
# Also see https://bugzilla.redhat.com/show_bug.cgi?id=1248514
# as to why we have to do this after all devices have been defined.
xml = Nokogiri::XML(domain.xml_desc)
xml.search('/domain/os/boot').each(&:remove)
# Parse the XML and find each defined drive and network interfacee
hd = xml.search("/domain/devices/disk[@device='disk']")
cdrom = xml.search("/domain/devices/disk[@device='cdrom']")
# implemented only for 1 network
nets = @boot_order.flat_map do |x|
x.class == Hash ? x : nil
end.compact
raise 'Defined only for 1 network for boot' if nets.size > 1
network = search_network(nets, xml)
# Parse the XML and find each defined drive and network interfacee
hd = xml.search("/domain/devices/disk[@device='disk']")
cdrom = xml.search("/domain/devices/disk[@device='cdrom']")
# implemented only for 1 network
nets = @boot_order.flat_map do |x|
x.class == Hash ? x : nil
end.compact
raise 'Defined only for 1 network for boot' if nets.size > 1
network = search_network(nets, xml)
# Generate an array per device group and a flattened
# array from all of those
devices = { 'hd' => hd,
'cdrom' => cdrom,
'network' => network }
# Generate an array per device group and a flattened
# array from all of those
devices = { 'hd' => hd,
'cdrom' => cdrom,
'network' => network }
final_boot_order = final_boot_order(@boot_order, devices)
# Loop over the entire defined boot order array and
# create boot order entries in the domain XML
final_boot_order.each_with_index do |node, index|
boot = "<boot order='#{index + 1}'/>"
node.add_child(boot)
logger_msg(node, index)
end
# Finally redefine the domain XML through Libvirt
# to apply the boot ordering
env[:machine].provider
.driver
.connection
.client
.define_domain_xml(xml.to_s)
final_boot_order = final_boot_order(@boot_order, devices)
# Loop over the entire defined boot order array and
# create boot order entries in the domain XML
final_boot_order.each_with_index do |node, index|
next if node.nil?
boot = "<boot order='#{index + 1}'/>"
node.add_child(boot)
logger_msg(node, index)
end
# Finally redefine the domain XML through Libvirt
# to apply the boot ordering
env[:machine].provider
.driver
.connection
.client
.define_domain_xml(xml.to_s)
@app.call(env)
end

View File

@ -0,0 +1,67 @@
# frozen_string_literal: true
require 'spec_helper'
require 'support/sharedcontext'
require 'support/libvirt_context'
require 'vagrant-libvirt/action/set_boot_order'
describe VagrantPlugins::ProviderLibvirt::Action::SetBootOrder do
subject { described_class.new(app, env) }
include_context 'unit'
include_context 'libvirt'
#before do
# allow(driver).to receive(:created?).and_return(true)
#end
describe '#call' do
it 'should return early' do
expect(connection).to_not receive(:client)
expect(subject.call(env)).to be_nil
end
context 'with boot_order defined' do
let(:domain_xml) { File.read(File.join(File.dirname(__FILE__), File.basename(__FILE__, '.rb'), test_file)) }
let(:updated_domain_xml) { File.read(File.join(File.dirname(__FILE__), File.basename(__FILE__, '.rb'), updated_test_file)) }
let(:test_file) { 'default.xml' }
let(:updated_test_file) { 'explicit_boot_order.xml' }
let(:vagrantfile_providerconfig) do
<<-EOF
libvirt.boot "hd"
libvirt.boot "cdrom"
libvirt.boot "network" => 'vagrant-libvirt'
EOF
end
before do
allow(connection).to receive(:client).and_return(libvirt_client)
allow(libvirt_client).to receive(:lookup_domain_by_uuid).and_return(libvirt_domain)
allow(libvirt_domain).to receive(:xml_desc).and_return(domain_xml)
allow(logger).to receive(:debug)
end
it 'should configure the boot order' do
expect(libvirt_client).to receive(:define_domain_xml).with(updated_domain_xml)
expect(subject.call(env)).to be_nil
end
context 'with multiple networks in bootorder' do
let(:vagrantfile_providerconfig) do
<<-EOF
libvirt.boot "hd"
libvirt.boot "cdrom"
libvirt.boot "network" => 'vagrant-libvirt'
libvirt.boot "network" => 'vagrant-libvirt'
EOF
end
it 'should raise an exception' do
expect { subject.call(env) }.to raise_error('Defined only for 1 network for boot')
end
end
end
end
end

View File

@ -0,0 +1,76 @@
<domain type="qemu">
<name>vagrant-libvirt_default</name>
<uuid>881a931b-0110-4d10-81aa-47a1a19f5726</uuid>
<description>Source: /home/test/vagrant-libvirt/Vagrantfile</description>
<memory unit="KiB">2097152</memory>
<currentMemory unit="KiB">2097152</currentMemory>
<vcpu placement="static">2</vcpu>
<os>
<type arch="x86_64" machine="pc-i440fx-6.0">hvm</type>
<boot dev="hd"/>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<cpu mode="host-model" check="partial"/>
<clock offset="utc"/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>destroy</on_crash>
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type="file" device="disk">
<driver name="qemu" type="qcow2"/>
<source file="/var/lib/libvirt/images/vagrant-libvirt_default.img"/>
<target dev="vda" bus="virtio"/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x03" function="0x0"/>
</disk>
<disk type="file" device="cdrom">
<driver name="qemu" type="raw"/>
<source file=""/>
<target dev="hda" bus="ide"/>
<readonly/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x06" function="0x0"/>
</disk>
<disk type="file" device="cdrom">
<driver name="qemu" type="raw"/>
<source file=""/>
<target dev="hdb" bus="ide"/>
<readonly/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x07" function="0x0"/>
</disk>
<controller type="usb" index="0" model="piix3-uhci">
<address type="pci" domain="0x0000" bus="0x00" slot="0x01" function="0x2"/>
</controller>
<controller type="pci" index="0" model="pci-root"/>
<interface type="network">
<mac address="52:54:00:7d:14:0e"/>
<source network="vagrant-libvirt"/>
<model type="virtio"/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x05" function="0x0"/>
</interface>
<serial type="pty">
<target type="isa-serial" port="0">
<model name="isa-serial"/>
</target>
</serial>
<console type="pty">
<target type="serial" port="0"/>
</console>
<input type="mouse" bus="ps2"/>
<input type="keyboard" bus="ps2"/>
<graphics type="vnc" port="-1" autoport="yes" listen="127.0.0.1" keymap="en-us">
<listen type="address" address="127.0.0.1"/>
</graphics>
<audio id="1" type="none"/>
<video>
<model type="cirrus" vram="16384" heads="1" primary="yes"/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x02" function="0x0"/>
</video>
<memballoon model="virtio">
<address type="pci" domain="0x0000" bus="0x00" slot="0x04" function="0x0"/>
</memballoon>
</devices>
</domain>

View File

@ -0,0 +1,77 @@
<?xml version="1.0"?>
<domain type="qemu">
<name>vagrant-libvirt_default</name>
<uuid>881a931b-0110-4d10-81aa-47a1a19f5726</uuid>
<description>Source: /home/test/vagrant-libvirt/Vagrantfile</description>
<memory unit="KiB">2097152</memory>
<currentMemory unit="KiB">2097152</currentMemory>
<vcpu placement="static">2</vcpu>
<os>
<type arch="x86_64" machine="pc-i440fx-6.0">hvm</type>
</os>
<features>
<acpi/>
<apic/>
<pae/>
</features>
<cpu mode="host-model" check="partial"/>
<clock offset="utc"/>
<on_poweroff>destroy</on_poweroff>
<on_reboot>restart</on_reboot>
<on_crash>destroy</on_crash>
<devices>
<emulator>/usr/bin/qemu-system-x86_64</emulator>
<disk type="file" device="disk">
<driver name="qemu" type="qcow2"/>
<source file="/var/lib/libvirt/images/vagrant-libvirt_default.img"/>
<target dev="vda" bus="virtio"/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x03" function="0x0"/>
<boot order="1"/></disk>
<disk type="file" device="cdrom">
<driver name="qemu" type="raw"/>
<source file=""/>
<target dev="hda" bus="ide"/>
<readonly/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x06" function="0x0"/>
<boot order="2"/></disk>
<disk type="file" device="cdrom">
<driver name="qemu" type="raw"/>
<source file=""/>
<target dev="hdb" bus="ide"/>
<readonly/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x07" function="0x0"/>
<boot order="3"/></disk>
<controller type="usb" index="0" model="piix3-uhci">
<address type="pci" domain="0x0000" bus="0x00" slot="0x01" function="0x2"/>
</controller>
<controller type="pci" index="0" model="pci-root"/>
<interface type="network">
<mac address="52:54:00:7d:14:0e"/>
<source network="vagrant-libvirt"/>
<model type="virtio"/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x05" function="0x0"/>
<boot order="4"/></interface>
<serial type="pty">
<target type="isa-serial" port="0">
<model name="isa-serial"/>
</target>
</serial>
<console type="pty">
<target type="serial" port="0"/>
</console>
<input type="mouse" bus="ps2"/>
<input type="keyboard" bus="ps2"/>
<graphics type="vnc" port="-1" autoport="yes" listen="127.0.0.1" keymap="en-us">
<listen type="address" address="127.0.0.1"/>
</graphics>
<audio id="1" type="none"/>
<video>
<model type="cirrus" vram="16384" heads="1" primary="yes"/>
<address type="pci" domain="0x0000" bus="0x00" slot="0x02" function="0x0"/>
</video>
<memballoon model="virtio">
<address type="pci" domain="0x0000" bus="0x00" slot="0x04" function="0x0"/>
</memballoon>
</devices>
</domain>