Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LIBCLOUD-703] packet bare metal cloud provider integration #527

Closed

Conversation

@crunchywelch
Copy link
Contributor

@crunchywelch crunchywelch commented May 22, 2015

I humbly submit this PR which adds support for our bare metal cloud hosting platform, Packet. Let me know if this looks ok, or if it requires additional work. The JIRA issue is here:

https://issues.apache.org/jira/browse/LIBCLOUD-703

Thanks!

@crunchywelch crunchywelch force-pushed the packethost:LIBCLOUD-703_packet_driver branch 4 times, most recently from a4bb91a to d526287 May 22, 2015
@crunchywelch
Copy link
Contributor Author

@crunchywelch crunchywelch commented May 22, 2015

I have faxed in my ICLA today as well. Thanks!

@Kami
Copy link
Member

@Kami Kami commented May 22, 2015

Great, thanks.

I will try to have a look ASAP (hopefully over the weekend).

method='DELETE')
return res.status == httplib.OK

def list_ssh_keys(self):

This comment has been minimized.

@Kami

Kami May 23, 2015
Member

Looks like you should be able to implement ssh key management functionality using key pair management methods which are part of the libcloud standard API - https://github.com/apache/libcloud/blob/trunk/libcloud/compute/base.py#L1183

This also means getting rid of SSHKey class in favor of libcloud.compute.base.KeyPair.

In your case, you can use store id in the name attribute and description in the extra dict.

This comment has been minimized.

@crunchywelch

crunchywelch May 23, 2015
Author Contributor

cool, works for me

"""
headers['Content-Type'] = 'application/json'
headers['X-Auth-Token'] = self.user_id
headers['X-Consumer-Token'] = \

This comment has been minimized.

@Kami

Kami May 23, 2015
Member

I use this is a testing token and should be self.key? :)

This comment has been minimized.

@crunchywelch

crunchywelch May 23, 2015
Author Contributor

heh, fair enough, will fix

This comment has been minimized.

@Kami

Kami May 23, 2015
Member

I actually meant the X-Consumer-Token. Or is this some kind of static token which is shared among all users?

This comment has been minimized.

@crunchywelch

crunchywelch May 23, 2015
Author Contributor

Yeah, exactly, we issue a consumer token for any platform or library that is integrated with us so we can tell what application requests are originating from. If calls are coming in with only an auth key, they will work, but be rate limited. This token in particular is the one we issued for libcloud. It doesn't need to be secured, people could technically abuse it or remove it if they wanted to, but then we'd rely on the auth token to relate those requests to a particular account and deal with it at that level. We have one for our mobile apps, docker machine, rancher, devo.ps, etc...

Does that make sense? Do you see an issue with it?

This comment has been minimized.

@Kami

Kami May 24, 2015
Member

Ah, I see, that makes sense. Thanks for the clarification.

P.S. We also try to be a good citizen and indicate that the request is made by Libcloud by sending a right user-agent header (e.g. User-Agent: libcloud/0.17.1-dev (Rackspace Cloud (Next Gen)) :)).


public_ips = []
private_ips = []
if 'ip_addresses' in data and data['ip_addresses'] is not None:

This comment has been minimized.

@Kami

Kami May 23, 2015
Member

This could be made a bit more readable if you refactor it in a new method - e.g. _get_ips_for_node or similar.

This comment has been minimized.

@crunchywelch

crunchywelch May 23, 2015
Author Contributor

A+, will do!

@@ -55,6 +55,7 @@ Provider list volumes create volume destroy volume
`Opsource`_ no no no no no no no
`Outscale INC`_ yes yes yes yes yes yes yes
`Outscale SAS`_ yes yes yes yes yes yes yes
`Packet`_ no no no no no no no

This comment has been minimized.

@Kami

Kami May 23, 2015
Member

Just for a future reference - in case you updated those files manually, they are generated automatically using ./contrib/generate_provider_feature_matrix_table.py script.

This comment has been minimized.

@crunchywelch

crunchywelch May 23, 2015
Author Contributor

cool, gtk thanks

'failed': NodeState.ERROR,
'active': NodeState.RUNNING}

def list_nodes(self, location):

This comment has been minimized.

@Kami

Kami May 23, 2015
Member

For consistency with the base API, location argument should be an instance of NodeLocation and not a string.

This comment has been minimized.

@crunchywelch

crunchywelch May 23, 2015
Author Contributor

so, this is not really a location, it is a project id which is a little unique to how we structure things on the platform. I'll rename the string to not cause confusion here, I hope that works

@crunchywelch
Copy link
Contributor Author

@crunchywelch crunchywelch commented May 23, 2015

thanks @Kami, will make some adjustments!

@crunchywelch crunchywelch force-pushed the packethost:LIBCLOUD-703_packet_driver branch from d526287 to f3bd4a5 May 23, 2015
@crunchywelch crunchywelch force-pushed the packethost:LIBCLOUD-703_packet_driver branch from f3bd4a5 to 762d803 May 23, 2015
@crunchywelch
Copy link
Contributor Author

@crunchywelch crunchywelch commented May 23, 2015

Ok, I think I got all those notes, lmk if you see anything else I should adjust!

Cheers,

@Kami
Copy link
Member

@Kami Kami commented May 24, 2015

I tries to test the driver using a dummy token and looks like an API returns 404 on invalid token. Something like 401 would be better and then Libcloud would also correctly throw InvalidCreds exception.

Or maybe exception is related to the project not existing? In any case, 401 would be more appropriate (and hopefully token validation happens before project validation anyway) :)

# -------- begin 140127380731600 request ----------
curl -i -X GET -H 'Host: api.packet.net' -H 'X-LC-Request-ID: 140127380731600' -H 'Accept-Encoding: gzip,deflate' -H 'X-Consumer-Token: kcrhMn7hwG8Ceo2hAhGFa2qpxLBvVHxEjS9ue8iqmsNkeeB2iQgMq4dNc1893pYu' -H 'X-Auth-Token: accesskeyid' -H 'Content-Type: application/json' -H 'User-Agent: libcloud/0.17.1-dev (Packet) ' --compress 'https://api.packet.net:443/projects/aaaaa/devices?include=plan'
# -------- begin 140127380731600:140127380758680 response ----------
HTTP/1.1 404 Not Found
Status: 404 Not Found
X-Request-Id: ec216d58-04b5-432b-92ad-60efc377e9fd
X-Xss-Protection: 1; mode=block
X-Content-Type-Options: nosniff
Transfer-Encoding: chunked
Server: nginx/1.6.2
X-Runtime: 0.005357
Cache-Control: no-cache
Date: Sun, 24 May 2015 14:49:02 GMT
X-Frame-Options: SAMEORIGIN
Content-Type: application/json; charset=utf-8

15
{"error":"Not found"}
0

# -------- end 140127380731600:140127380758680 response ----------
@asfgit asfgit closed this in 4d51ede May 24, 2015
@Kami
Copy link
Member

@Kami Kami commented May 24, 2015

I made a minor change (projectid, project_id -> ex_project_id since project id argument is not part of the standard API) and merged driver into trunk.

Thanks.

@crunchywelch
Copy link
Contributor Author

@crunchywelch crunchywelch commented May 24, 2015

great, thanks ! Do I need to provide the patch on the jira issue next? Looking at step 11 here:

http://libcloud.readthedocs.org/en/latest/development.html

@Kami
Copy link
Member

@Kami Kami commented May 24, 2015

No, it's fine, I already merged it using a patch from Github :)

@crunchywelch
Copy link
Contributor Author

@crunchywelch crunchywelch commented May 26, 2015

Sweet, thanks man! And thank you for being a maintainer!

On Sun, May 24, 2015 at 5:26 PM, Tomaz Muraus notifications@github.com
wrote:

No, it's fine, I already merged it using a patch from Github :)


Reply to this email directly or view it on GitHub
#527 (comment).

Aaron Welch

SVP of Product

212.933.9785 x301
welch@packet.net

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.