Merge pull request #2356 from fog/improve_vsphere_network_interfaces

Improve vsphere network interfaces

lib/fog/vsphere/models/compute/interface.rb

@@ -4,6 +4,8 @@ module Fog
 
       class Interface < Fog::Model
 
+        SAVE_MUTEX = Mutex.new
+
         identity :mac
         alias :id :mac
 
@@ -14,11 +16,18 @@ module Fog
         attribute :type
         attribute :key
         attribute :virtualswitch
+        attribute :server_id
+
+        def initialize(attributes = {})
+          # Assign server first to prevent race condition with persisted?
+          self.server_id = attributes.delete(:server_id)
 
-        def initialize(attributes={} )
           if attributes.has_key? :type and attributes[:type].is_a? String then
-             attributes[:type]=Fog.class_from_string(attributes[:type], "RbVmomi::VIM")
+            attributes[:type] = Fog.class_from_string(attributes[:type], "RbVmomi::VIM")
+          else
+            attributes[:type] = Fog.class_from_string("VirtualE1000", "RbVmomi::VIM")
           end
+
           super defaults.merge(attributes)
         end
 
@@ -26,6 +35,45 @@ module Fog
           name
         end
 
+        def server
+          requires :server_id
+          service.servers.get(server_id)
+        end
+
+        def destroy
+          requires :server_id, :key, :type
+
+          service.destroy_vm_interface(server_id, :key => key, :type => type)
+        end
+
+        def save
+          raise Fog::Errors::Error.new('Resaving an existing object may create a duplicate') if persisted?
+          requires :server_id, :type, :network
+
+          # Our approach of finding the newly created interface is rough.  We assume that the :key value always increments
+          # and thus the highest :key value must correspond to the created interface.  Since this has an inherent race
+          # condition we need to gate the saves.
+          SAVE_MUTEX.synchronize do
+            data = service.add_vm_interface(server_id, attributes)
+
+            if data['task_state'] == 'success'
+              # We have to query vSphere to get the interface attributes since the task handle doesn't include that info.
+              created = server.interfaces.all.sort_by(&:key).last
+
+              self.mac = created.mac
+              self.name = created.name
+              self.status = created.status
+              self.summary = created.summary
+              self.key = created.key
+              self.virtualswitch = created.virtualswitch
+
+              true
+            else
+              false
+            end
+          end
+        end
+
         private
 
         def defaults

lib/fog/vsphere/models/compute/interfaces.rb

@@ -9,36 +9,51 @@ module Fog
 
         model Fog::Compute::Vsphere::Interface
 
-        attr_accessor :vm
+        attribute :server
 
         def all(filters = {})
-          requires :vm
-          case vm
+          requires :server
+
+          case server
             when Fog::Compute::Vsphere::Server
-              load service.list_vm_interfaces(vm.id)
+              load service.list_vm_interfaces(server.id)
             when Fog::Compute::Vsphere::Template
-              load service.list_template_interfaces(vm.id)
+              load service.list_template_interfaces(server.id)
             else
             raise 'interfaces should have vm or template'
           end
+
+          self.each { |interface| interface.server_id = server.id }
+          self
         end
 
         def get(id)
-          requires :vm
-          case vm
+          requires :server
+
+          case server
             when Fog::Compute::Vsphere::Server
-              interface=service.get_vm_interface(vm.id, :key => id, :mac=> id, :name => id)
+              interface = service.get_vm_interface(server.id, :key => id, :mac=> id, :name => id)
             when Fog::Compute::Vsphere::Template
-              interface=service.get_template_interfaces(vm.id, :key => id, :mac=> id, :name => id)
+              interface = service.get_template_interfaces(server.id, :key => id, :mac=> id, :name => id)
             else
+
             raise 'interfaces should have vm or template'
           end
+
           if interface 
-            Fog::Compute::Vsphere::Interface.new(interface) 
+            Fog::Compute::Vsphere::Interface.new(interface.merge(:server_id => server.id, :service => service))
           else
             nil         
           end
         end
+
+        def new(attributes = {})
+          if server
+            super({ :server_id => server.id  }.merge(attributes))
+          else
+            super
+          end
+        end
      end
     end
   end

lib/fog/vsphere/models/compute/server.rb

@@ -184,7 +184,7 @@ module Fog
         end
 
         def interfaces
-          attributes[:interfaces] ||= id.nil? ? [] : service.interfaces( :vm => self )
+          attributes[:interfaces] ||= id.nil? ? [] : service.interfaces( :server => self )
         end
         
         def interface_ready? attrs
@@ -192,8 +192,9 @@ module Fog
         end
 
         def add_interface attrs
-          wait_for { not ready? } if interface_ready? attrs
-          service.add_vm_interface(id, attrs)
+          Fog::Logger.deprecation("<server>.add_interface is deprecated. Call <server>.interfaces.create instead.")
+
+          interfaces.create(attrs)
         end
 
         def update_interface attrs
@@ -202,8 +203,9 @@ module Fog
         end
 
         def destroy_interface attrs
-          wait_for { not ready? } if interface_ready? attrs
-          service.destroy_vm_interface(id, attrs)
+          Fog::Logger.deprecation("<server>.destroy_vm_interface is deprecated. Call <server>.interfaces.get(:key => <nic_key>).destroy instead.")
+
+          interfaces.get(attrs[:key] || attrs['key']).destroy
         end
 
         def volumes

lib/fog/vsphere/requests/compute/modify_vm_interface.rb

@@ -6,33 +6,36 @@ module Fog
         def add_vm_interface(vmid, options = {})
           raise ArgumentError, "instance id is a required parameter" unless vmid
 
-          interface=get_interface_from_options(vmid, options)
+          interface = get_interface_from_options(vmid, options.merge(:server_id => vmid))
           vm_reconfig_hardware('instance_uuid' => vmid, 'hardware_spec' => {'deviceChange'=>[create_interface(interface)]})
         end
 
         def destroy_vm_interface(vmid, options = {})
           raise ArgumentError, "instance id is a required parameter" unless vmid
 
-          interface=get_vm_interface(vmid, options)
+          interface = get_interface_from_options(vmid, options.merge(:server_id => vmid))
           vm_reconfig_hardware('instance_uuid' => vmid, 'hardware_spec' => {'deviceChange'=>[create_interface(interface, interface.key, :remove)]})
         end
         
         def update_vm_interface(vmid, options = {})
           raise ArgumentError, "instance id is a required parameter" unless vmid
 
-          interface=get_vm_interface(vmid, options)
-          vm_reconfig_hardware('instance_uuid' => vmid, 'hardware_spec' => {'deviceChange'=>[create_interface(interface, interface.key, :edit)]})          
+          interface = get_interface_from_options(vmid, options.merge(:server_id => vmid))
+          vm_reconfig_hardware('instance_uuid' => vmid, 'hardware_spec' => {'deviceChange'=>[create_interface(interface, interface.key, :edit)]})
         end
 
         private
         def get_interface_from_options(vmid, options)
           if options and options[:interface]
             options[:interface]
+
           elsif options[:key] and options[:key]>0
-            oldattributes=get_vm_interface(vmid, options)
-            Fog::Compute::Vsphere::Interface.new oldattributes.merge options
+            oldattributes = get_vm_interface(vmid, options)
+            Fog::Compute::Vsphere::Interface.new(oldattributes.merge(options))
+
           elsif options[:type] and options[:network]
             Fog::Compute::Vsphere::Interface.new options
+
           else
             raise ArgumentError, "interface is a required parameter or pass options with type and network"
           end