On Wed, 2011-11-23 at 17:23 +0100, [email protected] wrote:
> From: Michal Fojtik <[email protected]>
>
>
> Signed-off-by: Michal fojtik <[email protected]>
> ---
> server/lib/cimi/drivers/drivers.rb | 55
> +++++++++++++++++++++++++++
> server/lib/cimi/drivers/mock/mock_driver.rb | 53 ++++++++++++++++++++++++++
> server/lib/cimi/server.rb | 18 ++------
> 3 files changed, 113 insertions(+), 13 deletions(-)
> create mode 100644 server/lib/cimi/drivers/drivers.rb
> create mode 100644 server/lib/cimi/drivers/mock/mock_driver.rb
ACK; some comments:
> diff --git a/server/lib/cimi/drivers/drivers.rb
> b/server/lib/cimi/drivers/drivers.rb
> new file mode 100644
> index 0000000..4d1dd2e
> --- /dev/null
> +++ b/server/lib/cimi/drivers/drivers.rb
Short of copying this file wholesale, is their something more clever we
can do ? How about we start a new lib/common directory, which also has a
drivers.rb containing everything from the current
lib/deltacloud/drivers.rb except for driver_class and
driver_source_name, and make lib/cimi/drivers.rb
module CIMI::Drivers
include Common::Drivers
def driver_class ... same as in current drivers.rb ... end
def driver_source_name ... same as in current drivers.rb ...
end
end
and similar for Deltacloud::Drivers. Looking at how these two methods
differ between CIMI and Deltacloud, this could be refactored further, so
that CIMI::Drives and Deltacloud::Drivers only add two methods, one
returning "cimi"/"deltacloud", the other returning
CIMI::Drivers/Deltacloud::Drivers (could that be replaced by self ?)
> diff --git a/server/lib/cimi/drivers/mock/mock_driver.rb
> b/server/lib/cimi/drivers/mock/mock_driver.rb
> new file mode 100644
> index 0000000..bcb1e2c
> --- /dev/null
> +++ b/server/lib/cimi/drivers/mock/mock_driver.rb
> @@ -0,0 +1,53 @@
> + private
> +
> + def convert_machine_description(hwp)
> + MachineConfiguration.new(
> + :name => hwp.name,
> + :description => machine_conf_desc(hwp),
> + :cpu => hwp.cpu.value,
> + :created => Time.now.to_s,
> + :memory => { :quantity => hwp.memory.value, :units => hwp.memory.unit
> },
> + :disks => [ { :capacity => { :quantity => hwp.storage.value, :units =>
> hwp.storage.unit } } ]
> + )
> + end
This would be nicer as a class method on MachineConfiguration, say
MachineConfiguration.from_hwp or MachineConfiguration.convert_hwp
(and I wonder if we should alias *Configuration classes to *Config
classes)
> diff --git a/server/lib/cimi/server.rb b/server/lib/cimi/server.rb
> index ba925fa..39f72f5 100644
> --- a/server/lib/cimi/server.rb
> +++ b/server/lib/cimi/server.rb
> @@ -112,19 +112,11 @@ global_collection :machine_configurations do
> with_capability :hardware_profile
> param :id, :string, :required
> control do
> - @profile = driver.hardware_profile(credentials, params[:id])
> - if @profile
> - #setup the default values for a machine configuration
> - resource_default = get_resource_default "machine_configuration"
> - #get the actual values from profile
> - resource_value = { "name" => @profile.name,"uri" => @profile.name,
> - "href" => machine_configuration_url(@profile.name) }
> - #mixin actual values get from profile
> - @dmtfitem = resource_default["dmtfitem"].merge resource_value
> - show_resource "machine_configurations/show", "MachineConfiguration",
> - {"property" => "properties", "disk" => "disks", "operation" =>
> "operations"}
> - else
> - report_error(404)
> + machine_conf = driver.machine_configuration(credentials, :id =>
> params[:id])
> + machine_conf.uri = machine_configuration_url(params[:id])
Just to sound this out in my head: if we made this a method on MC, the
code would look like
machine_conf = MachineConfiguration.find(params[:id], self)
with
class MachineConfiguration
...
def self.find(id, ctx)
hwps = nil
if id == :all
hwps = ctx.driver.hardware_profiles(ctx.credentials)
else
hwps = ctx.driver.hardware_profile(ctx.credentials, id)
end
confs = hwps.map { |hwp| self.convert_hwp(hwp, ctx) }
id == :all ? confs : confs.first
end
private
def self.convert_hwp(hwp, ctx)
new(:name => hwp.name , :uri =>
ctx.machine_configuration_url(hwp.id), ...)
end
...
end
With that, pretty much all the methods in the current
CIMI::Drivers::MockDriver would just become methods in
MachineConfiguration (it would need a hardware_profiles method that
delegates to Deltacloud::Drivers::MockDriver).
I am not against having a CIMI::MockDriver, but I would like to try to
keep logic that is not driver specific in the model classes.
Essentially, the drivers are solely our internal cloud abstraction
layer.
David
