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)
Date: Fri, 10 Dec 2010 11:24:14 +0100
From: Michal Fojtik <mfojtik@redhat.com>
To: deltacloud-dev@incubator.apache.org
Subject: Re: [PATCH] Allow setting the provider on the server side, and
 implement region overriding in the ec2 driver based off of the provider.
Message-ID: <20101210102413.GA5499@redhat.com>
References: <1291928260-16117-1-git-send-email-tcrawley@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii; format=flowed
Content-Disposition: inline
In-Reply-To: <1291928260-16117-1-git-send-email-tcrawley@redhat.com>
User-Agent: Mutt/1.5.20 (2009-06-14)

On 09/12/10 15:57 -0500, tcrawley@redhat.com wrote:
>From: Tobias Crawley <tcrawley@redhat.com>
>
>The provider can now be set on the commandline to deltacloudd via -P/--provider
>(putting it in ENV['API_PROVIDER']), and also from the client via a header
>(putting it in Thread.current[:provider]).
>
>The ec2 driver is currently the only driver that understands providers, and
>looks in Thread.current[:provider], then ENV['API_PROVIDER'], then falls back
>to a default (currently 'us-east-1', the AWS default).

ACK. (Just one minor inline comment below).

   -- Michal

>---
> server/bin/deltacloudd                          |    8 +++++++-
> server/lib/deltacloud/drivers/ec2/ec2_driver.rb |   19 +++++++++++++++++--
> server/lib/sinatra/rack_driver_select.rb        |   20 ++++++++++++++------
> 3 files changed, 38 insertions(+), 9 deletions(-)
>
>diff --git a/server/bin/deltacloudd b/server/bin/deltacloudd
>index ed7883b..70be6ca 100755
>--- a/server/bin/deltacloudd
>+++ b/server/bin/deltacloudd
>@@ -36,6 +36,9 @@ BANNER
>   opts.on( '-p', '--port PORT', 'Use PORT (default: 3001)') do |port|
>     ENV["API_PORT"] = port
>   end
>+  opts.on( '-P', '--provider PROVIDER', 'Use PROVIDER (default is set in the driver)') do |provider|
>+    ENV['API_PROVIDER'] = provider
>+  end
>   opts.on( '-e', '--env ENV', 'Environment (default: "development")') { |env| options[:env] = env }
>   opts.on( '-h', '--help', '') { options[:help] = true }
> end
>@@ -55,7 +58,10 @@ end
> ENV["API_HOST"] = "localhost" unless ENV["API_HOST"]
> ENV["API_PORT"] = "3001" unless ENV["API_PORT"]
>
>-puts "Starting Deltacloud API :: #{ENV["API_DRIVER"]} :: http://#{ENV["API_HOST"]}:#{ENV["API_PORT"]}/api"
>+msg = "Starting Deltacloud API :: #{ENV["API_DRIVER"]} "
>+msg << ":: #{ENV['API_PROVIDER']} " if ENV['API_PROVIDER']
>+msg << ":: http://#{ENV["API_HOST"]}:#{ENV["API_PORT"]}/api"
>+puts msg
> puts
>
> dirname="#{File.dirname(__FILE__)}/.."
>diff --git a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>index efbfcc8..1bd4fc4 100644
>--- a/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>+++ b/server/lib/deltacloud/drivers/ec2/ec2_driver.rb
>@@ -106,6 +106,8 @@ class EC2Driver < Deltacloud::BaseDriver
>     stopped.to( :finish )         .automatically
>   end
>
>+  DEFAULT_REGION = 'us-east-1'
>+
>   #
>   # Images
>   #
>@@ -504,9 +506,9 @@ class EC2Driver < Deltacloud::BaseDriver
>   def new_client(credentials, provider_type = :ec2)
>     opts = {
>       :access_key_id => credentials.user,
>-      :secret_access_key => credentials.password
>+      :secret_access_key => credentials.password,
>+      :server => endpoint_for_service(provider_type)
>     }
>-    opts[:server] = ENV['DCLOUD_EC2_URL'] if ENV['DCLOUD_EC2_URL']
>     safely do
>       case provider_type
>         when :ec2
>@@ -517,6 +519,19 @@ class EC2Driver < Deltacloud::BaseDriver
>     end
>   end
>
>+  def endpoint_for_service(service)
>+    url = ""
>+    case service
>+    when :ec2
>+      url << 'ec2.'
>+    when :elb
>+      url << 'elasticloadbalancing.'
>+    end

This one can be written in this way, since 'case' statement already
returns value:

url += case service
         when :ec2
           "ec2."
         when :elb
           "elasticloadbalancing."
       end


>+    url << (Thread.current[:provider] || ENV['API_PROVIDER'] || DEFAULT_REGION)
>+    url << '.amazonaws.com'
>+    url
>+  end
>+
>   def convert_load_balancer(credentials, loadbalancer)
>     balancer_realms = loadbalancer.AvailabilityZones.member.collect do |m|
>       realm(credentials, m)
>diff --git a/server/lib/sinatra/rack_driver_select.rb b/server/lib/sinatra/rack_driver_select.rb
>index f00a2c8..3e7d038 100644
>--- a/server/lib/sinatra/rack_driver_select.rb
>+++ b/server/lib/sinatra/rack_driver_select.rb
>@@ -5,17 +5,25 @@ class RackDriverSelect
>     @opts = opts
>   end
>
>+  HEADER_TO_ENV_MAP = {
>+    'HTTP_X_DELTACLOUD_DRIVER' => :driver,
>+    'HTTP_X_DELTACLOUD_PROVIDER' => :provider
>+  }
>+
>   def call(env)
>-    original_driver = Thread.current[:driver]
>-    new_driver = extract_driver(env)
>-    Thread.current[:driver] = new_driver if new_driver
>+    original_settings = { }
>+    HEADER_TO_ENV_MAP.each do |header, name|
>+      original_settings[name] = Thread.current[name]
>+      new_setting = extract_header(env, header)
>+      Thread.current[name] = new_setting if new_setting
>+    end
>     @app.call(env)
>   ensure
>-    Thread.current[:driver] = original_driver
>+    original_settings.each { |name, value| Thread.current[name] = value }
>   end
>
>-  def extract_driver(env)
>-    driver_name = env['HTTP_X_DELTACLOUD_DRIVER'].downcase if env['HTTP_X_DELTACLOUD_DRIVER']
>+  def extract_header(env, header)
>+    env[header].downcase if env[header]
>   end
>
> end
>--
>1.7.3.2
>

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