RimuHosting branch

[Ivan]

Hi,

I work for RimuHosting and have been working on getting a libcloud
driver implemented. My fork is at http://github.com/hadashi/libcloud.

Code committed into the fork:
* List nodes
* List sizes
* List images
* Create Node
* Reboot Node
* Destroy Node
* Tests for the above
* Patched test/__init__.py to substitute - for _ in url-method
lookups.
* Added RimuHosting to types.py and providers.py

Comments/suggestions are welcome. Please let me know if there is
anything else that needs to be done to have it merged into the main
tree.

Regards,
Ivan Meredith

[Alex Polvi]

On Mon, Oct 19, 2009 at 5:35 PM, Ivan <iv...@ivan.net.nz> wrote:
>
> Hi,
>
> I work for RimuHosting and have been working on getting a libcloud
> driver implemented. My fork is at http://github.com/hadashi/libcloud.

This is great! Thanks Ivan.

I've started reviewing the code, and noticed a few places where it
seems to be out of "spec". In particular, the return from reboot and
destroy node is supposed to be the node object that was passed to the
function. This made me want to write some tests so developers such as
yourself can write just run tests to make sure that their code is
taking the right args and giving the correct response.

This following is a commit which adds a "TestCaseMixin". The mixin is
set of basic tests that should work on all providers. If there is a
better way to do this, I am totally open, but this was just a stab at
writing some code. Particularly, this checks that the response from
each of calls is the correct type. I would love your feedback on it.

http://github.com/polvi/libcloud/commit/57c436e5f0b5dbf1cf5bf8a377cd7e9410932bc3

This is the patch I had to create to get RimuHosting working (again,
this might not be the correct solution, but it causes the test to
pass):

http://github.com/polvi/libcloud/commit/32adafb00209de9328551cf66c6c4ebafc0d1cb0

Note: all this stuff is on my personal branch, just experimental for now.

Outside of the reboot/destroy response issue (which seems to be in
most of the drivers...), it looks like you added args to the init of
the driver itself? It should use a standard __init__... but this is
hard to test as it is not part of the zope interface. Still looking
for better ways to test this too.

Also, it appears we need to pick a standard way of doing provider
exceptions. Probably should create a base ProviderException class and
let the specific providers inherit from that.

Any feedback appreciated!

-Alex
--
co-founder, cloudkick.com
twitter.com/cloudkick
541 231 0624

[Ivan]

> I've started reviewing the code, and noticed a few places where it
> seems to be out of "spec". In particular, the return from reboot and
> destroy node is supposed to be the node object that was passed to the

Ah! I was confused about that. I couldn't find any documentation on
it... and weakly typed langauges just let me do whatever.

> function. This made me want to write some tests so developers such as
> yourself can write just run tests to make sure that their code is
> taking the right args and giving the correct response.
>

That would be really good.

> This following is a commit which adds a "TestCaseMixin". The mixin is
> set of basic tests that should work on all providers. If there is a
> better way to do this, I am totally open, but this was just a stab at
> writing some code. Particularly, this checks that the response from
> each of calls is the correct type. I would love your feedback on it.
>

> http://github.com/polvi/libcloud/commit/57c436e5f0b5dbf1cf5bf8a377cd7...

>



> This is the patch I had to create to get RimuHosting working (again,
> this might not be the correct solution, but it causes the test to
> pass):
>

> http://github.com/polvi/libcloud/commit/32adafb00209de9328551cf66c6c4...

>
> Note: all this stuff is on my personal branch, just experimental for now.
>

It looks good, atlhough our api blocks until the vps is restarted.
Therefore it would always be RUNNING. We can check to see if a VPS is
actaully pingable, which is what happens on the reboot.

> Outside of the reboot/destroy response issue (which seems to be in
> most of the drivers...), it looks like you added args to the init of
> the driver itself? It should use a standard __init__... but this is
> hard to test as it is not part of the zope interface. Still looking
> for better ways to test this too.
>

We run local tomcat instances of the site on our development PC's, so
its quite nice from our point of view to be able to easily use the
driver without needing to modify the source file.

I tried to do it in such a way that it had 'defaults'. If anyone has
suggestions on a better way to do this I would be willing to to change
it. As far as I know Python doesn't support constructor overloading
which seems more what I want maybe.

> Also, it appears we need to pick a standard way of doing provider
> exceptions. Probably should create a base ProviderException class and
> let the specific providers inherit from that.
>

Maybe a standard way of "failing" should be to raise a
ProviderException. This would make a standard way for the user of the
api (with any of the drivers) to be alerted to the fact there has been
an error. I saw that some drivers returned False if a reboot failed.
In our case we raise an exception because we get an error back.

The main reason I like the exception approach, is that I can raise it
in the response checking code, others could raise it in the reboot_node
() method if the Node.state != REBOOTING.

[Ivan]

Alex (or anyone else) Is there anything else we need to do to get
merged?

Thanks, Ivan

[Alex Polvi]

On Thu, Oct 22, 2009 at 1:43 PM, Ivan <iv...@ivan.net.nz> wrote:
>
> Alex (or anyone else) Is there anything else we need to do to get
> merged?

I just pushed some test cases to help determine if your return codes
are correct. If possible, please rebase your branch off the latest
cloudkick/master and update your test case to use the new
TestCaseMixin. The mixin is a set of tests that validates the response
from each of the API calls. If you have any questions about doing
that, do not hesitate to ask.

test_linode.py has an example of how to use the mixin, and how to
override if needed:

http://github.com/cloudkick/libcloud/blob/master/test/test_linode.py#L20

I had to override test_create_node_response because of the password
requirement for that particular driver.

-Alex

>
> Thanks, Ivan

[Ivan]

I see you changed it from returning nodes to returning a boolean?
Either way it works now.

ivan@starlight:~/git/libcloud$ trial test/test_rimuhosting.py
test.test_rimuhosting
RimuHostingTest

test_create_node ...
[OK]

test_create_node_response ...
[OK]

test_destroy_node ...
[OK]

test_destroy_node_response ...
[OK]

test_list_images ...
[OK]

test_list_images_response ...
[OK]

test_list_nodes ...
[OK]

test_list_nodes_response ...
[OK]

test_list_sizes ...
[OK]

test_list_sizes_response ...
[OK]

test_reboot_node ...
[OK]

test_reboot_node_response ...
[OK]

-------------------------------------------------------------------------------
Ran 12 tests in 0.034s

It has been pushed to http://github.com/hadashi/libcloud master branch

-Ivan

On Oct 23, 2:59 pm, Alex Polvi <po...@cloudkick.com> wrote:

[Alex Polvi]

On Thu, Oct 22, 2009 at 7:44 PM, Ivan <iv...@ivan.net.nz> wrote:
>
> I see you changed it from returning nodes to returning a boolean?
> Either way it works now.

I should have mentioned that, my bad. I merged RimuHosting in to the
main branch, and added to the support table on libcloud.org. Thank you
for all the help.

While I merged everything, you are not done yet! :)

If possible, please try to do some validation on the response from
reboot and destroy:

http://github.com/cloudkick/libcloud/commit/8d3cc77925896c22284950218ac4f64c4e42c755

Thanks again for all your help on this. Looking forward to your energy
and support as this community continues to grow!

Best,

-Alex