Clear and create forwarded ports on suspend/resume (#1679)

Ensure forwarded ports are cleared on suspend and recreated on resume.
Included tests to exercise the entire up/start behaviour to ensure the
expected actions are called in various scenarios, moving config validate
to only occur at the entrypoints and dropping it from being called
during start. Also eliminate a duplicate SetupComplete call.

Fixes: #1115
This commit is contained in:
Darragh Bailey
2022-11-23 13:44:32 +00:00
committed by GitHub
parent 451c6f6c53
commit f25a375d18
2 changed files with 402 additions and 18 deletions

View File

@@ -24,9 +24,13 @@ describe VagrantPlugins::ProviderLibvirt::Action do
allow(logger).to receive(:trace)
allow(logger).to receive(:debug)
allow(logger).to receive(:error)
allow(logger).to receive(:warn)
allow(connection.client).to receive(:libversion).and_return(6_002_000)
# ensure runner available
env[:action_runner] = runner
# patch out iterating synced_folders by emptying the list returned
# where vagrant us using a Collection, otherwise fallback to using
# the env value to disable the behaviour for older versions.
@@ -44,14 +48,289 @@ describe VagrantPlugins::ProviderLibvirt::Action do
results = responses.dup
allow_any_instance_of(action).to receive(:call) do |cls, env|
app = cls.instance_variable_get(:@app)
call_next(cls, env) do |_, env|
env[:result] = results[0]
if results.length > 1
results.shift
end
end
end
end
env[:result] = results[0]
if results.length > 1
results.shift
def receive_and_call_next(&block)
return receive(:call) { |cls, env| call_next(cls, env, &block) }
end
def call_next(cls, env)
app = cls.instance_variable_get(:@app)
yield(app, env) if block_given?
app.call(env)
end
describe '#action_up' do
before do
# typically set by the up command
env[:destroy_on_error] = true
end
context 'not created' do
before do
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsCreated, false)
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsSuspended, false)
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsRunning, false)
end
app.call(env)
it 'should create a new machine' do
expect_any_instance_of(Vagrant::Action::Builtin::BoxCheckOutdated).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CleanupOnFailure).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::Provision).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetNameOfDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::HandleStoragePool).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::HandleBox).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateDomainVolume).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworkInterfaces).to receive_and_call_next
# start action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::PrepareNFSValidIds).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SyncedFolderCleanup).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::SyncedFolders).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::PrepareNFSSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ShareFolders).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetBootOrder).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::StartDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::WaitTillUp).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::WaitForCommunicator).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ForwardPorts).to receive_and_call_next
# remaining up action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetHostname).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetupComplete).to receive_and_call_next
expect(runner.run(subject.action_up)).to match(hash_including({:machine => machine}))
end
context 'no box' do
before do
machine.config.vm.box = nil
end
it 'should create a new machine' do
expect_any_instance_of(Vagrant::Action::Builtin::BoxCheckOutdated).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CleanupOnFailure).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::Provision).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetNameOfDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworkInterfaces).to receive_and_call_next
# start action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetBootOrder).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::StartDomain).to receive_and_call_next
# remaining up action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetupComplete).to receive_and_call_next
expect(runner.run(subject.action_up)).to match(hash_including({:machine => machine}))
end
end
context 'on error' do
it 'should cleanup on error' do
expect_any_instance_of(Vagrant::Action::Builtin::BoxCheckOutdated).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::Provision).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetNameOfDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::HandleStoragePool).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::HandleBox).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateDomainVolume).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateDomain).to receive_and_call_next
# setup for error
expect(state).to receive(:id).and_return(:created)
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive(:call).and_raise(
::VagrantPlugins::ProviderLibvirt::Errors::CreateNetworkError.new(:error_message => 'errmsg')
)
expect(subject).to receive(:action_destroy).and_return(Vagrant::Action::Builder.new)
# remaining up
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetupComplete).to_not receive(:call)
expect { runner.run(subject.action_up) }.to raise_error(::VagrantPlugins::ProviderLibvirt::Errors::CreateNetworkError)
end
it 'should do nothing if already finished setup' do
expect(state).to receive(:id).and_return(:created)
expect_any_instance_of(Vagrant::Action::Builtin::BoxCheckOutdated).to receive_and_call_next
# don't intercept CleanupOnFailure or SetupComplete
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::Provision).to receive(:call) do |cls, env|
app = cls.instance_variable_get(:@app)
app.call(env)
raise Vagrant::Errors::VagrantError.new
end
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetNameOfDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::HandleStoragePool).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::HandleBox).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::HandleBoxImage).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateDomainVolume).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworkInterfaces).to receive_and_call_next
# start action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::PrepareNFSValidIds).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SyncedFolderCleanup).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::SyncedFolders).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::PrepareNFSSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ShareFolders).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetBootOrder).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::StartDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::WaitTillUp).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::WaitForCommunicator).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ForwardPorts).to receive_and_call_next
# remaining up action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetHostname).to receive_and_call_next
expect(subject).to_not receive(:action_destroy)
expect(subject).to_not receive(:action_halt)
expect { runner.run(subject.action_up) }.to raise_error(::Vagrant::Errors::VagrantError)
end
end
end
context 'halted' do
before do
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsCreated, true)
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsSuspended, false)
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsRunning, false)
end
it 'should start existing machine' do
expect_any_instance_of(Vagrant::Action::Builtin::BoxCheckOutdated).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CleanupOnFailure).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::Provision).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive_and_call_next
# start action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::PrepareNFSValidIds).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SyncedFolderCleanup).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::SyncedFolders).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::PrepareNFSSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ShareFolders).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetBootOrder).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::StartDomain).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::WaitTillUp).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::WaitForCommunicator).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ForwardPorts).to receive_and_call_next
# remaining up
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetupComplete).to receive_and_call_next
expect(runner.run(subject.action_up)).to match(hash_including({:machine => machine}))
end
context 'no box' do
before do
machine.config.vm.box = nil
end
it 'should start existing machine' do
expect_any_instance_of(Vagrant::Action::Builtin::BoxCheckOutdated).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CleanupOnFailure).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::Provision).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive_and_call_next
# start action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetBootOrder).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::StartDomain).to receive_and_call_next
# remaining up
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetupComplete).to receive_and_call_next
expect(runner.run(subject.action_up)).to match(hash_including({:machine => machine}))
end
end
context 'on error' do
it 'should call halt on error' do
expect_any_instance_of(Vagrant::Action::Builtin::BoxCheckOutdated).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CleanupOnFailure).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::Provision).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings).to receive_and_call_next
# setup for error
expect(state).to receive(:id).and_return(:created)
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive(:call).and_raise(
::VagrantPlugins::ProviderLibvirt::Errors::CreateNetworkError.new(:error_message => 'errmsg')
)
expect(subject).to receive(:action_halt).and_return(Vagrant::Action::Builder.new)
# remaining up
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetupComplete).to_not receive(:call)
expect { runner.run(subject.action_up) }.to raise_error(::VagrantPlugins::ProviderLibvirt::Errors::CreateNetworkError)
end
end
end
context 'suspended' do
before do
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsCreated, true)
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsSuspended, true)
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsRunning, false)
end
it 'should resume existing machine' do
expect_any_instance_of(Vagrant::Action::Builtin::BoxCheckOutdated).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CleanupOnFailure).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::Provision).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive_and_call_next
# start action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResumeDomain).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::WaitForCommunicator).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ForwardPorts).to receive_and_call_next
# remaining up
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetupComplete).to receive_and_call_next
expect(runner.run(subject.action_up)).to match(hash_including({:machine => machine}))
end
context 'no box' do
before do
machine.config.vm.box = nil
end
it 'should resume existing machine' do
expect_any_instance_of(Vagrant::Action::Builtin::BoxCheckOutdated).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CleanupOnFailure).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::Provision).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResolveDiskSettings).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive_and_call_next
# start action
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResumeDomain).to receive_and_call_next
# remaining up
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SetupComplete).to receive_and_call_next
expect(runner.run(subject.action_up)).to match(hash_including({:machine => machine}))
end
end
end
end
@@ -88,6 +367,11 @@ describe VagrantPlugins::ProviderLibvirt::Action do
expect(runner.run(subject.action_halt)).to match(hash_including({:machine => machine}))
end
it 'should clear forwarded ports' do
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ClearForwardedPorts).to receive(:call)
expect(runner.run(subject.action_halt)).to match(hash_including({:machine => machine}))
end
end
context 'when shutdown domain fails' do
@@ -105,6 +389,103 @@ describe VagrantPlugins::ProviderLibvirt::Action do
end
end
describe '#action_suspend' do
context 'not created' do
before do
expect(state).to receive(:id).and_return(:not_created)
end
it 'should execute without error' do
expect(ui).to receive(:info).with('Domain is not created. Please run `vagrant up` first.')
expect(runner.run(subject.action_suspend)).to match(hash_including({:machine => machine}))
end
end
context 'when created' do
before do
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsCreated, true)
end
context 'when running' do
before do
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsRunning, true)
end
it 'should clear ports and suspend the domain' do
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ClearForwardedPorts).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SuspendDomain).to receive_and_call_next
expect(runner.run(subject.action_suspend)).to match(hash_including({:machine => machine}))
end
end
context 'when not running' do
before do
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsRunning, false)
end
it 'should report not running' do
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ClearForwardedPorts).to_not receive(:call)
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::SuspendDomain).to_not receive(:call)
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::MessageNotRunning).to receive_and_call_next
expect(runner.run(subject.action_suspend)).to match(hash_including({:machine => machine}))
end
end
end
end
describe '#action_resume' do
context 'not created' do
before do
expect(state).to receive(:id).and_return(:not_created)
end
it 'should execute without error' do
expect(ui).to receive(:info).with('Domain is not created. Please run `vagrant up` first.')
expect(runner.run(subject.action_resume)).to match(hash_including({:machine => machine}))
end
end
context 'when created' do
before do
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsCreated, true)
end
context 'when suspended' do
before do
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsSuspended, true)
end
it 'should setup networking resume domain and forward ports' do
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResumeDomain).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::Provision).to receive_and_call_next
expect_any_instance_of(Vagrant::Action::Builtin::WaitForCommunicator).to receive_and_call_next
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ForwardPorts).to receive_and_call_next
expect(runner.run(subject.action_resume)).to match(hash_including({:machine => machine}))
end
end
context 'when not suspended' do
before do
allow_action_env_result(VagrantPlugins::ProviderLibvirt::Action::IsSuspended, false)
end
it 'should report not running' do
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::CreateNetworks).to_not receive(:call)
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::ResumeDomain).to_not receive(:call)
expect_any_instance_of(VagrantPlugins::ProviderLibvirt::Action::MessageNotSuspended).to receive_and_call_next
expect(runner.run(subject.action_resume)).to match(hash_including({:machine => machine}))
end
end
end
end
describe '#action_ssh' do
context 'when not created' do
before do