Mailing-List: contact deltacloud-dev-help@incubator.apache.org; run by ezmlm
Precedence: bulk
Reply-To: deltacloud-dev@incubator.apache.org
Received-SPF: pass (athena.apache.org: domain of mfojtik@redhat.com designates
 209.132.183.28 as permitted sender)
Subject: Re: [PATCH core] Added basic IP address managment support (for EC2)
Mime-Version: 1.0 (Apple Message framework v1084)
Content-Type: text/plain; charset=us-ascii
From: Michal Fojtik <mfojtik@redhat.com>
In-Reply-To: <1304378755.13626.49.camel@avon.watzmann.net>
Date: Tue, 3 May 2011 16:08:07 +0200
Content-Transfer-Encoding: quoted-printable
Message-Id: <2DF7D796-1F6B-45BA-9026-B91873EB4813@redhat.com>
References: <1304336456-23351-1-git-send-email-mfojtik@redhat.com>
 <1304336456-23351-2-git-send-email-mfojtik@redhat.com>
 <1304378755.13626.49.camel@avon.watzmann.net>
To: deltacloud-dev@incubator.apache.org

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>
>=20
> ACK after addressing the comments below:
>=20
>> 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
>>=20
>> +        def addresses(credentials, opts=3D{})
>> +          ec2 =3D new_client(credentials)
>> +          address_id =3D (opts and opts[:id]) ? opts[:id] : []
>=20
> Shouldn't address_id always be an array, i.e.,
>=20
>        address_id =3D (opts and opts[:id]) ? [opts[:id]] : []
>=20
> Also a request for http://localhost:3001/api/addresses?id=3D127.0.0.1
> leads to a backend error (error code 502)
>=20
> It would be more natural to me if that just resulted in an empty list.
>=20
> 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.

>=20
>> 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
>>=20
>> end
>> +
>> +get '/api/addresses/:id/associate' do
>> +  @instances =3D driver.instances(credentials)
>> +  @address =3D Address::new(:id =3D> 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 =3D driver.create_address(credentials, {})
>> +      respond_to do |format|
>> +        format.html { haml :"addresses/show" }
>> +        format.xml do
>> +          response.status =3D 201  # Created
>=20
> Also set the Location header to the URL for the new address.

Header set.

>=20
>=20
>> 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|
>> +    =3D haml :'address/show', :locals =3D> { :@address =3D> c, =
:partial =3D> true }
>=20
> Typo:
>                ^^^^ addresses/show


Fixed.

>=20
>> 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 =3D> address_url(@address.id), :id =3D> @address.id =
}
>=20
> We should provide the IP as a separate attribute; there's no need to
> cement ID =3D=3D 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 =3D> =
@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