Merge pull request #592 from kelseyhightower/ticket/master/10570_use_nil_for_missing_attributes

vSphere usability improvements

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,27 @@ 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
 
@@ -68,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]

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

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,19 +46,50 @@ 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
+
+        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
 
+        def get_folder_path(folder, root = nil)
+          nil
+        end
+
         def list_virtual_machines(options = {})
           case options['instance_uuid']
           when nil

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
+