commit 4269abbd3c1fba6deb50b4b8ede2fecbbed11ddd
Author: Tomas Strachota <tstrachota@redhat.com>
Date:   Thu Apr 30 15:33:32 2015 +0000

    Fixes #10248 - cloning volume information for virtual machines

diff --git a/app/assets/javascripts/compute_resources/ovirt/nic_info.js b/app/assets/javascripts/compute_resources/ovirt/nic_info.js
index d21ca4f24..52b4bebde 100644
--- a/app/assets/javascripts/compute_resources/ovirt/nic_info.js
+++ b/app/assets/javascripts/compute_resources/ovirt/nic_info.js
@@ -1,3 +1,3 @@
 providerSpecificNICInfo = function(form) {
-  return form.find('.ovirt_name').val() + ' @ ' + form.find('.ovirt_network').val();
+  return form.find('.ovirt_name').val() + ' @ ' + form.find('.ovirt_network').text();
 }
diff --git a/app/models/compute_resource.rb b/app/models/compute_resource.rb
index 8dc3b3466..2f3074566 100644
--- a/app/models/compute_resource.rb
+++ b/app/models/compute_resource.rb
@@ -244,7 +244,17 @@ class ComputeResource < ActiveRecord::Base
 
   def vm_compute_attributes_for(uuid)
     vm = find_vm_by_uuid(uuid)
-    vm.attributes.reject{|k,v| k == :id }
+    vm_attrs = vm.attributes rescue {}
+    vm_attrs = vm_attrs.reject{|k,v| k == :id }
+
+    if vm.respond_to?(:volumes)
+      volumes = vm.volumes || []
+      vm_attrs[:volumes_attributes] = Hash[volumes.each_with_index.map { |volume, idx| [idx.to_s, volume.attributes] }]
+    end
+    vm_attrs
+  rescue ActiveRecord::RecordNotFound
+    logger.warn("VM with UUID '#{uuid}' not found on #{self}")
+    {}
   end
 
   def user_data_supported?
diff --git a/app/models/compute_resources/foreman/model/ec2.rb b/app/models/compute_resources/foreman/model/ec2.rb
index 9965b78ea..b8c8d24ea 100644
--- a/app/models/compute_resources/foreman/model/ec2.rb
+++ b/app/models/compute_resources/foreman/model/ec2.rb
@@ -28,7 +28,7 @@ module Foreman::Model
     end
 
     def find_vm_by_uuid(uuid)
-      client.servers.get(uuid)
+      super
     rescue Fog::Compute::AWS::Error
       raise(ActiveRecord::RecordNotFound)
     end
diff --git a/app/models/compute_resources/foreman/model/libvirt.rb b/app/models/compute_resources/foreman/model/libvirt.rb
index 12f16c722..d6050fe1b 100644
--- a/app/models/compute_resources/foreman/model/libvirt.rb
+++ b/app/models/compute_resources/foreman/model/libvirt.rb
@@ -26,7 +26,7 @@ module Foreman::Model
     end
 
     def find_vm_by_uuid(uuid)
-      client.servers.get(uuid)
+      super
     rescue ::Libvirt::RetrieveError => e
       Foreman::Logging.exception("Failed retrieving libvirt vm by uuid #{ uuid }", e)
       raise ActiveRecord::RecordNotFound
@@ -150,6 +150,17 @@ module Foreman::Model
       associate_by("mac", vm.mac)
     end
 
+    def vm_compute_attributes_for(uuid)
+      vm_attrs = super
+      if vm_attrs[:memory_size].nil?
+        vm_attrs[:memory] = nil
+        logger.debug("Compute attributes for VM '#{uuid}' diddn't contain :memory_size")
+      else
+        vm_attrs[:memory] = vm_attrs[:memory_size]*1024 # value is returned in megabytes, we need bytes
+      end
+      vm_attrs
+    end
+
     protected
 
     def client
diff --git a/app/models/compute_resources/foreman/model/ovirt.rb b/app/models/compute_resources/foreman/model/ovirt.rb
index 9dfd5d7f5..d486aeaa1 100644
--- a/app/models/compute_resources/foreman/model/ovirt.rb
+++ b/app/models/compute_resources/foreman/model/ovirt.rb
@@ -17,6 +17,12 @@ module Foreman::Model
       [:build, :image]
     end
 
+    def find_vm_by_uuid(uuid)
+      super
+    rescue OVIRT::OvirtException
+      raise(ActiveRecord::RecordNotFound)
+    end
+
     def supports_update?
       true
     end
diff --git a/app/models/compute_resources/foreman/model/rackspace.rb b/app/models/compute_resources/foreman/model/rackspace.rb
index e4841189d..8051e78e5 100644
--- a/app/models/compute_resources/foreman/model/rackspace.rb
+++ b/app/models/compute_resources/foreman/model/rackspace.rb
@@ -16,7 +16,7 @@ module Foreman::Model
     end
 
     def find_vm_by_uuid(uuid)
-      client.servers.get(uuid)
+      super
     rescue Fog::Compute::Rackspace::Error
       raise(ActiveRecord::RecordNotFound)
     end
diff --git a/test/functional/api/v2/hosts_controller_test.rb b/test/functional/api/v2/hosts_controller_test.rb
index 161f2c008..4710fd17e 100644
--- a/test/functional/api/v2/hosts_controller_test.rb
+++ b/test/functional/api/v2/hosts_controller_test.rb
@@ -260,7 +260,7 @@ class Api::V2::HostsControllerTest < ActionController::TestCase
     get :vm_compute_attributes, { :id => host.to_param }
     assert_response :success
     data = JSON.parse(@response.body)
-    assert_equal data, "cpus" => 4
+    assert_equal data, "cpus" => 4, "memory" => nil
     ComputeResource.any_instance.unstub(:vm_compute_attributes_for)
   end
 
diff --git a/test/unit/compute_resource_test.rb b/test/unit/compute_resource_test.rb
index 223dc7605..e5f56fb32 100644
--- a/test/unit/compute_resource_test.rb
+++ b/test/unit/compute_resource_test.rb
@@ -170,4 +170,99 @@ class ComputeResourceTest < ActiveSupport::TestCase
     cr = FactoryGirl.build(:compute_resource)
     refute as_admin { cr.send(:associate_by, 'mac', '11:22:33:44:55:1a') }.readonly?
   end
+
+  describe "find_vm_by_uuid" do
+    before do
+      servers = mock()
+      servers.stubs(:get).returns(nil)
+
+      client = mock()
+      client.stubs(:servers).returns(servers)
+
+      @cr = ComputeResource.new
+      @cr.stubs(:client).returns(client)
+    end
+
+    it "raises RecordNotFound when the vm does not exist" do
+      assert_raises ActiveRecord::RecordNotFound do
+        @cr.find_vm_by_uuid('abc')
+      end
+    end
+  end
+
+  describe "vm_compute_attributes_for" do
+    before do
+      plain_attrs = {
+        :id => 'abc',
+        :cpus => 5
+      }
+      @vm = mock()
+      @vm.stubs(:attributes).returns(plain_attrs)
+
+      @cr = compute_resources(:vmware)
+      @cr.stubs(:find_vm_by_uuid).returns(@vm)
+
+      vol1 = mock()
+      vol1.stubs(:attributes).returns({:vol => 1})
+      vol2 = mock()
+      vol2.stubs(:attributes).returns({:vol => 2})
+
+      @volumes = [
+        vol1,
+        vol2
+      ]
+    end
+
+    test "returns vm attributes without id" do
+      @vm.stubs(:volumes).returns(@volumes)
+
+      expected_attrs = {
+        :cpus => 5,
+        :volumes_attributes => {
+          "0" => { :vol => 1 },
+          "1" => { :vol => 2 }
+        }
+      }
+      attrs = @cr.vm_compute_attributes_for('abc')
+
+      assert_equal expected_attrs, attrs
+    end
+
+    test "returns correct vm attributes when vm does not respond to volumes" do
+      expected_attrs = { :cpus => 5 }
+      attrs = @cr.vm_compute_attributes_for('abc')
+
+      assert_equal expected_attrs, attrs
+    end
+
+    test "returns correct vm attributes when vm volumes are nil" do
+      @vm.stubs(:volumes).returns(nil)
+
+      expected_attrs = {
+        :cpus => 5,
+        :volumes_attributes => {}
+      }
+      attrs = @cr.vm_compute_attributes_for('abc')
+
+      assert_equal expected_attrs, attrs
+    end
+
+    test "returns default attributes when the vm no longer exists" do
+      @cr.stubs(:find_vm_by_uuid).returns(nil)
+
+      expected_attrs = {}
+      attrs = @cr.vm_compute_attributes_for('abc')
+
+      assert_equal expected_attrs, attrs
+    end
+
+    test "returns default attributes when the vm no longer exists and provider raises exception" do
+      @cr.stubs(:find_vm_by_uuid).raises(ActiveRecord::RecordNotFound)
+
+      expected_attrs = {}
+      attrs = @cr.vm_compute_attributes_for('abc')
+
+      assert_equal expected_attrs, attrs
+    end
+  end
 end
diff --git a/test/unit/compute_resources/compute_resource_test_helpers.rb b/test/unit/compute_resources/compute_resource_test_helpers.rb
new file mode 100644
index 000000000..44be6cf3f
--- /dev/null
+++ b/test/unit/compute_resources/compute_resource_test_helpers.rb
@@ -0,0 +1,27 @@
+module ComputeResourceTestHelpers
+  def empty_servers
+    servers = mock()
+    servers.stubs(:get).returns(nil)
+    servers
+  end
+
+  def servers_raising_exception(ex)
+    servers = mock()
+    servers.stubs(:get).raises(ex)
+    servers
+  end
+
+  def mock_cr_servers(cr, servers)
+    client = mock()
+    client.stubs(:servers).returns(servers)
+
+    cr.stubs(:client).returns(client)
+    cr
+  end
+
+  def assert_find_by_uuid_raises(ex_class, cr)
+    assert_raises(ex_class) do
+      cr.find_vm_by_uuid('abc')
+    end
+  end
+end
diff --git a/test/unit/compute_resources/ec2_test.rb b/test/unit/compute_resources/ec2_test.rb
index de3dd0834..579926e7e 100644
--- a/test/unit/compute_resources/ec2_test.rb
+++ b/test/unit/compute_resources/ec2_test.rb
@@ -1,10 +1,25 @@
 require 'test_helper'
+require 'unit/compute_resources/compute_resource_test_helpers'
 
 class EC2Test < ActiveSupport::TestCase
+  include ComputeResourceTestHelpers
+
   test "#associated_host matches any NIC" do
     host = FactoryGirl.create(:host, :ip => '10.0.0.154')
     cr = FactoryGirl.build(:ec2_cr)
     iface = mock('iface1', :public_ip_address => '10.0.0.154', :private_ip_address => "10.1.1.1")
     assert_equal host, as_admin { cr.associated_host(iface) }
   end
-end
\ No newline at end of file
+
+  describe "find_vm_by_uuid" do
+    it "raises RecordNotFound when the vm does not exist" do
+      cr = mock_cr_servers(Foreman::Model::EC2.new, empty_servers)
+      assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
+    end
+
+    it "raises RecordNotFound when the compute raises rackspace error" do
+      cr = mock_cr_servers(Foreman::Model::EC2.new, servers_raising_exception(Fog::Compute::AWS::Error))
+      assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
+    end
+  end
+end
diff --git a/test/unit/compute_resources/libvirt_test.rb b/test/unit/compute_resources/libvirt_test.rb
index 0ec93298c..718105e23 100644
--- a/test/unit/compute_resources/libvirt_test.rb
+++ b/test/unit/compute_resources/libvirt_test.rb
@@ -1,10 +1,49 @@
 require 'test_helper'
+require 'unit/compute_resources/compute_resource_test_helpers'
 
 class LibvirtTest < ActiveSupport::TestCase
+  include ComputeResourceTestHelpers
+
   test "#associated_host matches any NIC" do
     host = FactoryGirl.create(:host, :mac => 'ca:d0:e6:32:16:97')
     cr = FactoryGirl.build(:libvirt_cr)
     iface = mock('iface1', :mac => 'ca:d0:e6:32:16:97')
     assert_equal host, as_admin { cr.associated_host(iface) }
   end
-end
\ No newline at end of file
+
+  describe "find_vm_by_uuid" do
+    it "raises RecordNotFound when the vm does not exist" do
+      cr = mock_cr_servers(Foreman::Model::Libvirt.new, empty_servers)
+      assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
+    end
+
+    it "raises RecordNotFound when the compute raises retrieve error" do
+      cr = mock_cr_servers(Foreman::Model::Libvirt.new, servers_raising_exception(Libvirt::RetrieveError))
+      assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
+    end
+  end
+
+  describe "compute_attributes_for" do
+    test "returns memory in bytes" do
+      vm = mock()
+      vm.stubs(:attributes).returns({ :memory_size => 6 })
+
+      cr = FactoryGirl.build(:libvirt_cr)
+      cr.stubs(:find_vm_by_uuid).returns(vm)
+
+      attrs = cr.vm_compute_attributes_for('abc')
+      assert_equal 6*1024, attrs[:memory]
+    end
+
+    test "returns nil memory when :memory_size is not provided" do
+      vm = mock()
+      vm.stubs(:attributes).returns({})
+
+      cr = FactoryGirl.build(:libvirt_cr)
+      cr.stubs(:find_vm_by_uuid).returns(vm)
+
+      attrs = cr.vm_compute_attributes_for('abc')
+      assert_equal nil, attrs[:memory]
+    end
+  end
+end
diff --git a/test/unit/compute_resources/openstack_test.rb b/test/unit/compute_resources/openstack_test.rb
index 8c73390a3..75aa0d636 100644
--- a/test/unit/compute_resources/openstack_test.rb
+++ b/test/unit/compute_resources/openstack_test.rb
@@ -1,6 +1,9 @@
 require 'test_helper'
+require 'unit/compute_resources/compute_resource_test_helpers'
 
 class OpenstackTest < ActiveSupport::TestCase
+  include ComputeResourceTestHelpers
+
   setup do
     @compute_resource = FactoryGirl.build(:openstack_cr)
   end
@@ -23,6 +26,13 @@ class OpenstackTest < ActiveSupport::TestCase
                                 :flavor_ref => 'foo_flavor', :image_ref => 'foo_image')
   end
 
+  describe "find_vm_by_uuid" do
+    it "raises RecordNotFound when the vm does not exist" do
+      cr = mock_cr_servers(Foreman::Model::Openstack.new, empty_servers)
+      assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
+    end
+  end
+
   private
 
   def mocked_key_pair
diff --git a/test/unit/compute_resources/ovirt_test.rb b/test/unit/compute_resources/ovirt_test.rb
index da6cb193f..ae7266dbf 100644
--- a/test/unit/compute_resources/ovirt_test.rb
+++ b/test/unit/compute_resources/ovirt_test.rb
@@ -1,6 +1,9 @@
 require 'test_helper'
+require 'unit/compute_resources/compute_resource_test_helpers'
 
 class OvirtTest < ActiveSupport::TestCase
+  include ComputeResourceTestHelpers
+
   test "#associated_host matches any NIC" do
     host = FactoryGirl.create(:host, :mac => 'ca:d0:e6:32:16:97')
     cr = FactoryGirl.build(:ovirt_cr)
@@ -9,4 +12,16 @@ class OvirtTest < ActiveSupport::TestCase
     vm = mock('vm', :interfaces => [iface1, iface2])
     assert_equal host, as_admin { cr.associated_host(vm) }
   end
+
+  describe "find_vm_by_uuid" do
+    it "raises RecordNotFound when the vm does not exist" do
+      cr = mock_cr_servers(Foreman::Model::Ovirt.new, empty_servers)
+      assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
+    end
+
+    it "raises RecordNotFound when the compute raises retrieve error" do
+      cr = mock_cr_servers(Foreman::Model::Ovirt.new, servers_raising_exception(OVIRT::OvirtException.new('VM not found')))
+      assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
+    end
+  end
 end
diff --git a/test/unit/compute_resources/rackspace_test.rb b/test/unit/compute_resources/rackspace_test.rb
index aa8540966..a8fbe7d14 100644
--- a/test/unit/compute_resources/rackspace_test.rb
+++ b/test/unit/compute_resources/rackspace_test.rb
@@ -1,10 +1,25 @@
 require 'test_helper'
+require 'unit/compute_resources/compute_resource_test_helpers'
 
 class RackspaceTest < ActiveSupport::TestCase
+  include ComputeResourceTestHelpers
+
   test "#associated_host matches any NIC" do
     host = FactoryGirl.create(:host, :ip => '10.0.0.154')
     cr = FactoryGirl.build(:rackspace_cr)
     iface = mock('iface1', :public_ip_address => '10.0.0.154', :private_ip_address => "10.1.1.1")
     assert_equal host, as_admin { cr.associated_host(iface) }
   end
-end
\ No newline at end of file
+
+  describe "find_vm_by_uuid" do
+    it "raises RecordNotFound when the vm does not exist" do
+      cr = mock_cr_servers(Foreman::Model::Rackspace.new, empty_servers)
+      assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
+    end
+
+    it "raises RecordNotFound when the compute raises rackspace error" do
+      cr = mock_cr_servers(Foreman::Model::Rackspace.new, servers_raising_exception(Fog::Compute::Rackspace::Error))
+      assert_find_by_uuid_raises(ActiveRecord::RecordNotFound, cr)
+    end
+  end
+end
diff --git a/test/unit/host_test.rb b/test/unit/host_test.rb
index bfd909e22..48077fd05 100644
--- a/test/unit/host_test.rb
+++ b/test/unit/host_test.rb
@@ -148,7 +148,7 @@ class HostTest < ActiveSupport::TestCase
   end
 
   test "can fetch vm compute attributes" do
-    host = FactoryGirl.create(:host, :compute_resource => compute_resources(:one))
+    host = FactoryGirl.create(:host, :compute_resource => compute_resources(:ec2))
     ComputeResource.any_instance.stubs(:vm_compute_attributes_for).returns({:cpus => 4})
     assert_equal host.vm_compute_attributes, :cpus => 4
   end
