ACK - nit inline (which imo should be fixed before pushed):
On 25/01/13 13:08, [email protected] wrote:
> From: Dies Koper <[email protected]>
>
> ---
> 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)
>
