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/22 18:16:58 UTC
[PATCH] Patch in preparation for eucalyptus driver
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 77257ee..78cc895 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)
@@ -104,11 +115,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 d1fe4b8..a1f597c 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 07a1029..488ab76 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 preparation for eucalyptus driver
Posted by Sang-Min Park <sa...@eucalyptus.com>.
Actually, the reason I didn't see your previous comments was that my email
client, Microsoft Outlook, just doesn't display your message. I found it
through Gmail, but not through Outloook, which I think a serious bug in the
Outlook.
Anyway, I did re-generate the new patch following your comments.
The reason I modified 'base_driver.rb' that way was to implement inheritence
first, and then do any fixes necessary, just like you suggested in your
early comments to define cleanup helpers. But you're right that we don't
actually need to use class variable for hardware profiles and instance
variable just works for Euca (I tested). Similarly, @instance_state_machine
can remain as instance variable by referencing EC2 driver in Euca driver.
Finally, yes, Michal's patch made 'remove_feature' unnecessary.
As a result, the new patch won't modify any code except for EC2 and
Eucalyptus driver. I'll send it shortly.
Sang-min
On Wed, Mar 23, 2011 at 4:43 PM, Sang-Min Park <sp...@eucalyptus.com> wrote:
> Oh, I didn't scroll down on that message.
> I'll work on it now.
>
> Sang-min
>
> -----Original Message-----
> From: David Lutterkort [mailto:lutter@redhat.com]
> Sent: Wednesday, March 23, 2011 4:36 PM
> To: Sang-Min Park
> Cc: deltacloud-dev@incubator.apache.org
> Subject: RE: [PATCH] Patch in preparation for eucalyptus driver
>
> On Wed, 2011-03-23 at 16:31 -0700, Sang-Min Park wrote:
> > Hmm, maybe I misunderstood something.
> > Yes, it is a repost of the previous patch.
> >
> > Did you send comments on that patch before?
> > I saw the comments for the other patch ([PATCH] Eucalyptus driver
> > support), but not for this one ([PATCH] Patch in preparation for
> > eucalyptus driver).
>
> Yes, if you scroll down on my reply to your original patch, there are a
> number of comments there.
>
> David
>
RE: [PATCH] Patch in preparation for eucalyptus driver
Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2011-03-23 at 16:31 -0700, Sang-Min Park wrote:
> Hmm, maybe I misunderstood something.
> Yes, it is a repost of the previous patch.
>
> Did you send comments on that patch before?
> I saw the comments for the other patch ([PATCH] Eucalyptus driver
> support), but not for this one ([PATCH] Patch in preparation for
> eucalyptus driver).
Yes, if you scroll down on my reply to your original patch, there are a
number of comments there.
David
RE: [PATCH] Patch in preparation for eucalyptus driver
Posted by Sang-Min Park <sp...@eucalyptus.com>.
Hmm, maybe I misunderstood something.
Yes, it is a repost of the previous patch.
Did you send comments on that patch before?
I saw the comments for the other patch ([PATCH] Eucalyptus driver
support), but not for this one ([PATCH] Patch in preparation for
eucalyptus driver).
Sang-min
-----Original Message-----
From: David Lutterkort [mailto:lutter@redhat.com]
Sent: Wednesday, March 23, 2011 4:18 PM
To: deltacloud-dev@incubator.apache.org
Cc: Sang-Min Park
Subject: Re: [PATCH] Patch in preparation for eucalyptus driver
On Tue, 2011-03-22 at 10:16 -0700, 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(-)
This seems to simply be a repost of your original patch, and doesn't
address any of the comments I made.
In particular, there should not be any need to do anything special for
hardware_profiles and features, since they are not inherited anymore;
rather each driver class has its own list of these.
David
Re: [PATCH] Patch in preparation for eucalyptus driver
Posted by David Lutterkort <lu...@redhat.com>.
On Tue, 2011-03-22 at 10:16 -0700, 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(-)
This seems to simply be a repost of your original patch, and doesn't
address any of the comments I made.
In particular, there should not be any need to do anything special for
hardware_profiles and features, since they are not inherited anymore;
rather each driver class has its own list of these.
David