incubator-deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Michael Mayo <m...@openstack.org>
Subject Re: OpenStack Support
Date Sat, 11 Sep 2010 00:02:17 GMT
There's not a public OpenStack installation yet.  OpenStack will fully support the Rackspace
API when it's released, so this contribution is slightly premature.

As for your review comments, I agree with them all and will send an updated patch soon.

Mike

On Sep 10, 2010, at 4:06 PM, David Lutterkort wrote:

> On Fri, 2010-09-10 at 12:41 -0700, Michael Mayo wrote:
>> I've attached a small patch to add OpenStack support to Deltacloud.
>> Since OpenStack == Rackspace essentially, I moved the Rackspace code
>> into new OpenStack code and added an option to set an auth API
>> endpoint.  Now, the Rackspace classes simply subclass the OpenStack
>> ones with the auth API endpoint defaulted to
>> auth.api.rackspacecloud.com.
> 
> Thanks for doing this. Is there some public OpenSTack test installation
> at which I could point the driver ? Ideally, Michal could do his
> capturing magic to capture responses from an OpenStack cloud, so that we
> can run feature tests against the canned responses.
> 
> To make the review easier, I'll quote the patch here:
> 
>> From 990009aaab537c3b1b12ab8e77e83db1756bca86 Mon Sep 17 00:00:00 2001
>> From: Mike Mayo <greenisus@gmail.com>
>> Date: Fri, 10 Sep 2010 12:24:26 -0700
>> Subject: [PATCH] openstack support
>> 
>> diff --git a/server/lib/deltacloud/drivers/openstack/openstack_driver.rb b/server/lib/deltacloud/drivers/openstack/openstack_driver.rb
>> new file mode 100644
>> index 0000000..bb4b4d2
>> --- /dev/null
>> +++ b/server/lib/deltacloud/drivers/openstack/openstack_driver.rb
>> @@ -0,0 +1,193 @@
>> +
>> +module Deltacloud
>> +  module Drivers
>> +    module OpenStack
>> +
>> +class OpenStackDriver < Deltacloud::BaseDriver
>> +
>> +  feature :instances, :user_name
>> +
>> +  def hardware_profiles(credentials, opts = nil)
>> +    racks = new_client( credentials, @opts )
> 
> Why pass @opts into new_client ? In fact, the second argument is always
> @opts, so you could just drop it and access it directily inside the
> new_client method.
> 
>> +  # OpenStack does not at this stage have realms... its all US/TX, all the time
(at least at time of writing)
>> +  def realms(credentials, opts=nil)
>> +    [Realm.new( {
>> +      :id=>"us",
>> +      :name=>"United States",
>> +      :state=> "AVAILABLE"
>> +    } )]
>> +  end
>> +
> 
> That should be changed for OpenStack -  I assume OpenStack also only has
> one realm, though there's no telling where it might be. Unless OpenStack
> gives you a way to learn more about the installation, I'd just return a
> realm with
> 
>  :id => "default", :name => "Default realm", :state => "AVAILABLE"
> 
> This raises a question we just discovered with the EC2 driver the other
> day: are there any subdivisions of an OpenStack cloud that are important
> at this level ? For example, in EC2 images are only usable within the
> same availability zone.
> 
>> +  def new_client(credentials, opts={})
> 
> As I said above, since every invocation of new_client has @opts as its
> second argument, I would just drop the second argument and use @opts
> inside new_client
> 
>> +    safely do
>> +      @auth_endpoint_uri = opts[:auth_endpoint_uri]
>> +      @opts = { :auth_endpoint_uri => @auth_endpoint_uri }
> 
> I am not sure why this is here - I really dislike setting @opts here as
> a side effect of new_client.
> 
> There also doesn't seem to be a place in the code that initializes the
> auth_endpoint_uri. Since we need to pass that in from the outside, there
> are two options:
> 
>      * Pass it in through the environment, as the EC2 driver does right
>        now via DCLOUD_EC2_URL
>      * Add an initialize method in BaseDriver to take some sort of
>        config hash that is filled from command line arguments to
>        deltacloudd (or env vars that deltacloudd sets and drivers.rb
>        consumes)
> 
> So my suggestion is that we add to BaseDriver
> 
>     attr_reader :endpoint_url
> 
>        def initialize(opts = {})
>          @endpoint_url = opts[:endpoint_url]
>        end
> 
> and in the driver method in drivers.rb something like
> 
>    opts = { :endpoint_url => ENV["DCLOUD_ENDPOINT_URL"] }
> 
> which deltacloudd sets from some appropriate command line argument; I
> assume that passenger provides a way to do the equivalent from the
> Apache config.
> 
> David
> 
> 


Mime
View raw message