Ensure state is correct reflected in global-status (#1292)

Vagrant relies on the side effect of checking the machine state to
trigger updating of the state in the global machine index. As a
consequence any action should not inspect the domain state directly and
instead should access the machine state.

Additionally as part of the up/start actions should switch to built-in
WaitForCommunicator which will inspect the machine states by default to
align with the expected state updating side effects that would be in
effect for any internal provider shipped with vagrant.

Closes: #948
Partial-Fix: #193

lib/vagrant-libvirt/action.rb

@@ -49,6 +49,7 @@ module VagrantPlugins
 
                 b2.use StartDomain
                 b2.use WaitTillUp
+                b2.use WaitForCommunicator, [:running]
 
                 b2.use ForwardPorts
                 b2.use SetHostname
@@ -107,6 +108,7 @@ module VagrantPlugins
                 # Machine should gain IP address when comming up,
                 # so wait for dhcp lease and store IP into machines data_dir.
                 b3.use WaitTillUp
+                b3.use WaitForCommunicator, [:running]
 
                 b3.use ForwardPorts
                 b3.use PrepareNFSSettings
@@ -369,6 +371,7 @@ module VagrantPlugins
       autoload :SyncedFolders, 'vagrant/action/builtin/synced_folders'
       autoload :SyncedFolderCleanup, 'vagrant/action/builtin/synced_folder_cleanup'
       autoload :ProvisionerCleanup, 'vagrant/action/builtin/provisioner_cleanup'
+      autoload :WaitForCommunicator, 'vagrant/action/builtin/wait_for_communicator'
     end
   end
 end

lib/vagrant-libvirt/action/is_running.rb

@@ -9,9 +9,7 @@ module VagrantPlugins
         end
 
         def call(env)
-          domain = env[:machine].provider.driver.connection.servers.get(env[:machine].id.to_s)
-          raise Errors::NoDomainError if domain.nil?
-          env[:result] = domain.state.to_s == 'running'
+          env[:result] = env[:machine].state.id == :running
 
           @app.call(env)
         end

lib/vagrant-libvirt/action/is_suspended.rb

@@ -16,9 +16,9 @@ module VagrantPlugins
           libvirt_domain = env[:machine].provider.driver.connection.client.lookup_domain_by_uuid(env[:machine].id)
           if config.suspend_mode == 'managedsave'
             if libvirt_domain.has_managed_save?
-              env[:result] = libvirt_domain.has_managed_save?
+              env[:result] = env[:machine].state.id == :shutoff
             else
-              env[:result] = domain.state.to_s == 'paused'
+              env[:result] = env[:machine].state.id == :paused
               if env[:result]
                 env[:ui].warn('One time switching to pause suspend mode, found a paused VM.')
                 config.suspend_mode = 'pause'
@@ -27,10 +27,10 @@ module VagrantPlugins
           else
             if libvirt_domain.has_managed_save?
               env[:ui].warn('One time switching to managedsave suspend mode, state found.')
-              env[:result] = true
+              env[:result] = [:shutoff, :paused].include?(env[:machine].state.id)
               config.suspend_mode = 'managedsave'
             else
-              env[:result] = domain.state.to_s == 'paused'
+              env[:result] = env[:machine].state.id == :paused
             end
           end
 

lib/vagrant-libvirt/action/wait_till_up.rb

@@ -47,30 +47,6 @@ module VagrantPlugins
           @logger.info("Got IP address #{env[:ip_address]}")
           @logger.info("Time for getting IP: #{env[:metrics]['instance_ip_time']}")
 
-          # Machine has ip address assigned, now wait till we are able to
-          # connect via ssh.
-          env[:metrics]['instance_ssh_time'] = Util::Timer.time do
-            env[:ui].info(I18n.t('vagrant_libvirt.waiting_for_ssh'))
-            retryable(on: Fog::Errors::TimeoutError, tries: 60) do
-              # If we're interrupted don't worry about waiting
-              next if env[:interrupted]
-
-              # Wait till we are able to connect via ssh.
-              loop do
-                # If we're interrupted then just back out
-                break if env[:interrupted]
-                break if env[:machine].communicate.ready?
-                sleep 2
-              end
-            end
-          end
-          # just return if interrupted and let the warden call recover
-          return if env[:interrupted]
-          @logger.info("Time for SSH ready: #{env[:metrics]['instance_ssh_time']}")
-
-          # Booted and ready for use.
-          # env[:ui].info(I18n.t("vagrant_libvirt.ready"))
-
           @app.call(env)
         end
 
@@ -80,7 +56,7 @@ module VagrantPlugins
         end
 
         def terminate(env)
-          if env[:machine].provider.state.id != :not_created
+          if env[:machine].state.id != :not_created
             # If we're not supposed to destroy on error then just return
             return unless env[:destroy_on_error]
 

spec/unit/action/wait_till_up_spec.rb

@@ -64,28 +64,9 @@ describe VagrantPlugins::ProviderLibvirt::Action::WaitTillUp do
           expect {subject.call(env) }.to raise_error(::Fog::Errors::TimeoutError)
         end
       end
-
-      context 'if interrupted waiting for SSH' do
-        before do
-          allow(domain).to receive(:wait_for).and_return(true)
-          allow(env).to receive(:[]).and_call_original
-          allow(env).to receive(:[]).with(:interrupted).and_return(false, true, true)
-          allow(driver).to receive(:get_domain_ipaddress).and_return('192.168.121.2')
-        end
-        it 'should exit after getting IP' do
-          expect(app).to_not receive(:call)
-          expect(ui).to receive(:info).with('Waiting for domain to get an IP address...')
-          expect(ui).to receive(:info).with('Waiting for SSH to become available...')
-          expect(logger).to receive(:debug).with(/Searching for IP for MAC address: .*/)
-          expect(logger).to receive(:info).with('Got IP address 192.168.121.2')
-          expect(logger).to receive(:info).with(/Time for getting IP: .*/)
-          expect(env[:machine].communicate).to_not receive(:ready?)
-          expect(subject.call(env)).to be_nil
-        end
-      end
     end
 
-    context 'when machine boots and ssh available' do
+    context 'when machine boots and ip available' do
       before do
         allow(domain).to receive(:wait_for).and_return(true)
         allow(env).to receive(:[]).and_call_original
@@ -95,12 +76,9 @@ describe VagrantPlugins::ProviderLibvirt::Action::WaitTillUp do
       it 'should call the next hook' do
         expect(app).to receive(:call)
         expect(ui).to receive(:info).with('Waiting for domain to get an IP address...')
-        expect(ui).to receive(:info).with('Waiting for SSH to become available...')
         expect(logger).to receive(:debug).with(/Searching for IP for MAC address: .*/)
         expect(logger).to receive(:info).with('Got IP address 192.168.121.2')
         expect(logger).to receive(:info).with(/Time for getting IP: .*/)
-        expect(logger).to receive(:info).with(/Time for SSH ready: .*/)
-        expect(env[:machine].communicate).to receive(:ready?).and_return(true)
         expect(subject.call(env)).to be_nil
       end
     end