Remove redis locks

lib/vmpooler/pool_manager.rb

@@ -558,20 +558,28 @@ module Vmpooler
       end
     end
 
+    def pool_mutex(poolname)
+      mutex = @reconfigure_pool[poolname] || @reconfigure_pool[poolname] = Mutex.new
+    end
+
+    def set_pool_template(pool)
+      pool_template = $redis.hget('vmpooler__config__template', pool['name'])
+      if pool_template
+        unless pool['template'] == pool_template
+          pool['template'] = pool_template
+        end
+      end
+    end
+
     def prepare_template(pool, provider)
       if $config[:config]['create_template_delta_disks']
         # Ensure templates are evaluated for delta disk creation on startup
-        return if $redis.hget('vmpooler__config__updating', pool['name'])
         unless $redis.hget('vmpooler__template__prepared', pool['name'])
-          mutex = @reconfigure_pool[pool['name']] || @reconfigure_pool[pool['name']] = Mutex.new
+          mutex = pool_mutex(pool['name'])
+          return if mutex.locked?
           mutex.synchronize do
-            begin
-              $redis.hset('vmpooler__config__updating', pool['name'], 1)
             provider.create_template_delta_disks(pool)
             $redis.hset('vmpooler__template__prepared', pool['name'], pool['template'])
-            ensure
-              $redis.hdel('vmpooler__config__updating', pool['name'])
-            end
           end
         end
       end
@@ -581,12 +589,10 @@ module Vmpooler
       $redis.hset('vmpooler__template', pool['name'], pool['template']) unless $redis.hget('vmpooler__template', pool['name'])
       return unless $redis.hget('vmpooler__config__template', pool['name'])
       unless $redis.hget('vmpooler__config__template', pool['name']) == $redis.hget('vmpooler__template', pool['name'])
-        return if $redis.hget('vmpooler__config__updating', pool['name'])
         # Ensure we are only updating a template once
-        mutex = @reconfigure_pool[pool['name']] || @reconfigure_pool[pool['name']] = Mutex.new
+        mutex = pool_mutex(pool['name'])
+        return if mutex.locked?
         mutex.synchronize do
-          begin
-            $redis.hset('vmpooler__config__updating', pool['name'], 1)
           old_template_name = $redis.hget('vmpooler__template', pool['name'])
           new_template_name = $redis.hget('vmpooler__config__template', pool['name'])
           pool['template'] = new_template_name
@@ -609,9 +615,6 @@ module Vmpooler
           $logger.log('s', "[*] [#{pool['name']}] creating template deltas")
           provider.create_template_delta_disks(pool)
           $logger.log('s', "[*] [#{pool['name']}] template deltas have been created")
-          ensure
-            $redis.hdel('vmpooler__config__updating', pool['name'])
-          end
         end
       end
     end
@@ -619,7 +622,7 @@ module Vmpooler
     def remove_excess_vms(pool, provider, ready, total)
       if total
         unless total == 0
-          mutex = @reconfigure_pool[pool['name']] || @reconfigure_pool[pool['name']] = Mutex.new
+          mutex = pool_mutex(pool['name'])
           mutex.synchronize do
             if total > pool['size']
               difference = ready - pool['size']
@@ -776,7 +779,8 @@ module Vmpooler
 
       # REPOPULATE
       # Do not attempt to repopulate a pool while a template is updating
-      unless $redis.hget('vmpooler__config__updating', pool['name'])
+      mutex = pool_mutex(pool['name'])
+      unless mutex.locked?
         ready = $redis.scard("vmpooler__ready__#{pool['name']}")
         total = $redis.scard("vmpooler__pending__#{pool['name']}") + ready
 
@@ -845,8 +849,6 @@ module Vmpooler
       $redis.set('vmpooler__tasks__clone', 0)
       # Clear out vmpooler__migrations since stale entries may be left after a restart
       $redis.del('vmpooler__migration')
-      # Clear out any configuration changes in flight that were interrupted
-      $redis.del('vmpooler__config__updating')
       # Ensure template deltas are created on each startup
       $redis.del('vmpooler__template__prepared')
 

spec/unit/pool_manager_spec.rb

@@ -1606,13 +1606,16 @@ EOT
     end
 
     context 'when already updating' do
+      let(:mutex) { Mutex.new }
       before(:each) do
         redis.hset('vmpooler__template', pool, template)
         redis.hset('vmpooler__config__template', pool, new_template)
-        redis.hset('vmpooler__config__updating', pool, 1)
+        expect(subject).to receive(:pool_mutex).with(pool).and_return(mutex)
       end
 
       it 'should return' do
+        mutex.lock
+
         expect(subject.update_pool_template(config[:pools][0], provider)).to be_nil
       end
     end
@@ -1670,6 +1673,7 @@ EOT
   end
 
   describe 'prepare_template' do
+    let(:mutex) { Mutex.new }
     let(:config) { YAML.load(<<-EOT
 ---
 :config:
@@ -1685,7 +1689,8 @@ EOT
     }
 
     it 'should return if a pool configuration is updating' do
-      redis.hset('vmpooler__config__updating', pool, 1)
+      expect(subject).to receive(:pool_mutex).with(pool).and_return(mutex)
+      mutex.lock
 
       expect(subject.prepare_template(config[:pools][0], provider)).to be_nil
     end
@@ -1701,10 +1706,11 @@ EOT
         allow(redis).to receive(:hset)
         allow(redis).to receive(:hdel)
         allow(provider).to receive(:create_template_delta_disks)
+        expect(subject).to receive(:pool_mutex).with(pool).and_return(mutex)
       end
 
       it 'should mark the pool as updating' do
-        expect(redis).to receive(:hset).with('vmpooler__config__updating', pool, 1)
+        mutex.lock
 
         subject.prepare_template(config[:pools][0], provider)
       end
@@ -1720,12 +1726,6 @@ EOT
 
         subject.prepare_template(config[:pools][0], provider)
       end
-
-      it' should mark the configuration as completed' do
-        expect(redis).to receive(:hdel).with('vmpooler__config__updating', pool)
-
-        subject.prepare_template(config[:pools][0], provider)
-      end
     end
   end
 
@@ -2050,7 +2050,6 @@ EOT
       it 'should run startup tasks only once' do
         expect(redis).to receive(:set).with('vmpooler__tasks__clone', 0).once
         expect(redis).to receive(:del).with('vmpooler__migration').once
-        expect(redis).to receive(:del).with('vmpooler__config__updating').once
         expect(redis).to receive(:del).with('vmpooler__template__prepared').once
 
         subject.execute!(maxloop,0)