You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by mf...@redhat.com on 2012/10/03 18:10:29 UTC

[PATCH core 1/4] CIMI: Removed already fixed Struct constant removal

From: Michal Fojtik <mf...@redhat.com>


Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/cimi/models/base.rb |    4 ----
 1 file changed, 4 deletions(-)

diff --git a/server/lib/cimi/models/base.rb b/server/lib/cimi/models/base.rb
index e36ad6e..86a8188 100644
--- a/server/lib/cimi/models/base.rb
+++ b/server/lib/cimi/models/base.rb
@@ -103,10 +103,6 @@ class CIMI::Model::Base
     def <<(model)
       clone_base_schema unless base_schema_cloned?
       member_name = model.name.split("::").last
-      if ::Struct.const_defined?("CIMI_#{member_name}")
-        puts "Removing struct"
-        ::Struct.send(:remove_const, "CIMI_#{member_name}")
-      end
       member_symbol = member_name.underscore.pluralize.to_sym
       members = CIMI::Model::Schema::Array.new(member_symbol)
       members.struct.schema.attributes = model.schema.attributes
-- 
1.7.10.2


Re: [PATCH core 3/4] CIMI: Removed event_logs and unused vars

Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2012-10-03 at 18:10 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>
> ---
>  server/lib/cimi/models/machine.rb      |    2 --
>  server/lib/cimi/models/network.rb      |    2 --
>  server/lib/cimi/models/network_port.rb |    2 --
>  server/lib/cimi/models/volume.rb       |   11 +++++------
>  4 files changed, 5 insertions(+), 12 deletions(-)

ACK, though you also need to remove eventLog in the test fixtures
(incremental patch attached)

> diff --git a/server/lib/cimi/models/volume.rb b/server/lib/cimi/models/volume.rb
> index 8d32c66..5e761f4 100644
> --- a/server/lib/cimi/models/volume.rb
> +++ b/server/lib/cimi/models/volume.rb
> @@ -68,14 +68,14 @@ class CIMI::Model::Volume < CIMI::Model::Base
>  
>    def self.find_to_attach_from_json(json_in, context)
>      json = JSON.parse(json_in)
> -    volumes = json["volumes"].map{|v| {:volume=>self.find(v["volume"]["href"].split("/volumes/").last, context),
> -                                       :attachment_point=>v["attachmentPoint"]  }}
> +    json["volumes"].map{|v| {:volume=>self.find(v["volume"]["href"].split("/volumes/").last, context),
> +                             :attachment_point=>v["attachmentPoint"]  }}
>    end
>  
>    def self.find_to_attach_from_xml(xml_in, context)
>      xml = XmlSimple.xml_in(xml_in)
> -    volumes = xml["volume"].map{|v| {:volume => self.find(v["href"].split("/volumes/").last, context),
> -                                      :attachment_point=>v["attachmentPoint"] }}
> +    xml["volume"].map{|v| {:volume => self.find(v["href"].split("/volumes/").last, context),
> +                           :attachment_point=>v["attachmentPoint"] }}
>    end

This hunk seems to only change formatting - please leave that out of
this patch.

David


[PATCH core 3/4] CIMI: Removed event_logs and unused vars

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>


Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/cimi/models/machine.rb      |    2 --
 server/lib/cimi/models/network.rb      |    2 --
 server/lib/cimi/models/network_port.rb |    2 --
 server/lib/cimi/models/volume.rb       |   11 +++++------
 4 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/server/lib/cimi/models/machine.rb b/server/lib/cimi/models/machine.rb
index 35a0354..334defc 100644
--- a/server/lib/cimi/models/machine.rb
+++ b/server/lib/cimi/models/machine.rb
@@ -22,8 +22,6 @@ class CIMI::Model::Machine < CIMI::Model::Base
 
   text :memory
 
-  href :event_log
-
   subcollection :disks
   subcollection :volumes, :use => :machine_volumes
 
diff --git a/server/lib/cimi/models/network.rb b/server/lib/cimi/models/network.rb
index 4e783ea..b08984a 100644
--- a/server/lib/cimi/models/network.rb
+++ b/server/lib/cimi/models/network.rb
@@ -29,8 +29,6 @@ class CIMI::Model::Network < CIMI::Model::Base
 
   href :forwarding_group
 
-  href :event_log
-
   array :meters do
     scalar :href
   end
diff --git a/server/lib/cimi/models/network_port.rb b/server/lib/cimi/models/network_port.rb
index 6905f5a..2951ffd 100644
--- a/server/lib/cimi/models/network_port.rb
+++ b/server/lib/cimi/models/network_port.rb
@@ -25,8 +25,6 @@ class CIMI::Model::NetworkPort < CIMI::Model::Base
 
   text :class_of_service
 
-  href :event_log
-
   array :meters do
     scalar :href
   end
diff --git a/server/lib/cimi/models/volume.rb b/server/lib/cimi/models/volume.rb
index 8d32c66..5e761f4 100644
--- a/server/lib/cimi/models/volume.rb
+++ b/server/lib/cimi/models/volume.rb
@@ -30,7 +30,7 @@ class CIMI::Model::Volume < CIMI::Model::Base
   array :meters do
     scalar :ref
   end
-  href :eventlog
+
   array :operations do
     scalar :rel, :href
   end
@@ -68,14 +68,14 @@ class CIMI::Model::Volume < CIMI::Model::Base
 
   def self.find_to_attach_from_json(json_in, context)
     json = JSON.parse(json_in)
-    volumes = json["volumes"].map{|v| {:volume=>self.find(v["volume"]["href"].split("/volumes/").last, context),
-                                       :attachment_point=>v["attachmentPoint"]  }}
+    json["volumes"].map{|v| {:volume=>self.find(v["volume"]["href"].split("/volumes/").last, context),
+                             :attachment_point=>v["attachmentPoint"]  }}
   end
 
   def self.find_to_attach_from_xml(xml_in, context)
     xml = XmlSimple.xml_in(xml_in)
-    volumes = xml["volume"].map{|v| {:volume => self.find(v["href"].split("/volumes/").last, context),
-                                      :attachment_point=>v["attachmentPoint"] }}
+    xml["volume"].map{|v| {:volume => self.find(v["href"].split("/volumes/").last, context),
+                           :attachment_point=>v["attachmentPoint"] }}
   end
 
   private
@@ -97,7 +97,6 @@ class CIMI::Model::Volume < CIMI::Model::Base
                 :supports_snapshots => "true", #fixme, will vary (true for ec2)
                 :snapshots => [], #fixme...
                 :guest_interface => "",
-                :eventlog => {:href=> "http://eventlogs"},#FIXME
                 :meters => []
             } )
   end
-- 
1.7.10.2


[PATCH core 4/4] CIMI: Added support for $expand in Network model

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>


Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/cimi/collections/networks.rb           |    2 +-
 server/lib/cimi/models.rb                         |    2 +-
 server/lib/cimi/models/network.rb                 |    7 ++++++-
 server/lib/cimi/models/network_port.rb            |   13 +++++++++++++
 server/lib/cimi/models/network_port_collection.rb |   14 --------------
 5 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/server/lib/cimi/collections/networks.rb b/server/lib/cimi/collections/networks.rb
index e006503..3b8b864 100644
--- a/server/lib/cimi/collections/networks.rb
+++ b/server/lib/cimi/collections/networks.rb
@@ -125,7 +125,7 @@ module CIMI::Collections
         description "Retrieve the Network's NetworkPortCollection"
         param :id, :string, :required
         control do
-          network_ports = NetworkPortCollection.for_network(params[:id], self)
+          network_ports = NetworkPort.collection_for_network(params[:id], self)
           respond_to do |format|
             format.json {network_ports.to_json}
             format.xml  {network_ports.to_xml}
diff --git a/server/lib/cimi/models.rb b/server/lib/cimi/models.rb
index f5ae48b..d726bd3 100644
--- a/server/lib/cimi/models.rb
+++ b/server/lib/cimi/models.rb
@@ -42,10 +42,10 @@ require_relative './models/machine_template'
 require_relative './models/machine_configuration'
 require_relative './models/machine_image'
 require_relative './models/credential'
+require_relative './models/network_port'
 require_relative './models/network'
 require_relative './models/network_template'
 require_relative './models/network_configuration'
-require_relative './models/network_port'
 require_relative './models/network_port_template'
 require_relative './models/network_port_configuration'
 require_relative './models/address'
diff --git a/server/lib/cimi/models/network.rb b/server/lib/cimi/models/network.rb
index b08984a..aa9c6ca 100644
--- a/server/lib/cimi/models/network.rb
+++ b/server/lib/cimi/models/network.rb
@@ -25,7 +25,7 @@ class CIMI::Model::Network < CIMI::Model::Base
 
   text :class_of_service
 
-  href :network_ports
+  subcollection :network_ports
 
   href :forwarding_group
 
@@ -44,6 +44,11 @@ class CIMI::Model::Network < CIMI::Model::Base
     else
       networks = context.driver.networks(context.credentials, {:id=>id, :env=>context})
     end
+    if context.cimi_expand.include? 'networkPorts'
+      networks.each do |network|
+        network.network_ports = CIMI::Model::NetworkPort.collection_for_network(network.id, context)
+      end
+    end
     networks
   end
 
diff --git a/server/lib/cimi/models/network_port.rb b/server/lib/cimi/models/network_port.rb
index 2951ffd..d69dc3f 100644
--- a/server/lib/cimi/models/network_port.rb
+++ b/server/lib/cimi/models/network_port.rb
@@ -74,6 +74,19 @@ class CIMI::Model::NetworkPort < CIMI::Model::Base
     end
   end
 
+  def self.collection_for_network(network_id, context)
+    net_url = context.network_url(network_id)
+    network_ports = CIMI::Model::NetworkPort.all(context)
+    ports_collection = network_ports.inject([]){|res, current| res << current if current.network.href == net_url ; res}
+    CIMI::Model::NetworkPortCollection.new(
+      :id => net_url+"/network_ports",
+      :name => 'default',
+      :created => Time.now,
+      :description => "#{context.driver.name.capitalize} NetworkPortCollection",
+      :count => ports_collection.size,
+      :network_ports => ports_collection
+    )
+  end
 
   private
 
diff --git a/server/lib/cimi/models/network_port_collection.rb b/server/lib/cimi/models/network_port_collection.rb
index 32642cf..8b4f62e 100644
--- a/server/lib/cimi/models/network_port_collection.rb
+++ b/server/lib/cimi/models/network_port_collection.rb
@@ -34,18 +34,4 @@ class CIMI::Model::NetworkPortCollection < CIMI::Model::Base
     )
   end
 
-  def self.for_network(network_id, context)
-    net_url = context.network_url(network_id)
-    network_ports = CIMI::Model::NetworkPort.all(context)
-    ports_collection = network_ports.inject([]){|res, current| res << current if current.network.href == net_url ; res}
-    self.new(
-      :id => net_url+"/network_ports",
-      :name => 'default',
-      :created => Time.now,
-      :description => "#{context.driver.name.capitalize} NetworkPortCollection",
-      :count => ports_collection.size,
-      :network_ports => ports_collection
-    )
-  end
-
 end
-- 
1.7.10.2


Re: [PATCH core 1/4] CIMI: Removed already fixed Struct constant removal

Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2012-10-03 at 18:10 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> 
> Signed-off-by: Michal fojtik <mf...@redhat.com>

ACK

David



Re: [PATCH core 2/4] CIMI: Initial implementation of $expand

Posted by Michal Fojtik <mf...@redhat.com>.
On Oct 6, 2012, at 2:51 AM, David Lutterkort <lu...@redhat.com> wrote:

> Hi Michal,
> 
> On Wed, 2012-10-03 at 18:10 +0200, mfojtik@redhat.com wrote:
>> From: Michal Fojtik <mf...@redhat.com>
>> 
>> * This PoC works only for MachineCollection now (disks, volumes)
> 
> this finally forced me to finish the stuff I had done around embedded
> collections. I'll send out a patch series for that.
> 
> The stuff you did around $expand would still be usable, I think (after
> some serious rebasing ;)

Rebase complete :-) However I found few issues:

* http://localhost:3001/cimi/machines/inst0/disks -> return

, [2012-10-08T12:28:10.823334 #96183] ERROR -- : [500] NoMethodError:undefined method `default' for CIMI::Model::DiskCollection:Class

/Users/mfojtik/code/core/server/lib/cimi/collections/machines.rb:126:in `block (3 levels) in <class:Machines>'

* The collections are always expanded. I don't think this is the right behavior
  here. AFAIK the point of collections was to display just 'href' to resource
  and then expand details on those you want (with $expand + $filter combo).
  But having everything expanded by default makes this useless from client point
  of view and also from backend POV (DC need always to query backend for extra details).

  -- Michal

Michal Fojtik
http://deltacloud.org
mfojtik@redhat.com




Re: [PATCH core 2/4] CIMI: Initial implementation of $expand

Posted by David Lutterkort <lu...@redhat.com>.
Hi Michal,

On Wed, 2012-10-03 at 18:10 +0200, mfojtik@redhat.com wrote:
> From: Michal Fojtik <mf...@redhat.com>
> 
> * This PoC works only for MachineCollection now (disks, volumes)

this finally forced me to finish the stuff I had done around embedded
collections. I'll send out a patch series for that.

The stuff you did around $expand would still be usable, I think (after
some serious rebasing ;)

David



[PATCH core 2/4] CIMI: Initial implementation of $expand

Posted by mf...@redhat.com.
From: Michal Fojtik <mf...@redhat.com>

* This PoC works only for MachineCollection now (disks, volumes)

Signed-off-by: Michal fojtik <mf...@redhat.com>
---
 server/lib/cimi/helpers/cimi_helper.rb |    4 ++++
 server/lib/cimi/models.rb              |    8 ++++----
 server/lib/cimi/models/machine.rb      |   20 +++++++++++++-------
 server/lib/cimi/models/schema.rb       |   19 ++++++++++++++++++-
 4 files changed, 39 insertions(+), 12 deletions(-)

diff --git a/server/lib/cimi/helpers/cimi_helper.rb b/server/lib/cimi/helpers/cimi_helper.rb
index ea1e89e..2da684f 100644
--- a/server/lib/cimi/helpers/cimi_helper.rb
+++ b/server/lib/cimi/helpers/cimi_helper.rb
@@ -16,6 +16,10 @@
 module CIMI
   module Helper
 
+    def cimi_expand
+      (params['$expand'] || '').split(',')
+    end
+
     def no_content_with_status(code=200)
       body ''
       status code
diff --git a/server/lib/cimi/models.rb b/server/lib/cimi/models.rb
index df328f5..f5ae48b 100644
--- a/server/lib/cimi/models.rb
+++ b/server/lib/cimi/models.rb
@@ -33,15 +33,15 @@ require_relative './models/machine_volume_collection'
 # in which the entities appear in the CEP
 require_relative './models/cloud_entry_point'
 require_relative './models/resource_metadata'
+require_relative './models/volume'
+require_relative './models/volume_template'
+require_relative './models/volume_configuration'
+require_relative './models/volume_image'
 require_relative './models/machine'
 require_relative './models/machine_template'
 require_relative './models/machine_configuration'
 require_relative './models/machine_image'
 require_relative './models/credential'
-require_relative './models/volume'
-require_relative './models/volume_template'
-require_relative './models/volume_configuration'
-require_relative './models/volume_image'
 require_relative './models/network'
 require_relative './models/network_template'
 require_relative './models/network_configuration'
diff --git a/server/lib/cimi/models/machine.rb b/server/lib/cimi/models/machine.rb
index da67eb7..35a0354 100644
--- a/server/lib/cimi/models/machine.rb
+++ b/server/lib/cimi/models/machine.rb
@@ -24,9 +24,8 @@ class CIMI::Model::Machine < CIMI::Model::Base
 
   href :event_log
 
-  href :disks
-
-  href :volumes
+  subcollection :disks
+  subcollection :volumes, :use => :machine_volumes
 
   href :network_interfaces
 
@@ -51,7 +50,6 @@ class CIMI::Model::Machine < CIMI::Model::Base
   end
 
   def self.create_from_json(body, context)
-    json = JSON.parse(body)
     hardware_profile_id = xml['machineTemplate']['machineConfig']["href"].split('/').last
     image_id = xml['machineTemplate']['machineImage']["href"].split('/').last
     instance = context.create_instance(context.credentials, image_id, { :hwp_id => hardware_profile_id })
@@ -118,7 +116,7 @@ class CIMI::Model::Machine < CIMI::Model::Base
   private
   def self.from_instance(instance, context)
     cpu =  memory = (instance.instance_profile.id == "opaque")? "n/a" : nil
-    self.new(
+    machine_spec = {
       :name => instance.id,
       :description => instance.name,
       :created => instance.launch_time,
@@ -129,9 +127,17 @@ class CIMI::Model::Machine < CIMI::Model::Base
       :disks => {:href => context.machine_url(instance.id)+"/disks"},
       :network_interfaces => {:href => context.machine_url(instance.id+"/network_interfaces")},
       :operations => convert_instance_actions(instance, context),
-      :volumes=>{:href=>context.machine_url(instance.id)+"/volumes"},
+      :volumes=> { :href=>context.machine_url(instance.id)+"/volumes" },
       :property => convert_instance_properties(instance, context)
-    )
+    }
+    if context.cimi_expand.include? 'disks'
+      machine_spec[:disks].merge!(CIMI::Model::DiskCollection.default(instance.id, context).attribute_values)
+    end
+    if context.cimi_expand.include? 'volumes'
+      machine_spec[:volumes].merge!(CIMI::Model::MachineVolumeCollection.default(instance.id, context).attribute_values)
+    end
+    machine = self.new(machine_spec)
+    machine
   end
 
   # FIXME: This will convert 'RUNNING' state to 'STARTED'
diff --git a/server/lib/cimi/models/schema.rb b/server/lib/cimi/models/schema.rb
index ed3541a..ba8946c 100644
--- a/server/lib/cimi/models/schema.rb
+++ b/server/lib/cimi/models/schema.rb
@@ -237,7 +237,9 @@ class CIMI::Model::Schema
   def to_xml(model, xml = nil)
     xml ||= OrderedHash.new
     @attributes.freeze
-    @attributes.each { |attr| attr.to_xml(model, xml) }
+    @attributes.each do |attr|
+      attr.to_xml(model, xml)
+    end
     xml
   end
 
@@ -266,11 +268,26 @@ class CIMI::Model::Schema
   # Requires that the class into which this is included has a
   # +add_attributes!+ method
   module DSL
+
     def href(*args)
       opts = args.extract_opts!
       args.each { |arg| struct(arg, opts) { scalar :href } }
     end
 
+    def subcollection(name, opts={})
+      collection_model_name = "#{(opts[:use] || name).to_s.singularize.camelize}Collection"
+      begin
+        collection_model = CIMI::Model::const_get(collection_model_name)
+        add_attributes!([name, {}], Struct, &Proc.new{
+          scalar :href
+          @attributes += collection_model.schema.attributes
+        })
+      rescue NameError
+        warn "WARNING: You need to require '#{collection_model_name}' first to use "+
+          "it as a subcollection for the #{self.name} model."
+      end
+    end
+
     def text(*args)
       args.expand_opts!(:text => :nested)
       scalar(*args)
-- 
1.7.10.2