This provides deploy_node for basic zones. Advanced zones will require more work.
I also fixed some consistency issues in create_node and it looks a lot more like the openstack create_node.
Fixed tests and documentation

Thanks.

There are a couple of issues in this patch which need to be addressed before it can be merged:

1. Missing "base64" and "b" import
2. pep8 / style issues
3. [ip for ip in private_ips if is_private_subnet(ip) == False] - should use "if not is_private_subnet(..) here"
4. I assume "if keypair in vm.keys():" should be "if 'keypair' in vm.keys()", because keypair is initialize to None. Same goes for the line bellow.
5. Need to add a note to documentation example which says that this functionality is only available in trunk
6. Also need to add some test cases which exercises the new functionality and catch the previously mentioned missing import issue.

This is a squash commit that address 1-5

About 6, I am not sure what you mean. How can we test deploy_node ?
I made sure that the existing tests passed. I did look at the other drivers but did not see a test for the import...

Getting this to you before heading to bed

ok I think I found the test you are thinking about test_create_node_with_ex_keyname...etc
But I am not seeing any test or try/except on the import of base64 or b, I looked at openstack and ec2 driver.

Sorry about those though, it's true that I did not test the userdata stuff, hence the missing imports.

Commit 54b778f264a1f85bb9e82763e2fea4f3b961bfab in branch refs/heads/trunk from sebastien goasguen
[ https://git-wip-us.apache.org/repos/asf?p=libcloud.git;h=54b778f ]

LIBCLOUD-388: Added deploy_node for basic zone, improved consistency of params in create_node

Signed-off-by: Tomaz Muraus <tomaz@apache.org>

Just FYI, tests failed under 3.2 and 3.3.

======================================================================
ERROR: test_list_nodes_response (libcloud.test.compute.test_cloudstack.CloudStackNodeDriverTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/w/lc/libcloud/libcloud/test/compute/_init_.py", line 26, in test_list_nodes_response
nodes = self.driver.list_nodes()
File "/w/lc/libcloud/libcloud/compute/drivers/cloudstack.py", line 259, in list_nodes
public_ips.extend([ip for ip in private_ips
AttributeError: 'dict_keys' object has no attribute 'extend'

...

This time I've fixed it (https://git-wip-us.apache.org/repos/asf?p=libcloud.git;a=commitdiff;h=260e305d2f7516a65423f31b3795d777c3af87ba), but next time please run "tox" before submitting a final patch and make sure all the tests pass.

There were also a bunch of issues with the documentation which I have also fixed (https://git-wip-us.apache.org/repos/asf?p=libcloud.git;a=commitdiff;h=b6fa496d7010938519e6fdecdbbf60674e7d9321 ):

• some text was out of date
• Issues with example code (undefined IMAGE_ID, extra_args was used in a wrong way, unused TEMPLATE_ID, ..).

Hello,

Sorry for not commenting on this sooner but i was only receiving notifications about GitHub pull requests - that won't happen again!

The behaviour of create_node/deploy_node has changed in trunk and the "features = {}" line is now used to indicate one of three things:

• If it contains "password", it should take a NodeAuthPassword via the "auth" argument and pass that password to the Node that is created
• If it contains "ssh_key" it should take a NodeAuthSSHKey object via the "auth" argument. That will contain a public key that should be used in teh authorized_keys file in the created node
• If it contains "generates_password" then the VM creates a password and returns it via an extra parameter.

I noticed your patch adds the "ssh_key" feature to CloudStack, but it does not take a NodeAuthSSHKey parameter. That breaks the API and tools that rely on the metadata will wrongly try and pass a NodeAuthSSHKey to your driver. So I am going to revert that part of your patch.

deploy_node does not rely on the features metadata any more so this should not impact your deploy_node fixes, but it would be good if you could do a follow up pull request that properly supports NodeAuthSSHKey. It looks that CloudStack is similar to EC2 here in that there is a keyring service of some kind. If you look at the EC2 driver you will see it has a method called ex_find_or_import_keypair_by_key_material which imports a key into the keyring automatically if it doesnt exist already and gives you the key name.

Hum, this is strange, I did run the tests...
let me check

@tomaz:
Ok I did not have 3.2 and 3.3 installed, I did install them now and run tox and I see the errors.
I was only running the 2.7 tests manually.
I will make sure to run tox next time, sorry about that.
About the docs that's my bad, I wanted to fix the security group weird string passing and did not properly re-read everythign for compliance. I would have fixed it in a next commit.

@john:
thanks for the review. I will look more closely into it. This is merely a first step to get deploy_node to work. Right now It only works by passing a ssh_key parameter and expecting a ex_keyname in the kwargs of create_node. I did not implement tested the auth object. base.py is actually a bit confusing there because ssh_key in the features is used to say that you need a auth as NodeauthSSHKey, yet you can pass ssh_key and it will go forward with the deploy. If deploy only works with ssh_key can we set features to an empty dict ?

Indeed we also have ex_import_keypair calls in the driver, I will add the import_by_key_material, but I don't think it's very intuitive. A User that has already import or generated a keypair will know it by name and can specify by name instead of passing the actual pubkey, but that's just my opinion

Thanks guys

sebastien goasguen,

In trunk deploy_node is totally decoupled from self.features['create_node']. That metadata is only used right now to indicate what kinds of values you can pass to the auth argument. Note that this doesn't mean you have to pass them to the auth argument, auth can be None. So you are correct - it is possible to provide NO authentication related information to create_node, but deploy_node would still work. This is by design. Some services don't actually let you specify a password, ssh key or keyring name at all, and deploy_node will still work for them. So it is possible for deploy_node to work even if the features dict is entirely empty!

I do agree that for services that have a keyring and people know what their key is called already then it makes sense to continue to support the ex_keyname API. So definitely don't remove the ex_keyname API. But there are hosting services that take the pubkey and don't have a keyring. The find_or_import approach means we can have a single API that works for services with or without a keyring, as well as the ex_keyname API for the services with a keyring.

Ok I think I get it. So it looks like you removed the ssh_key metadata.
We are good then ?

I will work on adding the find_or_import and supporting the two 'auth' mechanisms.

thanks a lot for the review

I think so, thanks for working on this

Closing because this one has been merged.