You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by ma...@redhat.com on 2012/12/04 13:23:06 UTC
Create volume with config by value - DTACLOUD-385
Patches above fix this issue - also available at
http://tracker.deltacloud.org/set/177
Scenarios for testing:
1. Create from template by reference - not yet supported - can likely be
added with new Datamapper (persistence layer) implementation
2. Create from template by value, with config by reference:
XML:
curl -v --user "mockuser:mockpassword" -H "Accept:application/xml" -X POST -d
'<VolumeCreate><name> marios_new_volume </name> <description> a new volume
</description><volumeTemplate> <volumeConfig href="http://localhost:3001/cimi/volume_configurations/2">
</volumeConfig></volumeTemplate></VolumeCreate>' http://localhost:3001//cimi/volumes
JSON:
curl -v --user "mockuser:mockpassword" -H "Accept:application/xml" -X POST -d
'{"name": "marios_new_volume", "description": "a new volume", "volumeTemplate":
{ "volumeConfig": {"href":"http://localhost:3001/cimi/volume_configurations/2" }}}'
http://localhost:3001//cimi/volumes
3. Create from template by value, with config by value (issue
reported/addressed here):
XML:
curl -v --user "mockuser:mockpassword" -H "Accept:application/xml" -X POST -d
'<VolumeCreate><name> marios_volume </name><description> a new volume </description>
<volumeTemplate><volumeConfig><type>http://schemas.dmtf.org/cimi/1/mapped</type>
<capacity> 1024 </capacity></volumeConfig></volumeTemplate> </VolumeCreate>'
http://localhost:3001//cimi/volumes
JSON:
curl --user "mockuser:mockpassword" -H "Accept:application/xml" -X POST -d
'{"name": "marios_new_volume", "description": "a new volume", "volumeTemplate":
{ "volumeConfig": {"type":"http://schemas.dmtf.org/cimi/1/mapped", "capacity": 1024 }}}'
http://localhost:3001/cimi/volumes
marios
Re: Create volume with config by value - DTACLOUD-385
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2012-12-04 at 14:23 +0200, marios@redhat.com wrote:
> Patches above fix this issue - also available at
> http://tracker.deltacloud.org/set/177
Seems like Ronelle ack'd them - I just pushed them.
David
Re: [PATCH 2/2] CIMI - collections use grab_content_type helper to
guess type if not supplied
Posted by "marios@redhat.com" <ma...@redhat.com>.
On 05/12/12 02:45, David Lutterkort wrote:
> On Tue, 2012-12-04 at 14:23 +0200, marios@redhat.com wrote:
>> From: marios <ma...@redhat.com>
>>
>>
>> Signed-off-by: marios <ma...@redhat.com>
>> ---
>> server/lib/cimi/collections/addresses.rb | 2 +-
>> server/lib/cimi/collections/credentials.rb | 2 +-
>> server/lib/cimi/collections/machine_templates.rb | 2 +-
>> server/lib/cimi/collections/machines.rb | 10 +++++-----
>> server/lib/cimi/collections/network_ports.rb | 6 +++---
>> server/lib/cimi/collections/networks.rb | 8 ++++----
>> 6 files changed, 15 insertions(+), 15 deletions(-)
>
> I think what grab_content_type is doing is overly lenient - we should
> reject requests that have a content-type other than application/xml or
> application/json with a 415. Note that the comparison of the
> Content-Type header with the above two values must be case-insensitive.
>
ok - I added this more for personal comfort - like when I forgot to add
the 'Content-Type' header to my cURL command. Digging a little deeper,
the CIMI spec defers to RFC 2616 and looking at that Content-Type is a
'should' rather than a 'must'. I enforced the strict 'xml or json only'
check by removing the call to 'guess_content_type' and raise a 415
appropriately.
I also added the unit test (as part of part_4test.rb). Patches are at
http://tracker.deltacloud.org/set/181
thanks, marios
> David
>
>
Re: [PATCH 2/2] CIMI - collections use grab_content_type helper to
guess type if not supplied
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2012-12-04 at 14:23 +0200, marios@redhat.com wrote:
> From: marios <ma...@redhat.com>
>
>
> Signed-off-by: marios <ma...@redhat.com>
> ---
> server/lib/cimi/collections/addresses.rb | 2 +-
> server/lib/cimi/collections/credentials.rb | 2 +-
> server/lib/cimi/collections/machine_templates.rb | 2 +-
> server/lib/cimi/collections/machines.rb | 10 +++++-----
> server/lib/cimi/collections/network_ports.rb | 6 +++---
> server/lib/cimi/collections/networks.rb | 8 ++++----
> 6 files changed, 15 insertions(+), 15 deletions(-)
I think what grab_content_type is doing is overly lenient - we should
reject requests that have a content-type other than application/xml or
application/json with a 415. Note that the comparison of the
Content-Type header with the above two values must be case-insensitive.
David
[PATCH 2/2] CIMI - collections use grab_content_type helper to guess type if not supplied
Posted by ma...@redhat.com.
From: marios <ma...@redhat.com>
Signed-off-by: marios <ma...@redhat.com>
---
server/lib/cimi/collections/addresses.rb | 2 +-
server/lib/cimi/collections/credentials.rb | 2 +-
server/lib/cimi/collections/machine_templates.rb | 2 +-
server/lib/cimi/collections/machines.rb | 10 +++++-----
server/lib/cimi/collections/network_ports.rb | 6 +++---
server/lib/cimi/collections/networks.rb | 8 ++++----
6 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/server/lib/cimi/collections/addresses.rb b/server/lib/cimi/collections/addresses.rb
index a5a5986..6f016b2 100644
--- a/server/lib/cimi/collections/addresses.rb
+++ b/server/lib/cimi/collections/addresses.rb
@@ -47,7 +47,7 @@ module CIMI::Collections
operation :create, :with_capability => :create_address do
description "Create a new Address"
control do
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
address = CIMI::Model::Address.create(request.body.read, self, :json)
else
address = CIMI::Model::Address.create(request.body.read, self, :xml)
diff --git a/server/lib/cimi/collections/credentials.rb b/server/lib/cimi/collections/credentials.rb
index 24c66c0..f2ca059 100644
--- a/server/lib/cimi/collections/credentials.rb
+++ b/server/lib/cimi/collections/credentials.rb
@@ -46,7 +46,7 @@ module CIMI::Collections
operation :create, :with_capability => :create_key do
description "Show specific machine admin"
control do
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
new_admin = Credential.create_from_json(request.body.read, self)
else
new_admin = Credential.create_from_xml(request.body.read, self)
diff --git a/server/lib/cimi/collections/machine_templates.rb b/server/lib/cimi/collections/machine_templates.rb
index bda57a0..a90f360 100644
--- a/server/lib/cimi/collections/machine_templates.rb
+++ b/server/lib/cimi/collections/machine_templates.rb
@@ -45,7 +45,7 @@ module CIMI::Collections
operation :create do
description "Create new machine template"
control do
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
new_machine_template = CIMI::Model::MachineTemplate.create_from_json(request.body.read, self)
else
new_machine_template = CIMI::Model::MachineTemplate.create_from_xml(request.body.read, self)
diff --git a/server/lib/cimi/collections/machines.rb b/server/lib/cimi/collections/machines.rb
index 20fcfe0..91fbc64 100644
--- a/server/lib/cimi/collections/machines.rb
+++ b/server/lib/cimi/collections/machines.rb
@@ -46,7 +46,7 @@ module CIMI::Collections
operation :create, :with_capability => :create_instance do
description "Create a new Machine entity."
control do
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
new_machine = Machine.create_from_json(request.body.read, self)
else
new_machine = Machine.create_from_xml(request.body.read, self)
@@ -73,7 +73,7 @@ module CIMI::Collections
param :id, :string, :required
control do
machine = Machine.find(params[:id], self)
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
action = Action.from_json(request.body.read)
else
action = Action.from_xml(request.body.read)
@@ -90,7 +90,7 @@ module CIMI::Collections
param :id, :string, :required
control do
machine = Machine.find(params[:id], self)
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
action = Action.from_json(request.body.read.gsub("restart", "reboot"))
else
action = Action.from_xml(request.body.read.gsub("restart", "reboot"))
@@ -107,7 +107,7 @@ module CIMI::Collections
param :id, :string, :required
control do
machine = Machine.find(params[:id], self)
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
action = Action.from_json(request.body.read)
else
action = Action.from_xml(request.body.read)
@@ -175,7 +175,7 @@ module CIMI::Collections
description "Attach CIMI Volume(s) to a machine."
param :id, :string, :required
control do
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
volume_to_attach, location = MachineVolume.find_to_attach_from_json(request.body.read, self)
else
volume_to_attach, location = MachineVolume.find_to_attach_from_xml(request.body.read, self)
diff --git a/server/lib/cimi/collections/network_ports.rb b/server/lib/cimi/collections/network_ports.rb
index 1eb71c6..9a009d6 100644
--- a/server/lib/cimi/collections/network_ports.rb
+++ b/server/lib/cimi/collections/network_ports.rb
@@ -47,7 +47,7 @@ module CIMI::Collections
operation :create, :with_capability => :create_network_port do
description "Create a new NetworkPort"
control do
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
network_port = CIMI::Model::NetworkPort.create(request.body.read, self, :json)
else
network_port = CIMI::Model::NetworkPort.create(request.body.read, self, :xml)
@@ -73,7 +73,7 @@ module CIMI::Collections
control do
network_port = NetworkPort.find(params[:id], self)
report_error(404) unless network_port
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
action = Action.from_json(request.body.read)
else
action = Action.from_xml(request.body.read)
@@ -91,7 +91,7 @@ module CIMI::Collections
control do
network_port = NetworkPort.find(params[:id], self)
report_error(404) unless network_port
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
action = Action.from_json(request.body.read)
else
action = Action.from_xml(request.body.read)
diff --git a/server/lib/cimi/collections/networks.rb b/server/lib/cimi/collections/networks.rb
index 002b828..6500c82 100644
--- a/server/lib/cimi/collections/networks.rb
+++ b/server/lib/cimi/collections/networks.rb
@@ -46,7 +46,7 @@ module CIMI::Collections
operation :create, :with_capability => :create_network do
description "Create a new Network"
control do
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
network = Network.create(request.body.read, self, :json)
else
network = Network.create(request.body.read, self, :xml)
@@ -72,7 +72,7 @@ module CIMI::Collections
control do
network = Network.find(params[:id], self)
report_error(404) unless network
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
action = Action.from_json(request.body.read)
else
action = Action.from_xml(request.body.read)
@@ -90,7 +90,7 @@ module CIMI::Collections
control do
network = Network.find(params[:id], self)
report_error(404) unless network
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
action = Action.from_json(request.body.read)
else
action = Action.from_xml(request.body.read)
@@ -108,7 +108,7 @@ module CIMI::Collections
control do
network = Network.find(params[:id], self)
report_error(404) unless network
- if request.content_type.end_with?("json")
+ if grab_content_type(request.content_type, request.body) == :json
action = Action.from_json(request.body.read)
else
action = Action.from_xml(request.body.read)
--
1.7.11.7
Re: Create volume with config by value - DTACLOUD-385
Posted by "marios@redhat.com" <ma...@redhat.com>.
appologies - fixed another issue - if testing please use the patchset at
http://tracker.deltacloud.org/set/179
(alternatively grab the patches at
https://issues.apache.org/jira/browse/DTACLOUD-385)
thanks, marios
On 04/12/12 14:23, marios@redhat.com wrote:
> Patches above fix this issue - also available at
> http://tracker.deltacloud.org/set/177
>
> Scenarios for testing:
>
> 1. Create from template by reference - not yet supported - can likely be
> added with new Datamapper (persistence layer) implementation
>
> 2. Create from template by value, with config by reference:
>
> XML:
>
> curl -v --user "mockuser:mockpassword" -H "Accept:application/xml" -X POST -d
> '<VolumeCreate><name> marios_new_volume </name> <description> a new volume
> </description><volumeTemplate> <volumeConfig href="http://localhost:3001/cimi/volume_configurations/2">
> </volumeConfig></volumeTemplate></VolumeCreate>' http://localhost:3001//cimi/volumes
>
> JSON:
>
> curl -v --user "mockuser:mockpassword" -H "Accept:application/xml" -X POST -d
> '{"name": "marios_new_volume", "description": "a new volume", "volumeTemplate":
> { "volumeConfig": {"href":"http://localhost:3001/cimi/volume_configurations/2" }}}'
> http://localhost:3001//cimi/volumes
>
>
> 3. Create from template by value, with config by value (issue
> reported/addressed here):
>
> XML:
>
> curl -v --user "mockuser:mockpassword" -H "Accept:application/xml" -X POST -d
> '<VolumeCreate><name> marios_volume </name><description> a new volume </description>
> <volumeTemplate><volumeConfig><type>http://schemas.dmtf.org/cimi/1/mapped</type>
> <capacity> 1024 </capacity></volumeConfig></volumeTemplate> </VolumeCreate>'
> http://localhost:3001//cimi/volumes
>
> JSON:
>
> curl --user "mockuser:mockpassword" -H "Accept:application/xml" -X POST -d
> '{"name": "marios_new_volume", "description": "a new volume", "volumeTemplate":
> { "volumeConfig": {"type":"http://schemas.dmtf.org/cimi/1/mapped", "capacity": 1024 }}}'
> http://localhost:3001/cimi/volumes
>
>
> marios
>
Re: [PATCH 1/2] CIMI - Adds Volume creation with template and
config by value - DTACLOUD-385
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2012-12-04 at 14:23 +0200, marios@redhat.com wrote:
> From: marios <ma...@redhat.com>
>
> https://issues.apache.org/jira/browse/DTACLOUD-385
>
> Signed-off-by: marios <ma...@redhat.com>
> ---
> server/lib/cimi/collections/volumes.rb | 11 +++-------
> server/lib/cimi/helpers/cimi_helper.rb | 39 ++++++++++++++++++++++++++++++----
> server/lib/cimi/models/volume.rb | 37 +++++++++++++++++++-------------
> 3 files changed, 60 insertions(+), 27 deletions(-)
Thanks for the quick turnaround; please also add a unit test that
demonstrates that this now works.
David
[PATCH 1/2] CIMI - Adds Volume creation with template and config by value - DTACLOUD-385
Posted by ma...@redhat.com.
From: marios <ma...@redhat.com>
https://issues.apache.org/jira/browse/DTACLOUD-385
Signed-off-by: marios <ma...@redhat.com>
---
server/lib/cimi/collections/volumes.rb | 11 +++-------
server/lib/cimi/helpers/cimi_helper.rb | 39 ++++++++++++++++++++++++++++++----
server/lib/cimi/models/volume.rb | 37 +++++++++++++++++++-------------
3 files changed, 60 insertions(+), 27 deletions(-)
diff --git a/server/lib/cimi/collections/volumes.rb b/server/lib/cimi/collections/volumes.rb
index 32912c0..1078014 100644
--- a/server/lib/cimi/collections/volumes.rb
+++ b/server/lib/cimi/collections/volumes.rb
@@ -49,15 +49,10 @@ module CIMI::Collections
operation :create, :with_capability => :create_storage_volume do
description "Create a new Volume."
control do
- content_type = (request.content_type.end_with?("json") ? :json : :xml)
- #((request.content_type.end_with?("xml")) ? :xml : report_error(415) ) FIXME
- case content_type
- when :json
- new_volume = Volume.create_from_json(request.body.read, self)
- when :xml
- new_volume = Volume.create_from_xml(request.body.read, self)
- end
+ content_type = grab_content_type(request.content_type, request.body)
+ new_volume = Volume.create(request.body.read, self, content_type)
status 201
+ headers 'Location' => new_volume.id
respond_to do |format|
format.json { new_volume.to_json }
format.xml { new_volume.to_xml }
diff --git a/server/lib/cimi/helpers/cimi_helper.rb b/server/lib/cimi/helpers/cimi_helper.rb
index 99af08a..9248f8a 100644
--- a/server/lib/cimi/helpers/cimi_helper.rb
+++ b/server/lib/cimi/helpers/cimi_helper.rb
@@ -34,22 +34,53 @@ module CIMI
def to_kibibyte(value, unit)
case unit
when "GB"
- (value.to_i*1024*1024).to_i
+ (value*1024*1024).to_i
when "MB"
- (value.to_i*1024).to_i
+ (value*1024).to_i
else
nil # should probably be exploding something here...
end
end
+ #e.g. convert volume to GB for deltacloud driver
def from_kibibyte(value, unit="GB")
case unit
- when "GB" then ((value.to_i)/1024/1024).to_i
- when "MB" then ((value.to_i)/1024).to_i
+ when "GB" then ((value.to_f)/1024/1024)
+ when "MB" then ((value.to_f)/1024)
else nil
end
end
+ def grab_content_type(request_content_type, request_body)
+ case request_content_type
+ when /xml$/i then :xml
+ when /json$/i then :json
+ else guess_content_type(request_body)
+ end
+ end
+
+ def guess_content_type(request_body)
+ xml = json = false
+ body = request_body.read
+ request_body.rewind
+ begin
+ XmlSimple.xml_in(body)
+ xml = true
+ rescue Exception
+ xml = false
+ end
+ begin
+ JSON.parse(body)
+ json = true
+ rescue Exception
+ json = false
+ end
+ if (json == xml) #both true or both false
+ raise CIMI::Model::BadRequest.new("Couldn't guess content type of: #{body}")
+ end
+ type = (xml)? :xml : :json
+ end
+
end
end
diff --git a/server/lib/cimi/models/volume.rb b/server/lib/cimi/models/volume.rb
index 7951a99..8461f82 100644
--- a/server/lib/cimi/models/volume.rb
+++ b/server/lib/cimi/models/volume.rb
@@ -48,21 +48,24 @@ class CIMI::Model::Volume < CIMI::Model::Base
def self.all(context); find(:all, context); end
- def self.create_from_json(json_in, context)
- json = JSON.parse(json_in)
- volume_config_id = json["volumeTemplate"]["volumeConfig"]["href"].split("/").last
- volume_image_id = (json["volumeTemplate"].has_key?("volumeImage") ?
- json["volumeTemplate"]["volumeImage"]["href"].split("/").last : nil)
- create_volume({:volume_config_id=>volume_config_id, :volume_image_id=>volume_image_id}, json, context)
+ def self.create(request_body, context, type)
+ #json = JSON.parse(json_in)
+ input = (type == :xml)? XmlSimple.xml_in(request_body, {"ForceArray"=>false,"NormaliseSpace"=>2}) : JSON.parse(request_body)
+ if input["volumeTemplate"]["href"] #template by reference
+ #FIXME - don't have volumeTemplates yet - datamapper volume_config =
+ else #template by value
+ volume_image_id = (input["volumeTemplate"].has_key?("volumeImage") ?
+ input["volumeTemplate"]["volumeImage"]["href"].split("/").last : nil)
+ if input["volumeTemplate"]["volumeConfig"]["href"] #with config by reference
+ volume_config_id = input["volumeTemplate"]["volumeConfig"]["href"].split("/").last
+ create_volume({:volume_config_id=>volume_config_id, :volume_image_id=>volume_image_id}, input, context)
+ else #with config by value
+ capacity = input["volumeTemplate"]["volumeConfig"]["capacity"]
+ create_volume({:capacity=>capacity, :volume_image_id=>volume_image_id}, input, context)
+ end
+ end
end
- def self.create_from_xml(xml_in, context)
- xml = XmlSimple.xml_in(xml_in)
- volume_config_id = xml["volumeTemplate"][0]["volumeConfig"][0]["href"].split("/").last
- volume_image_id = (xml["volumeTemplate"][0].has_key?("volumeImage") ?
- xml["volumeTemplate"][0]["volumeImage"][0]["href"].split("/").last : nil)
- create_volume({:volume_config_id=>volume_config_id, :volume_image_id=>volume_image_id}, xml, context)
- end
def self.delete!(id, context)
context.driver.destroy_storage_volume(context.credentials, {:id=>id} )
@@ -88,8 +91,12 @@ class CIMI::Model::Volume < CIMI::Model::Base
private
def self.create_volume(params, data, context)
- volume_config = CIMI::Model::VolumeConfiguration.find(params[:volume_config_id], context)
- opts = {:capacity=>context.from_kibibyte(volume_config.capacity, "GB"), :snapshot_id=>params[:volume_image_id] }
+ if params[:volume_config_id]
+ volume_config = CIMI::Model::VolumeConfiguration.find(params[:volume_config_id], context)
+ opts = {:capacity=>context.from_kibibyte(volume_config.capacity, "GB"), :snapshot_id=>params[:volume_image_id] }
+ elsif params[:capacity]
+ opts = {:capacity=>context.from_kibibyte(params[:capacity], "GB"), :snapshot_id=>params[:volume_image_id]}
+ end
storage_volume = context.driver.create_storage_volume(context.credentials, opts)
store_attributes_for(context, storage_volume, data)
from_storage_volume(storage_volume, context)
--
1.7.11.7