Consolidate ip address retreival to single method (#1199)

Refactor WaitTillUp action to make use of the domain IP address
retrieval code in the driver to ensure a single place to maintain.

Remove references to machine option for driver where already should be
available as an instance variable.

lib/vagrant-libvirt/action/wait_till_up.rb

@@ -34,33 +34,13 @@ module VagrantPlugins
           @logger.debug("Searching for IP for MAC address: #{domain.mac}")
           env[:ui].info(I18n.t('vagrant_libvirt.waiting_for_ip'))
 
-          if env[:machine].provider_config.qemu_use_session
-            env[:metrics]['instance_ip_time'] = Util::Timer.time do
-              retryable(on: Fog::Errors::TimeoutError, tries: 300) do
-                # just return if interrupted and let the warden call recover
-                return if env[:interrupted]
+          env[:metrics]['instance_ip_time'] = Util::Timer.time do
+            retryable(on: Fog::Errors::TimeoutError, tries: 300) do
+              # just return if interrupted and let the warden call recover
+              return if env[:interrupted]
 
-                # Wait for domain to obtain an ip address
-                domain.wait_for(2) do
-                  env[:ip_address] = env[:machine].provider.driver.get_ipaddress_system(domain.mac)
-                  !env[:ip_address].nil?
-                end
-              end
-            end
-          else
-            env[:metrics]['instance_ip_time'] = Util::Timer.time do
-              retryable(on: Fog::Errors::TimeoutError, tries: 300) do
-                # just return if interrupted and let the warden call recover
-                return if env[:interrupted]
-
-                # Wait for domain to obtain an ip address
-                domain.wait_for(2) do
-                  addresses.each_pair do |_type, ip|
-                    env[:ip_address] = ip[0] unless ip[0].nil?
-                  end
-                  !env[:ip_address].nil?
-                end
-              end
+              # Wait for domain to obtain an ip address
+              env[:ip_address] = env[:machine].provider.driver.get_domain_ipaddress(domain)
             end
           end
 

lib/vagrant-libvirt/driver.rb

@@ -60,12 +60,12 @@ module VagrantPlugins
         @@system_connection
       end
 
-      def get_domain(mid)
+      def get_domain
         begin
-          domain = connection.servers.get(mid)
+          domain = connection.servers.get(@machine.id)
         rescue Libvirt::RetrieveError => e
           if e.libvirt_code == ProviderLibvirt::Util::ErrorCodes::VIR_ERR_NO_DOMAIN
-            @logger.debug("machine #{mid} not found #{e}.")
+            @logger.debug("machine #{@machine.name} domain not found #{e}.")
             return nil
           else
             raise e
@@ -80,54 +80,38 @@ module VagrantPlugins
         !domain.nil?
       end
 
-      def get_ipaddress(machine)
+      def get_ipaddress
         # Find the machine
-        domain = get_domain(machine.id)
-        if @machine.provider_config.qemu_use_session
-          return get_ipaddress_system domain.mac
-        end
+        domain = get_domain(@machine.id)
 
         if domain.nil?
           # The machine can't be found
           return nil
         end
 
+        get_domain_ipaddress(domain)
+      end
+
+      def get_domain_ipaddress(domain)
+        if @machine.provider_config.qemu_use_session
+          return get_ipaddress_from_system domain.mac
+        end
+
         # Get IP address from arp table
-        ip_address = nil
         begin
-          domain.wait_for(2) do
-            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 unless ip[0].nil?
-            end
-            !ip_address.nil?
-          end
+          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 %s' % @machine.name)
         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 %s' % @machine.name)
           return nil
         end
 
         ip_address
       end
 
-      def get_ipaddress_system(mac)
-        ip_address = nil
-
-        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?
-          break if ip_address
-        end
-
-        return ip_address
-      end
-
       def state(machine)
         # may be other error states with initial retreival we can't handle
         begin
@@ -142,6 +126,37 @@ module VagrantPlugins
 
         domain.state.tr('-', '_').to_sym
       end
+
+      private
+
+      def get_ipaddress_from_system(mac)
+        ip_address = nil
+
+        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?
+          break if ip_address
+        end
+
+        ip_address
+      end
+
+      def get_ipaddress_from_domain(domain)
+        ip_address = nil
+        domain.wait_for(2) do
+          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
+          end
+
+          ip_address != nil
+        end
+
+        ip_address
+      end
+
     end
   end
 end

lib/vagrant-libvirt/provider.rb

@@ -55,7 +55,7 @@ module VagrantPlugins
         # be called from other threads of execution.
         return nil if state.id != :running
 
-        ip = driver.get_ipaddress(@machine)
+        ip = driver.get_ipaddress
 
         # if can't determine the IP, just return nil and let the core
         # deal with it, similar to the docker provider