deltacloud-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "marios@redhat.com" <mandr...@redhat.com>
Subject Re: [PATCH] Fgcp: changes and bug fixes to pass most api tests (rebased)
Date Fri, 25 Jan 2013 12:01:18 GMT
ACK - nit inline (which imo should be fixed before pushed):

On 25/01/13 13:08, diesk@fast.au.fujitsu.com wrote:
> From: Dies Koper <diesk@fast.au.fujitsu.com>
> 
> ---
>  server/lib/deltacloud/drivers/fgcp/fgcp_client.rb |  2 +-
>  server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb | 20 ++++++++++++++++----
>  tests/config.yaml                                 |  4 ++--
>  tests/deltacloud/common_tests_collections.rb      |  4 ++--
>  tests/deltacloud/instances_test.rb                |  8 ++++----
>  tests/deltacloud/test_setup.rb                    |  2 +-
>  6 files changed, 26 insertions(+), 14 deletions(-)
> 
> diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_client.rb b/server/lib/deltacloud/drivers/fgcp/fgcp_client.rb
> index 489e8c4..4d569da 100644
> --- a/server/lib/deltacloud/drivers/fgcp/fgcp_client.rb
> +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_client.rb
> @@ -167,7 +167,7 @@ class FgcpClient
>        <description>#{description}</description>
>      </locale>
>      <locale>
> -      <lcid>jp</lcid>
> +      <lcid>ja</lcid>
>        <name>#{name}</name>
>        <description>#{description}</description>
>      </locale>
> diff --git a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> index 716ac85..cbb670c 100644
> --- a/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> +++ b/server/lib/deltacloud/drivers/fgcp/fgcp_driver.rb
> @@ -109,6 +109,13 @@ class FgcpDriver < Deltacloud::BaseDriver
>        if xml['diskimages'] # not likely to not be so, but just in case
>          xml['diskimages'][0]['diskimage'].each do |img|
>  
> +          # 32bit CentOS/RHEL images are refused on hwps > 16GB (i.e. w_high, quad_high)
> +          os_arch = img['osName'][0].to_s =~ /.*32.?bit.*/ ? 'i386' : 'x86_64'
> +          os_centos_rhel = img['osName'][0] =~ /(CentOS|Red Hat).*/
> +          allowed_hwps = hwps.select do |hwp|
> +            hwp.memory.default.to_i < 16000 or os_arch == 'x86_64' or not os_centos_rhel
> +          end
> +
>            images << Image.new(
>              :id => img['diskimageId'][0],
>              :name => img['diskimageName'][0].to_s,
> @@ -118,8 +125,8 @@ class FgcpDriver < Deltacloud::BaseDriver
>              # This will determine image architecture using OS name.
>              # Usually the OS name includes '64bit' or '32bit'. If not,
>              # it will fall back to 64 bit.
> -            :architecture => img['osName'][0].to_s =~ /.*32.?bit.*/ ? 'i386' : 'x86_64',
> -            :hardware_profiles => hwps
> +            :architecture => os_arch,
> +            :hardware_profiles => allowed_hwps
>            ) if opts[:id].nil? or opts[:id] == img['diskimageId'][0]
>          end
>        end
> @@ -139,7 +146,7 @@ class FgcpDriver < Deltacloud::BaseDriver
>        if opts[:name].nil?
>          # default to instance name
>          instance = client.get_vserver_attributes(opts[:id])
> -        opts[:name] ||= instance['vserver'][0]['vserverName']
> +        opts[:name] = instance['vserver'][0]['vserverName']
>          opts[:description] ||= opts[:name]
>        end
>  
> @@ -1444,6 +1451,11 @@ eofwopxml
>        status 404 # Not Found
>      end
>  
> +    # trying to create an image that has never been booted
> +    on /NEVER_BOOTED/ do
> +      status 405 # Method Not Allowed
> +    end
> +


I think this should be a 409 - it's not the case that POST /api/images
isn't allowed ... rather POST /api/images using an instance that was
never booted, isn't allowed (i.e. 'conflict with the current state of
the resource').



marios




>      # reached maximum number of attempts while polling for an update
>      on /Server did not include public IP address in FW NAT rules/ do
>        status 504 # Gateway Timeout
> @@ -1630,7 +1642,7 @@ eofwopxml
>        :firewalls => server != 'FW' ? [client.extract_vsys_id(vserver['vserverId'][0])
+ '-S-0001'] : nil,
>        :owner_id => vserver['creator'][0]
>      }
> -    instance.merge!( {'create_image' => false}) if not server == 'vserver'
> +    instance.merge!( {'create_image' => false}) if server != 'vserver' or state_data[:state]
!= 'STOPPED'
>      instance.merge! state_data
>  
>      Instance::new(instance)
> diff --git a/tests/config.yaml b/tests/config.yaml
> index 2f18caa..078f6fa 100644
> --- a/tests/config.yaml
> +++ b/tests/config.yaml
> @@ -39,9 +39,9 @@ fgcp:
>    #used for instances tests:
>    preferred:
>      provider: "au"
> -    image: "IMG_3c9820_D30U8UNY6I9S"
> +    image: "IMG_3c9820_S24FWXU0Q9VH0JK"
>      hardware_profile: "economy"
> -    realm: "UZXC0GRT-ZG8ZJCJ07"
> +    realm: "UZXC0GRT-ZG8ZJCJ07-N-DMZ"
>  
>  
>  # CIMI testing
> diff --git a/tests/deltacloud/common_tests_collections.rb b/tests/deltacloud/common_tests_collections.rb
> index 5745296..717e3a7 100644
> --- a/tests/deltacloud/common_tests_collections.rb
> +++ b/tests/deltacloud/common_tests_collections.rb
> @@ -39,7 +39,7 @@ module CommonCollectionsTest
>        end
>  
>        it "must require authentication to access the #{test_collection} collection" do
> -        skip "Skipping for #{test_collection} as no auth required here" if ["hardware_profiles",
"instance_states"].include?(test_collection)
> +        skip "Skipping for #{test_collection} as auth may not be required here" if ["hardware_profiles",
"instance_states"].include?(test_collection)
>          proc {  get(test_collection, :noauth => true) }.must_raise RestClient::Request::Unauthorized
>        end
>  
> @@ -59,7 +59,7 @@ module CommonCollectionsTest
>      end
>    end
>  
> -  #run tests for both the top-level collection and it's members
> +  #run tests for both the top-level collection and its members
>    def self.run_collection_and_member_tests_for(test_collection)
>      #first run only 'top-level' collection tests (e.g. for 'images')
>      run_collection_tests_for(test_collection)
> diff --git a/tests/deltacloud/instances_test.rb b/tests/deltacloud/instances_test.rb
> index 73b5ed2..1522914 100644
> --- a/tests/deltacloud/instances_test.rb
> +++ b/tests/deltacloud/instances_test.rb
> @@ -84,11 +84,11 @@ puts "CLEANUP attempt finished... resources looks like: #{@@created_resources.in
>  
>    #Now run the instances-specific tests:
>  
> -  it 'must have the "state" element defined for each instance in collection' do
> +  it 'must have a legal "state" element defined for each instance in collection, or
no "state" at all' do
>      res = get(INSTANCES)
>      (res.xml/'instances/instance').each do |r|
> -      (r/'state').wont_be_empty
> -      (r/'state').first.must_match /(RUNNING|STOPPED|PENDING)/
> +      # provider may not return state for each instance in collection because of performance
reasons
> +      (r/'state').first.must_match /(RUNNING|STOPPED|PENDING)/ unless (r/'state').empty?
>      end
>    end
>  
> @@ -242,7 +242,7 @@ puts "CLEANUP attempt finished... resources looks like: #{@@created_resources.in
>      instance_actions = (res.xml/'actions/link').to_a.inject([]){|actions, current| actions
<< current[:rel]; actions}
>      skip "no create image support for instance #{@@my_instance_id}" unless instance_actions.include?("create_image")
>      #create image
> -    res = post("/images", :instance_id => @@my_instance_id, :name=>random_name)
> +    res = post("/images", :instance_id => @@my_instance_id, :name => random_name)
>      res.code.must_equal 201
>      my_image_id = (res.xml/'image')[0][:id]
>      #mark for deletion later:
> diff --git a/tests/deltacloud/test_setup.rb b/tests/deltacloud/test_setup.rb
> index b28e456..4d1943c 100644
> --- a/tests/deltacloud/test_setup.rb
> +++ b/tests/deltacloud/test_setup.rb
> @@ -172,7 +172,7 @@ module Deltacloud::Test::Methods
>  
>      def random_name
>        name = rand(36**10).to_s(36)
> -      name.insert(0, "apitest")
> +      name.insert(0, "dcapitest")
>      end
>  
>      def get_a(item)
> 


Mime
View raw message