incubator-deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michal Fojtik <mfoj...@redhat.com>
Subject Re: [PATCH core] Added basic IP address managment support (for EC2)
Date Tue, 03 May 2011 14:08:07 GMT
On May 3, 2011, at 1:25 AM, David Lutterkort wrote:

> On Mon, 2011-05-02 at 13:40 +0200, mfojtik@redhat.com wrote:
>> From: Michal Fojtik <mfojtik@redhat.com>
> 
> ACK after addressing the comments below:
> 
>> diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> index 4b9899a..fc9ab73 100644
>> --- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> +++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>> @@ -509,6 +509,56 @@ module Deltacloud
>>           end
>>         end
>> 
>> +        def addresses(credentials, opts={})
>> +          ec2 = new_client(credentials)
>> +          address_id = (opts and opts[:id]) ? opts[:id] : []
> 
> Shouldn't address_id always be an array, i.e.,
> 
>        address_id = (opts and opts[:id]) ? [opts[:id]] : []
> 
> Also a request for http://localhost:3001/api/addresses?id=127.0.0.1
> leads to a backend error (error code 502)
> 
> It would be more natural to me if that just resulted in an empty list.
> 
> A request for http://localhost:3001/api/addresses/127.0.0.1 also leads
> to a 502; that should be a 404

Fixed. Thanks for pointing to this.

> 
>> diff --git a/server/server.rb b/server/server.rb
>> index dcc76e5..344d4bf 100644
>> --- a/server/server.rb
>> +++ b/server/server.rb
>> @@ -858,3 +858,89 @@ collection :buckets do
>>   end
>> 
>> end
>> +
>> +get '/api/addresses/:id/associate' do
>> +  @instances = driver.instances(credentials)
>> +  @address = Address::new(:id => params[:id])
>> +  respond_to do |format|
>> +    format.html { haml :"addresses/associate" }
>> +  end
>> +end
>> +
>> +collection :addresses do
>> +  description "Manage IP addresses"
>> +
>> +  operation :index do
>> +    description "List IP addresses assigned to your account."
>> +    with_capability :addresses
>> +    control do
>> +      filter_all :addresses
>> +    end
>> +  end
>> +
>> +  operation :show do
>> +    description "Show details about IP addresses specified by given ID"
>> +    with_capability :address
>> +    param :id,  :string,  :required
>> +    control { show :address }
>> +  end
>> +
>> +  operation :create do
>> +    description "Acquire a new IP address for use with your account."
>> +    with_capability :create_address
>> +    control do
>> +      @address = driver.create_address(credentials, {})
>> +      respond_to do |format|
>> +        format.html { haml :"addresses/show" }
>> +        format.xml do
>> +          response.status = 201  # Created
> 
> Also set the Location header to the URL for the new address.

Header set.

> 
> 
>> diff --git a/server/views/addresses/index.xml.haml b/server/views/addresses/index.xml.haml
>> new file mode 100644
>> index 0000000..e2a564f
>> --- /dev/null
>> +++ b/server/views/addresses/index.xml.haml
>> @@ -0,0 +1,4 @@
>> +!!!XML
>> +%addresses
>> +  - @elements.each do |c|
>> +    = haml :'address/show', :locals => { :@address => c, :partial => true
}
> 
> Typo:
>                ^^^^ addresses/show


Fixed.

> 
>> diff --git a/server/views/addresses/show.xml.haml b/server/views/addresses/show.xml.haml
>> new file mode 100644
>> index 0000000..a450690
>> --- /dev/null
>> +++ b/server/views/addresses/show.xml.haml
>> @@ -0,0 +1,12 @@
>> +- unless defined?(partial)
>> +  !!! XML
>> +%address{ :href => address_url(@address.id), :id => @address.id }
> 
> We should provide the IP as a separate attribute; there's no need to
> cement ID == IP in the API.

Right now there no other way how to 'identify' IP address in EC2.
For us that means we don't have any other 'attribute' we can use as an 'id'

Also we are using this format for all other objects (:id => @object.id).

I put a new element called '<ip>xxx.xxx.xxx.xxx</ip>' inside <address> block.

Thanks! I'll push this patchset today.

  -- Michal

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


Mime
View raw message