From 1fe5a80516fb940b4232ce4ab8baaf3b98037dc5 Mon Sep 17 00:00:00 2001 From: Darragh Bailey Date: Thu, 20 May 2021 13:36:23 +0100 Subject: [PATCH] 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 | 3 +++ lib/vagrant-libvirt/action/is_running.rb | 4 +--- lib/vagrant-libvirt/action/is_suspended.rb | 8 +++---- lib/vagrant-libvirt/action/wait_till_up.rb | 26 +--------------------- spec/unit/action/wait_till_up_spec.rb | 24 +------------------- 5 files changed, 10 insertions(+), 55 deletions(-) diff --git a/lib/vagrant-libvirt/action.rb b/lib/vagrant-libvirt/action.rb index 9266f6b..0b36072 100644 --- a/lib/vagrant-libvirt/action.rb +++ b/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 diff --git a/lib/vagrant-libvirt/action/is_running.rb b/lib/vagrant-libvirt/action/is_running.rb index bcf23ab..324cf11 100644 --- a/lib/vagrant-libvirt/action/is_running.rb +++ b/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 diff --git a/lib/vagrant-libvirt/action/is_suspended.rb b/lib/vagrant-libvirt/action/is_suspended.rb index 9920468..27f1e37 100644 --- a/lib/vagrant-libvirt/action/is_suspended.rb +++ b/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 diff --git a/lib/vagrant-libvirt/action/wait_till_up.rb b/lib/vagrant-libvirt/action/wait_till_up.rb index 5181cd1..e731bc3 100644 --- a/lib/vagrant-libvirt/action/wait_till_up.rb +++ b/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] diff --git a/spec/unit/action/wait_till_up_spec.rb b/spec/unit/action/wait_till_up_spec.rb index 09a248d..2fa17e4 100644 --- a/spec/unit/action/wait_till_up_spec.rb +++ b/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