commit 80ad15ca975c741c2cbdcb1e44eb1eb161e1034c
Author: Dominic Cleal <dcleal@redhat.com>
Date:   Fri Apr 25 18:42:58 2014 +0100

    fixes #5453 - ensure all VMware compute attribute keys are symbolized

diff --git a/app/models/compute_resources/foreman/model/vmware.rb b/app/models/compute_resources/foreman/model/vmware.rb
index ce41c9b1b..433d7cc1c 100644
--- a/app/models/compute_resources/foreman/model/vmware.rb
+++ b/app/models/compute_resources/foreman/model/vmware.rb
@@ -231,8 +231,9 @@ module Foreman::Model
         raise "Unknown Network ID: #{interface["network"]}" if net.nil?
         interface["network"] = net.name
       end
+      args = args.symbolize_keys
 
-      # convert rails nested_attributes into a plain hash
+      # convert rails nested_attributes into a plain, symbolized hash
       [:interfaces, :volumes].each do |collection|
         nested_attrs = args.delete("#{collection}_attributes".to_sym)
         args[collection] = nested_attributes_for(collection, nested_attrs) if nested_attrs
@@ -248,7 +249,7 @@ module Foreman::Model
       test_connection
       return unless errors.empty?
 
-      if args["image_id"]
+      if args[:image_id].present?
         clone_vm(args)
       else
         vm = new_vm(args)
@@ -261,7 +262,7 @@ module Foreman::Model
     end
 
     def new_vm args
-      opts = vm_instance_defaults.merge(args.to_hash).symbolize_keys
+      opts = vm_instance_defaults.symbolize_keys.merge(args.symbolize_keys)
       client.servers.new opts
     end
 
@@ -282,21 +283,21 @@ module Foreman::Model
     def clone_vm args
       path_replace = %r{/Datacenters/#{datacenter}/vm(/|)}
 
-      interfaces = client.list_vm_interfaces(args["image_id"])
+      interfaces = client.list_vm_interfaces(args[:image_id])
       interface = interfaces.detect{|i| i[:name] == "Network adapter 1" }
       network_adapter_device_key = interface[:key]
 
       opts = {
         "datacenter" => datacenter,
-        "template_path" => args["image_id"],
-        "dest_folder" => args["path"].gsub(path_replace, ''),
+        "template_path" => args[:image_id],
+        "dest_folder" => args[:path].gsub(path_replace, ''),
         "power_on" => false,
-        "start" => args["start"],
-        "name" => args["name"],
-        "numCPUs" => args["cpus"],
-        "memoryMB" => args["memory_mb"],
-        "datastore" => args["volumes"].first["datastore"],
-        "network_label" => args["interfaces"].first["network"],
+        "start" => args[:start],
+        "name" => args[:name],
+        "numCPUs" => args[:cpus],
+        "memoryMB" => args[:memory_mb],
+        "datastore" => args[:volumes].first[:datastore],
+        "network_label" => args[:interfaces].first[:network],
         "network_adapter_device_key" => network_adapter_device_key
       }
       client.servers.get(client.vm_clone(opts)['new_vm']['id'])
diff --git a/test/factories/compute_resources.rb b/test/factories/compute_resources.rb
index 062a2dc94..49b00583f 100644
--- a/test/factories/compute_resources.rb
+++ b/test/factories/compute_resources.rb
@@ -8,6 +8,7 @@ FactoryGirl.define do
       user 'ec2user'
       password 'ec2password'
       url 'eu-west-1'
+      after_build { |host| host.class.skip_callback(:create, :after, :setup_key_pair) }
     end
 
     trait :gce do
@@ -25,12 +26,14 @@ FactoryGirl.define do
       provider 'Openstack'
       user 'osuser'
       password 'ospassword'
+      after_build { |host| host.class.skip_callback(:create, :after, :setup_key_pair) }
     end
 
     trait :ovirt do
       provider 'Ovirt'
       user 'ovirtuser'
       password 'ovirtpassword'
+      after_build { |host| host.class.skip_callback(:create, :before, :update_public_key) }
     end
 
     trait :rackspace do
@@ -46,14 +49,15 @@ FactoryGirl.define do
       password 'vpassword'
       sequence(:server) { |n| "#{n}.example.com" }
       datacenter 'vdatacenter'
+      after_build { |host| host.class.skip_callback(:create, :before, :update_public_key) }
     end
 
-    factory :ec2_cr, :traits => [:ec2]
-    factory :gce_cr, :traits => [:gce]
-    factory :libvirt_cr, :traits => [:libvirt]
-    factory :openstack_cr, :traits => [:openstack]
-    factory :ovirt_cr, :traits => [:ovirt]
-    factory :rackspace_cr, :traits => [:rackspace]
-    factory :vmware_cr, :traits => [:vmware]
+    factory :ec2_cr, :class => Foreman::Model::EC2, :traits => [:ec2]
+    factory :gce_cr, :class => Foreman::Model::GCE, :traits => [:gce]
+    factory :libvirt_cr, :class => Foreman::Model::Libvirt, :traits => [:libvirt]
+    factory :openstack_cr, :class => Foreman::Model::Openstack, :traits => [:openstack]
+    factory :ovirt_cr, :class => Foreman::Model::Ovirt, :traits => [:ovirt]
+    factory :rackspace_cr, :class => Foreman::Model::Rackspace, :traits => [:rackspace]
+    factory :vmware_cr, :class => Foreman::Model::Vmware, :traits => [:vmware]
   end
 end
diff --git a/test/unit/compute_resources/vmware_test.rb b/test/unit/compute_resources/vmware_test.rb
new file mode 100644
index 000000000..5bcc83364
--- /dev/null
+++ b/test/unit/compute_resources/vmware_test.rb
@@ -0,0 +1,54 @@
+require 'test_helper'
+
+class VmwareTest < ActiveSupport::TestCase
+  test "#create_vm calls new_vm when network provisioning" do
+    attrs_in = HashWithIndifferentAccess.new("cpus"=>"1", "interfaces_attributes"=>{"new_interfaces"=>{"type"=>"VirtualE1000", "network"=>"network-17", "_delete"=>""}, "0"=>{"type"=>"VirtualVmxnet3", "network"=>"network-17", "_delete"=>""}}, "volumes_attributes"=>{"new_volumes"=>{"size_gb"=>"10", "_delete"=>""}, "0"=>{"size_gb"=>"1", "_delete"=>""}})
+    # All keys must be symbolized
+    attrs_out = {:cpus=>"1", :interfaces=>[{:type=>"VirtualVmxnet3", :network=>"Test network", :_delete=>""}], :volumes=>[{:size_gb=>"1", :_delete=>""}]}
+
+    mock_vm = mock('vm')
+    mock_vm.expects(:save).returns(mock_vm)
+    mock_network = mock('network')
+    mock_network.stubs('id').returns('network-17')
+    mock_network.stubs('name').returns('Test network')
+
+    cr = FactoryGirl.build(:vmware_cr)
+    cr.expects(:new_vm).with(attrs_out).returns(mock_vm)
+    cr.expects(:test_connection)
+    cr.expects(:networks).returns([mock_network])
+    assert_equal mock_vm, cr.create_vm(attrs_in)
+  end
+
+  test "#new_vm merges defaults with user args and creates server" do
+    attrs_in = {:cpus=>"1", :interfaces=>[{:type=>"VirtualVmxnet3", :network=>"Test network", :_delete=>""}], :volumes=>[{:size_gb=>"1", :_delete=>""}]}
+    attrs_out = {:name => 'test', :cpus=>"1", :interfaces=>[{:type=>"VirtualVmxnet3", :network=>"Test network", :_delete=>""}], :volumes=>[{:size_gb=>"1", :_delete=>""}]}
+
+    mock_vm = mock('new server')
+    mock_servers = mock('client.servers')
+    mock_servers.expects(:new).with(attrs_out).returns(mock_vm)
+    mock_client = mock('client')
+    mock_client.expects(:servers).returns(mock_servers)
+
+    cr = FactoryGirl.build(:vmware_cr)
+    cr.expects(:vm_instance_defaults).returns(HashWithIndifferentAccess.new(:name => 'test', :cpus => '2', :interfaces => [mock('iface')], :volumes => [mock('vol')]))
+    cr.expects(:client).returns(mock_client)
+    assert_equal mock_vm, cr.new_vm(attrs_in)
+  end
+
+  test "#create_vm calls clone_vm when image provisioning" do
+    attrs_in = HashWithIndifferentAccess.new("image_id"=>"2","cpus"=>"1", "interfaces_attributes"=>{"new_interfaces"=>{"type"=>"VirtualE1000", "network"=>"network-17", "_delete"=>""}, "0"=>{"type"=>"VirtualVmxnet3", "network"=>"network-17", "_delete"=>""}}, "volumes_attributes"=>{"new_volumes"=>{"size_gb"=>"10", "_delete"=>""}, "0"=>{"size_gb"=>"1", "_delete"=>""}})
+    # All keys must be symbolized
+    attrs_out = {:image_id=>"2", :cpus=>"1", :interfaces=>[{:type=>"VirtualVmxnet3", :network=>"Test network", :_delete=>""}], :volumes=>[{:size_gb=>"1", :_delete=>""}]}
+
+    mock_vm = mock('vm')
+    mock_network = mock('network')
+    mock_network.stubs('id').returns('network-17')
+    mock_network.stubs('name').returns('Test network')
+
+    cr = FactoryGirl.build(:vmware_cr)
+    cr.expects(:clone_vm).with(attrs_out).returns(mock_vm)
+    cr.expects(:test_connection)
+    cr.expects(:networks).returns([mock_network])
+    assert_equal mock_vm, cr.create_vm(attrs_in)
+  end
+end
