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

@@ -320,30 +320,32 @@ module Vmpooler
       result
     end
 
-    def revert_vm_snapshot(vm, snapshot_name, provider)
+    def revert_vm_snapshot(pool_name, vm, snapshot_name, provider)
       Thread.new do
-        _revert_vm_snapshot(vm, snapshot_name, provider)
+        begin
+          _revert_vm_snapshot(pool_name, vm, snapshot_name, provider)
+        rescue => err
+          $logger.log('d', "[!] [#{pool_name}] '#{vm}' failed while reverting snapshot: #{err}")
+          raise
+        end
       end
     end
 
-    def _revert_vm_snapshot(vm, snapshot_name, provider)
+    def _revert_vm_snapshot(pool_name, vm_name, snapshot_name, provider)
-      host = provider.find_vm(vm)
+      $logger.log('s', "[ ] [snapshot_manager] 'Attempting to revert #{vm_name}' in pool #{pool_name} to snapshot '#{snapshot_name}'")
+      start = Time.now
 
-      if host
+      result = provider.revert_snapshot(pool_name, vm_name, snapshot_name)
-        snapshot = provider.find_snapshot(host, snapshot_name)
 
-        if snapshot
+      finish = '%.2f' % (Time.now - start)
-          $logger.log('s', "[ ] [snapshot_manager] '#{vm}' is being reverted to snapshot '#{snapshot_name}'")
 
-          start = Time.now
+      if result
-
+        $logger.log('s', "[+] [snapshot_manager] '#{vm_name}' reverted to snapshot '#{snapshot_name}' in #{finish} seconds")
-          snapshot.RevertToSnapshot_Task.wait_for_completion
+      else
-
+        $logger.log('s', "[+] [snapshot_manager] Failed to revert #{vm_name}' in pool #{pool_name} to snapshot '#{snapshot_name}'")
-          finish = '%.2f' % (Time.now - start)
-
-          $logger.log('s', "[<] [snapshot_manager] '#{vm}' reverted to snapshot in #{finish} seconds")
-        end
       end
+
+      result
     end
 
     def check_disk_queue(maxloop = 0, loop_delay = 5)

spec/unit/pool_manager_spec.rb

@@ -915,58 +915,88 @@ EOT
   end
 
   describe '#revert_vm_snapshot' do
-    let(:provider) { double('provider') }
     let(:snapshot_name) { 'snapshot' }
 
     before do
       expect(subject).not_to be_nil
     end
 
-    it 'calls _create_vm_snapshot' do
+    it 'calls _revert_vm_snapshot' do
       expect(Thread).to receive(:new).and_yield
-      expect(subject).to receive(:_revert_vm_snapshot).with(vm, snapshot_name, provider)
+      expect(subject).to receive(:_revert_vm_snapshot).with(pool, vm, snapshot_name, provider)
 
-      subject.revert_vm_snapshot(vm, snapshot_name, provider)
+      subject.revert_vm_snapshot(pool, vm, snapshot_name, provider)
     end
   end
 
   describe '#_revert_vm_snapshot' do
-    let(:provider) { double('provider') }
     let(:snapshot_name) { 'snapshot1' }
-    let(:snapshot_object) { double('snapshot_object') }
 
     before do
       expect(subject).not_to be_nil
     end
 
-    before(:each) do
+    context 'Given a Pool that does not exist' do
-      allow(provider).to receive(:find_vm).with(vm).and_return(host)
+      let(:missing_pool) { 'missing_pool' }
-      allow(snapshot_object).to receive_message_chain(:RevertToSnapshot_Task, :wait_for_completion)
+
-      allow(provider).to receive(:find_snapshot).with(host,snapshot_name).and_return(snapshot_object)
+      before(:each) do
+        expect(provider).to receive(:revert_snapshot).with(missing_pool, vm, snapshot_name).and_raise("Pool #{missing_pool} not found")
+      end
+
+      it 'should not log a result message' do
+        expect(logger).to receive(:log).with('s', /\[\+\] \[snapshot_manager\] '#{vm}' reverted to snapshot '#{snapshot_name}' in 0.[\d]+ seconds/).exactly(0).times
+        expect(logger).to receive(:log).with('s', "[+] [snapshot_manager] Failed to revert #{vm}' in pool #{missing_pool} to snapshot '#{snapshot_name}'").exactly(0).times
+
+        expect{ subject._revert_vm_snapshot(missing_pool, vm, snapshot_name, provider) }.to raise_error("Pool #{missing_pool} not found")
+      end
     end
 
-    it 'should not do anything if the VM does not exist' do
+    context 'Given a VM that does not exist' do
-      expect(provider).to receive(:find_vm).with(vm).and_return(nil)
+      let(:missing_vm) { 'missing_vm' }
-      expect(logger).to receive(:log).exactly(0).times
+      before(:each) do
-      subject._revert_vm_snapshot(vm, snapshot_name, provider)
+        expect(provider).to receive(:revert_snapshot).with(pool, missing_vm, snapshot_name).and_raise("VM #{missing_vm} not found")
+      end
+
+      it 'should not log a result message' do
+        expect(logger).to receive(:log).with('s', /\[\+\] \[snapshot_manager\] '#{missing_vm}' reverted to snapshot '#{snapshot_name}' in 0.[\d]+ seconds/).exactly(0).times
+        expect(logger).to receive(:log).with('s', "[+] [snapshot_manager] Failed to revert #{missing_vm}' in pool #{pool} to snapshot '#{snapshot_name}'").exactly(0).times
+
+        expect{ subject._revert_vm_snapshot(pool, missing_vm, snapshot_name, provider) }.to raise_error("VM #{missing_vm} not found")
+      end
     end
 
-    it 'should not do anything if the snapshot name is nil' do
+    context 'Given a snapshot revert that succeeds' do
-      expect(logger).to receive(:log).exactly(0).times
+      before(:each) do
-      expect(provider).to receive(:find_snapshot).with(host,nil).and_return nil
+        expect(provider).to receive(:revert_snapshot).with(pool, vm, snapshot_name).and_return(true)
-      subject._revert_vm_snapshot(vm, nil, provider)
+      end
+
+      it 'should log success messages' do
+        expect(logger).to receive(:log).with('s', "[ ] [snapshot_manager] 'Attempting to revert #{vm}' in pool #{pool} to snapshot '#{snapshot_name}'")
+        expect(logger).to receive(:log).with('s', /\[\+\] \[snapshot_manager\] '#{vm}' reverted to snapshot '#{snapshot_name}' in 0.[\d]+ seconds/)
+
+        subject._revert_vm_snapshot(pool, vm, snapshot_name, provider)
+      end
+
+      it 'should return true' do
+        expect(subject._revert_vm_snapshot(pool, vm, snapshot_name, provider)).to be true
+      end
     end
 
-    it 'should not do anything if the snapshot name is empty string' do
+    context 'Given a snapshot creation that fails' do
-      expect(logger).to receive(:log).exactly(0).times
+      before(:each) do
-      expect(provider).to receive(:find_snapshot).with(host,'').and_return nil
+        expect(provider).to receive(:revert_snapshot).with(pool, vm, snapshot_name).and_return(false)
-      subject._revert_vm_snapshot(vm, '', provider)
+      end
-    end
 
-    it 'should invoke provider to revert the VM to the snapshot' do
+      it 'should log failure messages' do
-      expect(logger).to receive(:log).with('s', "[ ] [snapshot_manager] '#{vm}' is being reverted to snapshot '#{snapshot_name}'")
+        expect(logger).to receive(:log).with('s', "[ ] [snapshot_manager] 'Attempting to revert #{vm}' in pool #{pool} to snapshot '#{snapshot_name}'")
-      expect(logger).to receive(:log).with('s', /\[\<\] \[snapshot_manager\] '#{vm}' reverted to snapshot in 0\.[\d]+ seconds/)
+        expect(logger).to receive(:log).with('s', "[+] [snapshot_manager] Failed to revert #{vm}' in pool #{pool} to snapshot '#{snapshot_name}'")
-      subject._revert_vm_snapshot(vm, snapshot_name, provider)
+
+        subject._revert_vm_snapshot(pool, vm, snapshot_name, provider)
+      end
+
+      it 'should return false' do
+        expect(subject._revert_vm_snapshot(pool, vm, snapshot_name, provider)).to be false
+      end
     end
   end