My replies are in-line.
On Thu, Dec 5, 2013 at 9:18 PM, John Carr <[email protected]> wrote:
> +1 to the general idea.
>
> Also in scope i think is the ex_keyname argument that is passed to
> create_node.
>
Yep, in this case we two options:
1. Take just a key pair name - e.g. keypair_name argument
2. Take a KeyPair instance
>
> I had been considering adding another NodeAuth type object for these
> public keys, so you could pass a NodeAuthKeyName to the auth kwarg. EC2 et
> al would then:
>
> class NodeDriver:
>
> features = {'create_node': ['ssh_key_name']}
>
This seems reasonable to me.
If user specifies NodeAuthKeyName for auth argument, you would then pass
"keypair_name" (or whatever we end up calling this argument) to the
create_node method, right?
Also, if we promote the key pair API we could make this method a mixin:
>
>
> https://github.com/apache/libcloud/blob/trunk/libcloud/compute/drivers/ec2.py#L1020
>
> It would have a better name though. Perhaps just get_or_import_keypair.
> The hashing part would be pluggable, as EC2 uses a weird hash for SSH
> public keys. With that in place, they would have the ssh_key feature AND
> the ssh_key_name feature.
>
> RE: your open questions:
>
> 1. I think keypair is a good name, without an ssh prefix. It’s the same
> terminology that EC2, OpenStack and CloudStack use.
>
After giving it more thought I also thinking that ssh prefix is not needed
and both you and Sebastien seem to agree with this :)
> 2. I like the look of list_keypairs (vs list_key_pairs). Keypair is
> obviously made up of 2 words, but its use in the wild (at least for
> openstack and cloudstack) seems to be as though it was a single word.
>
Yeah, most of the documentation and other materials refers to it using a
single word so maybe we should do the same.
> 3. I don’t like this - all the method names have key pair in them so i
> know what I’m dealing with. I sort of agree with you though. Maybe
> something like create_keypair_from_public_key?
>
Yeah, both of you have a good point here and I agree that all the methods
having a same common prefix makes it more obvious that they all belong in
the same group of functionality (key pair management in this case).
Cheers
> John
>
> On 5 Dec 2013, at 18:50, Tomaz Muraus <[email protected]> wrote:
>
> > This is something we have already talked about in the past and I believe
> > that now (pre 1.0 release) is the right time to do it.
> >
> > *Proposal*
> >
> > I propose the following methods for the key pair management to be
> promoted
> > and be a part of the standard / base compute API:
> >
> > * ex_list_keypairs -> list_keypairs()
> > * ex_create_keypair -> create_key_pair(name)
> > * ex_import_keypair -> import_key_pair_from_file(name, public_key_file)
> > * ex_import_keypair_fom_string -> import_key_pair_fom_string(name,
> > public_key_material)
> > * ex_delete_keypair -> delete_keypair(keypair)
> >
> > This also means adding a new "KeyPair" / "SSHKeyPair" class which
> > represents a key pair object.
> >
> > Whole proposal for those methods and a new class is also available in a
> > more readable format as a gist - https://gist.github.com/Kami/7810989
> >
> > Currently, this functionality is already implemented as the
> aforementioned
> > extension methods in the following drivers:
> >
> > * CloudStack
> > * OpenStack
> > * Rackspace
> > * EC2
> >
> > To preserve backward compatibility, we should also modify existing
> > extension methods to call new methods and emit a deprecation warning.
> >
> > *Open questions*
> >
> > Here are a couple of things I'm still not too sure about so a feedback
> > would be greatly appreciated.
> >
> > 1. Should all the methods and a key pair class be prefixed with "ssh" so
> > it's clear that it references a SSH key pair or just calling it key pair
> is
> > clear enough?
> >
> > 2. "keypair "vs "key_pair".
> >
> > Existing extension methods use #1, but to be consistent with other
> methods,
> > we should use #2.
> >
> > 3. "import_key_pair" vs "import_public_key".
> >
> > Existing extension methods use #1, but imo it's confusing and not obvious
> > since you are importing just a public key, so a better name would be
> > "import_public_key".
> >
> > Feedback is welcome.
> >
> > Thanks.
>
>
