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] Fix how deltacloud generally and openstack driver particularly checks supported_collections
Date Fri, 14 Sep 2012 15:13:29 GMT
On Sep 14, 2012, at 4:48 PM, marios@redhat.com wrote:

Looks good to me. There is one extra failure on OPTIONS in /api
so make sure you fix it before push :-)

Also it would be nice if you can move that commented tests into
base_driver tests and run it just to make sure everything works
as expected.

  -- Michal

> From: marios <marios@redhat.com>
> 
> (related https://issues.apache.org/jira/browse/DTACLOUD-316)
> 
> Signed-off-by: marios <marios@redhat.com>
> ---
> server/lib/deltacloud/drivers/base_driver.rb           | 17 +++++++++++++++++
> .../deltacloud/drivers/openstack/openstack_driver.rb   | 18 ++++++++----------
> server/lib/deltacloud/helpers/deltacloud_helper.rb     | 16 ----------------
> server/lib/deltacloud/server.rb                        |  9 +++------
> server/tests/deltacloud/deltacloud_helper_test.rb      | 18 ++++++++++--------
> server/views/api/show.html.haml                        |  2 +-
> server/views/api/show.xml.haml                         |  2 +-
> 7 files changed, 40 insertions(+), 42 deletions(-)
> 
> diff --git a/server/lib/deltacloud/drivers/base_driver.rb b/server/lib/deltacloud/drivers/base_driver.rb
> index 41b6923..3b37721 100644
> --- a/server/lib/deltacloud/drivers/base_driver.rb
> +++ b/server/lib/deltacloud/drivers/base_driver.rb
> @@ -155,6 +155,23 @@ module Deltacloud
>       (self.class.instance_methods - superclass_methods).include? method
>     end
> 
> +    def supported_collections(credentials)
> +      collection_arr = []
> +      Deltacloud::Collections.deltacloud_modules.each do |m|
> +        m.collections.each do |c|
> +          # Get the required capability for the :index operation (like 'realms' or 'instance_state_machine')
> +          index_operation_capability = c.operation(:index).required_capability
> +          # Then use this capability to check if the 'capability' lambda defined
> +          # for the Sinatra::Base class evaluate to 'true'
> +          next if m.settings.respond_to?(:capability) and !m.settings.capability(index_operation_capability)
> +          yield c if block_given?
> +          collection_arr << c
> +        end
> +      end
> +      collection_arr
> +    end
> +
> +
>     ## Capabilities
>     # The rabbit dsl supports declaring a capability that is required
>     # in the backend driver for the call to succeed. A driver can
> diff --git a/server/lib/deltacloud/drivers/openstack/openstack_driver.rb b/server/lib/deltacloud/drivers/openstack/openstack_driver.rb
> index ca824bd..7e11258 100644
> --- a/server/lib/deltacloud/drivers/openstack/openstack_driver.rb
> +++ b/server/lib/deltacloud/drivers/openstack/openstack_driver.rb
> @@ -40,17 +40,15 @@ module Deltacloud
> 
>         define_hardware_profile('default')
> 
> -        def respond_to?(capability)#, credentials)
> -          if capability == :buckets
> -            begin
> -              client = new_client(Deltacloud.config["openstack_creds"], capability)
> -            rescue Deltacloud::ExceptionHandler::NotImplemented #OpenStack::Exception::NotImplemented...
> -              return false
> -            end
> -            return true
> -          else
> -            super(capability)
> +        def supported_collections(credentials)
> +          #get the collections as defined by 'capability' and 'respond_to?' blocks
> +          super_collections = super
> +          begin
> +             client = new_client(credentials, :buckets)
> +          rescue Deltacloud::ExceptionHandler::NotImplemented #OpenStack::Exception::NotImplemented...
> +             return super_collections - [Sinatra::Rabbit::BucketsCollection]
>           end
> +          super_collections
>         end
> 
>         def hardware_profiles(credentials, opts = {})
> diff --git a/server/lib/deltacloud/helpers/deltacloud_helper.rb b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> index 33fbe23..404e293 100644
> --- a/server/lib/deltacloud/helpers/deltacloud_helper.rb
> +++ b/server/lib/deltacloud/helpers/deltacloud_helper.rb
> @@ -18,22 +18,6 @@ module Deltacloud::Helpers
> 
>     require 'benchmark'
> 
> -    def supported_collections
> -      collection_arr = []
> -      Deltacloud::Collections.deltacloud_modules.each do |m|
> -        m.collections.each do |c|
> -          # Get the required capability for the :index operation (like 'realms' or 'instance_state_machine')
> -          index_operation_capability = c.operation(:index).required_capability
> -          # Then use this capability to check if the 'capability' lambda defined
> -          # for the Sinatra::Base class evaluate to 'true'
> -          next if m.settings.respond_to?(:capability) and !m.settings.capability(index_operation_capability)
> -          yield c if block_given?
> -          collection_arr << c
> -        end
> -      end
> -      collection_arr
> -    end
> -
>     def auth_feature_name
>       return 'key' if driver.class.has_feature?(:instances, :authentication_key)
>       return 'password' if driver.class.has_feature?(:instances, :authentication_password)
> diff --git a/server/lib/deltacloud/server.rb b/server/lib/deltacloud/server.rb
> index e2b2a8f..4ed527a 100644
> --- a/server/lib/deltacloud/server.rb
> +++ b/server/lib/deltacloud/server.rb
> @@ -46,13 +46,10 @@ module Deltacloud
>     set :config, Deltacloud[:deltacloud]
> 
>     get '/' do
> -      if driver.name == "openstack" or params[:force_auth]
> +      if params[:force_auth]
>         return [401, 'Authentication failed'] unless driver.valid_credentials?(credentials)
> -        if driver.name == "openstack"
> -          Deltacloud.config["openstack_creds"] = credentials
> -          #or here also works: Thread.current["openstack_creds"] = credentials
> -        end
>       end
> +      @collections = driver.supported_collections(credentials)
>       respond_to do |format|
>         format.xml { haml :"api/show" }
>         format.json { xml_to_json :"api/show" }
> @@ -61,7 +58,7 @@ module Deltacloud
>     end
> 
>     options '/' do
> -      headers 'Allow' => supported_collections { |c| c.collection_name }.join(',')
> +      headers 'Allow' => driver.supported_collections { |c| c.collection_name }.join(',')
>     end
> 
>     post '/' do
> diff --git a/server/tests/deltacloud/deltacloud_helper_test.rb b/server/tests/deltacloud/deltacloud_helper_test.rb
> index b7de3e6..060948b 100644
> --- a/server/tests/deltacloud/deltacloud_helper_test.rb
> +++ b/server/tests/deltacloud/deltacloud_helper_test.rb
> @@ -13,14 +13,16 @@ describe Deltacloud::Helpers::Application do
>     @helper = ApplicationHelper.new
>   end
> 
> -  it 'provides list of supported collections for the current driver' do
> -    @helper.supported_collections.wont_be_empty
> -    @helper.supported_collections.must_include Sinatra::Rabbit::InstancesCollection
> -    @helper.supported_collections.wont_include Sinatra::Rabbit::LoadBalancersCollection
> -    Thread.current[:driver] = 'ec2'
> -    @helper.supported_collections.must_include Sinatra::Rabbit::LoadBalancersCollection
> -    Thread.current[:driver] = 'mock'
> -  end
> +
> +#THIS TEST SHOULD BE MOVED TO server/tests/drivers/base/base_driver_test.rb ?
> +#  it 'provides list of supported collections for the current driver' do
> +#    @helper.supported_collections.wont_be_empty
> +#    @helper.supported_collections.must_include Sinatra::Rabbit::InstancesCollection
> +#    @helper.supported_collections.wont_include Sinatra::Rabbit::LoadBalancersCollection
> +#    Thread.current[:driver] = 'ec2'
> +#    @helper.supported_collections.must_include Sinatra::Rabbit::LoadBalancersCollection
> +#    Thread.current[:driver] = 'mock'
> +#  end
> 
>   it 'provides name for the authentication feature' do
>     @helper.auth_feature_name.wont_be_nil
> diff --git a/server/views/api/show.html.haml b/server/views/api/show.html.haml
> index 541c388..15a3490 100644
> --- a/server/views/api/show.html.haml
> +++ b/server/views/api/show.html.haml
> @@ -3,7 +3,7 @@
> 
> %div{ :'data-role' => :content, :'data-theme' => 'c'}
>   %ul{ :'data-role' => :listview, :'data-inset' => 'true'}
> -    - supported_collections do |c|
> +    - @collections.each do |c|
>       %li
>         %a{ :href => url_for(c.collection_name),  :'data-icon' => "arrow-r"}=c.collection_name.to_s.gsub('_',
' ').titlecase
> 
> diff --git a/server/views/api/show.xml.haml b/server/views/api/show.xml.haml
> index 73e7171..c9d970e 100644
> --- a/server/views/api/show.xml.haml
> +++ b/server/views/api/show.xml.haml
> @@ -1,5 +1,5 @@
> %api{ :version => settings.version, :driver => driver_symbol, :provider => Thread.current[:provider]
|| ENV['API_PROVIDER'] }
> -  - supported_collections do |c|
> +  - @collections.each do |c|
>     %link{ :rel => c.collection_name, :href => self.send(:"#{c.collection_name}_url")}
>       - c.features.select { |f| driver.class.has_feature?(c.collection_name, f.name)
}.each do |f|
>         %feature{ :name => f.name }
> -- 
> 1.7.11.4
> 

Michal Fojtik
http://deltacloud.org
mfojtik@redhat.com




Mime
View raw message