You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by lu...@redhat.com on 2013/03/01 01:15:26 UTC
RFC: Separate CIMI (de)serialization from server functionality
This patch shows how we can split the current CIMI::Model classes into two
different class hierarchies, CIMI::Model and CIMI::Service. After the
split, the former will strictly be restricted to (de)serialization of CIMI
resources whereas the latter will be dedicated to interaction with drivers
and the DB.
One motivator for this change is that the (de)serialization functionality
of the current CIMI::Model classes is useful in other contexts, especially
for clients, be those blackbox tests or the CIMI webapp. Dealing with
drivers and the DB on the other hand is only useful for the server.
Once we have completed the split, we should extract the CIMI::Model classes
into a separate gem that can be used as the basis for CIMI clients. An
important aspect of this is that the new CIMI::Model classes should not
make any reference to a context (server) anymore.
The patch is by no means complete, it only shows what this split would look
like using the example of Machine. The tests in
tests/cimi/collection/machines_test.rb do pass though :)
Michal: feel free to expand this patch.
David
Re: RFC: Separate CIMI (de)serialization from server functionality
Posted by Michal Fojtik <mf...@redhat.com>.
On 02/28, lutter@redhat.com wrote:
>
> This patch shows how we can split the current CIMI::Model classes into two
> different class hierarchies, CIMI::Model and CIMI::Service. After the
> split, the former will strictly be restricted to (de)serialization of CIMI
> resources whereas the latter will be dedicated to interaction with drivers
> and the DB.
>
> One motivator for this change is that the (de)serialization functionality
> of the current CIMI::Model classes is useful in other contexts, especially
> for clients, be those blackbox tests or the CIMI webapp. Dealing with
> drivers and the DB on the other hand is only useful for the server.
>
> Once we have completed the split, we should extract the CIMI::Model classes
> into a separate gem that can be used as the basis for CIMI clients. An
> important aspect of this is that the new CIMI::Model classes should not
> make any reference to a context (server) anymore.
>
> The patch is by no means complete, it only shows what this split would look
> like using the example of Machine. The tests in
> tests/cimi/collection/machines_test.rb do pass though :)
Thanks! This all sounds very resonable and also will enable us to ship a
full featured CIMI ruby client (webapp/binding).
>
> Michal: feel free to expand this patch.
Working on it. I'll send rev2 today afternoon, so somebody else can also
jump in ;-)
-- Michal
--
Michal Fojtik <mf...@redhat.com>
Deltacloud API, CloudForms
[PATCH] CIMI: split models into model and service objects
Posted by lu...@redhat.com.
From: David Lutterkort <lu...@redhat.com>
The current CIMI::Model classes address two concerns:
* serialization/deserialization of CIMI objects
* interaction with the current driver and the DB
This patch splits these two concerns into two separate class hierarchies:
CIMI::Model for (de)serialization and CIMI::Service for interacting with
drivers/db
Besides cleaning up the code, this will make it possible to reuse
CIMI::Model classes as the basis for client code.
---
server/lib/cimi/collections/base.rb | 4 +-
server/lib/cimi/models/collection.rb | 27 ++----
server/lib/cimi/models/machine.rb | 125 --------------------------
server/lib/cimi/service.rb | 21 +++++
server/lib/cimi/service/base.rb | 169 +++++++++++++++++++++++++++++++++++
server/lib/cimi/service/machine.rb | 147 ++++++++++++++++++++++++++++++
6 files changed, 347 insertions(+), 146 deletions(-)
create mode 100644 server/lib/cimi/service.rb
create mode 100644 server/lib/cimi/service/base.rb
create mode 100644 server/lib/cimi/service/machine.rb
diff --git a/server/lib/cimi/collections/base.rb b/server/lib/cimi/collections/base.rb
index cf92143..f268080 100644
--- a/server/lib/cimi/collections/base.rb
+++ b/server/lib/cimi/collections/base.rb
@@ -13,14 +13,14 @@
# License for the specific language governing permissions and limitations
# under the License.
-require_relative '../models'
+require_relative '../service'
module CIMI::Collections
class Base < Sinatra::Base
include Sinatra::Rabbit
include Sinatra::Rabbit::Features
- include CIMI::Model
+ include CIMI::Service
helpers Deltacloud::Helpers::Drivers
helpers Deltacloud::Helpers::Database
diff --git a/server/lib/cimi/models/collection.rb b/server/lib/cimi/models/collection.rb
index f36c081..cc95ab7 100644
--- a/server/lib/cimi/models/collection.rb
+++ b/server/lib/cimi/models/collection.rb
@@ -120,26 +120,15 @@ module CIMI::Model
end
# Return a collection of entities
- def list(context)
- entries = find(:all, context)
- desc = "#{self.name.split("::").last} Collection for the #{context.driver.name.capitalize} driver"
- acts_as_root_entity unless collection_class
- id = context.send("#{collection_class.entry_name}_url")
- ops = []
- cimi_entity = collection_class.entry_name.to_s.singularize
- cimi_create = "create_#{cimi_entity}_url"
- dcloud_create = context.deltacloud_create_method_for(cimi_entity)
- if(context.respond_to?(cimi_create) &&
- context.driver.respond_to?(dcloud_create)) ||
- provides?(cimi_entity)
- url = context.send(cimi_create)
- ops << { :rel => "add", :href => url }
+ def list(id, entries, params = {})
+ params[:id] = id
+ params[:entries] = entries
+ params[:count] = params[:entries].size
+ if params[:add_url]
+ params[:operations] ||= []
+ params[:operations] << { :rel => "add", :href => params.delete(:add_url) }
end
- collection_class.new(:id => id,
- :count => entries.size,
- :entries => entries,
- :operations => ops,
- :description => desc)
+ collection_class.new(params)
end
def all(context)
diff --git a/server/lib/cimi/models/machine.rb b/server/lib/cimi/models/machine.rb
index 3beb2f7..6990fb9 100644
--- a/server/lib/cimi/models/machine.rb
+++ b/server/lib/cimi/models/machine.rb
@@ -38,129 +38,4 @@ class CIMI::Model::Machine < CIMI::Model::Base
scalar :rel, :href
end
- def self.find(id, context)
- instances = []
- if id == :all
- instances = context.driver.instances(context.credentials)
- instances.map { |instance| from_instance(instance, context) }.compact
- else
- instance = context.driver.instance(context.credentials, :id => id)
- raise CIMI::Model::NotFound unless instance
- from_instance(instance, context)
- end
- end
-
- def perform(action, context, &block)
- begin
- if context.driver.send(:"#{action.name}_instance", context.credentials, self.id.split("/").last)
- block.callback :success
- else
- raise "Operation failed to execute on given Machine"
- end
- rescue => e
- block.callback :failure, e.message
- end
- end
-
- def self.delete!(id, context)
- context.driver.destroy_instance(context.credentials, id)
- new(:id => id).destroy
- end
-
- #returns the newly attach machine_volume
- def self.attach_volume(volume, location, context)
- context.driver.attach_storage_volume(context.credentials,
- {:id=>volume, :instance_id=>context.params[:id], :device=>location})
- CIMI::Model::MachineVolume.find(context.params[:id], context, volume)
- end
-
- #returns the machine_volume_collection for the given machine
- def self.detach_volume(volume, location, context)
- context.driver.detach_storage_volume(context.credentials,
- {:id=>volume, :instance_id=>context.params[:id], :device=>location})
- CIMI::Model::MachineVolume.collection_for_instance(context.params[:id], context)
- end
-
- def self.from_instance(instance, context)
- cpu = memory = (instance.instance_profile.id == "opaque")? "n/a" : nil
- machine_conf = CIMI::Model::MachineConfiguration.find(instance.instance_profile.name, context)
- machine_spec = {
- :name => instance.name,
- :created => instance.launch_time.nil? ? Time.now.xmlschema : Time.parse(instance.launch_time.to_s).xmlschema,
- :description => "No description set for Machine #{instance.name}",
- :id => context.machine_url(instance.id),
- :state => convert_instance_state(instance.state),
- :cpu => cpu || convert_instance_cpu(instance.instance_profile, context),
- :memory => memory || convert_instance_memory(instance.instance_profile, context),
- :disks => { :href => context.machine_url(instance.id)+"/disks"},
- :volumes => { :href=>context.machine_url(instance.id)+"/volumes"},
- :operations => convert_instance_actions(instance, context)
- }
- if context.expand? :disks
- machine_spec[:disks] = CIMI::Model::Disk.find(instance, machine_conf, context, :all)
- end
- if context.expand? :volumes
- machine_spec[:volumes] = CIMI::Model::MachineVolume.find(instance.id, context, :all)
- end
- machine_spec[:realm] = instance.realm_id if instance.realm_id
- machine_spec[:machine_image] = { :href => context.machine_image_url(instance.image_id) } if instance.image_id
- self.new(machine_spec)
- end
-
- # FIXME: This will convert 'RUNNING' state to 'STARTED'
- # which is defined in CIMI (p65)
- #
- def self.convert_instance_state(state)
- case state
- when "RUNNING" then "STARTED"
- when "PENDING" then "CREATING" #aruba only exception... could be "STARTING" here
- else state
- end
- end
-
- def self.convert_instance_cpu(profile, context)
- cpu_override = profile.overrides.find { |p, v| p == :cpu }
- if cpu_override.nil?
- CIMI::Model::MachineConfiguration.find(profile.id, context).cpu
- else
- cpu_override[1]
- end
- end
-
- def self.convert_instance_memory(profile, context)
- machine_conf = CIMI::Model::MachineConfiguration.find(profile.name, context)
- memory_override = profile.overrides.find { |p, v| p == :memory }
- memory_override.nil? ? machine_conf.memory.to_i : context.to_kibibyte(memory_override[1].to_i,"MB")
- end
-
- def self.convert_instance_addresses(instance)
- (instance.public_addresses + instance.private_addresses).collect do |address|
- {
- :hostname => address.is_hostname? ? address : nil,
- :mac_address => address.is_mac? ? address : nil,
- :state => 'Active',
- :protocol => 'IPv4',
- :address => address.is_ipv4? ? address : nil,
- :allocation => 'Static'
- }
- end
- end
-
- def self.convert_instance_actions(instance, context)
- actions = instance.actions.collect do |action|
- action = :restart if action == :reboot
- name = action
- name = :delete if action == :destroy # In CIMI destroy operation become delete
- { :href => context.send(:"#{action}_machine_url", instance.id), :rel => "http://schemas.dmtf.org/cimi/1/action/#{name}" }
- end
- actions << { :href => context.send(:"machine_images_url"), :rel => "http://schemas.dmtf.org/cimi/1/action/capture" } if instance.can_create_image?
- actions
- end
-
- def self.convert_storage_volumes(instance, context)
- instance.storage_volumes ||= [] #deal with nilpointers
- instance.storage_volumes.map{|vol| {:href=>context.volume_url(vol.keys.first),
- :initial_location=>vol.values.first} }
- end
-
end
diff --git a/server/lib/cimi/service.rb b/server/lib/cimi/service.rb
new file mode 100644
index 0000000..c9b662d
--- /dev/null
+++ b/server/lib/cimi/service.rb
@@ -0,0 +1,21 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless require_relatived by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+#
+
+module CIMI::Service; end
+
+require_relative './models'
+require_relative './service/base'
+require_relative './service/machine'
diff --git a/server/lib/cimi/service/base.rb b/server/lib/cimi/service/base.rb
new file mode 100644
index 0000000..9c281b7
--- /dev/null
+++ b/server/lib/cimi/service/base.rb
@@ -0,0 +1,169 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+require 'xmlsimple'
+
+require_relative '../helpers/database_helper'
+
+# Service objects implement the server functionality of CIMI resources; in
+# particular, these objects are responsible for interacting with the
+# current driver. They use the CIMI::Model objects for (de)serialization
+module CIMI::Service
+
+ class Base
+
+ # Extend the base model with database methods
+ extend Deltacloud::Helpers::Database
+
+ attr_reader :model, :context
+
+ class << self
+ def model_class
+ CIMI::Model.const_get(name.split('::').last)
+ end
+
+ def model_name
+ name.split('::').last.underscore.to_sym
+ end
+
+ def collection_name
+ name.split('::').last.underscore.pluralize.to_sym
+ end
+
+ def inherited(subclass)
+ # Decorate all the attributes of the model class
+ schema = subclass.model_class.schema
+ schema.attribute_names.each do |name|
+ define_method(name) { self[name] }
+ define_method(:"#{name}=") { |newval| self[name] = newval }
+ end
+ end
+ end
+
+ def initialize(context, opts)
+ if opts[:values]
+ @model = model_class.new(opts[:values])
+ elsif opts[:model]
+ @model = opts[:model]
+ else
+ @model = model_class.new({})
+ end
+ @context = context
+ retrieve_entity
+ end
+
+ def model_class
+ self.class.model_class
+ end
+
+ # Decorate some model methods
+ def []=(a, v)
+ v = (@model[a] = v)
+ retrieve_entity if a == :id
+ v
+ end
+
+ def [](a)
+ @model[a]
+ end
+
+ def to_xml
+ @model.to_xml
+ end
+
+ def to_json
+ @model.to_json
+ end
+
+ def select_attributes(attr_list)
+ @model.select_attributes(attr_list)
+ end
+
+ def self.list(ctx)
+ id = ctx.send("#{collection_name}_url")
+ entries = find(:all, ctx)
+ params = {}
+ params[:desc] = "#{self.name.split("::").last} Collection for the #{ctx.driver.name.capitalize} driver"
+ params[:add_url] = create_url(ctx)
+ model_class.list(id, entries, params)
+ end
+
+ def self.create_url(ctx)
+ cimi_create = "create_#{model_name}_url"
+ dcloud_create = ctx.deltacloud_create_method_for(model_name)
+ if(ctx.respond_to?(cimi_create) &&
+ ctx.respond_to?(dcloud_create)) || provides?(model_name)
+ ctx.send(cimi_create)
+ end
+ end
+
+ # Save the common attributes name, description, and properties to the
+ # database
+ def save
+ if @entity
+ @entity.name = @model.name
+ @entity.description = @model.description
+ @entity.properties = @model.property
+ @entity.save
+ end
+ self
+ end
+
+ # Destroy the database attributes for this model
+ def destroy
+ @entity.destroy
+ self
+ end
+
+ # FIXME: Kludge around the fact that we do not have proper *Create
+ # objects that deserialize properties by themselves
+ def extract_properties!(data)
+ h = {}
+ if data['property']
+ # Data came from XML
+ h = data['property'].inject({}) do |r,v|
+ r[v['key']] = v['content']
+ r
+ end
+ elsif data['properties']
+ h = data['properties']
+ end
+ property ||= {}
+ property.merge!(h)
+ end
+
+ def ref_id(ref_url)
+ ref_url.split('/').last if ref_url
+ end
+
+ private
+
+ # Load an existing database entity for this object, or create a new one
+ def retrieve_entity
+ if self.id
+ @entity = Deltacloud::Database::Entity::retrieve(self)
+ if @entity.exists?
+ @model.name = @entity.name
+ @model.description = @entity.description
+ @model.property ||= {}
+ @model.property.merge!(@entity.properties)
+ end
+ else
+ @entity = nil
+ end
+ end
+
+ end
+end
diff --git a/server/lib/cimi/service/machine.rb b/server/lib/cimi/service/machine.rb
new file mode 100644
index 0000000..dedb0ad
--- /dev/null
+++ b/server/lib/cimi/service/machine.rb
@@ -0,0 +1,147 @@
+# Licensed to the Apache Software Foundation (ASF) under one or more
+# contributor license agreements. See the NOTICE file distributed with
+# this work for additional information regarding copyright ownership. The
+# ASF licenses this file to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance with the
+# License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+# License for the specific language governing permissions and limitations
+# under the License.
+
+class CIMI::Service::Machine < CIMI::Service::Base
+
+ def initialize(ctx, opts)
+ super
+ end
+
+ def self.find(id, context)
+ instances = []
+ if id == :all
+ instances = context.driver.instances(context.credentials)
+ instances.map { |instance| from_instance(instance, context) }.compact
+ else
+ instance = context.driver.instance(context.credentials, :id => id)
+ raise CIMI::Model::NotFound unless instance
+ from_instance(instance, context)
+ end
+ end
+
+ def perform(action, context, &block)
+ begin
+ if context.driver.send(:"#{action.name}_instance", context.credentials, self.id.split("/").last)
+ block.callback :success
+ else
+ raise "Operation failed to execute on given Machine"
+ end
+ rescue => e
+ block.callback :failure, e.message
+ end
+ end
+
+ def self.delete!(id, context)
+ context.driver.destroy_instance(context.credentials, id)
+ new(:id => id).destroy
+ end
+
+ #returns the newly attach machine_volume
+ def self.attach_volume(volume, location, context)
+ context.driver.attach_storage_volume(context.credentials,
+ {:id=>volume, :instance_id=>context.params[:id], :device=>location})
+ CIMI::Model::MachineVolume.find(context.params[:id], context, volume)
+ end
+
+ #returns the machine_volume_collection for the given machine
+ def self.detach_volume(volume, location, context)
+ context.driver.detach_storage_volume(context.credentials,
+ {:id=>volume, :instance_id=>context.params[:id], :device=>location})
+ CIMI::Model::MachineVolume.collection_for_instance(context.params[:id], context)
+ end
+
+ def self.from_instance(instance, context)
+ cpu = memory = (instance.instance_profile.id == "opaque")? "n/a" : nil
+ machine_conf = CIMI::Model::MachineConfiguration.find(instance.instance_profile.name, context)
+ machine_spec = {
+ :name => instance.name,
+ :created => instance.launch_time.nil? ? Time.now.xmlschema : Time.parse(instance.launch_time.to_s).xmlschema,
+ :description => "No description set for Machine #{instance.name}",
+ :id => context.machine_url(instance.id),
+ :state => convert_instance_state(instance.state),
+ :cpu => cpu || convert_instance_cpu(instance.instance_profile, context),
+ :memory => memory || convert_instance_memory(instance.instance_profile, context),
+ :disks => { :href => context.machine_url(instance.id)+"/disks"},
+ :volumes => { :href=>context.machine_url(instance.id)+"/volumes"},
+ :operations => convert_instance_actions(instance, context)
+ }
+ if context.expand? :disks
+ machine_spec[:disks] = CIMI::Model::Disk.find(instance, machine_conf, context, :all)
+ end
+ if context.expand? :volumes
+ machine_spec[:volumes] = CIMI::Model::MachineVolume.find(instance.id, context, :all)
+ end
+ machine_spec[:realm] = instance.realm_id if instance.realm_id
+ machine_spec[:machine_image] = { :href => context.machine_image_url(instance.image_id) } if instance.image_id
+ self.new(context, :values => machine_spec)
+ end
+
+ # FIXME: This will convert 'RUNNING' state to 'STARTED'
+ # which is defined in CIMI (p65)
+ #
+ def self.convert_instance_state(state)
+ case state
+ when "RUNNING" then "STARTED"
+ when "PENDING" then "CREATING" #aruba only exception... could be "STARTING" here
+ else state
+ end
+ end
+
+ def self.convert_instance_cpu(profile, context)
+ cpu_override = profile.overrides.find { |p, v| p == :cpu }
+ if cpu_override.nil?
+ CIMI::Model::MachineConfiguration.find(profile.id, context).cpu
+ else
+ cpu_override[1]
+ end
+ end
+
+ def self.convert_instance_memory(profile, context)
+ machine_conf = CIMI::Model::MachineConfiguration.find(profile.name, context)
+ memory_override = profile.overrides.find { |p, v| p == :memory }
+ memory_override.nil? ? machine_conf.memory.to_i : context.to_kibibyte(memory_override[1].to_i,"MB")
+ end
+
+ def self.convert_instance_addresses(instance)
+ (instance.public_addresses + instance.private_addresses).collect do |address|
+ {
+ :hostname => address.is_hostname? ? address : nil,
+ :mac_address => address.is_mac? ? address : nil,
+ :state => 'Active',
+ :protocol => 'IPv4',
+ :address => address.is_ipv4? ? address : nil,
+ :allocation => 'Static'
+ }
+ end
+ end
+
+ def self.convert_instance_actions(instance, context)
+ actions = instance.actions.collect do |action|
+ action = :restart if action == :reboot
+ name = action
+ name = :delete if action == :destroy # In CIMI destroy operation become delete
+ { :href => context.send(:"#{action}_machine_url", instance.id), :rel => "http://schemas.dmtf.org/cimi/1/action/#{name}" }
+ end
+ actions << { :href => context.send(:"machine_images_url"), :rel => "http://schemas.dmtf.org/cimi/1/action/capture" } if instance.can_create_image?
+ actions
+ end
+
+ def self.convert_storage_volumes(instance, context)
+ instance.storage_volumes ||= [] #deal with nilpointers
+ instance.storage_volumes.map{|vol| {:href=>context.volume_url(vol.keys.first),
+ :initial_location=>vol.values.first} }
+ end
+
+end
--
1.8.1.2