From ac7d7009d20e0bc29f49069a947f471f46253457 Mon Sep 17 00:00:00 2001 From: Glenn Sarti Date: Wed, 1 Mar 2017 21:47:06 -0800 Subject: [PATCH] (POOLER-70) Refactor clone_vm to take pool configuration object Previously, the clone_vm method took various VSphere specific parameters e.g. template folder. However in order make VMPooler less VSphere specific this method should just take the pool configuration and then it can determine the appropriate settings itself. This commit also moves the threading to a clone_vm while the actual method which does the work is now _clone_vm as per all other multithread worker methods in pool_manager. This commit also updates the spec tests appropriately. --- lib/vmpooler/pool_manager.rb | 199 +++++++++++++++++---------------- spec/unit/pool_manager_spec.rb | 113 ++++++++++++------- 2 files changed, 175 insertions(+), 137 deletions(-) diff --git a/lib/vmpooler/pool_manager.rb b/lib/vmpooler/pool_manager.rb index f9103ef..af3237d 100644 --- a/lib/vmpooler/pool_manager.rb +++ b/lib/vmpooler/pool_manager.rb @@ -185,105 +185,113 @@ module Vmpooler end # Clone a VM - def clone_vm(template, folder, datastore, target, vsphere) + def clone_vm(pool, vsphere) Thread.new do begin - vm = {} - - if template =~ /\// - templatefolders = template.split('/') - vm['template'] = templatefolders.pop - end - - if templatefolders - vm[vm['template']] = vsphere.find_folder(templatefolders.join('/')).find(vm['template']) - else - fail 'Please provide a full path to the template' - end - - if vm['template'].length == 0 - fail "Unable to find template '#{vm['template']}'!" - end - - # Generate a randomized hostname - o = [('a'..'z'), ('0'..'9')].map(&:to_a).flatten - vm['hostname'] = $config[:config]['prefix'] + o[rand(25)] + (0...14).map { o[rand(o.length)] }.join - - # Add VM to Redis inventory ('pending' pool) - $redis.sadd('vmpooler__pending__' + vm['template'], vm['hostname']) - $redis.hset('vmpooler__vm__' + vm['hostname'], 'clone', Time.now) - $redis.hset('vmpooler__vm__' + vm['hostname'], 'template', vm['template']) - - # Annotate with creation time, origin template, etc. - # Add extraconfig options that can be queried by vmtools - configSpec = RbVmomi::VIM.VirtualMachineConfigSpec( - annotation: JSON.pretty_generate( - name: vm['hostname'], - created_by: $config[:vsphere]['username'], - base_template: vm['template'], - creation_timestamp: Time.now.utc - ), - extraConfig: [ - { key: 'guestinfo.hostname', - value: vm['hostname'] - } - ] - ) - - # Choose a clone target - if target - $clone_target = vsphere.find_least_used_host(target) - elsif $config[:config]['clone_target'] - $clone_target = vsphere.find_least_used_host($config[:config]['clone_target']) - end - - # Put the VM in the specified folder and resource pool - relocateSpec = RbVmomi::VIM.VirtualMachineRelocateSpec( - datastore: vsphere.find_datastore(datastore), - host: $clone_target, - diskMoveType: :moveChildMostDiskBacking - ) - - # Create a clone spec - spec = RbVmomi::VIM.VirtualMachineCloneSpec( - location: relocateSpec, - config: configSpec, - powerOn: true, - template: false - ) - - # Clone the VM - $logger.log('d', "[ ] [#{vm['template']}] '#{vm['hostname']}' is being cloned from '#{vm['template']}'") - - begin - start = Time.now - vm[vm['template']].CloneVM_Task( - folder: vsphere.find_folder(folder), - name: vm['hostname'], - spec: spec - ).wait_for_completion - finish = '%.2f' % (Time.now - start) - - $redis.hset('vmpooler__clone__' + Date.today.to_s, vm['template'] + ':' + vm['hostname'], finish) - $redis.hset('vmpooler__vm__' + vm['hostname'], 'clone_time', finish) - - $logger.log('s', "[+] [#{vm['template']}] '#{vm['hostname']}' cloned from '#{vm['template']}' in #{finish} seconds") - rescue => err - $logger.log('s', "[!] [#{vm['template']}] '#{vm['hostname']}' clone failed with an error: #{err}") - $redis.srem('vmpooler__pending__' + vm['template'], vm['hostname']) - raise - end - - $redis.decr('vmpooler__tasks__clone') - - $metrics.timing("clone.#{vm['template']}", finish) + _clone_vm(pool, vsphere) rescue => err - $logger.log('s', "[!] [#{vm['template']}] '#{vm['hostname']}' failed while preparing to clone with an error: #{err}") + $logger.log('s', "[!] [#{pool['name']}] failed while cloning VM with an error: #{err}") raise end end end + def _clone_vm(pool, vsphere) + template = pool['template'] + folder = pool['folder'] + datastore = pool['datastore'] + target = pool['clone_target'] + vm = {} + + if template =~ /\// + templatefolders = template.split('/') + vm['template'] = templatefolders.pop + end + + if templatefolders + vm[vm['template']] = vsphere.find_folder(templatefolders.join('/')).find(vm['template']) + else + fail 'Please provide a full path to the template' + end + + if vm['template'].length == 0 + fail "Unable to find template '#{vm['template']}'!" + end + + # Generate a randomized hostname + o = [('a'..'z'), ('0'..'9')].map(&:to_a).flatten + vm['hostname'] = $config[:config]['prefix'] + o[rand(25)] + (0...14).map { o[rand(o.length)] }.join + + # Add VM to Redis inventory ('pending' pool) + $redis.sadd('vmpooler__pending__' + vm['template'], vm['hostname']) + $redis.hset('vmpooler__vm__' + vm['hostname'], 'clone', Time.now) + $redis.hset('vmpooler__vm__' + vm['hostname'], 'template', vm['template']) + + # Annotate with creation time, origin template, etc. + # Add extraconfig options that can be queried by vmtools + configSpec = RbVmomi::VIM.VirtualMachineConfigSpec( + annotation: JSON.pretty_generate( + name: vm['hostname'], + created_by: $config[:vsphere]['username'], + base_template: vm['template'], + creation_timestamp: Time.now.utc + ), + extraConfig: [ + { key: 'guestinfo.hostname', + value: vm['hostname'] + } + ] + ) + + # Choose a clone target + if target + $clone_target = vsphere.find_least_used_host(target) + elsif $config[:config]['clone_target'] + $clone_target = vsphere.find_least_used_host($config[:config]['clone_target']) + end + + # Put the VM in the specified folder and resource pool + relocateSpec = RbVmomi::VIM.VirtualMachineRelocateSpec( + datastore: vsphere.find_datastore(datastore), + host: $clone_target, + diskMoveType: :moveChildMostDiskBacking + ) + + # Create a clone spec + spec = RbVmomi::VIM.VirtualMachineCloneSpec( + location: relocateSpec, + config: configSpec, + powerOn: true, + template: false + ) + + # Clone the VM + $logger.log('d', "[ ] [#{vm['template']}] '#{vm['hostname']}' is being cloned from '#{vm['template']}'") + + begin + start = Time.now + vm[vm['template']].CloneVM_Task( + folder: vsphere.find_folder(folder), + name: vm['hostname'], + spec: spec + ).wait_for_completion + finish = '%.2f' % (Time.now - start) + + $redis.hset('vmpooler__clone__' + Date.today.to_s, vm['template'] + ':' + vm['hostname'], finish) + $redis.hset('vmpooler__vm__' + vm['hostname'], 'clone_time', finish) + + $logger.log('s', "[+] [#{vm['template']}] '#{vm['hostname']}' cloned from '#{vm['template']}' in #{finish} seconds") + rescue => err + $logger.log('s', "[!] [#{vm['template']}] '#{vm['hostname']}' clone failed with an error: #{err}") + $redis.srem('vmpooler__pending__' + vm['template'], vm['hostname']) + raise + end + + $redis.decr('vmpooler__tasks__clone') + + $metrics.timing("clone.#{vm['template']}", finish) + end + # Destroy a VM def destroy_vm(vm, pool, vsphere) Thread.new do @@ -710,14 +718,7 @@ module Vmpooler if $redis.get('vmpooler__tasks__clone').to_i < $config[:config]['task_limit'].to_i begin $redis.incr('vmpooler__tasks__clone') - - clone_vm( - pool['template'], - pool['folder'], - pool['datastore'], - pool['clone_target'], - vsphere - ) + clone_vm(pool,vsphere) rescue => err $logger.log('s', "[!] [#{pool['name']}] clone failed during check_pool with an error: #{err}") $redis.decr('vmpooler__tasks__clone') diff --git a/spec/unit/pool_manager_spec.rb b/spec/unit/pool_manager_spec.rb index 825d082..de684e1 100644 --- a/spec/unit/pool_manager_spec.rb +++ b/spec/unit/pool_manager_spec.rb @@ -505,13 +505,7 @@ EOT end describe '#clone_vm' do - before do - expect(subject).not_to be_nil - end - - before(:each) do - expect(Thread).to receive(:new).and_yield - end + let(:vsphere) { double('vsphere') } let(:config) { YAML.load(<<-EOT @@ -520,9 +514,41 @@ EOT prefix: "prefix" :vsphere: username: "vcenter_user" +:pools: + - name: #{pool} EOT ) } + let (:pool_object) { config[:pools][0] } + + before do + expect(subject).not_to be_nil + end + + it 'calls _clone_vm' do + expect(Thread).to receive(:new).and_yield + expect(subject).to receive(:_clone_vm).with(pool_object,vsphere) + + subject.clone_vm(pool_object,vsphere) + end + + it 'logs a message if an error is raised' do + expect(Thread).to receive(:new).and_yield + expect(logger).to receive(:log) + expect(subject).to receive(:_clone_vm).with(pool_object,vsphere).and_raise('an_error') + + expect{subject.clone_vm(pool_object,vsphere)}.to raise_error(/an_error/) + end + end + + describe '#_clone_vm' do + before do + expect(subject).not_to be_nil + end + + before(:each) do + #expect(Thread).to receive(:new).and_yield + end let (:folder) { 'vmfolder' } let (:folder_object) { double('folder_object') } @@ -530,34 +556,47 @@ EOT let (:template) { "template/#{template_name}" } let (:datastore) { 'datastore' } let (:target) { 'clone_target' } + + let(:config) { + YAML.load(<<-EOT +--- +:config: + prefix: "prefix" +:vsphere: + username: "vcenter_user" +:pools: + - name: #{pool} + template: '#{template}' + folder: '#{folder}' + datastore: '#{datastore}' + clone_target: '#{target}' +EOT + ) + } + let (:vsphere) { double('vsphere') } let (:template_folder_object) { double('template_folder_object') } let (:template_vm_object) { double('template_vm_object') } let (:clone_task) { double('clone_task') } + let (:pool_object) { config[:pools][0] } context 'no template specified' do - it 'should raise an error' do - - expect{subject.clone_vm(nil,folder,datastore,target,vsphere)}.to raise_error(/Please provide a full path to the template/) + before(:each) do + pool_object['template'] = nil end - it 'should log a message' do - expect(logger).to receive(:log).with('s', "[!] [] '' failed while preparing to clone with an error: Please provide a full path to the template") - - expect{subject.clone_vm(nil,folder,datastore,target,vsphere)}.to raise_error(RuntimeError) + it 'should raise an error' do + expect{subject._clone_vm(pool_object,vsphere)}.to raise_error(/Please provide a full path to the template/) end end context 'a template with no forward slash in the string' do - it 'should raise an error' do - - expect{subject.clone_vm('vm1',folder,datastore,target,vsphere)}.to raise_error(/Please provide a full path to the template/) + before(:each) do + pool_object['template'] = template_name end - it 'should log a message' do - expect(logger).to receive(:log).with('s', "[!] [] '' failed while preparing to clone with an error: Please provide a full path to the template") - - expect{subject.clone_vm('vm1',folder,datastore,target,vsphere)}.to raise_error(RuntimeError) + it 'should raise an error' do + expect{subject._clone_vm(pool_object,vsphere)}.to raise_error(/Please provide a full path to the template/) end end @@ -571,9 +610,9 @@ EOT context "Template name does not match pool name (Implementation Bug)" do let (:template_name) { 'template_vm' } - # The implementaion of clone_vm incorrectly uses the VM Template name instead of the pool name. The VM Template represents the + # The implementaion of _clone_vm incorrectly uses the VM Template name instead of the pool name. The VM Template represents the # name of the VM to clone in vSphere whereas pool is the name of the pool in Pooler. The tests below document the behaviour of - # clone_vm if the Template and Pool name differ. It is expected that these test will fail once this bug is removed. + # _clone_vm if the Template and Pool name differ. It is expected that these test will fail once this bug is removed. context 'a valid template' do before(:each) do @@ -595,7 +634,7 @@ EOT expect(logger).to receive(:log).at_least(:once) expect(redis.scard("vmpooler__pending__#{pool}")).to eq(0) - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) expect(redis.scard("vmpooler__pending__#{template_name}")).to eq(1) # Get the new VM Name from the pending pool queue as it should be the only entry @@ -610,14 +649,14 @@ EOT expect(logger).to receive(:log).with('d',/\[ \] \[#{template_name}\] '(.+)' is being cloned from '#{template_name}'/) allow(logger).to receive(:log) - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) end it 'should log a message that it completed being cloned' do expect(logger).to receive(:log).with('s',/\[\+\] \[#{template_name}\] '(.+)' cloned from '#{template_name}' in [0-9.]+ seconds/) allow(logger).to receive(:log) - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) end end @@ -639,7 +678,7 @@ EOT it 'should raise an error within the Thread' do expect(logger).to receive(:log).at_least(:once) - expect{subject.clone_vm(template,folder,datastore,target,vsphere)}.to raise_error(/SomeError/) + expect{subject._clone_vm(pool_object,vsphere)}.to raise_error(/SomeError/) end it 'should log a message that is being cloned from a template' do @@ -648,19 +687,18 @@ EOT # Swallow the error begin - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) rescue end end it 'should log messages that the clone failed' do expect(logger).to receive(:log).with('s', /\[!\] \[#{template_name}\] '(.+)' clone failed with an error: SomeError/) - expect(logger).to receive(:log).with('s', /\[!\] \[#{template_name}\] '(.+)' failed while preparing to clone with an error: SomeError/) allow(logger).to receive(:log) # Swallow the error begin - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) rescue end end @@ -688,7 +726,7 @@ EOT expect(logger).to receive(:log).at_least(:once) expect(redis.scard("vmpooler__pending__#{pool}")).to eq(0) - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) expect(redis.scard("vmpooler__pending__#{pool}")).to eq(1) # Get the new VM Name from the pending pool queue as it should be the only entry @@ -703,7 +741,7 @@ EOT redis.incr('vmpooler__tasks__clone') redis.incr('vmpooler__tasks__clone') expect(redis.get('vmpooler__tasks__clone')).to eq('2') - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) expect(redis.get('vmpooler__tasks__clone')).to eq('1') end @@ -711,14 +749,14 @@ EOT expect(logger).to receive(:log).with('d',/\[ \] \[#{pool}\] '(.+)' is being cloned from '#{template_name}'/) allow(logger).to receive(:log) - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) end it 'should log a message that it completed being cloned' do expect(logger).to receive(:log).with('s',/\[\+\] \[#{pool}\] '(.+)' cloned from '#{template_name}' in [0-9.]+ seconds/) allow(logger).to receive(:log) - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) end end @@ -740,7 +778,7 @@ EOT it 'should raise an error within the Thread' do expect(logger).to receive(:log).at_least(:once) - expect{subject.clone_vm(template,folder,datastore,target,vsphere)}.to raise_error(/SomeError/) + expect{subject._clone_vm(pool_object,vsphere)}.to raise_error(/SomeError/) end it 'should log a message that is being cloned from a template' do @@ -749,19 +787,18 @@ EOT # Swallow the error begin - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) rescue end end it 'should log messages that the clone failed' do expect(logger).to receive(:log).with('s', /\[!\] \[#{pool}\] '(.+)' clone failed with an error: SomeError/) - expect(logger).to receive(:log).with('s', /\[!\] \[#{pool}\] '(.+)' failed while preparing to clone with an error: SomeError/) allow(logger).to receive(:log) # Swallow the error begin - subject.clone_vm(template,folder,datastore,target,vsphere) + subject._clone_vm(pool_object,vsphere) rescue end end