From b53416abce6eb9de2d358e9516540d50a58f8e87 Mon Sep 17 00:00:00 2001 From: Carl Caum Date: Fri, 28 Oct 2011 09:03:02 -0400 Subject: [PATCH 1/4] (#10055) Search vmFolder inventory vs children This patch changes how the `list_all_virtual_machine_mobs` method searches for virtual machines. Previously the search was based on children of the `dc.vmFolder` object. This makes the false assumption that all children are virtual machines. Now the search is based on the entire inventory of the `dc.vmFolder` object, which gives us more control over what we consider a virtual machine. This patch also adds a new `find_all_in_inventory` method; a generic method that can be used to search an inventory for any VMware object exposed by RbVmomi. Using `find_all_in_inventory` we are able to traverse VMware folders and pickout specific VMware object, in this case `RbVmomi::VIM::VirtualMachine` --- .../requests/compute/list_virtual_machines.rb | 31 ++++++++++++++++--- 1 file changed, 26 insertions(+), 5 deletions(-) diff --git a/lib/fog/vsphere/requests/compute/list_virtual_machines.rb b/lib/fog/vsphere/requests/compute/list_virtual_machines.rb index 0d2761511..47818773c 100644 --- a/lib/fog/vsphere/requests/compute/list_virtual_machines.rb +++ b/lib/fog/vsphere/requests/compute/list_virtual_machines.rb @@ -34,6 +34,7 @@ module Fog { 'virtual_machines' => virtual_machines } end + # NOTE: This is a private instance method required by the vm_clone # request. It's very hard to get the Managed Object Reference # of a Template because we can't search for it by instance_uuid @@ -45,15 +46,35 @@ module Fog datacenters = @connection.rootFolder.children.find_all do |child| child.kind_of? RbVmomi::VIM::Datacenter end - # Next, look in the "vmFolder" of each data center: + # Next, search the "vmFolder" inventory of each data center: datacenters.each do |dc| - dc.vmFolder.children.each do |vm| - virtual_machines << vm - end + inventory = dc.vmFolder.inventory( 'VirtualMachine' => :all ) + virtual_machines << find_all_in_inventory(inventory, :type => RbVmomi::VIM::VirtualMachine, :property => 'name' ) end - virtual_machines + + virtual_machines.flatten end + def find_all_in_inventory(inventory, properties = { :type => RbVmomi::VIM::VirtualMachine, :property => nil } ) + results = Array.new + + inventory.each do |k,v| + + # If we have a VMware folder we need to traverse the directory + # to ensure we pick VMs inside folders. So we do a bit of recursion + # here. + results << find_all_in_inventory(v) if k.is_a? RbVmomi::VIM::Folder + + if v[0].is_a? properties[:type] + if properties[:property].nil? + results << v[0] + else + results << v[1][properties[:property]] + end + end + end + results.flatten + end end class Mock From 762aa624ac07f76d774bc852e4737637273cea30 Mon Sep 17 00:00:00 2001 From: Carl Caum Date: Fri, 28 Oct 2011 10:10:37 -0400 Subject: [PATCH 2/4] Adding a path attribute to the vm_mob_ref hash This patch adds a new `path` attribute to the vm_mob_ref hash returned by the `convert_vm_mob_ref_to_attr_hash` method. In order to support the new `path` attribute, this patch also adds a new `get_folder_path` method to the `list_virtual_machines` vsphere module. --- lib/fog/vsphere/compute.rb | 1 + lib/fog/vsphere/models/compute/server.rb | 1 + lib/fog/vsphere/requests/compute/list_virtual_machines.rb | 7 +++++++ 3 files changed, 9 insertions(+) diff --git a/lib/fog/vsphere/compute.rb b/lib/fog/vsphere/compute.rb index a2e85a916..214e04b0a 100644 --- a/lib/fog/vsphere/compute.rb +++ b/lib/fog/vsphere/compute.rb @@ -58,6 +58,7 @@ module Fog attrs['mo_ref'] = vm_mob_ref._ref attrs['hypervisor'] = attrs['host'].name attrs['mac_addresses'] = vm_mob_ref.macs + attrs['path'] = get_folder_path(vm_mob_ref.parent) end end diff --git a/lib/fog/vsphere/models/compute/server.rb b/lib/fog/vsphere/models/compute/server.rb index 49406a33b..0c7aa4738 100644 --- a/lib/fog/vsphere/models/compute/server.rb +++ b/lib/fog/vsphere/models/compute/server.rb @@ -33,6 +33,7 @@ module Fog attribute :is_a_template attribute :connection_state attribute :mo_ref + attribute :path def start(options = {}) requires :instance_uuid diff --git a/lib/fog/vsphere/requests/compute/list_virtual_machines.rb b/lib/fog/vsphere/requests/compute/list_virtual_machines.rb index 47818773c..1e68ddf0d 100644 --- a/lib/fog/vsphere/requests/compute/list_virtual_machines.rb +++ b/lib/fog/vsphere/requests/compute/list_virtual_machines.rb @@ -75,6 +75,13 @@ module Fog end results.flatten end + + def get_folder_path(folder, root = nil) + if ( not folder.methods.include?('parent') ) or ( folder == root ) + return + end + "#{get_folder_path(folder.parent)}/#{folder.name}" + end end class Mock From bc74e06d128ba2cdef60534eecb27174af388219 Mon Sep 17 00:00:00 2001 From: Kelsey Hightower Date: Mon, 7 Nov 2011 07:08:54 -0500 Subject: [PATCH 3/4] (#10570) Use nil in-place of missing attributes Without this patch, `Fog::Compute::Vsphere#convert_vm_mob_ref_to_attr_hash` method produces unhandled exceptions during VMware cloning and listing operations. The root cause of these exceptions are based on the fact that some VMware virtual machine attributes: hypervisor name, and macaddress, are not available until the cloning process has finished. These exceptions can be triggered when external events take place within the VMware infrastructure such as end-users cloning machines via some other VMware management tool. This patch solves the problem by catching any exceptions that occur during attribute lookups for both the hypervisor name and the virtual machine macaddress, and setting them to nil. This patch also removes the host attribute from the hash generated by the `Fog::Compute::Vsphere#convert_vm_mob_ref_to_attr_hash` method. The hypervisor attribute is added instead, which is an alias to host. This patch changes the behaviour of the `Fog::Compute::Vsphere#convert_vm_mob_ref_to_attr_hash` method by catching exceptions for missing attributes, and setting them to nil. --- lib/fog/vsphere/compute.rb | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/lib/fog/vsphere/compute.rb b/lib/fog/vsphere/compute.rb index 214e04b0a..55acba26d 100644 --- a/lib/fog/vsphere/compute.rb +++ b/lib/fog/vsphere/compute.rb @@ -39,7 +39,7 @@ module Fog :ipaddress => 'guest.ipAddress', :power_state => 'runtime.powerState', :connection_state => 'runtime.connectionState', - :host => 'runtime.host', + :hypervisor => 'runtime.host', :tools_state => 'guest.toolsStatus', :tools_version => 'guest.toolsVersionStatus', :is_a_template => 'config.template', @@ -53,11 +53,26 @@ module Fog return nil unless vm_mob_ref props = vm_mob_ref.collect! *ATTR_TO_PROP.values.uniq + # NOTE: Object.tap is in 1.8.7 and later. + # Here we create the hash object that this method returns, but first we need + # to add a few more attributes that require additional calls to the vSphere + # API. The hypervisor name and mac_addresses attributes may not be available + # so we need catch any exceptions thrown during lookup and set them to nil. + # + # The use of the "tap" method here is a convience, it allows us to update the + # hash object without expliclty returning the hash at the end of the method. Hash[ATTR_TO_PROP.map { |k,v| [k.to_s, props[v]] }].tap do |attrs| attrs['id'] ||= vm_mob_ref._ref attrs['mo_ref'] = vm_mob_ref._ref - attrs['hypervisor'] = attrs['host'].name - attrs['mac_addresses'] = vm_mob_ref.macs + # The name method "magically" appears after a VM is ready and + # finished cloning. + if attrs['hypervisor'].kind_of?(RbVmomi::VIM::HostSystem) then + # If it's not ready, set the hypervisor to nil + attrs['hypervisor'] = attrs['hypervisor'].name rescue nil + end + # This inline rescue catches any standard error. While a VM is + # cloning, a call to the macs method will throw and NoMethodError + attrs['mac_addresses'] = vm_mob_ref.macs rescue nil attrs['path'] = get_folder_path(vm_mob_ref.parent) end end From a65db7f594c6c5fde37d4c393e6438e1d0f71163 Mon Sep 17 00:00:00 2001 From: Kelsey Hightower Date: Tue, 8 Nov 2011 00:47:27 -0500 Subject: [PATCH 4/4] (#10570) Update `Fog::Compute::Vsphere` tests Update `Fog::Compute::Vsphere` tests to reflect the way the `Fog::Compute::Vsphere#convert_vm_mob_ref_to_attr_hash` method currently converts a `RbVmomi::VIM::ManagedObject` object to a hash. Without this patch, tests related to converting an instance of `RbVmomi::VIM::ManagedObject` to a hash will raise an exception: NoMethodError: undefined method `collect!' for Hash; causing the test to fail. This patch solves the problem by mocking `RbVmomi::VIM::ManagedObject` with a new `MockManagedObject` class, which provides a `collect!` method, and the `_ref` and `parent` attributes. This patch renames fake_vm to fake_vm_mob_ref in order to provide a more descriptive name for what's actually being tested. The `Fog::Compute::Vshpere::Mock` class has been updated to require the rbvmomi library, which prevents an `NameError` exception from being raised due to the `Fog::Compute::Vsphere::Shared::RbVmomi` constant not being initialized. The `Fog::Compute::Vshpere::Mock` class has been updated with a `get_folder_path` method, which prevents a `NoMethodError` exception from being raised due to the `get_folder_path` method being undefined. --- lib/fog/vsphere/compute.rb | 1 + .../requests/compute/list_virtual_machines.rb | 4 +++ tests/vsphere/compute_tests.rb | 28 ++++++++++++------- 3 files changed, 23 insertions(+), 10 deletions(-) diff --git a/lib/fog/vsphere/compute.rb b/lib/fog/vsphere/compute.rb index 55acba26d..7292cb01e 100644 --- a/lib/fog/vsphere/compute.rb +++ b/lib/fog/vsphere/compute.rb @@ -84,6 +84,7 @@ module Fog include Shared def initialize(options={}) + require 'rbvmomi' @vsphere_username = options[:vsphere_username] @vsphere_password = 'REDACTED' @vsphere_server = options[:vsphere_server] diff --git a/lib/fog/vsphere/requests/compute/list_virtual_machines.rb b/lib/fog/vsphere/requests/compute/list_virtual_machines.rb index 1e68ddf0d..84a48cb40 100644 --- a/lib/fog/vsphere/requests/compute/list_virtual_machines.rb +++ b/lib/fog/vsphere/requests/compute/list_virtual_machines.rb @@ -86,6 +86,10 @@ module Fog class Mock + def get_folder_path(folder, root = nil) + nil + end + def list_virtual_machines(options = {}) case options['instance_uuid'] when nil diff --git a/tests/vsphere/compute_tests.rb b/tests/vsphere/compute_tests.rb index 13ff9c946..57a02f2e9 100644 --- a/tests/vsphere/compute_tests.rb +++ b/tests/vsphere/compute_tests.rb @@ -3,21 +3,28 @@ Shindo.tests('Fog::Compute[:vsphere]', ['vsphere']) do compute = Fog::Compute[:vsphere] tests("| convert_vm_mob_ref_to_attr_hash") do - require 'ostruct' + # Mock the RbVmomi::VIM::ManagedObject class + class MockManagedObject - fake_vm = OpenStruct.new({ - :_ref => 'vm-123', - :name => 'fakevm', - :summary => OpenStruct.new(:guest => OpenStruct.new), - :runtime => OpenStruct.new, - }) + attr_reader :parent, :_ref + + def initialize + @parent = @_ref = 'vm-123' + end + + def collect! *pathSet + { '_ref' => 'vm-123', 'name' => 'fakevm' } + end + end + + fake_vm_mob_ref = MockManagedObject.new tests("When converting an incomplete vm object") do test("it should return a Hash") do - compute.convert_vm_mob_ref_to_attr_hash(fake_vm).kind_of? Hash + compute.convert_vm_mob_ref_to_attr_hash(fake_vm_mob_ref).kind_of? Hash end tests("The converted Hash should") do - attr_hash = compute.convert_vm_mob_ref_to_attr_hash(fake_vm) + attr_hash = compute.convert_vm_mob_ref_to_attr_hash(fake_vm_mob_ref) test("have a name") { attr_hash['name'] == 'fakevm' } test("have a mo_ref") {attr_hash['mo_ref'] == 'vm-123' } test("have an id") { attr_hash['id'] == 'vm-123' } @@ -38,10 +45,11 @@ Shindo.tests('Fog::Compute[:vsphere]', ['vsphere']) do test("it should respond to #{attr}") { compute.respond_to? attr } end end + tests("Compute collections") do %w{ servers }.each do |collection| test("it should respond to #{collection}") { compute.respond_to? collection } end end - end +