From c64bd973106c2468a5b10d4f4b85d7d56bd815c3 Mon Sep 17 00:00:00 2001 From: "kirby@puppetlabs.com" Date: Thu, 24 May 2018 16:45:51 -0700 Subject: [PATCH] Remove redis locks --- lib/vmpooler/pool_manager.rb | 82 +++++++++++++++++----------------- spec/unit/pool_manager_spec.rb | 19 ++++---- 2 files changed, 51 insertions(+), 50 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index e176103..4bb95da 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/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 + provider.create_template_delta_disks(pool) + $redis.hset('vmpooler__template__prepared', pool['name'], pool['template']) end end end @@ -581,37 +589,32 @@ 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 - $redis.hset('vmpooler__template', pool['name'], new_template_name) - $logger.log('s', "[*] [#{pool['name']}] template updated from #{old_template_name} to #{new_template_name}") - # Remove all ready and pending VMs so new instances are created from the new template - if $redis.scard("vmpooler__ready__#{pool['name']}") > 0 - $logger.log('s', "[*] [#{pool['name']}] removing ready instances") - $redis.smembers("vmpooler__ready__#{pool['name']}").each do |vm| - move_vm_queue(pool['name'], vm, 'ready', 'completed') - end + old_template_name = $redis.hget('vmpooler__template', pool['name']) + new_template_name = $redis.hget('vmpooler__config__template', pool['name']) + pool['template'] = new_template_name + $redis.hset('vmpooler__template', pool['name'], new_template_name) + $logger.log('s', "[*] [#{pool['name']}] template updated from #{old_template_name} to #{new_template_name}") + # Remove all ready and pending VMs so new instances are created from the new template + if $redis.scard("vmpooler__ready__#{pool['name']}") > 0 + $logger.log('s', "[*] [#{pool['name']}] removing ready instances") + $redis.smembers("vmpooler__ready__#{pool['name']}").each do |vm| + move_vm_queue(pool['name'], vm, 'ready', 'completed') end - if $redis.scard("vmpooler__pending__#{pool['name']}") > 0 - $logger.log('s', "[*] [#{pool['name']}] removing pending instances") - $redis.smembers("vmpooler__pending__#{pool['name']}").each do |vm| - move_vm_queue(pool['name'], vm, 'pending', 'completed') - end - end - # Prepare template for deployment - $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 + if $redis.scard("vmpooler__pending__#{pool['name']}") > 0 + $logger.log('s', "[*] [#{pool['name']}] removing pending instances") + $redis.smembers("vmpooler__pending__#{pool['name']}").each do |vm| + move_vm_queue(pool['name'], vm, 'pending', 'completed') + end + end + # Prepare template for deployment + $logger.log('s', "[*] [#{pool['name']}] creating template deltas") + provider.create_template_delta_disks(pool) + $logger.log('s', "[*] [#{pool['name']}] template deltas have been created") 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') diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 3499576..a93207a 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/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)