* Added some more tests

* Lots of fixes and some refactoring

lib/fog/xenserver.rb

@@ -37,7 +37,7 @@ module Fog
               response = eval("@factory.call('#{method}', '#{@credentials}', #{params.map {|p|  p.is_a?(String) ? "'#{p}'" : p}.join(',')})")
             end
           end
-          raise RequestFailed.new(response["ErrorDescription"].to_s) unless response["Status"].eql? "Success"
+          raise RequestFailed.new("#{method}: " + response["ErrorDescription"].to_s) unless response["Status"].eql? "Success"
           if parser
             parser.parse( response["Value"] )
             response = parser.response

lib/fog/xenserver/compute.rb

@@ -73,12 +73,13 @@ module Fog
 
         def default_template
           return nil if @defaults[:template].nil?
-          servers.all(:name_matches => @defaults[:template], :include_templates => true,
+          (servers.custom_templates + servers.builtin_templates).find do |s|
-                      :include_custom_templates => true ).first
+            (s.name == @defaults[:template]) or (s.uuid == @defaults[:template])
+          end
         end
         
         def default_network
-          Fog::XenServer::Network.new( get_network( @defaults[:network] ) ) if @defaults[:network]
+          net = networks.find { |n| n.name == (@defaults[:network] || "Pool-wide network associated with eth0") }
         end
         
       end

lib/fog/xenserver/models/compute/host.rb

@@ -23,16 +23,6 @@ module Fog
         attribute :__pifs,            :aliases => :PIFs
         attribute :__resident_vms,      :aliases => :resident_VMs
         
-        def templates
-          connection.servers.all(:include_templates => true).delete_if { |s| !s.is_a_template }
-        end
-
-        def custom_templates
-          connection.servers.all(:include_custom_templates => true).delete_if do |s| 
-            !s.is_a_template
-          end
-        end
-
         def pifs
           __pifs.collect { |pif| connection.pifs.get pif }
         end

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

@@ -50,9 +50,15 @@ module Fog
         end
 
         def destroy
-          stop('hard') if running?
+          # Make sure it's halted
+          stop('hard')
           vbds.each do |vbd|
-            connection.destroy_vdi( vbd.vdi.reference ) if vbd.type == "Disk"
+            if vbd.type == "Disk"
+              connection.destroy_vbd( vbd.reference ) \
+                if vbd.allowed_operations.include?("unplug")
+              connection.destroy_vdi( vbd.vdi.reference ) \
+                if vbd.vdi.allowed_operations.include?("destroy")
+            end
           end
           connection.destroy_server( reference )
           true
@@ -65,12 +71,12 @@ module Fog
         end
 
         def vifs
-          networks
+          __vifs.collect { |vif| connection.vifs.get vif }
         end
 
         # associations
         def networks
-          __vifs.collect { |vif| vifs.get vif }
+          vifs.collect { |v| v.network }
         end
 
         def resident_on
@@ -102,11 +108,13 @@ module Fog
         end
         
         def running?
-          power_state =~ /Running/
+          reload
+          power_state == "Running"
         end
         
         def halted?
-          power_state =~ /Halted/
+          reload
+          power_state == "Halted"
         end
         
         # operations
@@ -118,8 +126,9 @@ module Fog
 
         def save(params = {})
           requires :name
-          new_vm = connection.create_server( name, template_name, nil) 
+          networks = params[:networks] || []
-          merge_attributes(new_vm.attributes)
+          attributes = connection.get_record(connection.create_server( name, template_name, networks ), 'VM')
+          merge_attributes attributes
           true
         end
 
@@ -139,7 +148,7 @@ module Fog
         def stop(stype = 'clean')
           return false if !running?
           connection.shutdown_server( reference, stype )
-          wait_for { !running? }
+          wait_for { power_state == 'Halted' }
           true
         end
 

lib/fog/xenserver/models/compute/servers.rb

@@ -9,25 +9,30 @@ module Fog
 
         model Fog::Compute::XenServer::Server
 
+        def templates
+          custom_templates + builtin_templates
+        end
+
+        def custom_templates
+          data = connection.get_records 'VM'
+          data.delete_if do |vm|
+            !vm[:is_a_template] or !vm[:other_config]['default_template'].nil? 
+          end
+          load(data)
+        end
+        
+        def builtin_templates
+          data = connection.get_records 'VM'
+          data.delete_if do |vm|
+            !vm[:is_a_template] or vm[:other_config]['default_template'].nil?
+          end
+          load(data)
+        end
+
         def all(options = {})
           data = connection.get_records 'VM'
           # Exclude templates
-          data.delete_if { |vm| 
+          data.delete_if { |vm| vm[:is_control_domain] or vm[:is_a_template] }
-            vm[:is_a_template] and (!options[:include_templates] and !options[:include_custom_templates])
-          }
-          data.delete_if { |vm| 
-            # VM is a custom template
-            if vm[:is_a_template] and vm[:allowed_operations].include?("destroy")
-              !options[:include_custom_templates]
-            end
-          }
-          data.delete_if { |vm| 
-            # VM is a built-in template
-            if vm[:is_a_template] and !vm[:allowed_operations].include?("destroy")
-              !options[:include_templates]
-            end
-          }
-          data.delete_if { |vm| vm[:is_control_domain] }
           data.delete_if { |vm| vm[:is_a_snapshot] and !options[:include_snapshots] }
           data.delete_if { |vm| options[:name_matches] and (vm[:name_label] !~ /#{Regexp.escape(options[:name_matches])}/i ) }
           data.delete_if { |vm| options[:name_equals] and (vm[:name_label] != options[:name_equals] ) }
@@ -35,7 +40,8 @@ module Fog
         end
 
         def get_by_name( name )
-          all(:name_equals => name).first
+          ref = connection.get_vm_by_name( name )
+          get ref
         end
 
         def get( vm_ref )

lib/fog/xenserver/models/compute/vbd.rb

@@ -12,14 +12,22 @@ module Fog
         
         attribute :uuid
         attribute :currently_attached
+        attribute :allowed_operations
+        attribute :current_operations
         attribute :reserved
         attribute :__vdi,               :aliases => :VDI
         attribute :__vm,                :aliases => :VM
         attribute :device
         attribute :status_detail
+        attribute :status_code
         attribute :type
         attribute :userdevice
-        
+        attribute :empty
+        attribute :type
+        attribute :mode        
+        attribute :runtime_properties
+        attribute :unpluggable
+
         #
         # May return nil
         #

lib/fog/xenserver/models/compute/vbds.rb

@@ -15,7 +15,6 @@ module Fog
 
         def all(options = {})
           data = connection.get_records 'VBD'
-          #data.delete_if { |vm| vm[:is_a_template] and !options[:include_templates] }
           load(data)
         end
 

lib/fog/xenserver/models/compute/vdi.rb

@@ -20,6 +20,8 @@ module Fog
         attribute :__sr, :aliases => :SR
         attribute :sharable
         attribute :readonly
+        attribute :current_operations
+        attribute :allowed_operations
         
         def initialize(attributes={})
           @uuid ||= 0

lib/fog/xenserver/models/compute/vdis.rb

@@ -15,7 +15,6 @@ module Fog
 
         def all(options = {})
           data = connection.get_records 'VDI'
-          #data.delete_if { |vm| vm[:is_a_template] and !options[:include_templates] }
           load(data)
         end
 

lib/fog/xenserver/parser.rb

@@ -14,15 +14,17 @@ module Fog
         end
         
         def parse( data )
-          if data.is_a? Hash
+          if data.kind_of? Hash
             @response = data.symbolize_keys!
             @response.each do |k,v|
               if @response[k] == "OpaqueRef:NULL"
                 @response[k] = nil
               end
             end
-          elsif data.is_a? Array
+          elsif data.kind_of? Array
             @response = data.first
+          elsif data.kind_of?(String) and data =~ /OpaqueRef:/
+            @response = data
           end
           
           @response

lib/fog/xenserver/parsers/get_vms.rb

@@ -1,6 +1,7 @@
 module Fog
   module Parsers
     module XenServer
+
       class GetVms < Fog::Parsers::XenServer::Base
         
         def reset

lib/fog/xenserver/requests/compute/create_server.rb

@@ -2,22 +2,30 @@ module Fog
   module Compute
     class XenServer
       class Real
+        
+        require 'fog/xenserver/parsers/get_vms'
 
         def get_vm_by_name(label)
           @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VM.get_by_name_label' }, label)
         end
         
-        def create_server( name_label, template = nil, network = nil, extra_args = {})
+        def create_server( name_label, template = nil, networks = [], extra_args = {})
-          template ||= default_template
+          if !networks.kind_of? Array
-          network ||= default_network
+            raise "Invalid networks argument"
-          
-          if template.nil?
-            raise "Invalid template"
           end
 
           if template.kind_of? String
             template_string = template
-            template = servers.get get_vm_by_name(template_string)
+            # try template by UUID 
+            template = servers.templates.find { |s| s.uuid == template_string }
+            if template.nil?
+              # Try with the template name just in case
+              template = servers.get get_vm_by_name(template_string)
+            end
+          end
+
+          if template.nil?
+            raise "Invalid template"
           end
 
           #FIXME: need to check that template exist actually
@@ -25,16 +33,18 @@ module Fog
           raise 'Clone Operation not Allowed' unless template.allowed_operations.include?('clone')
 
           # Clone the VM template
-          @connection.request(
+          ref = @connection.request(
             {:parser => Fog::Parsers::XenServer::Base.new, :method => 'VM.clone'}, 
             template.reference, name_label
           )
-          new_vm = servers.get get_vm_by_name( name_label )
+          networks.each do |n|
-
+            create_vif ref, n.reference
-          @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VM.provision'}, new_vm.reference)
+          end
-          start_vm( new_vm.reference ) unless extra_args[:auto_start] == false
+          #new_vm = servers.get get_vm_by_name( name_label )
+          @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VM.provision'}, ref)
+          start_vm( ref ) unless extra_args[:auto_start] == false
           
-          new_vm
+          ref
         end
         
       end

lib/fog/xenserver/requests/compute/create_vif.rb

@@ -5,7 +5,8 @@ module Fog
       class Real
         
         def create_vif( vm_ref, network_ref )
-          @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VIF.create'}, default_vif_config(vm_ref, network_ref) )
+          vif_config = default_vif_config(vm_ref, network_ref)
+          @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VIF.create'}, vif_config )
         end
         
         def default_vif_config( vm_ref, network_ref, device_number = '0' )

lib/fog/xenserver/requests/compute/destroy_vbd.rb

@@ -5,7 +5,7 @@ module Fog
       class Real
 
         def destroy_vbd( vbd_ref, extra_args = {})
-          @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VBD.destroy'}, vbd_ref)
+          @connection.request({:parser => Fog::Parsers::XenServer::Base.new, :method => 'VBD.unplug'}, vbd_ref)
         end
 
       end

tests/xenserver/compute_tests.rb

@@ -30,6 +30,10 @@ Shindo.tests('Fog::Compute[:xenserver]', ['xenserver']) do
     # (not sure if that's gonna be a real scenario though)
     tests("Networks collection") do
       test("should have at least one Network") { compute.networks.size >= 1 }
+      tests("each network should be a Fog::Compute::XenServer::Network") do
+        ok = true
+        compute.networks.each { |n| ok = false if n.kind_of? Fog::Compute::XenServer::Network } 
+      end
     end
   end
 
@@ -38,7 +42,7 @@ Shindo.tests('Fog::Compute[:xenserver]', ['xenserver']) do
     # This template exists in our XenServer
     compute.default_template = 'squeeze-test'
     test("it should have a default template if template exists") { compute.default_template.name == 'squeeze-test' }
-    test("default template must be a Fog::Compute::XenServer::Server") { compute.default_template.is_a? Fog::Compute::XenServer::Server }
+    test("it should be a Fog::Compute::XenServer::Server") { compute.default_template.is_a? Fog::Compute::XenServer::Server }
     test("it should return nil when not found") do
       compute.default_template = 'asdfasdfasfdwe'
       compute.default_template.nil?

tests/xenserver/models/compute/host_tests.rb

@@ -64,26 +64,5 @@ Shindo.tests('Fog::Compute[:xenserver] | host model', ['xenserver']) do
 
   end
 
-  tests("The host template list should") do
-    test("include a #{test_template_name} template in custom_templates") do
-      found = false
-      host.custom_templates.each do |s|
-        found = (s.name == test_template_name)
-      end
-      found
-    end 
-    test("include only one custom template") { host.custom_templates.size == 1 }
-    tests("not include built-in templates in custom_templates") do
-      host.custom_templates.each do |s|
-        test("#{s.name} is NOT a built-in template") {s.allowed_operations.include?('destroy') }
-      end
-    end
-    test("include more than one built-in templates") { host.templates.size >= 1 }
-    tests("not include real servers") do
-      host.templates.each do |s|
-        test("#{s.name} is not a real server") { s.is_a_template }
-      end
-    end
-  end
 
 end

tests/xenserver/models/compute/server_tests.rb

@@ -1,7 +1,8 @@
 
 Shindo.tests('Fog::Compute[:xenserver] | server model', ['xenserver']) do
 
-  servers = Fog::Compute[:xenserver].servers
+  connection = Fog::Compute[:xenserver]
+  servers = connection.servers
   # pre-flight cleanup
   (servers.all :name_matches => test_ephemeral_vm_name).each do |s|
     s.destroy
@@ -66,6 +67,29 @@ Shindo.tests('Fog::Compute[:xenserver] | server model', ['xenserver']) do
 
   end
 
+  tests("Creating a server") do
+    tests("it should create a server") do
+      s = nil
+      test("named FOOBARSTUFF") do
+        s = servers.create(:name => "FOOBARSTUFF", 
+                           :template_name => test_template_name)
+        servers.get(s.reference).name == "FOOBARSTUFF"
+      end
+      test("and destroy it afterwards") { s.destroy }
+    end
+    tests("it should create a server") do
+      s = nil
+      test("with 3 NICs") do
+        s = servers.create(:name => "FOOBARSTUFF", 
+                           :template_name => test_template_name,
+                           :networks => [connection.default_network, connection.default_network, connection.default_network])
+        s.reload
+        s.networks.size == 3
+      end
+      test("and destroy it afterwards") { s.destroy }
+    end
+  end
+
   tests("A real server should") do
     tests("return valid vbds") do
       test("as an array") { server.vbds.kind_of? Array }
@@ -73,6 +97,19 @@ Shindo.tests('Fog::Compute[:xenserver] | server model', ['xenserver']) do
           test("and each VBD should be a Fog::Compute::XenServer::VBD") { i.kind_of? Fog::Compute::XenServer::VBD }
       } 
     end
+
+    test("have 0 or more networks") { server.networks.kind_of? Array }
+
+    tests("have networks if networks > 0") do
+      if server.networks.size > 0
+        server.networks.each do |n|
+          test("and network is of type Fog::Compute::XenServer::Network") do
+            n.kind_of? Fog::Compute::XenServer::Network
+          end
+        end
+      end
+    end
+
     test("reside on a Fog::Compute::XenServer::Host") { server.resident_on.kind_of? Fog::Compute::XenServer::Host }
     #test("have Fog::Compute::XenServer::GuestMetrics") { server.guest_metrics.kind_of? Fog::Compute::XenServer::GuestMetrics }
     test("be able to refresh itself") { server.refresh }

tests/xenserver/models/compute/servers_tests.rb

@@ -6,15 +6,23 @@ Shindo.tests('Fog::Compute[:xenserver] | servers collection', ['xenserver']) do
   (conn.servers.all :name_matches => test_ephemeral_vm_name).each do |s|
     s.destroy
   end
-  # Create some test data
+  servers = conn.servers
-  server = conn.servers.create(:name => test_ephemeral_vm_name, 
+  templates = conn.servers.templates
-                               :template_name => test_template_name)
+    
-  server.wait_for { running? }
-
   tests('The servers collection') do
     servers = conn.servers.all
 
-    test('should not be empty') { !servers.empty? }
+    test('should be empty') do
+      servers.empty? 
+    end
+
+    server = conn.servers.create(:name => test_ephemeral_vm_name, 
+                                 :template_name => test_template_name)
+    test('should NOT be empty') do
+      servers.reload
+      !servers.empty? 
+    end
+    server.destroy
 
     test('should be a kind of Fog::Compute::XenServer::Servers') { servers.kind_of? Fog::Compute::XenServer::Servers }
 
@@ -26,11 +34,54 @@ Shindo.tests('Fog::Compute[:xenserver] | servers collection', ['xenserver']) do
       end
     end
 
+    test("should return a list of templates") do
+      templates.kind_of? Array
+    end
+
+    tests("The servers template list should") do
+      test("should include only templates in servers.templates") do
+        ok = true
+        templates.each { |t| ok = false if !t.is_a_template }
+        ok
+      end
+      test("include a #{test_template_name} template in custom_templates") do
+        found = false
+        servers.custom_templates.each do |s|
+          found = (s.name == test_template_name)
+        end
+        found
+      end 
+      test("NOT include a #{test_template_name} template in built-in templates") do
+        found = false
+        servers.builtin_templates.each do |s|
+          found = (s.name != test_template_name)
+        end
+        found
+      end 
+      # This may fail in other test scenarios with more than one built-in template
+      # present
+      test("include only one custom template") { servers.custom_templates.size == 1 }
+      tests("not include built-in templates in custom_templates") do
+        servers.custom_templates.each do |s|
+          test("#{s.name} is NOT a built-in template") {s.allowed_operations.include?('destroy') }
+        end
+      end
+      test("include more than one built-in templates") { servers.builtin_templates.size >= 1 }
+      tests("not include real servers") do
+        servers.builtin_templates.each do |s|
+          test("#{s.name} is not a real server") { s.is_a_template }
+        end
+      end
+    end
+
     tests('should be able to reload itself').succeeds { servers.reload }
 
     tests('should be able to get a model') do
-      tests('by name').succeeds { servers.get_by_name test_ephemeral_vm_name }
+      server = conn.servers.create(:name => test_ephemeral_vm_name, 
-      tests('by instance uuid').succeeds { servers.get(servers.get_by_name(test_ephemeral_vm_name).reference) }
+                                   :template_name => test_template_name)
+      test('by name') { servers.get_by_name(test_ephemeral_vm_name).kind_of? Fog::Compute::XenServer::Server }
+      test('by instance reference') { servers.get(server.reference).kind_of? Fog::Compute::XenServer::Server }
+      server.destroy
     end
 
   end

tests/xenserver/requests/compute/create_server_tests.rb

@@ -11,9 +11,14 @@ Shindo.tests('Fog::Compute[:xenserver] | create_server request', ['xenserver'])
     raises(StandardError, 'raise exception when template nil') do
       compute.create_server 'fooserver', nil
     end
-    test('create a VM') do
+
-      compute.create_server test_ephemeral_vm_name, test_template_name
+    ref = compute.create_server test_ephemeral_vm_name, test_template_name
-      !compute.get_vm_by_name(test_ephemeral_vm_name).nil?
+    test('return a valid reference') do
+      if (ref != "OpaqueRef:NULL") and (ref.split("1") != "NULL")
+        true
+      else
+        false
+      end
     end
   end