Tidy up driver code (#1505)

Clean up the driver code to allow it to be close to passing with a
rubocop scan.

lib/vagrant-libvirt/driver.rb

@@ -32,16 +32,17 @@ module VagrantPlugins
         config = @machine.provider_config
         uri = config.uri
 
-        conn_attr = {}
-        conn_attr[:provider] = 'libvirt'
-        conn_attr[:libvirt_uri] = uri
-        conn_attr[:libvirt_username] = config.username if config.username
-        conn_attr[:libvirt_password] = config.password if config.password
-
         # Setup command for retrieving IP address for newly created machine
         # with some MAC address. Get it from dnsmasq leases table
         ip_command = %q( awk "/$mac/ {print \$1}" /proc/net/arp )
-        conn_attr[:libvirt_ip_command] = ip_command
+
+        conn_attr = {
+          provider: 'libvirt',
+          libvirt_uri: uri,
+          libvirt_ip_command: ip_command,
+        }
+        conn_attr[:libvirt_username] = config.username if config.username
+        conn_attr[:libvirt_password] = config.password if config.password
 
         @logger.info("Connecting to Libvirt (#{uri}) ...")
         begin
@@ -61,7 +62,7 @@ module VagrantPlugins
 
         config = @machine.provider_config
 
-        @system_connection = Libvirt::open_read_only(config.system_uri)
+        @system_connection = Libvirt.open_read_only(config.system_uri)
         @system_connection
       end
 
@@ -69,12 +70,10 @@ module VagrantPlugins
         begin
           domain = connection.servers.get(machine.id)
         rescue Libvirt::RetrieveError => e
-          if e.libvirt_code == ProviderLibvirt::Util::ErrorCodes::VIR_ERR_NO_DOMAIN
+          raise e unless e.libvirt_code == ProviderLibvirt::Util::ErrorCodes::VIR_ERR_NO_DOMAIN
-            @logger.debug("machine #{machine.name} domain not found #{e}.")
+
-            return nil
+          @logger.debug("machine #{machine.name} domain not found #{e}.")
-          else
+          return nil
-            raise e
-          end
         end
 
         domain
@@ -99,26 +98,24 @@ module VagrantPlugins
 
       def get_domain_ipaddress(machine, domain)
         # attempt to get ip address from qemu agent
-        if @machine.provider_config.qemu_use_agent == true
+        if machine.provider_config.qemu_use_agent == true
           @logger.info('Get IP via qemu agent')
-          return get_ipaddress_from_qemu_agent(domain, machine.id)
+          return get_ipaddress_from_qemu_agent(domain, machine.id, machine.config.vm.boot_timeout)
         end
 
-        if @machine.provider_config.qemu_use_session
+        return get_ipaddress_from_system domain.mac 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)
         rescue Fog::Errors::TimeoutError
-          @logger.info('Timeout at waiting for an ip address for machine %s' % machine.name)
+          @logger.info("Timeout at waiting for an ip address for machine #{machine.name}")
 
           raise
         end
 
         unless ip_address
-          @logger.info('No arp table entry found for machine %s' % machine.name)
+          @logger.info("No arp table entry found for machine #{machine.name}")
           return nil
         end
 
@@ -201,7 +198,7 @@ module VagrantPlugins
           end
         end
 
-        return state
+        state
       end
 
       private
@@ -212,43 +209,44 @@ module VagrantPlugins
         system_connection.list_all_networks.each do |net|
           leases = net.dhcp_leases(mac, 0)
           # Assume the lease expiring last is the current IP address
-          ip_address = leases.sort_by { |lse| lse["expirytime"] }.last["ipaddr"] if !leases.empty?
+          ip_address = leases.max_by { |lse| lse['expirytime'] }['ipaddr'] unless leases.empty?
           break if ip_address
         end
 
         ip_address
       end
 
-      def get_ipaddress_from_qemu_agent(domain, machine_id)
+      def get_ipaddress_from_qemu_agent(domain, machine_id, timeout)
         ip_address = nil
         addresses = nil
         libvirt_domain = connection.client.lookup_domain_by_uuid(machine_id)
         begin
-          response = libvirt_domain.qemu_agent_command('{"execute":"guest-network-get-interfaces"}', timeout=10)
+          response = libvirt_domain.qemu_agent_command('{"execute":"guest-network-get-interfaces"}', timeout)
-          @logger.debug("Got Response from qemu agent")
+          @logger.debug('Got Response from qemu agent')
           @logger.debug(response)
           addresses = JSON.parse(response)
-        rescue => e
+        rescue StandardError => e
-          @logger.debug("Unable to receive IP via qemu agent: [%s]" % e.message)
+          puts "Unable to receive IP via qemu agent: [#{e.message}]"
+          @logger.debug("Unable to receive IP via qemu agent: [#{e.message}]")
         end
 
         unless addresses.nil?
-          addresses["return"].each{ |interface|
+          addresses['return'].each do |interface|
-            if domain.mac.downcase == interface["hardware-address"].downcase
+            next unless domain.mac.downcase == interface['hardware-address'].downcase
-              @logger.debug("Found mathing interface: [%s]" % interface["name"])
+
-              if interface.has_key?("ip-addresses")
+            @logger.debug("Found matching interface: [#{interface['name']}]")
-                interface["ip-addresses"].each{ |ip|
+            next unless interface.key?('ip-addresses')
-                  # returning ipv6 addresses might break windows guests because
+
-                  # winrm cant handle connection, winrm fails with "invalid uri"
+            interface['ip-addresses'].each do |ip|
-                  if ip["ip-address-type"] == "ipv4"
+              # returning ipv6 addresses might break windows guests because
-                    ip_address = ip["ip-address"]
+              # winrm cant handle connection, winrm fails with "invalid uri"
-                    @logger.debug("Return IP: [%s]" % ip_address)
+              next unless ip['ip-address-type'] == 'ipv4'
-                    break
+
-                  end
+              ip_address = ip['ip-address']
-                  }
+              @logger.debug("Return IP: [#{ip_address}]")
-              end
+              break
             end
-          }
+          end
         end
         ip_address
       end
@@ -256,13 +254,13 @@ module VagrantPlugins
       def get_ipaddress_from_domain(domain)
         ip_address = nil
         domain.wait_for(2) do
-          addresses.each_pair do |type, ip|
+          addresses.each_pair do |_type, ip|
             # Multiple leases are separated with a newline, return only
             # the most recent address
-            ip_address = ip[0].split("\n").first if ip[0] != nil
+            ip_address = ip[0].split("\n").first unless ip[0].nil?
           end
 
-          ip_address != nil
+          !ip_address.nil?
         end
 
         ip_address
@@ -272,17 +270,14 @@ module VagrantPlugins
         begin
           libvirt_domain = connection.client.lookup_domain_by_uuid(machine.id)
         rescue Libvirt::RetrieveError => e
-          if e.libvirt_code == ProviderLibvirt::Util::ErrorCodes::VIR_ERR_NO_DOMAIN
+          raise e unless e.libvirt_code == ProviderLibvirt::Util::ErrorCodes::VIR_ERR_NO_DOMAIN
-            @logger.debug("machine #{machine.name} not found #{e}.")
+
-            return nil
+          @logger.debug("machine #{machine.name} not found #{e}.")
-          else
+          return nil
-            raise e
-          end
         end
 
         libvirt_domain
       end
-
     end
   end
 end

spec/acceptance/use_qemu_agent_for_connectivity_spec.rb

@@ -15,7 +15,8 @@ describe 'use qemu agent to determine machine private address', acceptance: true
 
   it 'should succeed' do
     status('Test: machine is created successfully')
-    expect(environment.execute('vagrant', 'up')).to exit_with(0)
+    result = environment.execute('vagrant', 'up')
+    expect(result).to exit_with(0)
 
     # extract SSH IP address emitted as it should be the private network since
     # the mgmt network has not been attached