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 2011/12/23 17:31:19 UTC
[PATCH 1/6] Fix deltacloud HTML UI attach volume form
From: marios <ma...@redhat.com>
Signed-off-by: marios <ma...@redhat.com>
---
server/lib/deltacloud/server.rb | 10 ++++++++++
server/views/storage_volumes/show.html.haml | 2 +-
2 files changed, 11 insertions(+), 1 deletions(-)
diff --git a/server/lib/deltacloud/server.rb b/server/lib/deltacloud/server.rb
index 88250a6..2fa81c7 100644
--- a/server/lib/deltacloud/server.rb
+++ b/server/lib/deltacloud/server.rb
@@ -664,6 +664,16 @@ collection :storage_volumes do
end
end
+ operation :attach_instance, :method=>:get, :member=>true do
+ description "A form to attach a storage volume to an instance"
+ control do
+ @instances = driver.instances(credentials)
+ respond_to do |format|
+ format.html{ haml :"storage_volumes/attach"}
+ end
+ end
+ end
+
operation :attach, :method => :post, :member => true do
description "Attach storage volume to instance"
with_capability :attach_storage_volume
diff --git a/server/views/storage_volumes/show.html.haml b/server/views/storage_volumes/show.html.haml
index 585990e..aabdd85 100644
--- a/server/views/storage_volumes/show.html.haml
+++ b/server/views/storage_volumes/show.html.haml
@@ -32,6 +32,6 @@
=link_to_action "Snapshot", api_url_for("storage_snapshots/new?volume_id=#{@storage_volume.id}"), :get
- unless @storage_volume.instance_id
=link_to_action "Delete", destroy_storage_volume_url(@storage_volume.id), :delete
- =link_to_action "Attach", api_url_for("storage_volumes/attach?id=#{@storage_volume.id}"), :get
+ =link_to_action "Attach", api_url_for("storage_volumes/#{@storage_volume.id}/attach_instance"), :get
- if @storage_volume.instance_id
=link_to_action "Detach", detach_storage_volume_url(@storage_volume.id), :post
--
1.7.6.4
[PATCH 5/6] CIMI deal with nilpointers in convert storage volumes (+swap volume=>device)
Posted by ma...@redhat.com.
From: marios <ma...@redhat.com>
Signed-off-by: marios <ma...@redhat.com>
---
server/lib/cimi/model/machine.rb | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/server/lib/cimi/model/machine.rb b/server/lib/cimi/model/machine.rb
index fddadd7..c55c4eb 100644
--- a/server/lib/cimi/model/machine.rb
+++ b/server/lib/cimi/model/machine.rb
@@ -219,8 +219,9 @@ class CIMI::Model::Machine < CIMI::Model::Base
end
def self.convert_storage_volumes(instance, context)
- instance.storage_volumes.map{|vol| {:href=>context.volume_url(vol.values.first),
- :attachment_point=>vol.keys.first} }
+ instance.storage_volumes ||= [] #deal with nilpointers
+ instance.storage_volumes.map{|vol| {:href=>context.volume_url(vol.keys.first),
+ :attachment_point=>vol.values.first} }
end
end
--
1.7.6.4
[PATCH 6/6] CIMI Adds attach/detach to Volumes cucumber tests
Posted by ma...@redhat.com.
From: marios <ma...@redhat.com>
Signed-off-by: marios <ma...@redhat.com>
---
.../features/step_definitions/volumes_steps.rb | 38 ++++++++++++++++++++
server/tests/cimi/features/volumes.feature | 14 +++++++
2 files changed, 52 insertions(+), 0 deletions(-)
diff --git a/server/tests/cimi/features/step_definitions/volumes_steps.rb b/server/tests/cimi/features/step_definitions/volumes_steps.rb
index 922cb04..a5abc9d 100644
--- a/server/tests/cimi/features/step_definitions/volumes_steps.rb
+++ b/server/tests/cimi/features/step_definitions/volumes_steps.rb
@@ -63,6 +63,44 @@ Then /^client should verify that this Volume was created correctly$/ do |capacit
@@retrieved_volume.capacity[:quantity].should == capacity.raw[0][1]
end
+When /^client specifies a running Machine using$/ do |machine|
+ @machine_id = machine.raw[0][1]
+ authorize 'mockuser', 'mockpassword'
+ header 'Content-Type', 'application/xml'
+ get "/cimi/machines/#{@machine_id}?format=xml"
+ last_response.status.should==200
+ @@machine = Machine.from_xml(last_response.body)
+ @@machine.name.should == @machine_id
+ @@machine.state.should == "STARTED"
+end
+
+When /^client specifies the new Volume with attachment point using$/ do |attach|
+ @builder = Nokogiri::XML::Builder.new do |xml|
+ xml.VolumeAttach {
+ xml.volume( :href => @@created_volume.uri, :attachmentPoint=>attach.raw[0][1])
+ }
+ end
+end
+
+Then /^client should be able to attach the new volume to the Machine$/ do
+ authorize 'mockuser', 'mockpassword'
+ header 'Content-Type', 'application/CIMI-Machine+xml'
+ put "/cimi/machines/#{@@machine.name}/attach_volume?format=xml", @builder.to_xml
+ last_response.status.should == 200
+end
+
+When /^client should be able to detach the volume$/ do
+ authorize 'mockuser', 'mockpassword'
+ header 'Content-Type', 'application/CIMI-Machine+xml'
+ @builder = Nokogiri::XML::Builder.new do |xml|
+ xml.VolumeDetach {
+ xml.volume(:href => @@created_volume.uri)
+ }
+ end
+ put "/cimi/machines/#{@@machine.name}/detach_volume", @builder.to_xml
+ last_response.status.should == 200
+end
+
When /^client deletes the newly created Volume$/ do
authorize 'mockuser', 'mockpassword'
delete "/cimi/volumes/#{@@created_volume.name}"
diff --git a/server/tests/cimi/features/volumes.feature b/server/tests/cimi/features/volumes.feature
index c8d5282..7835e2a 100644
--- a/server/tests/cimi/features/volumes.feature
+++ b/server/tests/cimi/features/volumes.feature
@@ -25,6 +25,20 @@ Scenario: Query the newly created Volume
Then client should verify that this Volume was created correctly
| capacity | 2 |
+Scenario: Attach the newly created Volume to a Machine
+ Given Cloud Entry Point URL is provided
+ And client retrieve the Cloud Entry Point
+ When client specifies a running Machine using
+ | name | inst0 |
+ And client specifies the new Volume with attachment point using
+ | attachment_point | /dev/sdc |
+ Then client should be able to attach the new volume to the Machine
+
+Scenario: Detach the newly created Volume from the Machine
+ Given Cloud Entry Point URL is provided
+ And client retrieve the Cloud Entry Point
+ Then client should be able to detach the volume
+
Scenario: Delete the newly created Volume
Given Cloud Entry Point URL is provided
And client retrieve the Cloud Entry Point
--
1.7.6.4
Re: [PATCH 1/6] Fix deltacloud HTML UI attach volume form
Posted by "marios@redhat.com" <ma...@redhat.com>.
On 02/01/12 14:03, Michal Fojtik wrote:
> Hi,
>
> ACK.
thanks for review
>
> Just one question/idea, not related to this patch.
>
> Wouldn't be cleaner to rather than creating a 'special' operation to add 'update' operation
> that will change the 'instance_id' in storage_volume resource?
>
yeah... in CIMI, attaching a volume is an operation of Machine - you
attach the volume to a Machine, by PUTting the Machine description
including the volume(s) you want attaching (or omitting the ones you
want to detach). For now we have not implemented this in this way,
instead using a 'special' attach_volume operation of Machine. It is not
intended to be permanent, but just something for us to use as a starting
point before implementing eventually the 'proper' way (parsing the
passed in Machine description and picking apart all the components of
the Machine, in this case the Volumes).
> I mean something like:
>
> PUT /api/storage_volumes/vol1
>
> Where the params will look like:
>
> { 'instance_id' => 'inst1' }
>
> For me it sounds more 'natural' that we want to 'PUT' (literally) an instance
> to this storage_volume.
OK, for me it sounds more natural to PUT the volume to the instance (I
think that's what you suggest below though so not sure if I understood
the above correctly). In any case though, the fact remains that in CIMI,
attaching a Volume is an operation on a Machine, not on the Volume (in
contrast, Deltacloud 'attach_instance' is an operation of StorageVolume).
Or _even_ 'more' natural will be that we will 'PUT' the
> storage_volume to instance. In that case the 'instances' collection will define:
>
> collection :instances do
> operation :volumes, :method => :put, :member => true do
> end
> end
>
> Then client will do:
>
> PUT /api/instances/inst1/volumes { 'storage_volume_id' => 'vol1' }
THis is how the current (temporary) CIMI attach_volume works:
PUT /cimi/Machines/machine1/attach_volume {
-H "Content-Type: application/CIMI-Machine+xml"
-d '<VolumeAttach><volume
href="http://localhost:3001/cimi/volumes/vol-019dfd6c"
attachmentPoint="/dev/sdc"/></VolumeAttach>'}
>
> Then we somehow need to list the volumes in 'instance'. Just my 50c ;-)
>
yes, this is why I added the 'storage_volumes' attribute to Deltacloud
Instance model - luckily EC2 API will return this attribute for
description of an Instance, so you get the volume_id and its device
(e.g. /dev/sdc) for any attached volumes
marios
> -- Michal
>
> On Dec 23, 2011, at 5:31 PM, marios@redhat.com wrote:
>
>> From: marios <ma...@redhat.com>
>>
>>
>> Signed-off-by: marios <ma...@redhat.com>
>> ---
>> server/lib/deltacloud/server.rb | 10 ++++++++++
>> server/views/storage_volumes/show.html.haml | 2 +-
>> 2 files changed, 11 insertions(+), 1 deletions(-)
>>
>> diff --git a/server/lib/deltacloud/server.rb b/server/lib/deltacloud/server.rb
>> index 88250a6..2fa81c7 100644
>> --- a/server/lib/deltacloud/server.rb
>> +++ b/server/lib/deltacloud/server.rb
>> @@ -664,6 +664,16 @@ collection :storage_volumes do
>> end
>> end
>>
>> + operation :attach_instance, :method=>:get, :member=>true do
>> + description "A form to attach a storage volume to an instance"
>> + control do
>> + @instances = driver.instances(credentials)
>> + respond_to do |format|
>> + format.html{ haml :"storage_volumes/attach"}
>> + end
>> + end
>> + end
>> +
>> operation :attach, :method => :post, :member => true do
>> description "Attach storage volume to instance"
>> with_capability :attach_storage_volume
>> diff --git a/server/views/storage_volumes/show.html.haml b/server/views/storage_volumes/show.html.haml
>> index 585990e..aabdd85 100644
>> --- a/server/views/storage_volumes/show.html.haml
>> +++ b/server/views/storage_volumes/show.html.haml
>> @@ -32,6 +32,6 @@
>> =link_to_action "Snapshot", api_url_for("storage_snapshots/new?volume_id=#{@storage_volume.id}"), :get
>> - unless @storage_volume.instance_id
>> =link_to_action "Delete", destroy_storage_volume_url(@storage_volume.id), :delete
>> - =link_to_action "Attach", api_url_for("storage_volumes/attach?id=#{@storage_volume.id}"), :get
>> + =link_to_action "Attach", api_url_for("storage_volumes/#{@storage_volume.id}/attach_instance"), :get
>
> I think, you can also use:
>
> attach_instance
>
>> - if @storage_volume.instance_id
>> =link_to_action "Detach", detach_storage_volume_url(@storage_volume.id), :post
>> --
>> 1.7.6.4
>>
>
> ------------------------------------------------------
> Michal Fojtik, mfojtik@redhat.com
> Deltacloud API: http://deltacloud.org
>
Re: [PATCH 1/6] Fix deltacloud HTML UI attach volume form
Posted by Michal Fojtik <mf...@redhat.com>.
Hi,
ACK.
Just one question/idea, not related to this patch.
Wouldn't be cleaner to rather than creating a 'special' operation to add 'update' operation
that will change the 'instance_id' in storage_volume resource?
I mean something like:
PUT /api/storage_volumes/vol1
Where the params will look like:
{ 'instance_id' => 'inst1' }
For me it sounds more 'natural' that we want to 'PUT' (literally) an instance
to this storage_volume. Or _even_ 'more' natural will be that we will 'PUT' the
storage_volume to instance. In that case the 'instances' collection will define:
collection :instances do
operation :volumes, :method => :put, :member => true do
end
end
Then client will do:
PUT /api/instances/inst1/volumes { 'storage_volume_id' => 'vol1' }
Then we somehow need to list the volumes in 'instance'. Just my 50c ;-)
-- Michal
On Dec 23, 2011, at 5:31 PM, marios@redhat.com wrote:
> From: marios <ma...@redhat.com>
>
>
> Signed-off-by: marios <ma...@redhat.com>
> ---
> server/lib/deltacloud/server.rb | 10 ++++++++++
> server/views/storage_volumes/show.html.haml | 2 +-
> 2 files changed, 11 insertions(+), 1 deletions(-)
>
> diff --git a/server/lib/deltacloud/server.rb b/server/lib/deltacloud/server.rb
> index 88250a6..2fa81c7 100644
> --- a/server/lib/deltacloud/server.rb
> +++ b/server/lib/deltacloud/server.rb
> @@ -664,6 +664,16 @@ collection :storage_volumes do
> end
> end
>
> + operation :attach_instance, :method=>:get, :member=>true do
> + description "A form to attach a storage volume to an instance"
> + control do
> + @instances = driver.instances(credentials)
> + respond_to do |format|
> + format.html{ haml :"storage_volumes/attach"}
> + end
> + end
> + end
> +
> operation :attach, :method => :post, :member => true do
> description "Attach storage volume to instance"
> with_capability :attach_storage_volume
> diff --git a/server/views/storage_volumes/show.html.haml b/server/views/storage_volumes/show.html.haml
> index 585990e..aabdd85 100644
> --- a/server/views/storage_volumes/show.html.haml
> +++ b/server/views/storage_volumes/show.html.haml
> @@ -32,6 +32,6 @@
> =link_to_action "Snapshot", api_url_for("storage_snapshots/new?volume_id=#{@storage_volume.id}"), :get
> - unless @storage_volume.instance_id
> =link_to_action "Delete", destroy_storage_volume_url(@storage_volume.id), :delete
> - =link_to_action "Attach", api_url_for("storage_volumes/attach?id=#{@storage_volume.id}"), :get
> + =link_to_action "Attach", api_url_for("storage_volumes/#{@storage_volume.id}/attach_instance"), :get
I think, you can also use:
attach_instance
> - if @storage_volume.instance_id
> =link_to_action "Detach", detach_storage_volume_url(@storage_volume.id), :post
> --
> 1.7.6.4
>
------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org
[PATCH 2/6] Adds attach/detach volume to deltacloud mock driver
Posted by ma...@redhat.com.
From: marios <ma...@redhat.com>
Signed-off-by: marios <ma...@redhat.com>
---
server/lib/deltacloud/drivers/mock/mock_driver.rb | 37 +++++++++++++++++++++
1 files changed, 37 insertions(+), 0 deletions(-)
diff --git a/server/lib/deltacloud/drivers/mock/mock_driver.rb b/server/lib/deltacloud/drivers/mock/mock_driver.rb
index 5d07eed..bee75a0 100644
--- a/server/lib/deltacloud/drivers/mock/mock_driver.rb
+++ b/server/lib/deltacloud/drivers/mock/mock_driver.rb
@@ -264,6 +264,17 @@ module Deltacloud::Drivers::Mock
@client.destroy(:storage_volumes, opts[:id])
end
+ #opts: {:id=,:instance_id,:device}
+ def attach_storage_volume(credentials, opts)
+ check_credentials(credentials)
+ attach_volume_instance(opts[:id], opts[:device], opts[:instance_id])
+ end
+
+ def detach_storage_volume(credentials, opts)
+ check_credentials(credentials)
+ detach_volume_instance(opts[:id], opts[:instance_id])
+ end
+
#
# Storage Snapshots
#
@@ -440,6 +451,32 @@ module Deltacloud::Drivers::Mock
end
end
+ def attach_volume_instance(volume_id, device, instance_id)
+ volume = @client.load(:storage_volumes, volume_id)
+ instance = @client.load(:instances, instance_id)
+ volume[:instance_id] = instance_id
+ volume[:device] = device
+ volume[:state] = "IN-USE"
+ instance[:storage_volumes] ||= []
+ instance[:storage_volumes] << {volume_id=>device}
+ @client.store(:storage_volumes, volume)
+ @client.store(:instances, instance)
+ StorageVolume.new(volume)
+ end
+
+ def detach_volume_instance(volume_id, instance_id)
+ volume = @client.load(:storage_volumes, volume_id)
+ instance = @client.load(:instances, instance_id)
+ volume[:instance_id] = nil
+ device = volume[:device]
+ volume[:device] = nil
+ volume[:state] = "AVAILABLE"
+ instance[:storage_volumes].delete({volume_id => device}) unless instance[:storage_volumes].nil?
+ @client.store(:storage_volumes, volume)
+ @client.store(:instances, instance)
+ StorageVolume.new(volume)
+ end
+
exceptions do
on /AuthFailure/ do
--
1.7.6.4
[PATCH 3/6] Adds display of storage volumes to deltacloud show instance
Posted by ma...@redhat.com.
From: marios <ma...@redhat.com>
Signed-off-by: marios <ma...@redhat.com>
---
server/views/instances/show.html.haml | 5 +++++
server/views/instances/show.xml.haml | 4 ++++
2 files changed, 9 insertions(+), 0 deletions(-)
diff --git a/server/views/instances/show.html.haml b/server/views/instances/show.html.haml
index 6891f1b..b91b71e 100644
--- a/server/views/instances/show.html.haml
+++ b/server/views/instances/show.html.haml
@@ -46,6 +46,11 @@
- if @instance.firewalls
%li{ :'data-role' => 'list-divider'} Firewalls
%li=@instance.firewalls.join(", ")
+ - if @instance.storage_volumes
+ %li{ :'data-role' => 'list-divider'} Attached Storage Volumes
+ -@instance.storage_volumes.each do |vol|
+ %li
+ %p{ :'data-role' => 'fieldcontain'}="#{vol.keys.first} <---> #{vol.values.first}"
%li{ :'data-role' => 'list-divider'} Actions
%li
%div{ :'data-role' => 'controlgroup', :'data-type' => "horizontal" }
diff --git a/server/views/instances/show.xml.haml b/server/views/instances/show.xml.haml
index 5b929f4..b2fd5bf 100644
--- a/server/views/instances/show.xml.haml
+++ b/server/views/instances/show.xml.haml
@@ -41,6 +41,10 @@
%firewalls<
- @instance.firewalls.each do |firewall|
%firewall{:href => firewall_url(firewall), :id => firewall }
+ - if @instance.storage_volumes
+ %storage_volumes<
+ - @instance.storage_volumes.each do |volume|
+ %storage_volume{:href=> storage_volume_url(volume.keys.first), :id => volume.keys.first, :device => volume.values.first}
- if driver_has_auth_features?
%authentication{ :type => driver_auth_feature_name }
- if @instance.authn_feature_failed?
--
1.7.6.4
[PATCH 4/6] Swap storage_volume device mapping for ec2 driver (volume=>attachment)
Posted by ma...@redhat.com.
From: marios <ma...@redhat.com>
Signed-off-by: marios <ma...@redhat.com>
---
server/lib/deltacloud/drivers/ec2/ec2_driver.rb | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
index 8f8394d..a6e2380 100644
--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
@@ -803,7 +803,7 @@ module Deltacloud
:public_addresses => [InstanceAddress.new(instance[:dns_name], :type => :hostname)],
:private_addresses => [InstanceAddress.new(instance[:private_dns_name], :type => :hostname)],
:firewalls => instance[:aws_groups],
- :storage_volumes => instance[:block_device_mappings],
+ :storage_volumes => instance[:block_device_mappings].map{|vol| {vol.values.first=>vol.keys.first } },
:create_image => can_create_image
)
end
--
1.7.6.4