From acf32a3f7b97d5cceb32b9f91cb4937561b4e7e6 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Fri, 31 Mar 2017 14:19:24 -0700 Subject: [PATCH] (POOLER-70) Update check_snapshot_queue for VM Provider Previously the Pool Manager would use vSphere objects directly. This commit - Modifies the pool_manager to use the VM provider methods instead --- lib/vmpooler/pool_manager.rb | 42 +++++---- spec/unit/pool_manager_spec.rb | 156 +++++++++++++++++++++++---------- 2 files changed, 134 insertions(+), 64 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index 22e4283..3a58e03 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -402,12 +402,10 @@ module Vmpooler def check_snapshot_queue(maxloop = 0, loop_delay = 5) $logger.log('d', "[*] [snapshot_manager] starting worker thread") - $providers['snapshot_manager'] ||= Vmpooler::VsphereHelper.new $config, $metrics - $threads['snapshot_manager'] = Thread.new do loop_count = 1 loop do - _check_snapshot_queue $providers['snapshot_manager'] + _check_snapshot_queue sleep(loop_delay) unless maxloop.zero? @@ -418,26 +416,38 @@ module Vmpooler end end - def _check_snapshot_queue(provider) - vm = $redis.spop('vmpooler__tasks__snapshot') + def _check_snapshot_queue + task_detail = $redis.spop('vmpooler__tasks__snapshot') - unless vm.nil? + unless task_detail.nil? begin - vm_name, snapshot_name = vm.split(':') - create_vm_snapshot(vm_name, snapshot_name, provider) - rescue - $logger.log('s', "[!] [snapshot_manager] snapshot appears to have failed") + vm_name, snapshot_name = task_detail.split(':') + pool_name = get_pool_name_for_vm(vm_name) + raise("Unable to determine which pool #{vm_name} is a member of") if pool_name.nil? + + provider = get_provider_for_pool(pool_name) + raise("Missing Provider for vm #{vm_name} in pool #{pool_name}") if provider.nil? + + create_vm_snapshot(pool_name, vm_name, snapshot_name, provider) + rescue => err + $logger.log('s', "[!] [snapshot_manager] snapshot create appears to have failed: #{err}") end end - vm = $redis.spop('vmpooler__tasks__snapshot-revert') + task_detail = $redis.spop('vmpooler__tasks__snapshot-revert') - unless vm.nil? + unless task_detail.nil? begin - vm_name, snapshot_name = vm.split(':') - revert_vm_snapshot(vm_name, snapshot_name, provider) - rescue - $logger.log('s', "[!] [snapshot_manager] snapshot revert appears to have failed") + vm_name, snapshot_name = task_detail.split(':') + pool_name = get_pool_name_for_vm(vm_name) + raise("Unable to determine which pool #{vm_name} is a member of") if pool_name.nil? + + provider = get_provider_for_pool(pool_name) + raise("Missing Provider for vm #{vm_name} in pool #{pool_name}") if provider.nil? + + revert_vm_snapshot(pool_name, vm_name, snapshot_name, provider) + rescue => err + $logger.log('s', "[!] [snapshot_manager] snapshot revert appears to have failed: #{err}") end end end diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 3bd85dc..dd633de 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -1251,10 +1251,10 @@ EOT before(:each) do expect(Thread).to receive(:new).and_yield - allow(subject).to receive(:_check_snapshot_queue) + allow(subject).to receive(:_check_snapshot_queue).with(no_args) end - it 'should log the disk manager is starting' do + it 'should log the snapshot manager is starting' do expect(logger).to receive(:log).with('d', "[*] [snapshot_manager] starting worker thread") expect($threads.count).to be(0) @@ -1270,7 +1270,7 @@ EOT end it 'should call _check_snapshot_queue' do - expect(subject).to receive(:_check_snapshot_queue).with(Vmpooler::VsphereHelper) + expect(subject).to receive(:_check_snapshot_queue).with(no_args) subject.check_snapshot_queue(1,0) end @@ -1286,12 +1286,9 @@ EOT end it 'when a non-default loop delay is specified' do - start_time = Time.now - subject.check_snapshot_queue(maxloop,loop_delay) - finish_time = Time.now + expect(subject).to receive(:sleep).with(loop_delay).exactly(maxloop).times - # Use a generous delta to take into account various CPU load etc. - expect(finish_time - start_time).to be_within(0.75).of(maxloop * loop_delay) + subject.check_snapshot_queue(maxloop,loop_delay) end end @@ -1305,7 +1302,7 @@ EOT end it 'should call _check_snapshot_queue 5 times' do - expect(subject).to receive(:_check_snapshot_queue).with(Vmpooler::VsphereHelper).exactly(maxloop).times + expect(subject).to receive(:_check_snapshot_queue).with(no_args).exactly(maxloop).times subject.check_snapshot_queue(maxloop,0) end @@ -1313,8 +1310,6 @@ EOT end describe '#_check_snapshot_queue' do - let(:provider) { double('provider') } - before do expect(subject).not_to be_nil end @@ -1323,64 +1318,146 @@ EOT context 'when no VMs in the queue' do it 'should not call create_vm_snapshot' do expect(subject).to receive(:create_vm_snapshot).exactly(0).times - subject._check_snapshot_queue(provider) + subject._check_snapshot_queue + end + end + + context 'when VM in the queue does not exist' do + before(:each) do + snapshot_vm(vm,"snapshot_#{vm}") + end + + it 'should log an error' do + expect(logger).to receive(:log).with('s', /Unable to determine which pool #{vm} is a member of/) + + subject._check_snapshot_queue + end + + it 'should not call create_vm_snapshot' do + expect(subject).to receive(:create_vm_snapshot).exactly(0).times + + subject._check_snapshot_queue + end + end + + context 'when specified provider does not exist' do + before(:each) do + snapshot_vm(vm,"snapshot_#{vm}") + create_running_vm(pool, vm, token) + expect(subject).to receive(:get_provider_for_pool).and_return(nil) + end + + it 'should log an error' do + expect(logger).to receive(:log).with('s', /Missing Provider for/) + + subject._check_snapshot_queue + end + + it 'should not call create_vm_snapshot' do + expect(subject).to receive(:create_vm_snapshot).exactly(0).times + + subject._check_snapshot_queue end end context 'when multiple VMs in the queue' do before(:each) do - snapshot_vm('vm1','snapshot1') - snapshot_vm('vm2','snapshot2') - snapshot_vm('vm3','snapshot3') + ['vm1', 'vm2', 'vm3'].each do |vm_name| + snapshot_vm(vm_name,"snapshot_#{vm_name}") + create_running_vm(pool, vm_name, token) + end + + allow(subject).to receive(:get_provider_for_pool).with(pool).and_return(provider) end it 'should call create_vm_snapshot once' do expect(subject).to receive(:create_vm_snapshot).exactly(1).times - subject._check_snapshot_queue(provider) + subject._check_snapshot_queue end it 'should snapshot the first VM in the queue' do - expect(subject).to receive(:create_vm_snapshot).with('vm1','snapshot1',provider) - subject._check_snapshot_queue(provider) + expect(subject).to receive(:create_vm_snapshot).with(pool,'vm1','snapshot_vm1',provider) + subject._check_snapshot_queue end it 'should log an error if one occurs' do expect(subject).to receive(:create_vm_snapshot).and_raise(RuntimeError,'MockError') - expect(logger).to receive(:log).with('s', "[!] [snapshot_manager] snapshot appears to have failed") - subject._check_snapshot_queue(provider) + expect(logger).to receive(:log).with('s', "[!] [snapshot_manager] snapshot create appears to have failed: MockError") + subject._check_snapshot_queue end end end - context 'revert_vm_snapshot queue' do + context 'vmpooler__tasks__snapshot-revert queue' do context 'when no VMs in the queue' do it 'should not call revert_vm_snapshot' do expect(subject).to receive(:revert_vm_snapshot).exactly(0).times - subject._check_snapshot_queue(provider) + subject._check_snapshot_queue + end + end + + context 'when VM in the queue does not exist' do + before(:each) do + snapshot_revert_vm(vm,"snapshot_#{vm}") + end + + it 'should log an error' do + expect(logger).to receive(:log).with('s', /Unable to determine which pool #{vm} is a member of/) + + subject._check_snapshot_queue + end + + it 'should not call revert_vm_snapshot' do + expect(subject).to receive(:revert_vm_snapshot).exactly(0).times + + subject._check_snapshot_queue + end + end + + context 'when specified provider does not exist' do + before(:each) do + snapshot_revert_vm(vm,"snapshot_#{vm}") + create_running_vm(pool, vm, token) + expect(subject).to receive(:get_provider_for_pool).and_return(nil) + end + + it 'should log an error' do + expect(logger).to receive(:log).with('s', /Missing Provider for/) + + subject._check_snapshot_queue + end + + it 'should not call revert_vm_snapshot' do + expect(subject).to receive(:revert_vm_snapshot).exactly(0).times + + subject._check_snapshot_queue end end context 'when multiple VMs in the queue' do before(:each) do - snapshot_revert_vm('vm1','snapshot1') - snapshot_revert_vm('vm2','snapshot2') - snapshot_revert_vm('vm3','snapshot3') + ['vm1', 'vm2', 'vm3'].each do |vm_name| + snapshot_revert_vm(vm_name,"snapshot_#{vm_name}") + create_running_vm(pool, vm_name, token) + end + + allow(subject).to receive(:get_provider_for_pool).with(pool).and_return(provider) end it 'should call revert_vm_snapshot once' do expect(subject).to receive(:revert_vm_snapshot).exactly(1).times - subject._check_snapshot_queue(provider) + subject._check_snapshot_queue end - it 'should revert snapshot the first VM in the queue' do - expect(subject).to receive(:revert_vm_snapshot).with('vm1','snapshot1',provider) - subject._check_snapshot_queue(provider) + it 'should snapshot the first VM in the queue' do + expect(subject).to receive(:revert_vm_snapshot).with(pool,'vm1','snapshot_vm1',provider) + subject._check_snapshot_queue end it 'should log an error if one occurs' do expect(subject).to receive(:revert_vm_snapshot).and_raise(RuntimeError,'MockError') - expect(logger).to receive(:log).with('s', "[!] [snapshot_manager] snapshot revert appears to have failed") - subject._check_snapshot_queue(provider) + expect(logger).to receive(:log).with('s', "[!] [snapshot_manager] snapshot revert appears to have failed: MockError") + subject._check_snapshot_queue end end end @@ -2496,21 +2573,4 @@ EOT end end end - - describe '#_check_snapshot_queue' do - let(:pool_helper) { double('pool') } - let(:provider) { {pool => pool_helper} } - - before do - expect(subject).not_to be_nil - $provider = provider - end - - it 'checks appropriate redis queues' do - expect(redis).to receive(:spop).with('vmpooler__tasks__snapshot') - expect(redis).to receive(:spop).with('vmpooler__tasks__snapshot-revert') - - subject._check_snapshot_queue(provider) - end - end end