deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Fojtik <mfoj...@redhat.com>
Subject Re: EC2 driver rewrite (rev 3)
Date Tue, 30 Nov 2010 17:18:22 GMT
On 29/11/10 14:10 -0500, Toby Crawley wrote:
>On 11/29/2010 08:16 AM, mfojtik@redhat.com wrote:
>>Hi guys,
>>
>>This is third edition of new EC2 driver. I removed / merged all
>>patches which was not related to new EC2 driver.
>>
>>To get this patchset running, you need latest AWS gem which is
>>now pending 2 pull commits (Tobi 1.8.6 compatibility patch and my
>>S3 location patch). For now, you can install AWS from this location:
>>
>>https://github.com/mifo/aws/raw/master/pkg/aws-2.3.9999.gem
>>
>>This version has all necessary patches/fixed applied.
>>
>>  --- Michal
>>
>
>Michal:
>
>I'm going to NACK this patch as it is currently, for the following reasons:
>
>* the attaching a public ip feature did not work for me at instance 
>launch time - it appears that AWS requires an instance to be in the 
>RUNNING state before it will attach an ip address (though I did not 
>see references to this in the AWS docs). Here is Dc log output for 
>the request:
>
>13:29:07,427 INFO  [STDOUT] ##### Aws::Ec2 returned an error: 400 Bad Request
>13:29:07,427 INFO  [STDOUT] <?xml version="1.0" encoding="UTF-8"?>
>13:29:07,427 INFO  [STDOUT] 
><Response><Errors><Error><Code>InvalidInstanceId</Code><Message>The

>instance 'i-77e3ac1a' is not in the 'running' state</Message></Error></Errors><RequestID>507d46ee-6968-4774-8c65-71932b64d971</RequestID></Response>

>#####
>13:29:07,427 INFO  [STDOUT] ##### Aws::Ec2 request: ec2.amazonaws.com:443/?AWSAccessKeyId=<redacted>&Action=AssociateAddress&InstanceId=i-77e3ac1a&PublicIp=50.16.242.45&SignatureMethod=HmacSHA256&SignatureVersion=2&Timestamp=2010-11-29T18%3A29%3A07.000Z&Version=2010-08-31&Signature=VAMkRQle2H0DhLm3SDRpI3bSSMifwum2e%2FnXY1rh6oI%3D

>####
>13:29:07,431 ERROR [STDERR] Deltacloud::BackendError - InvalidInstanceId: The instance
'i-77e3ac1a' is not in the 'running' state

I'm sorry, I don't test this feature. We can move this 'feature' to be
a separate action on instance (PUT /api/instances/i-12345/ip or something
similar). 

>* the new aws gem returns the in use state for a storage_volume as 
>'IN_USE', whereas amazon-ec2 returned 'IN-USE'. The storage_volume 
>index still makes reference to IN-USE 
>(server/views/storage_volumes/index.html.haml#26

Sure, good catch. I'll fix that to be amazon-ec2 compatible.

>* I had to make the following change to have the mounted instance information be reported
correctly on a storage_volume:
>
>diff --git a/client/lib/deltacloud.rb b/client/lib/deltacloud.rb
>index c632594..30e0a1d 100644
>--- a/client/lib/deltacloud.rb
>+++ b/client/lib/deltacloud.rb
>@@ -167,7 +167,7 @@ module DeltaCloud
>         end
>
>         if attribute.name == 'mount'
>-          obj.add_link!("instance", (attribute/"./instance/@id").first)
>+          obj.add_link!("instance", (attribute/"./instance/@id").first.value)
>           obj.add_text!("device", (attribute/"./device/@name").first.value)
>           next
>         end
>
>* I had to make the following two changes to the instance code to launch in the correct
realm, and to properly return the public hostname:
>
>diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/
>index 59206d7..af7a1e6 100644
>--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>@@ -170,7 +170,7 @@ module Deltacloud
>           instance_options = {}
>           instance_options.merge!(:user_data => opts[:user_data]) if opts[:user_data]
>           instance_options.merge!(:key_name => opts[:key_name]) if opts[:key_name]
>-          instance_options.merge!(:availability_zone => opts[:availability_zone])
if opts[:avail
>+          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!(:group_ids => opts[:security_group]) if opts[:security_group]
>           safely do
>@@ -451,7 +451,7 @@ module Deltacloud
>             :instance_profile => InstanceProfile.new(instance[:aws_instance_type]),
>             :realm_id => instance[:aws_availability_zone],
>             :private_addresses => instance[:private_dns_name],
>-            :public_addresses => instance[:public_addresses]
>+            :public_addresses => instance[:dns_name]
>           )
>         end
>
>
>Both of the above patches are the same changes I sent last week, but 
>these patches should apply cleanly to your current tree on top of the 
>patch set.

Ah, yes I forgot to apply your patches, will apply them + fix
storage_volume and move IP assignment elsewhere and resend this patchset
again. 

>I only tested instance and storage volume creation & management. I didn't test any
of the blob storage features.

We already did some testing with Marios and seems like it works without
problems. We just discover that 'aws' gem has problems with Asia and US
west region, but it was fixed in AWS and I issued a pull request for that.

However, if you find some other bugs/problems with this new gem, please
report them so it can be fixed and applied as full-working driver.


   -- Michal

-- 
--------------------------------------------------------
Michal Fojtik, mfojtik@redhat.com
Deltacloud API: http://deltacloud.org
--------------------------------------------------------

Mime
View raw message