You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by sa...@eucalyptus.com on 2011/03/11 00:25:23 UTC
[PATCH] patch in preparating for Eucalyptus driver support
From: Sang-Min Park <sp...@eucalyptus.com>
---
server/lib/deltacloud/base_driver/base_driver.rb | 25 +++++++---
server/lib/deltacloud/base_driver/features.rb | 9 +++
server/lib/deltacloud/drivers/ec2/ec2_driver.rb | 59 +++++++++++++--------
server/lib/deltacloud/validation.rb | 5 ++
4 files changed, 68 insertions(+), 30 deletions(-)
diff --git a/server/lib/deltacloud/base_driver/base_driver.rb b/server/lib/deltacloud/base_driver/base_driver.rb
index 2442385..2a7a758 100644
--- a/server/lib/deltacloud/base_driver/base_driver.rb
+++ b/server/lib/deltacloud/base_driver/base_driver.rb
@@ -44,11 +44,10 @@ module Deltacloud
class BaseDriver
def self.define_hardware_profile(name,&block)
- @hardware_profiles ||= []
- hw_profile = @hardware_profiles.find{|e| e.name == name}
+ hw_profile = hardware_profiles.find{|e| e.name == name}
return if hw_profile
hw_profile = ::Deltacloud::HardwareProfile.new( name, &block )
- @hardware_profiles << hw_profile
+ hardware_profiles << hw_profile
hw_params = hw_profile.params
unless hw_params.empty?
feature :instances, :hardware_profiles do
@@ -56,10 +55,22 @@ module Deltacloud
end
end
end
+
+ def self.clear_hardware_profile
+ hardware_profiles.each do |e|
+ hw_params = e.params
+ unless hw_params.empty?
+ feature :instance, :hardware_profiles do
+ decl.operation(:create) { delete_params(hw_params) }
+ end
+ end
+ end
+ hardware_profiles.clear
+ end
def self.hardware_profiles
- @hardware_profiles ||= []
- @hardware_profiles
+ @@hardware_profiles ||= []
+ @@hardware_profiles
end
def hardware_profiles(credentials, opts = nil)
@@ -103,11 +114,11 @@ module Deltacloud
def self.define_instance_states(&block)
machine = ::Deltacloud::StateMachine.new(&block)
- @instance_state_machine = machine
+ @@instance_state_machine = machine
end
def self.instance_state_machine
- @instance_state_machine
+ @@instance_state_machine
end
def instance_state_machine
diff --git a/server/lib/deltacloud/base_driver/features.rb b/server/lib/deltacloud/base_driver/features.rb
index 1f07843..86f44cd 100644
--- a/server/lib/deltacloud/base_driver/features.rb
+++ b/server/lib/deltacloud/base_driver/features.rb
@@ -136,6 +136,15 @@ module Deltacloud
end
features[collection] << Feature.new(decl, &block)
end
+
+ # A driver, inherited from another driver, may need to delete feature declaration from the base driver
+ # The example is Eucalyptus driver inherited from EC2
+ def self.remove_feature(collection, name)
+ features[collection] ||= []
+ if f = features[collection].find { |f| f.name == name }
+ features[collection].delete(f)
+ end
+ end
def features(collection)
self.class.features[collection] || []
diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
index 9a2abb6..70259f0 100644
--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
@@ -135,9 +135,9 @@ module Deltacloud
end
return img_arr
end
- owner_id = opts[:owner_id] || "amazon"
+ owner_id = opts[:owner_id] || default_image_owner
safely do
- img_arr = ec2.describe_images_by_owner(owner_id, "machine").collect do |image|
+ img_arr = ec2.describe_images_by_owner(owner_id, default_image_type).collect do |image|
convert_image(image)
end
end
@@ -171,16 +171,18 @@ module Deltacloud
inst_arr = ec2.describe_instances.collect do |instance|
convert_instance(instance) if instance
end.flatten
- tags = ec2.describe_tags(
- 'Filter.1.Name' => 'resource-type', 'Filter.1.Value' => 'instance'
- )
- inst_arr.each do |inst|
- name_tag = tags.select { |t| (t[:aws_resource_id] == inst.id) and t[:aws_key] == 'name' }
- unless name_tag.empty?
- inst.name = name_tag.first[:aws_value]
+ if tagging?
+ tags = ec2.describe_tags(
+ 'Filter.1.Name' => 'resource-type', 'Filter.1.Value' => 'instance'
+ )
+ inst_arr.each do |inst|
+ name_tag = tags.select { |t| (t[:aws_resource_id] == inst.id) and t[:aws_key] == 'name' }
+ unless name_tag.empty?
+ inst.name = name_tag.first[:aws_value]
+ end
end
- end
- delete_unused_tags(credentials, inst_arr.collect {|inst| inst.id})
+ delete_unused_tags(credentials, inst_arr.collect {|inst| inst.id})
+ end
end
inst_arr = filter_on( inst_arr, :id, opts )
filter_on( inst_arr, :state, opts )
@@ -192,7 +194,7 @@ module Deltacloud
instance_options.merge!(:user_data => opts[:user_data]) if opts[:user_data]
instance_options.merge!(:key_name => opts[:keyname]) if opts[:keyname]
instance_options.merge!(:availability_zone => opts[:realm_id]) if opts[:realm_id]
- instance_options.merge!(:instance_type => opts[:hwp_id]) if opts[:hwp_id]
+ instance_options.merge!(:instance_type => opts[:hwp_id]) if opts[:hwp_id] && opts[:hwp_id].length > 0
instance_options.merge!(:group_ids => opts[:security_group]) if opts[:security_group]
instance_options.merge!(
:min_count => opts[:instance_count],
@@ -202,15 +204,12 @@ module Deltacloud
new_instance = convert_instance(ec2.launch_instances(image_id, instance_options).first)
# TODO: Rework this to use client_id for name instead of tag
# Tags should be keept as an optional feature
- if opts[:name]
- tag_instance(credentials, new_instance, opts[:name])
- end
+ tag_instance(credentials, new_instance, opts[:name])
# Register Instance to Load Balancer if load_balancer_id is set.
# This parameter is a feature parameter
- if opts[:load_balancer_id] and opts[:load_balancer_id].length>0
- elb = new_client(credentials, :elb)
- elb.register_instances_with_load_balancer(opts[:load_balancer_id], [new_instance.id])
- end
+ if opts[:load_balancer_id]
+ lb = lb_register_instance(credentials, {'id' => opts[:load_balancer_id], 'instance_id' => new_instance.id})
+ end
new_instance
end
end
@@ -523,6 +522,18 @@ module Deltacloud
end
klass.new(credentials.user, credentials.password, {:server => endpoint_for_service(type), :connection_mode => :per_thread})
end
+
+ def default_image_owner
+ "amazon"
+ end
+
+ def default_image_type
+ "machine"
+ end
+
+ def tagging?
+ true
+ end
def endpoint_for_service(service)
endpoint = (Thread.current[:provider] || ENV['API_PROVIDER'] || DEFAULT_REGION)
@@ -532,11 +543,13 @@ module Deltacloud
end
def tag_instance(credentials, instance, name)
- ec2 = new_client(credentials)
- safely do
- ec2.create_tag(instance.id, 'name', name)
+ if name
+ ec2 = new_client(credentials)
+ safely do
+ ec2.create_tag(instance.id, 'name', name)
+ end
end
- end
+ end
def untag_instance(credentials, instance_id)
ec2 = new_client(credentials)
diff --git a/server/lib/deltacloud/validation.rb b/server/lib/deltacloud/validation.rb
index cfac4ce..efc9c84 100644
--- a/server/lib/deltacloud/validation.rb
+++ b/server/lib/deltacloud/validation.rb
@@ -73,6 +73,11 @@ module Deltacloud::Validation
# to add_params should be cumulative
new.each { |p| @params[p.name] = p }
end
+
+ # Delete the parameters from the hash if found
+ def delete_params(old)
+ old.each { |p| @params.delete(p) }
+ end
def each_param(&block)
params.each_value { |p| yield p }
--
1.7.1
Re: [PATCH] patch in preparating for Eucalyptus driver support
Posted by David Lutterkort <lu...@redhat.com>.
Sorry for the delay, somehow I thought I had reviewed this already -
obviously not.
On Thu, 2011-03-10 at 15:25 -0800, sang-min.park@eucalyptus.com wrote:
> From: Sang-Min Park <sp...@eucalyptus.com>
>
> ---
> server/lib/deltacloud/base_driver/base_driver.rb | 25 +++++++---
> server/lib/deltacloud/base_driver/features.rb | 9 +++
> server/lib/deltacloud/drivers/ec2/ec2_driver.rb | 59 +++++++++++++--------
> server/lib/deltacloud/validation.rb | 5 ++
> 4 files changed, 68 insertions(+), 30 deletions(-)
>
> diff --git a/server/lib/deltacloud/base_driver/base_driver.rb b/server/lib/deltacloud/base_driver/base_driver.rb
> index 2442385..2a7a758 100644
> --- a/server/lib/deltacloud/base_driver/base_driver.rb
> +++ b/server/lib/deltacloud/base_driver/base_driver.rb
> @@ -44,11 +44,10 @@ module Deltacloud
> class BaseDriver
>
> def self.define_hardware_profile(name,&block)
> - @hardware_profiles ||= []
> - hw_profile = @hardware_profiles.find{|e| e.name == name}
> + hw_profile = hardware_profiles.find{|e| e.name == name}
> return if hw_profile
> hw_profile = ::Deltacloud::HardwareProfile.new( name, &block )
> - @hardware_profiles << hw_profile
> + hardware_profiles << hw_profile
>From what we learned about instance variables for classes today, have
you tried if the original code gives you the expected results ? The
@hardware_profiles should be a variable per class in the class
hierarchy, so that there should be no inheritance of hardware profiles,
which is exactly what we want.
> @@ -56,10 +55,22 @@ module Deltacloud
> end
> end
> end
> +
> + def self.clear_hardware_profile
> + hardware_profiles.each do |e|
> + hw_params = e.params
> + unless hw_params.empty?
> + feature :instance, :hardware_profiles do
> + decl.operation(:create) { delete_params(hw_params) }
> + end
I am pretty sure this does nothing, since the collection is
called :instances, not :instance.
> + end
> + end
> + hardware_profiles.clear
> + end
This operation should not be necessary if we use @hardware_profiles
instead of @@hardware_profiles
> @@ -103,11 +114,11 @@ module Deltacloud
>
> def self.define_instance_states(&block)
> machine = ::Deltacloud::StateMachine.new(&block)
> - @instance_state_machine = machine
> + @@instance_state_machine = machine
> end
>
> def self.instance_state_machine
> - @instance_state_machine
> + @@instance_state_machine
> end
How about using @instance_state_machine instead of
@@instance_state_machine, and making the accessor look in parent
classes ? That way, we'd simulate inheritance, without actually having
it.
> diff --git a/server/lib/deltacloud/base_driver/features.rb b/server/lib/deltacloud/base_driver/features.rb
> index 1f07843..86f44cd 100644
> --- a/server/lib/deltacloud/base_driver/features.rb
> +++ b/server/lib/deltacloud/base_driver/features.rb
> @@ -136,6 +136,15 @@ module Deltacloud
> end
> features[collection] << Feature.new(decl, &block)
> end
> +
> + # A driver, inherited from another driver, may need to delete feature declaration from the base driver
> + # The example is Eucalyptus driver inherited from EC2
> + def self.remove_feature(collection, name)
> + features[collection] ||= []
> + if f = features[collection].find { |f| f.name == name }
> + features[collection].delete(f)
> + end
> + end
This should not be needed anymore, because of Michal's r1083755 today.
> diff --git a/server/lib/deltacloud/validation.rb b/server/lib/deltacloud/validation.rb
> index cfac4ce..efc9c84 100644
> --- a/server/lib/deltacloud/validation.rb
> +++ b/server/lib/deltacloud/validation.rb
> @@ -73,6 +73,11 @@ module Deltacloud::Validation
> # to add_params should be cumulative
> new.each { |p| @params[p.name] = p }
> end
> +
> + # Delete the parameters from the hash if found
> + def delete_params(old)
> + old.each { |p| @params.delete(p) }
> + end
We won't need this either if we don't need clear_hardware_profiles.
David