From 6f645bccf4747071b5a206bbeb143bbacd59d99c Mon Sep 17 00:00:00 2001 From: Guillaume Giamarchi Date: Wed, 25 Oct 2017 10:04:13 +0200 Subject: [PATCH] Fix deadlock for parallel `vagrant up` When some floating IPs are allocated but unused and when floating_ip_pool_always_allocate = false this can lead into a situation where several servers are trying to use the same IP address and it leads to a deadlock because they all wait to have the IP assigned and of course only one of them can have it. The mutex has to be at a higher level than it was to ensure floating IP assignment to be well done in an atomic way. Of course, doing that, floating IP assignment does not occur in parallel anymore but it looks to be the only way to solve the problem. --- samples/03_multimachine_loop/Vagrantfile | 2 +- samples/tests.bats | 81 ++++++++++++++++++- .../action/create_server.rb | 7 +- .../config_resolver.rb | 19 ++--- 4 files changed, 93 insertions(+), 16 deletions(-) diff --git a/samples/03_multimachine_loop/Vagrantfile b/samples/03_multimachine_loop/Vagrantfile index 3b725b3..d472468 100644 --- a/samples/03_multimachine_loop/Vagrantfile +++ b/samples/03_multimachine_loop/Vagrantfile @@ -17,7 +17,7 @@ Vagrant.configure('2') do |config| os.networks << ENV['OS_NETWORK'] end - (1..3).each do |i| + (1..6).each do |i| config.vm.define "server-#{i}" do |s| s.vm.provider :openstack do |os, override| os.server_name = "03_multimachine_loop-#{i}" diff --git a/samples/tests.bats b/samples/tests.bats index 30abeb9..382911a 100755 --- a/samples/tests.bats +++ b/samples/tests.bats @@ -35,11 +35,11 @@ teardown() { flush_out [ "$status" -eq 0 ] [ $(openstack floating ip list -f value | wc -l) -eq 1 ] # Check one IP is allocated - + run bundle exec vagrant ssh -c "true" flush_out [ "$status" -eq 0 ] - + run bundle exec vagrant destroy flush_out [ "$status" -eq 0 ] @@ -213,6 +213,83 @@ teardown() { [ "$status" -eq 0 ] } +@test "03 - Multimachine loop / in parallel" { + title "$BATS_TEST_DESCRIPTION" + + export VAGRANT_CWD=$BATS_TEST_DIRNAME/03_multimachine_loop + + run bundle exec vagrant up --parallel + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-1 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-2 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-3 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-4 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-5 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-6 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant destroy + flush_out + [ "$status" -eq 0 ] +} + +@test "03 - Multimachine loop / in parallel / with pre allocated floating IP" { + title "$BATS_TEST_DESCRIPTION" + + allocate_4_floating_ip + export VAGRANT_CWD=$BATS_TEST_DIRNAME/03_multimachine_loop + + run bundle exec vagrant up --parallel + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-1 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-2 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-3 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-4 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-5 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant ssh -c "true" server-6 + flush_out + [ "$status" -eq 0 ] + + run bundle exec vagrant destroy + flush_out + [ "$status" -eq 0 ] +} + @test "04 - Heat Stack" { title "$BATS_TEST_DESCRIPTION" diff --git a/source/lib/vagrant-openstack-provider/action/create_server.rb b/source/lib/vagrant-openstack-provider/action/create_server.rb index 1ca11b9..e186812 100644 --- a/source/lib/vagrant-openstack-provider/action/create_server.rb +++ b/source/lib/vagrant-openstack-provider/action/create_server.rb @@ -19,6 +19,7 @@ def initialize(app, _env, resolver = ConfigResolver.new, utils = Utils.new) @logger = Log4r::Logger.new('vagrant_openstack::action::create_server') @resolver = resolver @utils = utils + @@mutex = Mutex.new end def execute(env) @@ -49,8 +50,10 @@ def execute(env) env[:machine].id = server_id waiting_for_server_to_be_built(env, server_id) - assign_floating_ip(env, server_id) - waiting_for_floating_ip_to_be_assigned(env, server_id) + @@mutex.synchronize do + assign_floating_ip(env, server_id) + waiting_for_floating_ip_to_be_assigned(env, server_id) + end attach_volumes(env, server_id, options[:volumes]) unless options[:volumes].empty? @app.call(env) diff --git a/source/lib/vagrant-openstack-provider/config_resolver.rb b/source/lib/vagrant-openstack-provider/config_resolver.rb index e3b2da4..2b6ac7b 100644 --- a/source/lib/vagrant-openstack-provider/config_resolver.rb +++ b/source/lib/vagrant-openstack-provider/config_resolver.rb @@ -3,7 +3,6 @@ module Openstack class ConfigResolver def initialize @logger = Log4r::Logger.new('vagrant_openstack::action::config_resolver') - @mutex = Mutex.new end def resolve_ssh_port(env) @@ -160,16 +159,14 @@ def resolve_image_internal(env, image_name) end def search_free_ip(config, nova, env) - @mutex.synchronize do - @logger.debug 'Retrieving all allocated floating ips on tenant' - all_floating_ips = nova.get_all_floating_ips(env) - all_floating_ips.each do |floating_ip| - log_attach = floating_ip.instance_id ? "attached to #{floating_ip.instance_id}" : 'not attached' - @logger.debug "#{floating_ip.ip} #{log_attach}" if config.floating_ip_pool.include? floating_ip.pool - return floating_ip.ip if (config.floating_ip_pool.include? floating_ip.pool) && floating_ip.instance_id.nil? - end unless config.floating_ip_pool_always_allocate - @logger.debug 'No free ip found' - end + @logger.debug 'Retrieving all allocated floating ips on tenant' + all_floating_ips = nova.get_all_floating_ips(env) + all_floating_ips.each do |floating_ip| + log_attach = floating_ip.instance_id ? "attached to #{floating_ip.instance_id}" : 'not attached' + @logger.debug "#{floating_ip.ip} #{log_attach}" if config.floating_ip_pool.include? floating_ip.pool + return floating_ip.ip if (config.floating_ip_pool.include? floating_ip.pool) && floating_ip.instance_id.nil? + end unless config.floating_ip_pool_always_allocate + @logger.debug 'No free ip found' nil end