Merge pull request #345 from ggiamarchi/fix-parallel-race-condition

Fix deadlock for parallel `vagrant up`
ggiamarchi · Oct 25, 2017 · 02e13ac · 02e13ac
2 parents 56181bc + 6f645bc
commit 02e13ac
Show file tree

Hide file tree

Showing 4 changed files with 93 additions and 16 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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