You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@deltacloud.apache.org by Eric Woods <wo...@gmail.com> on 2011/01/28 23:51:31 UTC

IBM SBC Driver Available

Hi All,

As promised, I've submitted a patch for the IBM SBC driver to JIRA: https://issues.apache.org/jira/browse/DTACLOUD-15.

The patch is fully functional but has a few remaining (minor) todos.  I'd like to reach out for help on two issues:

1) sbc_client.rb:180:  For error handling, I raise Deltacloud:BackendErrors with error codes & messages that align with IBM's cloud.  These attributes are always ignored and the client sees a 500 Internal Server Error.  Is this the expected (generic) behavior, or should the client see error codes & messages specific to each cloud provider?  Perhaps this could be a standardization opportunity.

2) sbc_driver.rb:198: While converting each IBM instance to DeltaCloud's instances, I construct the an InstanceProfile by passing a HardwareProfile's name (the hardware profiles are defined at the bottom of the driver).  However, InstanceProfile does not to correctly lookup the corresponding HardwareProfile by name.  The InstanceProfile's id is set correctly, but cpu, storage, memory, and architecture are all null.  I can't find what I'm doing wrong or different from the other drivers.

Help with these issues and technical feedback is very appreciated.  Thanks!

Cheers,
- Eric W.

Re: IBM SBC Driver Available

Posted by Eric Woods <wo...@gmail.com>.
That did the trick!  Thanks, Michael!

On Feb 10, 2011, at 11:15 AM, Michal Fojtik wrote:

> On 10/02/11 10:59 -0500, Eric Woods wrote:
>> David, I am able to run the cucumber tests from your advice below.  However, I'm still unable to run the unit tests even after installing rspec (version 2.5.0) via "sudo gem install rspec"  Same error: "no such file to load -- spec (LoadError)"
>> 
>> I see within server/Rakefile that "spec" is initialized as "spec = Gem::Specification.load('deltacloud-core.gemspec')"  Could this be inaccessible to the tests or not run at all when running "rake test" from the server directory?
>> 
>> Here's the full message:
>> (in /Users/Eric/Development/workspace/deltacloud/110128_1718_deltacloud/server)
>> /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/bin/ruby -I"lib" "/Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb" "tests/drivers/mock/api_test.rb" "tests/drivers/mock/hardware_profiles_test.rb" "tests/drivers/mock/images_test.rb" "tests/drivers/mock/instance_states_test.rb" "tests/drivers/mock/instances_test.rb" "tests/drivers/mock/realms_test.rb" "tests/drivers/mock/url_for_test.rb" "tests/rabbit_test.rb"
>> warning: Insecure world writable dir /Users/Eric in PATH, mode 040777
>> /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/rubygems/custom_require.rb:31:in `gem_original_require': no such file to load -- spec (LoadError)
>> 	from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/rubygems/custom_require.rb:31:in `require'
>> 	from ./tests/drivers/mock/../../../tests/common.rb:6
>> 	from ./tests/drivers/mock/api_test.rb:2:in `require'
>> 	from ./tests/drivers/mock/api_test.rb:2
>> 	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5:in `load'
>> 	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5
>> 	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5:in `each'
>> 	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5
>> rake aborted!
>> Command failed with status (1): [/System/Library/Frameworks/Ruby.framework/...]
> 
> Hi Eric,
> 
> I suppose your have installed rspec version 2.0 which is not backward
> compatible with 1.3 which we are using there.
> 
> (gem install rspec --version=1.3.0 should help here).
> 
>  -- Michal
> 
>> 
>> Thanks for your help,
>> - Eric W.
>> 
>> 
>> On Feb 7, 2011, at 7:05 PM, David Lutterkort wrote:
>> 
>>> On Mon, 2011-02-07 at 17:15 -0500, Eric Woods wrote:
>>>> I've updated the patch to address your comment (https://issues.apache.org/jira/browse/DTACLOUD-15).  The driver should be complete and ready to be committed.
>>>> 
>>>> Now I'm turning my attention to tests.  I'm running into issues when running either test set:
>>>> 
>>>> 1) When I run "rake test" within the server directory, I get no
>>>> output.  After inspecting server/Rakefile, I changed the test
>>>> FileList.new(...) to "t.test_files =
>>>> FileList.new('tests/**/**/*_test.rb')" to pull in the "mock"
>>>> directory.  This triggers common.rb to execute, but I then receive a
>>>> LoadError for "spec."  I can't find this gem installed locally or
>>> 
>>> The tests pass for me without any changes to the Rakefile (IIRC, '**'
>>> matches directory hierarchies of arbitrary depths)
>>> 
>>> 
>>>> remotely.  I'm trying to install it via "sudo gem install spec."
>>> 
>>> The gem is cleverly called 'rspec'
>>> 
>>>> 2) When I run "API_DRIVER={ec2, mock} rake cucumber" within top level
>>>> tests, the rake aborts from not being able to load "rack/test"
>>> 
>>> You need to 'gem install rack-test'. There's a Gemfile that should list
>>> all the dependencies (look in the :test section) If anything is missing
>>> from that list, we need to add it.
>>> 
>>>> Only ec2, mock, and rhevm are supported for cucumber tests, and only
>>>> mock is supported for the unit tests.  Have these tests been updated
>>>> recently and we know they should run?  Of course I'm new to ruby, so I
>>>> could be making a trivial mistake.
>>> 
>>> Yes, all those tests pass for me - clearly, we need to document the
>>> process of setting up a dev environment better. If you still have it,
>>> can you collect a list of the gems you had to install for this
>>> exercise ? We need to make sure it goes into the Gemfile and the
>>> gemspec.
>>> 
>>> David
>>> 
>>> 
>> 
> 
> -- 
> --------------------------------------------------------
> Michal Fojtik, mfojtik@redhat.com
> Deltacloud API: http://deltacloud.org
> --------------------------------------------------------


Re: IBM SBC Driver Available

Posted by Michal Fojtik <mf...@redhat.com>.
On 10/02/11 10:59 -0500, Eric Woods wrote:
>David, I am able to run the cucumber tests from your advice below.  However, I'm still unable to run the unit tests even after installing rspec (version 2.5.0) via "sudo gem install rspec"  Same error: "no such file to load -- spec (LoadError)"
>
>I see within server/Rakefile that "spec" is initialized as "spec = Gem::Specification.load('deltacloud-core.gemspec')"  Could this be inaccessible to the tests or not run at all when running "rake test" from the server directory?
>
>Here's the full message:
>(in /Users/Eric/Development/workspace/deltacloud/110128_1718_deltacloud/server)
>/System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/bin/ruby -I"lib" "/Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb" "tests/drivers/mock/api_test.rb" "tests/drivers/mock/hardware_profiles_test.rb" "tests/drivers/mock/images_test.rb" "tests/drivers/mock/instance_states_test.rb" "tests/drivers/mock/instances_test.rb" "tests/drivers/mock/realms_test.rb" "tests/drivers/mock/url_for_test.rb" "tests/rabbit_test.rb"
>warning: Insecure world writable dir /Users/Eric in PATH, mode 040777
>/System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/rubygems/custom_require.rb:31:in `gem_original_require': no such file to load -- spec (LoadError)
>	from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/rubygems/custom_require.rb:31:in `require'
>	from ./tests/drivers/mock/../../../tests/common.rb:6
>	from ./tests/drivers/mock/api_test.rb:2:in `require'
>	from ./tests/drivers/mock/api_test.rb:2
>	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5:in `load'
>	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5
>	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5:in `each'
>	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5
>rake aborted!
>Command failed with status (1): [/System/Library/Frameworks/Ruby.framework/...]

Hi Eric,

I suppose your have installed rspec version 2.0 which is not backward
compatible with 1.3 which we are using there.

(gem install rspec --version=1.3.0 should help here).

   -- Michal

>
>Thanks for your help,
>- Eric W.
>
>
>On Feb 7, 2011, at 7:05 PM, David Lutterkort wrote:
>
>> On Mon, 2011-02-07 at 17:15 -0500, Eric Woods wrote:
>>> I've updated the patch to address your comment (https://issues.apache.org/jira/browse/DTACLOUD-15).  The driver should be complete and ready to be committed.
>>>
>>> Now I'm turning my attention to tests.  I'm running into issues when running either test set:
>>>
>>> 1) When I run "rake test" within the server directory, I get no
>>> output.  After inspecting server/Rakefile, I changed the test
>>> FileList.new(...) to "t.test_files =
>>> FileList.new('tests/**/**/*_test.rb')" to pull in the "mock"
>>> directory.  This triggers common.rb to execute, but I then receive a
>>> LoadError for "spec."  I can't find this gem installed locally or
>>
>> The tests pass for me without any changes to the Rakefile (IIRC, '**'
>> matches directory hierarchies of arbitrary depths)
>>
>>
>>> remotely.  I'm trying to install it via "sudo gem install spec."
>>
>> The gem is cleverly called 'rspec'
>>
>>> 2) When I run "API_DRIVER={ec2, mock} rake cucumber" within top level
>>> tests, the rake aborts from not being able to load "rack/test"
>>
>> You need to 'gem install rack-test'. There's a Gemfile that should list
>> all the dependencies (look in the :test section) If anything is missing
>> from that list, we need to add it.
>>
>>> Only ec2, mock, and rhevm are supported for cucumber tests, and only
>>> mock is supported for the unit tests.  Have these tests been updated
>>> recently and we know they should run?  Of course I'm new to ruby, so I
>>> could be making a trivial mistake.
>>
>> Yes, all those tests pass for me - clearly, we need to document the
>> process of setting up a dev environment better. If you still have it,
>> can you collect a list of the gems you had to install for this
>> exercise ? We need to make sure it goes into the Gemfile and the
>> gemspec.
>>
>> David
>>
>>
>

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

Re: IBM SBC Driver Available

Posted by Eric Woods <wo...@gmail.com>.
David, I am able to run the cucumber tests from your advice below.  However, I'm still unable to run the unit tests even after installing rspec (version 2.5.0) via "sudo gem install rspec"  Same error: "no such file to load -- spec (LoadError)"

I see within server/Rakefile that "spec" is initialized as "spec = Gem::Specification.load('deltacloud-core.gemspec')"  Could this be inaccessible to the tests or not run at all when running "rake test" from the server directory?

Here's the full message:
(in /Users/Eric/Development/workspace/deltacloud/110128_1718_deltacloud/server)
/System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/bin/ruby -I"lib" "/Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb" "tests/drivers/mock/api_test.rb" "tests/drivers/mock/hardware_profiles_test.rb" "tests/drivers/mock/images_test.rb" "tests/drivers/mock/instance_states_test.rb" "tests/drivers/mock/instances_test.rb" "tests/drivers/mock/realms_test.rb" "tests/drivers/mock/url_for_test.rb" "tests/rabbit_test.rb" 
warning: Insecure world writable dir /Users/Eric in PATH, mode 040777
/System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/rubygems/custom_require.rb:31:in `gem_original_require': no such file to load -- spec (LoadError)
	from /System/Library/Frameworks/Ruby.framework/Versions/1.8/usr/lib/ruby/1.8/rubygems/custom_require.rb:31:in `require'
	from ./tests/drivers/mock/../../../tests/common.rb:6
	from ./tests/drivers/mock/api_test.rb:2:in `require'
	from ./tests/drivers/mock/api_test.rb:2
	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5:in `load'
	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5
	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5:in `each'
	from /Library/Ruby/Gems/1.8/gems/rake-0.8.7/lib/rake/rake_test_loader.rb:5
rake aborted!
Command failed with status (1): [/System/Library/Frameworks/Ruby.framework/...]

Thanks for your help,
- Eric W.


On Feb 7, 2011, at 7:05 PM, David Lutterkort wrote:

> On Mon, 2011-02-07 at 17:15 -0500, Eric Woods wrote:
>> I've updated the patch to address your comment (https://issues.apache.org/jira/browse/DTACLOUD-15).  The driver should be complete and ready to be committed.
>> 
>> Now I'm turning my attention to tests.  I'm running into issues when running either test set:
>> 
>> 1) When I run "rake test" within the server directory, I get no
>> output.  After inspecting server/Rakefile, I changed the test
>> FileList.new(...) to "t.test_files =
>> FileList.new('tests/**/**/*_test.rb')" to pull in the "mock"
>> directory.  This triggers common.rb to execute, but I then receive a
>> LoadError for "spec."  I can't find this gem installed locally or 
> 
> The tests pass for me without any changes to the Rakefile (IIRC, '**'
> matches directory hierarchies of arbitrary depths)
> 
> 
>> remotely.  I'm trying to install it via "sudo gem install spec."
> 
> The gem is cleverly called 'rspec'
> 
>> 2) When I run "API_DRIVER={ec2, mock} rake cucumber" within top level
>> tests, the rake aborts from not being able to load "rack/test"
> 
> You need to 'gem install rack-test'. There's a Gemfile that should list
> all the dependencies (look in the :test section) If anything is missing
> from that list, we need to add it.
> 
>> Only ec2, mock, and rhevm are supported for cucumber tests, and only
>> mock is supported for the unit tests.  Have these tests been updated
>> recently and we know they should run?  Of course I'm new to ruby, so I
>> could be making a trivial mistake.
> 
> Yes, all those tests pass for me - clearly, we need to document the
> process of setting up a dev environment better. If you still have it,
> can you collect a list of the gems you had to install for this
> exercise ? We need to make sure it goes into the Gemfile and the
> gemspec.
> 
> David
> 
> 


Re: IBM SBC Driver Available

Posted by David Lutterkort <lu...@redhat.com>.
On Mon, 2011-02-07 at 17:15 -0500, Eric Woods wrote:
> I've updated the patch to address your comment (https://issues.apache.org/jira/browse/DTACLOUD-15).  The driver should be complete and ready to be committed.
> 
> Now I'm turning my attention to tests.  I'm running into issues when running either test set:
> 
> 1) When I run "rake test" within the server directory, I get no
> output.  After inspecting server/Rakefile, I changed the test
> FileList.new(...) to "t.test_files =
> FileList.new('tests/**/**/*_test.rb')" to pull in the "mock"
> directory.  This triggers common.rb to execute, but I then receive a
> LoadError for "spec."  I can't find this gem installed locally or 

The tests pass for me without any changes to the Rakefile (IIRC, '**'
matches directory hierarchies of arbitrary depths)


> remotely.  I'm trying to install it via "sudo gem install spec."

The gem is cleverly called 'rspec'

> 2) When I run "API_DRIVER={ec2, mock} rake cucumber" within top level
> tests, the rake aborts from not being able to load "rack/test"

You need to 'gem install rack-test'. There's a Gemfile that should list
all the dependencies (look in the :test section) If anything is missing
from that list, we need to add it.

> Only ec2, mock, and rhevm are supported for cucumber tests, and only
> mock is supported for the unit tests.  Have these tests been updated
> recently and we know they should run?  Of course I'm new to ruby, so I
> could be making a trivial mistake.

Yes, all those tests pass for me - clearly, we need to document the
process of setting up a dev environment better. If you still have it,
can you collect a list of the gems you had to install for this
exercise ? We need to make sure it goes into the Gemfile and the
gemspec.

David



Re: IBM SBC Driver Available

Posted by David Lutterkort <lu...@redhat.com>.
On Mon, 2011-02-07 at 17:15 -0500, Eric Woods wrote:
> I've updated the patch to address your comment
> (https://issues.apache.org/jira/browse/DTACLOUD-15).  The driver
> should be complete and ready to be committed.

I've just committed the driver and closed the Jira issue. There are
still a few TODO's that I would like to see addressed soon:

      * Documentation on the usage of the driver; most importantly
        instructions on where to get username/password - the information
        for the overall drivers table[1]
      * A row for the table listing driver capabilities [2]
      * sbc_driver (convert_instance): the InstanceProfile does not look
        up hte underlying HWP - the connection is solely by using the
        same name
      * sbc_driver (convert_image): architecture needs to be set
        correctly
      * sbc_driver (hardware profiles): you are right that using HWP
        names that contain '/' will lead to tears. Could we just use ':'
        as a separator instead ? With that switch we'd need to make sure
        that HWP names are translated correctly everywhere, in
        particular, when getting a HWP name from SBC or sending on to it

David

[1] http://incubator.apache.org/deltacloud/documentation.html
[2] http://incubator.apache.org/deltacloud/drivers.html



Re: IBM SBC Driver Available

Posted by Eric Woods <wo...@gmail.com>.
I've updated the patch to address your comment (https://issues.apache.org/jira/browse/DTACLOUD-15).  The driver should be complete and ready to be committed.

Now I'm turning my attention to tests.  I'm running into issues when running either test set:

1) When I run "rake test" within the server directory, I get no output.  After inspecting server/Rakefile, I changed the test FileList.new(...) to "t.test_files = FileList.new('tests/**/**/*_test.rb')" to pull in the "mock" directory.  This triggers common.rb to execute, but I then receive a LoadError for "spec."  I can't find this gem installed locally or remotely.  I'm trying to install it via "sudo gem install spec."

2) When I run "API_DRIVER={ec2, mock} rake cucumber" within top level tests, the rake aborts from not being able to load "rack/test"

Only ec2, mock, and rhevm are supported for cucumber tests, and only mock is supported for the unit tests.  Have these tests been updated recently and we know they should run?  Of course I'm new to ruby, so I could be making a trivial mistake.

Thanks
Eric W.

On Feb 2, 2011, at 7:54 PM, David Lutterkort wrote:
> On Wed, 2011-02-02 at 15:30 -0500, Eric Woods wrote:
>> Unfortunately I'm not able to provide a test account for IBM's cloud,
>> so we'll have to "fake it" for test cases.
> 
> Too bad - we'll figure something else out.
> 
>>  Is writing/running tests documented anywhere?
> 
> There's two levels of testing that are relevant here: 
>     1. unit tests in server/tests that you run with 'rake tests' from
>        the server directory[1]
>     2. blackbox cucumber tests in the toplevel tests/ directory. They
>        are run with 'API_DRIVER=xyz rake cucumber' where API_DRIVER
>        selects for which driver to run tests; currently, we have tests
>        for the mock, ec2 and rhevm drivers. These tests can either run
>        directly against an actual cloud or off of canned data captured
>        from a 'real' test run - how to capture data is a bit of a black
>        art, and Michal needs to document that
> 
> The underlying test frameworks though (Test::Unit and Cucumber) are
> standard Ruby test frameworks.
> 
>> The issue is not all hardware profiles are valid for all images.  For
>> each image, a list of valid HWP configurations are provided.  This
>> mandates that I check the corresponding image to determine a valid
>> "default."  I noticed that images() is called when a create request is
>> issued, so I stored the image there rather than performing an
>> additional lookup later.
> 
> Aah .. it all makes sense now; I hadn't noticed that. Just to be on the
> safe side then, just check that the cached image is really what we want,
> i.e. on line 81 etc. instead of
> 
>            if @last_image.nil?
>              @last_image = sbc_client.list_images(image_id).map[0]
>            end
> write
>            if @last_image.nil? || @last_image.id != image_id
>              @last_image = sbc_client.list_images(image_id).map[0]
>            end
> 
> And maybe add a comment in images why it's ok to cache an image in
> @last_image.
> 
>> http://incubator.apache.org/deltacloud/framework.html.  Near the
>> bottom where reboot_instance() and stop_instance() are documented.
> 
> Aah .. cool; fixed that.
> 
> David
> 
> [1] and yes, the coverage of these tests is embarrassing
> 
> 


Re: IBM SBC Driver Available

Posted by David Lutterkort <lu...@redhat.com>.
On Wed, 2011-02-02 at 15:30 -0500, Eric Woods wrote:
> Unfortunately I'm not able to provide a test account for IBM's cloud,
> so we'll have to "fake it" for test cases.

Too bad - we'll figure something else out.

>   Is writing/running tests documented anywhere?

There's two levels of testing that are relevant here: 
     1. unit tests in server/tests that you run with 'rake tests' from
        the server directory[1]
     2. blackbox cucumber tests in the toplevel tests/ directory. They
        are run with 'API_DRIVER=xyz rake cucumber' where API_DRIVER
        selects for which driver to run tests; currently, we have tests
        for the mock, ec2 and rhevm drivers. These tests can either run
        directly against an actual cloud or off of canned data captured
        from a 'real' test run - how to capture data is a bit of a black
        art, and Michal needs to document that

The underlying test frameworks though (Test::Unit and Cucumber) are
standard Ruby test frameworks.

> The issue is not all hardware profiles are valid for all images.  For
> each image, a list of valid HWP configurations are provided.  This
> mandates that I check the corresponding image to determine a valid
> "default."  I noticed that images() is called when a create request is
> issued, so I stored the image there rather than performing an
> additional lookup later.

Aah .. it all makes sense now; I hadn't noticed that. Just to be on the
safe side then, just check that the cached image is really what we want,
i.e. on line 81 etc. instead of

            if @last_image.nil?
              @last_image = sbc_client.list_images(image_id).map[0]
            end
write
            if @last_image.nil? || @last_image.id != image_id
              @last_image = sbc_client.list_images(image_id).map[0]
            end

And maybe add a comment in images why it's ok to cache an image in
@last_image.

> http://incubator.apache.org/deltacloud/framework.html.  Near the
> bottom where reboot_instance() and stop_instance() are documented.

Aah .. cool; fixed that.

David

[1] and yes, the coverage of these tests is embarrassing



Re: IBM SBC Driver Available

Posted by Eric Woods <wo...@gmail.com>.
Thanks, David.  See responses below.

Unfortunately I'm not able to provide a test account for IBM's cloud, so we'll have to "fake it" for test cases.  Is writing/running tests documented anywhere?

Thanks,
- Eric W.

On Feb 1, 2011, at 6:26 PM, David Lutterkort wrote:

> On Mon, 2011-01-31 at 14:37 -0500, Eric Woods wrote:
>> I've updated the driver based on David's feedback and uploaded a
>> patch: https://issues.apache.org/jira/browse/DTACLOUD-15.  I'm new to
>> ruby, so this feedback is really appreciated.
> 
> That looks good now, with the one exception of the @last_image business
> (see below)
> 
>> Some notes:
>> 2) I maintained an @last_image field to avoid an extra image lookup
>> for performance reasons.  I made this more robust by checking for nil
>> and performing a lookup if necessary.
> 
> Still don't like it, since it makes the default realm and HWP hard to
> predict. You might get used to the fact that you'll always get HWP x
> when you create an image without specifying the HWP explicitly, but then
> one day your code changes, and suddenly you get HWP y.
> 
> Since we already enumerate all the HWP explicitly, just pick one as the
> default (I assume COP32.1/2048/60 would be the simplest) As for realm,
> just hardcode one fairly arbitrary one.
> 
> These choices at least make the defaults for create instance predictable
> (and documentable ;)

The issue is not all hardware profiles are valid for all images.  For each image, a list of valid HWP configurations are provided.  This mandates that I check the corresponding image to determine a valid "default."  I noticed that images() is called when a create request is issued, so I stored the image there rather than performing an additional lookup later.  I recognize that this is a workaround, but I'm not sure how to pick a default when valid configurations are on a per-instance-basis.  Please advise.

> 
>> 3) The online documentation states that nothing is returned from
>> reboot/stop/destroy.  We should update the documentation to indicate
>> an instance is returned for each of these.
> 
> Where do you see that ? Definitely needs to be fixed.

http://incubator.apache.org/deltacloud/framework.html.  Near the bottom where reboot_instance() and stop_instance() are documented.

> 
>> 4) For instances(credentials, opts={}), this method/function is being
>> passed opts=nil which causes an exception, so I kept the line opts ||=
>> {}
> 
> Ok .. works for me.
> 
> David
> 
> 


Re: IBM SBC Driver Available

Posted by David Lutterkort <lu...@redhat.com>.
On Mon, 2011-01-31 at 14:37 -0500, Eric Woods wrote:
> I've updated the driver based on David's feedback and uploaded a
> patch: https://issues.apache.org/jira/browse/DTACLOUD-15.  I'm new to
> ruby, so this feedback is really appreciated.

That looks good now, with the one exception of the @last_image business
(see below)

> Some notes:
> 2) I maintained an @last_image field to avoid an extra image lookup
> for performance reasons.  I made this more robust by checking for nil
> and performing a lookup if necessary.

Still don't like it, since it makes the default realm and HWP hard to
predict. You might get used to the fact that you'll always get HWP x
when you create an image without specifying the HWP explicitly, but then
one day your code changes, and suddenly you get HWP y.

Since we already enumerate all the HWP explicitly, just pick one as the
default (I assume COP32.1/2048/60 would be the simplest) As for realm,
just hardcode one fairly arbitrary one.

These choices at least make the defaults for create instance predictable
(and documentable ;)

> 3) The online documentation states that nothing is returned from
> reboot/stop/destroy.  We should update the documentation to indicate
> an instance is returned for each of these.

Where do you see that ? Definitely needs to be fixed.

> 4) For instances(credentials, opts={}), this method/function is being
> passed opts=nil which causes an exception, so I kept the line opts ||=
> {}

Ok .. works for me.

David



Re: IBM SBC Driver Available

Posted by Eric Woods <wo...@gmail.com>.
I've updated the driver based on David's feedback and uploaded a patch: https://issues.apache.org/jira/browse/DTACLOUD-15.  I'm new to ruby, so this feedback is really appreciated.

Some notes:
1) Michael: I'm interested in writing tests for the driver.  What's involved?
2) I maintained an @last_image field to avoid an extra image lookup for performance reasons.  I made this more robust by checking for nil and performing a lookup if necessary.
3) The online documentation states that nothing is returned from reboot/stop/destroy.  We should update the documentation to indicate an instance is returned for each of these.
4) For instances(credentials, opts={}), this method/function is being passed opts=nil which causes an exception, so I kept the line opts ||= {}
5) General cleanup based on David's feedback (thanks!)

- Eric W.

On Jan 28, 2011, at 7:44 PM, David Lutterkort wrote:

> On Fri, 2011-01-28 at 17:51 -0500, Eric Woods wrote:
>> Hi All,
>> 
>> As promised, I've submitted a patch for the IBM SBC driver to JIRA:
>> https://issues.apache.org/jira/browse/DTACLOUD-15.
> 
> Excellent ! Thanks a ton for writing the driver.
> 
>> The patch is fully functional but has a few remaining (minor) todos.
>> I'd like to reach out for help on two issues:
>> 
>> 1) sbc_client.rb:180:  For error handling, I raise
>> Deltacloud:BackendErrors with error codes & messages that align with
>> IBM's cloud.  These attributes are always ignored and the client sees
>> a 500 Internal Server Error.  Is this the expected (generic) behavior,
>> or should the client see error codes & messages specific to each cloud
>> provider?  Perhaps this could be a standardization opportunity.
> 
> What we do in other places is return a 502 Bad Gateway to indicate a
> generic backend error. Of course, if, for example, authentication fails,
> you should raise a Deltacloud::AuthException.
> 
> There's a bit of machinery for that: you can implement
> catched_excpetion_list, and enclose anything where you want to have
> backend errors mapped in a safely do .. end. (see the end of
> server/lib/deltacloud/base_driver/base_driver.rb for details)
> 
> Or, in this case, you can raise the proper Deltacloud exception directly
> from the SBC client.
> 
>> 2) sbc_driver.rb:198: While converting each IBM instance to
>> DeltaCloud's instances, I construct the an InstanceProfile by passing
>> a HardwareProfile's name (the hardware profiles are defined at the
>> bottom of the driver).  However, InstanceProfile does not to correctly
>> lookup the corresponding HardwareProfile by name.  The
>> InstanceProfile's id is set correctly, but cpu, storage, memory, and
>> architecture are all null.  I can't find what I'm doing wrong or
>> different from the other drivers.
> 
> That's the right behavior. cpu/storage/memory in InstanceProfile are
> only set if the instance's sizing deviates from what's already in the
> HWP. For SBC, that's not possible, but some other clouds let you choose,
> for example, memory in some range.
> 
>> Help with these issues and technical feedback is very appreciated.  Thanks!
> 
> One general thing: can you talk to Michal and figure out a way to add
> cucumber tests for the IBM SBC cloud ? We need to make sure we have some
> minimal test coverage to spot bitrot early on.
> 
> Also, is there some sort of test account we could use to run the driver
> against the actual cloud ?
> 
> Some more detailed comments on the driver:
> 
> First nit: please do not use tabs in the source; the rest of the code
> uses 2 space indentation - please reformat accordingly.
> 
> Index: server/lib/drivers.rb
> ===================================================================
> --- server/lib/drivers.rb	(revision 1064909)
> +++ server/lib/drivers.rb	(working copy)
> @@ -20,6 +20,7 @@
> module Deltacloud
>   DRIVERS = {
>     :ec2 => { :name => "EC2" },
> +	:sbc => { :name => "SBC" },
>     :rackspace => { :name => "Rackspace" },
>     :gogrid => { :name => "Gogrid" },
>     :rhevm => { :name => "RHEVM" },
> Index: server/lib/deltacloud/drivers/sbc/sbc_client.rb
> ===================================================================
> --- server/lib/deltacloud/drivers/sbc/sbc_client.rb	(revision 0)
> +++ server/lib/deltacloud/drivers/sbc/sbc_client.rb	(revision 0)
> @@ -0,0 +1,199 @@
> +#
> +# Copyright (C) 2009, 2010  Red Hat, Inc.
> 
> That should be either copyright you or IBM, depending what your IP
> agreement is with your employer. And it should be 'Copyright (C) 2011'
> 
> +module Deltacloud
> +  module Drivers
> +    module SBC
> +
> +#
> +# Client for the IBM Smart Business Cloud (SBC).
> +#
> +# Author: Eric Woods
> +# Date: 18 January 2011
> +#
> +class SBCClient
> +	
> +	@@API_URL = URI.parse('https://www-147.ibm.com/computecloud/enterprise/api/rest/20100331')
> 
> This should be a constant ('API_URL = ' rather than '@@API_URL = ')
> 
> +	#
> +	# Retrieve instances
> +	#
> +	def list_instances(instance_id=nil)
> +		if instance_id.nil?
> +			JSON.parse(get('/instances', default_headers))['instances']
> +		else
> +			instances = []
> +			instances << JSON.parse(get('/instances/' + instance_id, default_headers))
> +			instances
> 
> You can write the else branch more concisely as
> 
>                        else
>                                [ JSON.parse(get('/instances/' + instance_id, default_headers)) ]
>                        end
> 
> +	#
> +	# Retrieve locations; returns an XML document.
> +	#
> +	def list_locations
> +		puts 'retrieving locations...'
> 
> Leftover from debugging ?
> 
> +	
> +	#
> +	# Utility to URL encode a hash.
> +	#
> +	def urlencode(hash)
> +		query = ''
> +		hash.each do |name, value|
> +			if !query.empty?
> +				query += '&'
> +			end
> +			query += URI.encode(name.to_s) + '=' + URI.encode(value.to_s)
> +		end
> +		query
> +	end
> 
> Here's the more Rubyish oneliner to do the same:
> 
>                def urlencode(hash)
>                  hash.keys.map { |k| "#{k}=#{hash[k]}" }.join("&")
>                end
> 
> Index: server/lib/deltacloud/drivers/sbc/sbc_driver.rb
> ===================================================================
> --- server/lib/deltacloud/drivers/sbc/sbc_driver.rb	(revision 0)
> +++ server/lib/deltacloud/drivers/sbc/sbc_driver.rb	(revision 0)
> @@ -0,0 +1,300 @@
> +#
> +# Copyright (C) 2009, 2010  Red Hat, Inc.
> 
> Same comment about copyright notice.
> 
> +class SBCDriver < Deltacloud::BaseDriver
> +	#
> +	# Retrieves images
> +	#
> +	def images(credentials, opts={})
> +		sbc_client = new_client(credentials)
> +		opts ||= {}
> +		images = []
> +		images = sbc_client.list_images(opts[:id]).map do |image|
> +			@last_image = image
> +			convert_image(image)
> +		end
> 
> Don't store @last_image ... code farther down will blow up if it is
> called before images is ever called. The above should simply be
> 
>                        images = sbc_client.list_images(opts[:id]).map { |img| convert_image(img) }
> 
> +		images = filter_on(images, :architecture, opts)
> +		images = filter_on(images, :owner_id, opts)
> +		images
> +	end
> +	
> +	#
> +	# Retrieves realms
> +	#
> +	def realms(credentials, opts={})
> +		sbc_client = new_client(credentials)
> +		locations = []
> +		doc = sbc_client.list_locations
> +		doc.xpath('ns2:DescribeLocationsResponse/Location').each do |location|
> +			locations << convert_location(location)
> +		end
> +		locations
> +	end
> +	
> 
> There's no need for an explicit locations array; the function can be simplified to
> 
>                def realms(credentials, opts={})
>                        sbc_client = new_client(credentials)
>                        doc = sbc_client.list_locations
>                        doc.xpath('ns2:DescribeLocationsResponse/Location').map { |loc| convert_location(location) }
>                end
> 
> +	#
> +	# Retrieves instances
> +	#
> +	def instances(credentials, opts={})
> +		sbc_client = new_client(credentials)
> +		opts ||= {}
> 
> This should be unnecessary unless somebody passes in an explicit nil (in
> which case they deserve the ensuing exception)
> 	
> +		instances = []
> +		instances = sbc_client.list_instances(opts[:id]).map do |instance|
> +			convert_instance(instance)
> +		end
> +		instances
> 
> The variable 'instances' isn't needed.
> 	
> +	#
> +	# Creates an instance
> +	#
> +	def create_instance(credentials, image_id, opts={})
> +		sbc_client = new_client(credentials)
> +		
> +		# Copy opts to body; keywords are mapped later
> +		body = opts.clone
> 
> You mean opts.dup here (it's subtle, just believe me ;)
> 
> +		body.delete('image_id')
> +		body.delete('hwp_id')
> +		body.delete('realm_id')
> +		
> +		# Map DeltaCloud keywords to SBC
> +		body['imageID'] = opts[:image_id]
> +		body['location'] = opts[:realm_id] || @last_image['location']
> +		body['instanceType'] = opts[:hwp_id] || @last_image['supportedInstanceTypes'][0]['id']
> 
> This is very dangerous, since @last_image might be nil. There's two ways
> to get out of missing realm_id/hwp_id parameters: (1) Hardcode a default
> in the driver or (2) look up the image that gets passed in and use its
> location/instanceType.
> 
> +		# Submit instance
> +		instances = []
> +		instances = sbc_client.create_instance(body).map do |instance|
> +			convert_instance(instance)
> +		end
> +		instances
> 
> You can simply return "sbc_client.create_instance(body)" and omit any mention of instances.
> 
> +	end
> +	
> +	#
> +	# Reboots an instance
> +	#
> +	def reboot_instance(credentials, instance_id)
> +		sbc_client = new_client(credentials)
> +		sbc_client.reboot_instance(instance_id)
> +	end
> 
> The client simply returns the body of the response from reboot_instance;
> this method though needs to return an object of class Instance. The same
> is true for hte other instance actions stop and destroy.
> 
> David
> 
> 


Re: IBM SBC Driver Available

Posted by David Lutterkort <lu...@redhat.com>.
On Fri, 2011-01-28 at 17:51 -0500, Eric Woods wrote:
> Hi All,
> 
> As promised, I've submitted a patch for the IBM SBC driver to JIRA:
> https://issues.apache.org/jira/browse/DTACLOUD-15.

Excellent ! Thanks a ton for writing the driver.

> The patch is fully functional but has a few remaining (minor) todos.
> I'd like to reach out for help on two issues:
> 
> 1) sbc_client.rb:180:  For error handling, I raise
> Deltacloud:BackendErrors with error codes & messages that align with
> IBM's cloud.  These attributes are always ignored and the client sees
> a 500 Internal Server Error.  Is this the expected (generic) behavior,
> or should the client see error codes & messages specific to each cloud
> provider?  Perhaps this could be a standardization opportunity.

What we do in other places is return a 502 Bad Gateway to indicate a
generic backend error. Of course, if, for example, authentication fails,
you should raise a Deltacloud::AuthException.

There's a bit of machinery for that: you can implement
catched_excpetion_list, and enclose anything where you want to have
backend errors mapped in a safely do .. end. (see the end of
server/lib/deltacloud/base_driver/base_driver.rb for details)

Or, in this case, you can raise the proper Deltacloud exception directly
from the SBC client.

> 2) sbc_driver.rb:198: While converting each IBM instance to
> DeltaCloud's instances, I construct the an InstanceProfile by passing
> a HardwareProfile's name (the hardware profiles are defined at the
> bottom of the driver).  However, InstanceProfile does not to correctly
> lookup the corresponding HardwareProfile by name.  The
> InstanceProfile's id is set correctly, but cpu, storage, memory, and
> architecture are all null.  I can't find what I'm doing wrong or
> different from the other drivers.

That's the right behavior. cpu/storage/memory in InstanceProfile are
only set if the instance's sizing deviates from what's already in the
HWP. For SBC, that's not possible, but some other clouds let you choose,
for example, memory in some range.

> Help with these issues and technical feedback is very appreciated.  Thanks!

One general thing: can you talk to Michal and figure out a way to add
cucumber tests for the IBM SBC cloud ? We need to make sure we have some
minimal test coverage to spot bitrot early on.

Also, is there some sort of test account we could use to run the driver
against the actual cloud ?

Some more detailed comments on the driver:

First nit: please do not use tabs in the source; the rest of the code
uses 2 space indentation - please reformat accordingly.

Index: server/lib/drivers.rb
===================================================================
--- server/lib/drivers.rb	(revision 1064909)
+++ server/lib/drivers.rb	(working copy)
@@ -20,6 +20,7 @@
 module Deltacloud
   DRIVERS = {
     :ec2 => { :name => "EC2" },
+	:sbc => { :name => "SBC" },
     :rackspace => { :name => "Rackspace" },
     :gogrid => { :name => "Gogrid" },
     :rhevm => { :name => "RHEVM" },
Index: server/lib/deltacloud/drivers/sbc/sbc_client.rb
===================================================================
--- server/lib/deltacloud/drivers/sbc/sbc_client.rb	(revision 0)
+++ server/lib/deltacloud/drivers/sbc/sbc_client.rb	(revision 0)
@@ -0,0 +1,199 @@
+#
+# Copyright (C) 2009, 2010  Red Hat, Inc.

That should be either copyright you or IBM, depending what your IP
agreement is with your employer. And it should be 'Copyright (C) 2011'

+module Deltacloud
+  module Drivers
+    module SBC
+
+#
+# Client for the IBM Smart Business Cloud (SBC).
+#
+# Author: Eric Woods
+# Date: 18 January 2011
+#
+class SBCClient
+	
+	@@API_URL = URI.parse('https://www-147.ibm.com/computecloud/enterprise/api/rest/20100331')

This should be a constant ('API_URL = ' rather than '@@API_URL = ')

+	#
+	# Retrieve instances
+	#
+	def list_instances(instance_id=nil)
+		if instance_id.nil?
+			JSON.parse(get('/instances', default_headers))['instances']
+		else
+			instances = []
+			instances << JSON.parse(get('/instances/' + instance_id, default_headers))
+			instances

You can write the else branch more concisely as

                        else
                                [ JSON.parse(get('/instances/' + instance_id, default_headers)) ]
                        end

+	#
+	# Retrieve locations; returns an XML document.
+	#
+	def list_locations
+		puts 'retrieving locations...'

Leftover from debugging ?

+	
+	#
+	# Utility to URL encode a hash.
+	#
+	def urlencode(hash)
+		query = ''
+		hash.each do |name, value|
+			if !query.empty?
+				query += '&'
+			end
+			query += URI.encode(name.to_s) + '=' + URI.encode(value.to_s)
+		end
+		query
+	end

Here's the more Rubyish oneliner to do the same:

                def urlencode(hash)
                  hash.keys.map { |k| "#{k}=#{hash[k]}" }.join("&")
                end

Index: server/lib/deltacloud/drivers/sbc/sbc_driver.rb
===================================================================
--- server/lib/deltacloud/drivers/sbc/sbc_driver.rb	(revision 0)
+++ server/lib/deltacloud/drivers/sbc/sbc_driver.rb	(revision 0)
@@ -0,0 +1,300 @@
+#
+# Copyright (C) 2009, 2010  Red Hat, Inc.

Same comment about copyright notice.

+class SBCDriver < Deltacloud::BaseDriver
+	#
+	# Retrieves images
+	#
+	def images(credentials, opts={})
+		sbc_client = new_client(credentials)
+		opts ||= {}
+		images = []
+		images = sbc_client.list_images(opts[:id]).map do |image|
+			@last_image = image
+			convert_image(image)
+		end

Don't store @last_image ... code farther down will blow up if it is
called before images is ever called. The above should simply be

                        images = sbc_client.list_images(opts[:id]).map { |img| convert_image(img) }

+		images = filter_on(images, :architecture, opts)
+		images = filter_on(images, :owner_id, opts)
+		images
+	end
+	
+	#
+	# Retrieves realms
+	#
+	def realms(credentials, opts={})
+		sbc_client = new_client(credentials)
+		locations = []
+		doc = sbc_client.list_locations
+		doc.xpath('ns2:DescribeLocationsResponse/Location').each do |location|
+			locations << convert_location(location)
+		end
+		locations
+	end
+	

There's no need for an explicit locations array; the function can be simplified to

                def realms(credentials, opts={})
                        sbc_client = new_client(credentials)
                        doc = sbc_client.list_locations
                        doc.xpath('ns2:DescribeLocationsResponse/Location').map { |loc| convert_location(location) }
                end

+	#
+	# Retrieves instances
+	#
+	def instances(credentials, opts={})
+		sbc_client = new_client(credentials)
+		opts ||= {}

This should be unnecessary unless somebody passes in an explicit nil (in
which case they deserve the ensuing exception)
	
+		instances = []
+		instances = sbc_client.list_instances(opts[:id]).map do |instance|
+			convert_instance(instance)
+		end
+		instances

The variable 'instances' isn't needed.
	
+	#
+	# Creates an instance
+	#
+	def create_instance(credentials, image_id, opts={})
+		sbc_client = new_client(credentials)
+		
+		# Copy opts to body; keywords are mapped later
+		body = opts.clone

You mean opts.dup here (it's subtle, just believe me ;)

+		body.delete('image_id')
+		body.delete('hwp_id')
+		body.delete('realm_id')
+		
+		# Map DeltaCloud keywords to SBC
+		body['imageID'] = opts[:image_id]
+		body['location'] = opts[:realm_id] || @last_image['location']
+		body['instanceType'] = opts[:hwp_id] || @last_image['supportedInstanceTypes'][0]['id']

This is very dangerous, since @last_image might be nil. There's two ways
to get out of missing realm_id/hwp_id parameters: (1) Hardcode a default
in the driver or (2) look up the image that gets passed in and use its
location/instanceType.

+		# Submit instance
+		instances = []
+		instances = sbc_client.create_instance(body).map do |instance|
+			convert_instance(instance)
+		end
+		instances

You can simply return "sbc_client.create_instance(body)" and omit any mention of instances.

+	end
+	
+	#
+	# Reboots an instance
+	#
+	def reboot_instance(credentials, instance_id)
+		sbc_client = new_client(credentials)
+		sbc_client.reboot_instance(instance_id)
+	end

The client simply returns the body of the response from reboot_instance;
this method though needs to return an object of class Instance. The same
is true for hte other instance actions stop and destroy.

David