diff --git a/lib/vagrant-libvirt/config.rb b/lib/vagrant-libvirt/config.rb index a81bbfe..ae3742c 100644 --- a/lib/vagrant-libvirt/config.rb +++ b/lib/vagrant-libvirt/config.rb @@ -970,7 +970,7 @@ module VagrantPlugins # Additional QEMU commandline environment variables @qemu_env = {} if @qemu_env == UNSET_VALUE - @qemu_use_agent = true if @qemu_use_agent != UNSET_VALUE + @qemu_use_agent = false if @qemu_use_agent == UNSET_VALUE @serials = [{:type => 'pty', :source => nil}] if @serials == [] end @@ -986,6 +986,9 @@ module VagrantPlugins end end + unless @qemu_use_agent == true || @qemu_use_agent == false + errors << "libvirt.qemu_use_agent must be a boolean." + end if @qemu_use_agent == true # if qemu agent is used to optain domain ip configuration, at least diff --git a/lib/vagrant-libvirt/driver.rb b/lib/vagrant-libvirt/driver.rb index 672aab6..2474196 100644 --- a/lib/vagrant-libvirt/driver.rb +++ b/lib/vagrant-libvirt/driver.rb @@ -98,16 +98,16 @@ module VagrantPlugins end def get_domain_ipaddress(machine, domain) - if @machine.provider_config.qemu_use_session - return get_ipaddress_from_system domain.mac - end - # attempt to get ip address from qemu agent if @machine.provider_config.qemu_use_agent == true @logger.info('Get IP via qemu agent') return get_ipaddress_from_qemu_agent(domain, machine.id) end + if @machine.provider_config.qemu_use_session + return get_ipaddress_from_system domain.mac + end + # Get IP address from dhcp leases table begin ip_address = get_ipaddress_from_domain(domain) @@ -168,9 +168,9 @@ module VagrantPlugins def get_ipaddress_from_qemu_agent(domain, machine_id) ip_address = nil addresses = nil - dom = system_connection.lookup_domain_by_uuid(machine_id) + libvirt_domain = connection.client.lookup_domain_by_uuid(machine_id) begin - response = dom.qemu_agent_command('{"execute":"guest-network-get-interfaces"}', timeout=10) + response = libvirt_domain.qemu_agent_command('{"execute":"guest-network-get-interfaces"}', timeout=10) @logger.debug("Got Response from qemu agent") @logger.debug(response) addresses = JSON.parse(response) @@ -180,7 +180,7 @@ module VagrantPlugins unless addresses.nil? addresses["return"].each{ |interface| - if domain.mac == interface["hardware-address"] + if domain.mac.downcase == interface["hardware-address"].downcase @logger.debug("Found mathing interface: [%s]" % interface["name"]) if interface.has_key?("ip-addresses") interface["ip-addresses"].each{ |ip| diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 71f055a..cb7a814 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -42,4 +42,12 @@ RSpec.configure do |config| config.before(:suite) do ENV.delete('LIBVIRT_DEFAULT_URI') end + + config.mock_with :rspec do |mocks| + # This option should be set when all dependencies are being loaded + # before a spec run, as is the case in a typical spec helper. It will + # cause any verifying double instantiation for a class that does not + # exist to raise, protecting against incorrectly spelt names. + mocks.verify_doubled_constant_names = true + end end diff --git a/spec/support/libvirt_context.rb b/spec/support/libvirt_context.rb index 74f55d1..3adfe4a 100644 --- a/spec/support/libvirt_context.rb +++ b/spec/support/libvirt_context.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true require 'fog/libvirt' +require 'fog/libvirt/models/compute/server' +require 'libvirt' shared_context 'libvirt' do include_context 'unit' @@ -8,7 +10,9 @@ shared_context 'libvirt' do let(:libvirt_context) { true } let(:id) { 'dummy-vagrant_dummy' } let(:connection) { double('connection') } - let(:domain) { double('domain') } + let(:domain) { instance_double('::Fog::Libvirt::Compute::Server') } + let(:libvirt_client) { instance_double('::Libvirt::Connect') } + let(:libvirt_domain) { instance_double('::Libvirt::Domain') } let(:logger) { double('logger') } def connection_result(options = {}) @@ -22,11 +26,10 @@ shared_context 'libvirt' do stub_const('::Fog::Compute', connection) # drivers also call vm_exists? during init; - allow(connection).to receive(:servers).with(kind_of(String)) + allow(connection).to receive(:servers) .and_return(connection_result(result: nil)) - # return some information for domain when needed - allow(domain).to receive(:mac).and_return('9C:D5:53:F1:5A:E7') + allow(connection).to receive(:client).and_return(libvirt_client) allow(machine).to receive(:id).and_return(id) allow(Log4r::Logger).to receive(:new).and_return(logger) diff --git a/spec/unit/action/shutdown_domain_spec.rb b/spec/unit/action/shutdown_domain_spec.rb index 7ca8357..ced72d3 100644 --- a/spec/unit/action/shutdown_domain_spec.rb +++ b/spec/unit/action/shutdown_domain_spec.rb @@ -50,7 +50,7 @@ describe VagrantPlugins::ProviderLibvirt::Action::ShutdownDomain do end it "should not shutdown" do - expect(domain).not_to receive(:shutoff) + expect(domain).not_to receive(:poweroff) subject.call(env) end diff --git a/spec/unit/action/wait_till_up_spec.rb b/spec/unit/action/wait_till_up_spec.rb index 48bfce7..7b00b2f 100644 --- a/spec/unit/action/wait_till_up_spec.rb +++ b/spec/unit/action/wait_till_up_spec.rb @@ -22,6 +22,8 @@ describe VagrantPlugins::ProviderLibvirt::Action::WaitTillUp do .and_return(driver) allow(driver).to receive(:get_domain).and_return(domain) allow(driver).to receive(:state).and_return(:running) + # return some information for domain when needed + allow(domain).to receive(:mac).and_return('9C:D5:53:F1:5A:E7') end context 'when machine does not exist' do diff --git a/spec/unit/driver_spec.rb b/spec/unit/driver_spec.rb index ace277b..8359e4e 100644 --- a/spec/unit/driver_spec.rb +++ b/spec/unit/driver_spec.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require 'fog/libvirt/requests/compute/dhcp_leases' + require 'spec_helper' require 'support/binding_proc' require 'support/sharedcontext' @@ -8,6 +10,7 @@ require 'vagrant-libvirt/driver' describe VagrantPlugins::ProviderLibvirt::Driver do include_context 'unit' + include_context 'libvirt' subject { described_class.new(machine) } @@ -27,6 +30,7 @@ describe VagrantPlugins::ProviderLibvirt::Driver do end EOF end + # need to override the default package iso_env as using a different # name for the test machines above. let(:machine) { iso_env.machine(:test1, :libvirt) } @@ -36,6 +40,14 @@ describe VagrantPlugins::ProviderLibvirt::Driver do let(:system_connection1) { double("system connection 1") } let(:system_connection2) { double("system connection 2") } + # make it easier for distros that want to switch the default value for + # qemu_use_session to true by ensuring it is explicitly false for tests. + before do + allow(machine.provider_config).to receive(:qemu_use_session).and_return(false) + allow(logger).to receive(:info) + allow(logger).to receive(:debug) + end + describe '#connection' do it 'should configure a separate connection per machine' do expect(Fog::Compute).to receive(:new).with( @@ -84,6 +96,129 @@ describe VagrantPlugins::ProviderLibvirt::Driver do end end + describe '#get_ipaddress' do + context 'when domain exists' do + # not used yet, but this is the form that is returned from addresses + let(:addresses) { { + :public => ["192.168.122.111"], + :private => ["192.168.122.111"], + } } + + before do + allow(subject).to receive(:get_domain).and_return(domain) + end + + it 'should retrieve the address via domain fog-libvirt API' do + # ideally should be able to yield a block to wait_for and check that + # the 'addresses' function on the domain is called correctly. + expect(domain).to receive(:wait_for).and_return(nil) + expect(subject.get_ipaddress(machine)).to eq(nil) + end + + context 'when qemu_use_agent is enabled' do + let(:qemu_agent_interfaces) { + <<-EOF + { + "return": [ + { + "name": "lo", + "ip-addresses": [ + { + "ip-address-type": "ipv4", + "ip-address": "127.0.0.1", + "prefix": 8 + } + ], + "hardware-address": "00:00:00:00:00:00" + }, + { + "name": "eth0", + "ip-addresses": [ + { + "ip-address-type": "ipv4", + "ip-address": "192.168.122.42", + "prefix": 24 + } + ], + "hardware-address": "52:54:00:f8:67:98" + } + ] + } + EOF + } + + before do + allow(machine.provider_config).to receive(:qemu_use_agent).and_return(true) + end + + it 'should retrieve the address via the agent' do + expect(subject).to receive(:connection).and_return(connection) + expect(libvirt_client).to receive(:lookup_domain_by_uuid).and_return(libvirt_domain) + expect(libvirt_domain).to receive(:qemu_agent_command).and_return(qemu_agent_interfaces) + expect(domain).to receive(:mac).and_return("52:54:00:f8:67:98").exactly(2).times + + expect(subject.get_ipaddress(machine)).to eq("192.168.122.42") + end + + context 'when qemu_use_session is enabled' do + before do + allow(machine.provider_config).to receive(:qemu_use_session).and_return(true) + end + + it 'should still retrieve the address via the agent' do + expect(subject).to receive(:connection).and_return(connection) + expect(libvirt_client).to receive(:lookup_domain_by_uuid).and_return(libvirt_domain) + expect(libvirt_domain).to receive(:qemu_agent_command).and_return(qemu_agent_interfaces) + expect(domain).to receive(:mac).and_return("52:54:00:f8:67:98").exactly(2).times + + expect(subject.get_ipaddress(machine)).to eq("192.168.122.42") + end + end + end + + context 'when qemu_use_session is enabled' do + let(:networks) { [instance_double('::Fog::Libvirt::Compute::Real')] } + let(:dhcp_leases) { + { + "iface" =>"virbr0", + "expirytime" =>1636287162, + "type" =>0, + "mac" =>"52:54:00:8b:dc:5f", + "ipaddr" =>"192.168.122.43", + "prefix" =>24, + "hostname" =>"vagrant-default_test", + "clientid" =>"ff:00:8b:dc:5f:00:01:00:01:29:1a:65:42:52:54:00:8b:dc:5f", + } + } + + before do + allow(machine.provider_config).to receive(:qemu_use_session).and_return(true) + end + + it 'should retreive the address via the system dhcp-leases API' do + expect(domain).to receive(:mac).and_return("52:54:00:8b:dc:5f") + expect(subject).to receive(:system_connection).and_return(system_connection1) + expect(system_connection1).to receive(:list_all_networks).and_return(networks) + expect(networks[0]).to receive(:dhcp_leases).and_return([dhcp_leases]) + + expect(subject.get_ipaddress(machine)).to eq("192.168.122.43") + end + + context 'when qemu_use_agent is enabled' do + before do + allow(machine.provider_config).to receive(:qemu_use_agent).and_return(true) + end + + it 'should retrieve the address via the agent' do + expect(subject).to receive(:get_ipaddress_from_qemu_agent).and_return("192.168.122.44") + + expect(subject.get_ipaddress(machine)).to eq("192.168.122.44") + end + end + end + end + end + describe '#state' do let(:domain) { double('domain') }