(POOLER-70) Update create_vm_snapshot 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

@@ -291,33 +291,33 @@ module Vmpooler
       result
     end
 
-    def create_vm_snapshot(vm, snapshot_name, provider)
+    def create_vm_snapshot(pool_name, vm, snapshot_name, provider)
       Thread.new do
-        _create_vm_snapshot(vm, snapshot_name, provider)
+        begin
+          _create_vm_snapshot(pool_name, vm, snapshot_name, provider)
+        rescue => err
+          $logger.log('d', "[!] [#{pool_name}] '#{vm}' failed while creating snapshot: #{err}")
+          raise
+        end
       end
     end
 
-    def _create_vm_snapshot(vm, snapshot_name, provider)
+    def _create_vm_snapshot(pool_name, vm_name, snapshot_name, provider)
-      host = provider.find_vm(vm)
+      $logger.log('s', "[ ] [snapshot_manager] 'Attempting to snapshot #{vm_name} in pool #{pool_name}")
-
-      if (host) && ((! snapshot_name.nil?) && (! snapshot_name.empty?))
-        $logger.log('s', "[ ] [snapshot_manager] '#{vm}' is being snapshotted")
-
       start = Time.now
 
-        host.CreateSnapshot_Task(
+      result = provider.create_snapshot(pool_name, vm_name, snapshot_name)
-          name: snapshot_name,
-          description: 'vmpooler',
-          memory: true,
-          quiesce: true
-        ).wait_for_completion
 
       finish = '%.2f' % (Time.now - start)
 
-        $redis.hset('vmpooler__vm__' + vm, 'snapshot:' + snapshot_name, Time.now.to_s)
+      if result
-
+        $redis.hset('vmpooler__vm__' + vm_name, 'snapshot:' + snapshot_name, Time.now.to_s)
-        $logger.log('s', "[+] [snapshot_manager] '#{vm}' snapshot created in #{finish} seconds")
+        $logger.log('s', "[+] [snapshot_manager] '#{vm_name}' snapshot created in #{finish} seconds")
+      else
+        $logger.log('s', "[+] [snapshot_manager] Failed to snapshot '#{vm_name}'")
       end
+
+      result
     end
 
     def revert_vm_snapshot(vm, snapshot_name, provider)

spec/unit/pool_manager_spec.rb

@@ -823,7 +823,6 @@ EOT
   end
 
   describe '#create_vm_snapshot' do
-    let(:provider) { double('provider') }
     let(:snapshot_name) { 'snapshot' }
 
     before do
@@ -832,57 +831,89 @@ EOT
 
     it 'calls _create_vm_snapshot' do
       expect(Thread).to receive(:new).and_yield
-      expect(subject).to receive(:_create_vm_snapshot).with(vm, snapshot_name, provider)
+      expect(subject).to receive(:_create_vm_snapshot).with(pool, vm, snapshot_name, provider)
 
-      subject.create_vm_snapshot(vm, snapshot_name, provider)
+      subject.create_vm_snapshot(pool, vm, snapshot_name, provider)
     end
   end
 
   describe '#_create_vm_snapshot' do
-    let(:provider) { double('provider') }
     let(:snapshot_name) { 'snapshot1' }
-    let(:snapshot_task) { double('snapshot_task') }
 
     before do
       expect(subject).not_to be_nil
     end
 
     before(:each) do
-      allow(provider).to receive(:find_vm).with(vm).and_return(host)
-      allow(snapshot_task).to receive(:wait_for_completion).and_return(nil)
-      allow(host).to receive(:CreateSnapshot_Task).with({:name=>snapshot_name, :description=>"vmpooler", :memory=>true, :quiesce=>true}).and_return(snapshot_task)
       create_running_vm(pool,vm,token)
     end
 
-    it 'should not do anything if the VM does not exist' do
+    context 'Given a Pool that does not exist' do
-      expect(provider).to receive(:find_vm).with(vm).and_return(nil)
+      let(:missing_pool) { 'missing_pool' }
-      expect(logger).to receive(:log).exactly(0).times
+
-      subject._create_vm_snapshot(vm, snapshot_name, provider)
+      before(:each) do
+        expect(provider).to receive(:create_snapshot).with(missing_pool, vm, snapshot_name).and_raise("Pool #{missing_pool} not found")
       end
 
-    it 'should not do anything if the snapshot name is nil' do
+      it 'should not update redis' do
-      expect(logger).to receive(:log).exactly(0).times
+        expect(redis.hget("vmpooler__vm__#{vm}", "snapshot:#{snapshot_name}")).to be_nil
-      subject._create_vm_snapshot(vm, nil, provider)
+        expect{ subject._create_vm_snapshot(missing_pool, vm, snapshot_name, provider) }.to raise_error("Pool #{missing_pool} not found")
+        expect(redis.hget("vmpooler__vm__#{vm}", "snapshot:#{snapshot_name}")).to be_nil
+      end
     end
 
-    it 'should not do anything if the snapshot name is empty string' do
+    context 'Given a VM that does not exist' do
-      expect(logger).to receive(:log).exactly(0).times
+      let(:missing_vm) { 'missing_vm' }
-      subject._create_vm_snapshot(vm, '', provider)
+      before(:each) do
+        expect(provider).to receive(:create_snapshot).with(pool, missing_vm, snapshot_name).and_raise("VM #{missing_vm} not found")
       end
 
-    it 'should invoke provider to snapshot the VM' do
+      it 'should not update redis' do
-      expect(logger).to receive(:log).with('s', "[ ] [snapshot_manager] '#{vm}' is being snapshotted")
+        expect(redis.hget("vmpooler__vm__#{vm}", "snapshot:#{snapshot_name}")).to be_nil
+        expect{ subject._create_vm_snapshot(pool, missing_vm, snapshot_name, provider) }.to raise_error("VM #{missing_vm} not found")
+        expect(redis.hget("vmpooler__vm__#{vm}", "snapshot:#{snapshot_name}")).to be_nil
+      end
+    end
+
+    context 'Given a snapshot creation that succeeds' do
+      before(:each) do
+        expect(provider).to receive(:create_snapshot).with(pool, vm, snapshot_name).and_return(true)
+      end
+
+      it 'should log messages' do
+        expect(logger).to receive(:log).with('s', "[ ] [snapshot_manager] 'Attempting to snapshot #{vm} in pool #{pool}")
         expect(logger).to receive(:log).with('s', /\[\+\] \[snapshot_manager\] '#{vm}' snapshot created in 0.[\d]+ seconds/)
-      subject._create_vm_snapshot(vm, snapshot_name, provider)
+
+        subject._create_vm_snapshot(pool, vm, snapshot_name, provider)
       end
 
       it 'should add snapshot redis information' do
         expect(redis.hget("vmpooler__vm__#{vm}", "snapshot:#{snapshot_name}")).to be_nil
-      subject._create_vm_snapshot(vm, snapshot_name, provider)
+        subject._create_vm_snapshot(pool, vm, snapshot_name, provider)
         expect(redis.hget("vmpooler__vm__#{vm}", "snapshot:#{snapshot_name}")).to_not be_nil
       end
     end
 
+    context 'Given a snapshot creation that fails' do
+      before(:each) do
+        expect(provider).to receive(:create_snapshot).with(pool, vm, snapshot_name).and_return(false)
+      end
+
+      it 'should log messages' do
+        expect(logger).to receive(:log).with('s', "[ ] [snapshot_manager] 'Attempting to snapshot #{vm} in pool #{pool}")
+        expect(logger).to receive(:log).with('s', "[+] [snapshot_manager] Failed to snapshot '#{vm}'")
+
+        subject._create_vm_snapshot(pool, vm, snapshot_name, provider)
+      end
+
+      it 'should not update redis' do
+        expect(redis.hget("vmpooler__vm__#{vm}", "snapshot:#{snapshot_name}")).to be_nil
+        subject._create_vm_snapshot(pool, vm, snapshot_name, provider)
+        expect(redis.hget("vmpooler__vm__#{vm}", "snapshot:#{snapshot_name}")).to be_nil
+      end
+    end
+  end
+
   describe '#revert_vm_snapshot' do
     let(:provider) { double('provider') }
     let(:snapshot_name) { 'snapshot' }