Merge pull request #3323 from brandondunne/fix_block_device_mappings

Fix block device mappings

Rakefile

@@ -69,7 +69,9 @@ namespace :test do
   task :ovirt do
       sh("export FOG_MOCK=#{mock} && bundle exec shindont tests/ovirt")
   end
-
+  task :openstack do
+      sh("export FOG_MOCK=#{mock} && bundle exec shindont tests/openstack")
+  end
 end
 
 desc 'Run mocked tests for a specific provider'

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

@@ -50,7 +50,7 @@ class Server < Fog::Compute::Server
 
         attr_reader :password
         attr_writer :image_ref, :flavor_ref, :nics, :os_scheduler_hints
-        attr_accessor :block_device_mapping
+        attr_accessor :block_device_mapping, :block_device_mapping_v2
 
         def initialize(attributes={})
           # Old 'connection' is renamed as service and should be used instead
@@ -62,6 +62,7 @@ def initialize(attributes={})
           self.nics = attributes.delete(:nics)
           self.os_scheduler_hints = attributes.delete(:os_scheduler_hints)
           self.block_device_mapping = attributes.delete(:block_device_mapping)
+          self.block_device_mapping_v2 = attributes.delete(:block_device_mapping_v2)
 
           super
         end
@@ -326,7 +327,7 @@ def detach_volume(volume_id)
         def save
           raise Fog::Errors::Error.new('Resaving an existing object may create a duplicate') if persisted?
           requires :flavor_ref, :name
-          requires_one :image_ref, :block_device_mapping
+          requires_one :image_ref, :block_device_mapping, :block_device_mapping_v2
           options = {
             'personality' => personality,
             'accessIPv4' => accessIPv4,
@@ -340,7 +341,8 @@ def save
             'max_count'   => @max_count,
             'nics' => @nics,
             'os:scheduler_hints' => @os_scheduler_hints,
-            'block_device_mapping' => @block_device_mapping
+            'block_device_mapping' => @block_device_mapping,
+            'block_device_mapping_v2' => @block_device_mapping_v2,
           }
           options['metadata'] = metadata.to_hash unless @metadata.nil?
           options = options.reject {|key, value| value.nil?}

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

@@ -57,37 +57,29 @@ def create_server(name, image_ref, flavor_ref, options = {})
             data['os:scheduler_hints'] = options['os:scheduler_hints']
           end
 
-          if options['block_device_mapping']
-            [options['block_device_mapping']].flatten.map do |mapping|
-              if mapping[:api_ver] == "v2"
-                data['server']['block_device_mapping_v2'] =
-                [
-                  {
-                    'device_name' => mapping[:device_name],
-                    'source_type' => mapping[:source_type],
-                    'destination_type' => mapping[:destination_type],
-                    'delete_on_termination' => mapping[:delete_on_termination],
-                    'uuid' => mapping[:uuid],
-                    'boot_index' => mapping[:boot_index]
-                  }
-                ]
-              else
-                data['server']['block_device_mapping'] =
-                  {
-                    'volume_size' => mapping[:volume_size],
-                    'volume_id' => mapping[:volume_id],
-                    'delete_on_termination' => mapping[:delete_on_termination],
-                    'device_name' => mapping[:device_name]
-                  }
-              end
+          if (block_device_mapping = options['block_device_mapping_v2'])
+            data['server']['block_device_mapping_v2'] = [block_device_mapping].flatten.collect do |mapping|
+              {
+                'boot_index'            => mapping[:boot_index],
+                'delete_on_termination' => mapping[:delete_on_termination],
+                'destination_type'      => mapping[:destination_type],
+                'device_name'           => mapping[:device_name],
+                'source_type'           => mapping[:source_type],
+                'uuid'                  => mapping[:uuid],
+              }
+            end
+          elsif (block_device_mapping = options['block_device_mapping'])
+            data['server']['block_device_mapping'] = [block_device_mapping].flatten.collect do |mapping|
+              {
+                'delete_on_termination' => mapping[:delete_on_termination],
+                'device_name'           => mapping[:device_name],
+                'volume_id'             => mapping[:volume_id],
+                'volume_size'           => mapping[:volume_size],
+              }
             end
           end
 
-          path = if data['server']['block_device_mapping']
-                   'os-volumes_boot.json'
-                 else
-                   'servers.json'
-                 end
+          path = block_device_mapping ? 'os-volumes_boot.json' : 'servers.json'
 
           request(
             :body     => Fog::JSON.encode(data),
@@ -156,6 +148,12 @@ def create_server(name, image_ref, flavor_ref, options = {})
             }
           end
 
+          if block_devices = options["block_device_mapping_v2"]
+            block_devices.each { |bd| compute.volumes.get(bd[:uuid]).attach(server_id, bd[:device_name]) }
+          elsif block_device = options["block_device_mapping"]
+            compute.volumes.get(block_device[:volume_id]).attach(server_id, block_device[:device_name])
+          end
+
           self.data[:last_modified][:servers][server_id] = Time.now
           self.data[:servers][server_id] = mock_data
           if security_groups = options['security_groups'] then

tests/openstack/requests/compute/helper.rb

@@ -10,16 +10,22 @@ module Formats
   end
 end
 
+def compute
+  Fog::Compute[:openstack]
+end
+
 def get_flavor_ref
-  compute = Fog::Compute[:openstack]
   ENV['OPENSTACK_FLAVOR_REF'] || compute.list_flavors.body['flavors'].first['id']
 end
 
 def get_image_ref
-  compute = Fog::Compute[:openstack]
   ENV['OPENSTACK_IMAGE_REF'] || compute.list_images.body['images'].first['id']
 end
 
+def get_volume_ref
+  ENV['OPENSTACK_VOLUME_REF'] || compute.list_volumes.body['volumes'].first['id']
+end
+
 def get_flavor_ref_resize
   # by default we simply add one to the default flavor ref
   ENV['OPENSTACK_FLAVOR_REF_RESIZE'] || (get_flavor_ref.to_i + 1).to_s
@@ -31,6 +37,5 @@ def set_password_enabled
 end
 
 def get_security_group_ref
-  compute = Fog::Compute[:openstack]
   ENV['OPENSTACK_SECURITY_GROUP_REF'] || compute.list_security_groups.body['security_groups'].first['name']
 end

tests/openstack/requests/compute/server_tests.rb

@@ -1,24 +1,25 @@
 Shindo.tests('Fog::Compute[:openstack] | server requests', ['openstack']) do
 
-  @detailed_server_format = {
-    'id'         => String,
-    'addresses'  => Hash,
-    'flavor'     => Hash,
-    'hostId'     => String,
-    'image'      => Hash,
-    'metadata'   => Hash,
-    'name'       => String,
-    'progress'   => Integer,
-    'status'     => String,
-    'accessIPv4' => Fog::Nullable::String,
-    'accessIPv6' => Fog::Nullable::String,
-    'links'      => Array,
-    'created'    => String,
-    'updated'    => String,
-    'user_id'    => String,
+  @base_server_format = {
+    'id'           => String,
+    'addresses'    => Hash,
+    'flavor'       => Hash,
+    'hostId'       => String,
+    'metadata'     => Hash,
+    'name'         => String,
+    'progress'     => Integer,
+    'status'       => String,
+    'accessIPv4'   => Fog::Nullable::String,
+    'accessIPv6'   => Fog::Nullable::String,
+    'links'        => Array,
+    'created'      => String,
+    'updated'      => String,
+    'user_id'      => String,
     'config_drive' => String,
   }
 
+  @server_from_image_format = @base_server_format.merge('image' => Hash)
+
   @create_format = {
     'adminPass'       => String,
     'id'              => String,
@@ -51,6 +52,60 @@
     @flavor_id = get_flavor_ref
     @security_group_name = get_security_group_ref
 
+    #CREATE_SERVER_WITH_BLOCK_DEVICE_MAPPING
+    tests('#create_server("test", nil , #{@flavor_id}) with a block_device_mapping').formats(@create_format, false) do
+      @volume1_id = compute.create_volume('test', 'this is a test volume', 1).body["volume"]["id"]
+      volume_data = {
+        :delete_on_termination => true,
+        :device_name           => "vda",
+        :volume_id             => @volume1_id,
+        :volume_size           => 1,
+      }
+      data = compute.create_server("test", nil, @flavor_id, "block_device_mapping" => volume_data).body['server']
+      @server_id = data['id']
+      data
+    end
+
+    tests("#get_server_details(#{@server_id})").formats(@base_server_format, false) do
+      compute.get_server_details(@server_id).body['server']
+    end
+
+    tests("#block_device_mapping").succeeds do
+      compute.servers.get(@server_id).volumes.first.id == @volume1_id
+    end
+
+    #CREATE_SERVER_WITH_BLOCK_DEVICE_MAPPING_V2
+    tests('#create_server("test", nil , #{@flavor_id}) with multiple block_device_mapping_v2').formats(@create_format, false) do
+      @volume2_id = compute.create_volume('test', 'this is a test volume', 1).body["volume"]["id"]
+      volume_data = [{
+        :boot_index            => 0,
+        :uuid                  => @volume1_id,
+        :device_name           => "vda",
+        :source_type           => "volume",
+        :destination_type      => "volume",
+        :delete_on_termination => true,
+      }, {
+        :boot_index            => 1,
+        :uuid                  => @volume2_id,
+        :device_name           => "vdb",
+        :source_type           => "volume",
+        :destination_type      => "volume",
+        :delete_on_termination => true,
+      }]
+      data = compute.create_server("test", nil, @flavor_id, "block_device_mapping_v2" => volume_data).body['server']
+      @server_id = data['id']
+      data
+    end
+
+    tests("#get_server_details(#{@server_id})").formats(@base_server_format, false) do
+      compute.get_server_details(@server_id).body['server']
+    end
+
+    tests("#block_device_mapping_v2").succeeds do
+      compute.servers.get(@server_id).volumes.collect(&:id).sort == [@volume1_id, @volume2_id].sort
+    end
+
+    #CREATE_SINGLE_FROM_IMAGE
     tests('#create_server("test", #{@image_id} , 19)').formats(@create_format, false) do
       data = Fog::Compute[:openstack].create_server("test", @image_id, @flavor_id).body['server']
       @server_id = data['id']
@@ -59,12 +114,11 @@
 
     Fog::Compute[:openstack].servers.get(@server_id).wait_for { ready? }
 
-    #CREATE
-    tests("#get_server_details(#{@server_id})").formats(@detailed_server_format, false) do
+    tests("#get_server_details(#{@server_id})").formats(@server_from_image_format, false) do
       Fog::Compute[:openstack].get_server_details(@server_id).body['server']
     end
 
-    #MULTI_CREATE
+    #MULTI_CREATE_FROM_IMAGE
     tests('#create_server("test", #{@image_id} , 19, {"min_count" => 2, "return_reservation_id" => "True"})').formats(@reservation_format, false) do
       data = Fog::Compute[:openstack].create_server("test", @image_id, @flavor_id, {"min_count" => 2, "return_reservation_id" => "True"}).body
       @reservation_id = data['reservation_id']
@@ -96,7 +150,7 @@
     end
 
     #DETAILS
-    tests('#list_servers_detail').formats({'servers' => [@detailed_server_format]}, false) do
+    tests('#list_servers_detail').formats({'servers' => [@server_from_image_format]}, false) do
       Fog::Compute[:openstack].list_servers_detail.body
     end
 
@@ -133,7 +187,7 @@
     Fog::Compute[:openstack].images.get(@snapshot_id).wait_for { ready? }
 
     #REBUILD
-    tests("#rebuild_server(#{@server_id}, #{@snapshot_id}, 'fog')").formats({'server' => @detailed_server_format}, false) do
+    tests("#rebuild_server(#{@server_id}, #{@snapshot_id}, 'fog')").formats({'server' => @server_from_image_format}, false) do
       Fog::Compute[:openstack].rebuild_server(@server_id, @snapshot_id, 'fog', 'newpass', {"foo" => "bar"}).body
     end
     Fog::Compute[:openstack].servers.get(@server_id).wait_for { ready? } if not Fog.mocking?