[PATCH 2/2] Storage volumes/snapshots: massage a little

[lu...@redhat.com]

From: David Lutterkort <lu...@redhat.com>

Some minor changes to storage volumes/snapshots, mostly with a view of not
exposing things we might not be able to support:

  - include a unit in the capacity of a volume

  - put the instance/device pair at which a volume might be mounted into a
    <mount/> element, in case we ever need to talk about add'l instances in
    connection with a volume

  - do not report a state for snapshots and volumes; those need to be
    normalized first
---
 client/lib/deltacloud.rb                      |   24 +++++++++++++++---------
 client/specs/storage_volume_spec.rb           |    2 --
 server/views/storage_snapshots/index.xml.haml |    2 --
 server/views/storage_snapshots/show.xml.haml  |    2 --
 server/views/storage_volumes/index.xml.haml   |   12 ++++++------
 server/views/storage_volumes/show.xml.haml    |   14 +++++++-------
 tests/mock/storage_snapshots.feature          |    1 -
 tests/mock/storage_volumes.feature            |    5 +----
 8 files changed, 29 insertions(+), 33 deletions(-)

diff --git a/client/lib/deltacloud.rb b/client/lib/deltacloud.rb
index f797223..c632594 100644
--- a/client/lib/deltacloud.rb
+++ b/client/lib/deltacloud.rb
@@ -90,7 +90,7 @@ module DeltaCloud
     # Define methods based on 'rel' attribute in entry point
     # Two methods are declared: 'images' and 'image'
     def declare_entry_points_methods(entry_points)
-      
+
       API.instance_eval do
         entry_points.keys.select {|k| [:instance_states].include?(k)==false }.each do |model|
 
@@ -99,13 +99,13 @@ module DeltaCloud
               base_object_collection(model, response)
             end
           end
-          
+
           define_method :"#{model.to_s.singularize}" do |*args|
             request(:get, "#{entry_points[model]}/#{args[0]}") do |response|
               base_object(model, response)
             end
           end
-          
+
           define_method :"fetch_#{model.to_s.singularize}" do |url|
             id = url.grep(/\/#{model}\/(.*)$/)
             self.send(model.to_s.singularize.to_sym, $1)
@@ -129,7 +129,7 @@ module DeltaCloud
 
     # Convert XML response to defined Ruby Class
     def xml_to_class(base_object, item)
-      
+
       return nil unless item
 
       params = {
@@ -166,11 +166,17 @@ module DeltaCloud
           end && next
         end
 
+        if attribute.name == 'mount'
+          obj.add_link!("instance", (attribute/"./instance/@id").first)
+          obj.add_text!("device", (attribute/"./device/@name").first.value)
+          next
+        end
+
         # Deal with collections like public-addresses, private-addresses
         if (attribute/'./*').length > 0
           obj.add_collection!(attribute.name, (attribute/'*').collect { |value| value.text }) && next
         end
-        
+
         # Anything else is treaten as text object
         obj.add_text!(attribute.name, attribute.text.convert)
       end
@@ -185,15 +191,15 @@ module DeltaCloud
         api_xml = Nokogiri::XML(response)
         @driver_name = api_xml.xpath('/api').first['driver']
         @api_version = api_xml.xpath('/api').first['version']
-        
+
         api_xml.css("api > link").each do |entry_point|
           rel, href = entry_point['rel'].to_sym, entry_point['href']
           @entry_points.store(rel, href)
-          
+
           entry_point.css("feature").each do |feature|
             @features[rel] ||= []
             @features[rel] << feature['name'].to_sym
-            
+
           end
         end
       end
@@ -247,7 +253,7 @@ module DeltaCloud
       if conf[:query_args] != {}
         conf[:path] += '?' + URI.escape(conf[:query_args].collect{ |key, value| "#{key}=#{value}" }.join('&')).to_s
       end
-      
+
       if conf[:method].eql?(:post)
         RestClient.send(:post, conf[:path], conf[:form_data], default_headers) do |response, request, block|
           handle_backend_error(response) if response.code.eql?(500)
diff --git a/client/specs/storage_volume_spec.rb b/client/specs/storage_volume_spec.rb
index f57d9c1..4ed23b7 100644
--- a/client/specs/storage_volume_spec.rb
+++ b/client/specs/storage_volume_spec.rb
@@ -44,7 +44,6 @@ describe "storage volumes" do
       storage_volume = client.storage_volume( 'vol3' )
       storage_volume.id.should eql( 'vol3' )
       storage_volume.uri.should eql( API_URL + '/storage_volumes/vol3' )
-      storage_volume.state.should eql( 'IN-USE' )
       storage_volume.capacity.should eql( 1.0 )
       storage_volume.device.should eql( '/dev/sda1' )
       storage_volume.instance.should_not be_nil
@@ -61,7 +60,6 @@ describe "storage volumes" do
       storage_volume.should_not be_nil
       storage_volume.id.should eql( 'vol3' )
       storage_volume.uri.should eql( API_URL + '/storage_volumes/vol3' )
-      storage_volume.state.should eql( 'IN-USE' )
       storage_volume.capacity.should eql( 1.0 )
       storage_volume.device.should eql( '/dev/sda1' )
       storage_volume.instance.should_not be_nil
diff --git a/server/views/storage_snapshots/index.xml.haml b/server/views/storage_snapshots/index.xml.haml
index 4813798..8748532 100644
--- a/server/views/storage_snapshots/index.xml.haml
+++ b/server/views/storage_snapshots/index.xml.haml
@@ -4,6 +4,4 @@
     %storage_snapshot{ :href => storage_snapshot_url(snapshot.id), :id => snapshot.id }
       %created<
         =snapshot.created
-      %state<
-        =snapshot.state
       %storage_volume{ :href => storage_volume_url(snapshot.storage_volume_id), :id => snapshot.storage_volume_id }
diff --git a/server/views/storage_snapshots/show.xml.haml b/server/views/storage_snapshots/show.xml.haml
index 8043d1b..3cfa8a6 100644
--- a/server/views/storage_snapshots/show.xml.haml
+++ b/server/views/storage_snapshots/show.xml.haml
@@ -2,6 +2,4 @@
 %storage_snapshot{:href => storage_snapshot_url(@storage_snapshot.id), :id => @storage_snapshot.id }
   %created<
     =@storage_snapshot.created
-  %state<
-    =@storage_snapshot.state
   %storage_volume{:href => storage_volume_url(@storage_snapshot.storage_volume_id), :id => @storage_snapshot.storage_volume_id}
diff --git a/server/views/storage_volumes/index.xml.haml b/server/views/storage_volumes/index.xml.haml
index a96ab61..471169c 100644
--- a/server/views/storage_volumes/index.xml.haml
+++ b/server/views/storage_volumes/index.xml.haml
@@ -4,10 +4,10 @@
     %storage_volume{ :href => storage_volume_url(volume.id), :id => volume.id }
       %created<
         =volume.created
-      %capacity<
-        =volume.capacity
-      - unless volume.device.nil?
-        %device<
-          =volume.device
+      %capacity{ :unit => "GB" }<
+        = volume.capacity
       - unless volume.instance_id.nil?
-        %instance{:href => instance_url(volume.instance_id), :id => volume.instance_id}
+        %mount
+          %instance{:href => instance_url(volume.instance_id), :id => volume.instance_id}
+          - unless volume.device.nil?
+            %device{ :name => volume.device }
diff --git a/server/views/storage_volumes/show.xml.haml b/server/views/storage_volumes/show.xml.haml
index b64ff87..ebf1b24 100644
--- a/server/views/storage_volumes/show.xml.haml
+++ b/server/views/storage_volumes/show.xml.haml
@@ -2,10 +2,10 @@
 %storage_volume{ :href => storage_volume_url(@storage_volume.id), :id => @storage_volume.id}
   %created<
     =@storage_volume.created
-  %capacity<
-    =@storage_volume.capacity
-  %device<
-    =@storage_volume.device
-  %state<
-    =@storage_volume.state
-  %instance{:href => @storage_volume.instance_id, :id => @storage_volume.instance_id}
+  %capacity{ :unit => "GB" }<
+    = @storage_volume.capacity
+  - unless @storage_volume.instance_id.nil?
+    %mount
+      %instance{:href => @storage_volume.instance_id, :id => @storage_volume.instance_id}
+      - unless @storage_volume.device.nil?
+        %device{ :name => @storage_volume.device }
diff --git a/tests/mock/storage_snapshots.feature b/tests/mock/storage_snapshots.feature
index 6870aff..fdaff9c 100644
--- a/tests/mock/storage_snapshots.feature
+++ b/tests/mock/storage_snapshots.feature
@@ -23,5 +23,4 @@ Feature: Accessing storage snapshots
     Then client should get this storage_snapshot
     And this storage_snapshot should have:
     | created |
-    | state |
     | storage_volume |
diff --git a/tests/mock/storage_volumes.feature b/tests/mock/storage_volumes.feature
index f431bc8..3011cf5 100644
--- a/tests/mock/storage_volumes.feature
+++ b/tests/mock/storage_volumes.feature
@@ -6,7 +6,7 @@ Feature: Accessing storage volumes
  # expected: ["capacity", "created", "device", "instance", "state"],
  #    got: ["capacity", "created"] (using ==)
  #
- # When I remove device, instance and state I got back 
+ # When I remove device, instance and state I got back
  # ["capacity", "created", "device", "instance", "state"]
  #
  # Anyway this test isn't usefull, because storage volumes will be
@@ -39,6 +39,3 @@ Feature: Accessing storage volumes
     And this storage_volume should have:
     | created |
     | capacity |
-    | state |
-    | device |
-    | instance |
-- 
1.7.2.3