Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(668)

Issue 6874049: Add more OpenStack provider implementation (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
9 years, 1 month ago by wallyworld
Modified:
9 years, 1 month ago
Reviewers:
mp+137471
Visibility:
Public.

Description

Add more OpenStack provider implementation This branch starts with the initial stub OpenStack provider and fleshes out more infrastructure and code. Initial impementations of Environ methods Instances() and AllInstances() are done, as well as a bunch more boilerplate to glue in the provider config etc. The relevant goose library APIs are used to access a live Canonistack instance. For the purpose of the live tests, test server instances are create and destroyed. At the moment, a knowm image is used to create the servers. There is at least one limitation in the underlying goose nova client which needs to be addressed - the ability to filter the results returned by ListServers(). This is required to exclude deleted and other irrelevant servers from AllInstances() but there is no support yet for this in goose. I've also restructured the initial tests to hook into the juju test infrastructure so that the standard juju tests are run with this OpenStack provider. Of course, they currently fail but will pass as soon as all of the provider methods are done. I've also included individual tests for each implemented API in the provider's live test module. These compliment the juju tests which in this case appear more integration rather than unit focussed. https://code.launchpad.net/~wallyworld/juju-core/more-openstatck-provider-work/+merge/137471 (do not edit description out of merge proposal)

Patch Set 1 #

Patch Set 2 : Add more OpenStack provider implementation #

Total comments: 19

Patch Set 3 : Add more OpenStack provider implementation #

Patch Set 4 : Add more OpenStack provider implementation #

Total comments: 5

Patch Set 5 : Add more OpenStack provider implementation #

Patch Set 6 : Add more OpenStack provider implementation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+306 lines, -59 lines) Patch
A [revision details] View 1 2 3 4 5 1 chunk +2 lines, -0 lines 0 comments Download
M environs/openstack/config.go View 3 chunks +8 lines, -27 lines 0 comments Download
M environs/openstack/config_test.go View 1 2 3 4 3 chunks +10 lines, -6 lines 0 comments Download
A environs/openstack/live_test.go View 1 2 3 4 5 1 chunk +153 lines, -0 lines 0 comments Download
M environs/openstack/local_test.go View 1 chunk +2 lines, -8 lines 0 comments Download
M environs/openstack/provider.go View 1 2 3 4 5 4 chunks +114 lines, -18 lines 0 comments Download
A environs/openstack/provider_test.go View 1 chunk +17 lines, -0 lines 0 comments Download

Messages

Total messages: 7
wallyworld
Please take a look.
9 years, 1 month ago (2012-12-03 05:33:09 UTC) #1
niemeyer
Thanks, that's pretty much ready to go. Just a couple of suggestions, plus one relevant ...
9 years, 1 month ago (2012-12-03 22:35:02 UTC) #2
wallyworld
Please take a look. https://codereview.appspot.com/6874049/diff/2001/environs/openstack/config.go File environs/openstack/config.go (right): https://codereview.appspot.com/6874049/diff/2001/environs/openstack/config.go#newcode82 environs/openstack/config.go:82: cred, err := identity.CompleteCredentialsFromEnv() On ...
9 years, 1 month ago (2012-12-04 01:12:07 UTC) #3
niemeyer
https://codereview.appspot.com/6874049/diff/2001/environs/openstack/config.go File environs/openstack/config.go (right): https://codereview.appspot.com/6874049/diff/2001/environs/openstack/config.go#newcode82 environs/openstack/config.go:82: cred, err := identity.CompleteCredentialsFromEnv() On 2012/12/04 01:12:07, wallyworld wrote: ...
9 years, 1 month ago (2012-12-04 01:33:54 UTC) #4
wallyworld
Please take a look. https://codereview.appspot.com/6874049/diff/2001/environs/openstack/live_test.go File environs/openstack/live_test.go (right): https://codereview.appspot.com/6874049/diff/2001/environs/openstack/live_test.go#newcode67 environs/openstack/live_test.go:67: t.testServers, err = t.createInstances(2) On ...
9 years, 1 month ago (2012-12-04 05:09:39 UTC) #5
niemeyer
LGTM, assuming the following changes are introduced as indicated: https://codereview.appspot.com/6874049/diff/10008/environs/openstack/config_test.go File environs/openstack/config_test.go (right): https://codereview.appspot.com/6874049/diff/10008/environs/openstack/config_test.go#newcode134 environs/openstack/config_test.go:134: ...
9 years, 1 month ago (2012-12-04 16:37:06 UTC) #6
wallyworld
9 years, 1 month ago (2012-12-05 02:01:31 UTC) #7
*** Submitted:

Add more OpenStack provider implementation

This branch starts with the initial stub OpenStack provider and fleshes out more
infrastructure and code. Initial impementations of Environ methods
Instances() and AllInstances() are done, as well as a bunch more boilerplate to
glue in the provider config etc.

The relevant goose library APIs are used to access a live Canonistack instance.
For the purpose of the live tests, test server instances are create and
destroyed. At the
moment, a knowm image is used to create the servers.

There is at least one limitation in the underlying goose nova client which needs
to be addressed - the ability to filter the results returned by ListServers().
This
is required to exclude deleted and other irrelevant servers from AllInstances()
but there is no support yet for this in goose.

I've also restructured the initial tests to hook into the juju test
infrastructure so that the standard juju tests are run with this OpenStack
provider. Of course,
they currently fail but will pass as soon as all of the provider methods are
done. I've also included individual tests for each implemented API in the
provider's
live test module. These compliment the juju tests which in this case appear more
integration rather than unit focussed.

R=niemeyer
CC=
https://codereview.appspot.com/6874049
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b