openstack backend #170
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,7 +23,8 @@ Spread | |
| [Fetching artifacts](#artifacts) | ||
| [LXD backend](#lxd) | ||
| [QEMU backend](#qemu) | ||
| [Google backend](#google) | ||
ZeyadYasser marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| [Openstack backend](#openstack) | ||
| [Linode backend](#linode) | ||
| [AdHoc backend](#adhoc) | ||
| [More on parallelism](#parallelism) | ||
|
|
@@ -899,6 +900,66 @@ and need to be removed by hand. | |
| For long term use, a dedicated project in the Google Cloud Platform is | ||
| recommended to prevent automated manipulation of important machines. | ||
|
|
||
| <a name="openstack"/> | ||
|
|
||
| ## Openstack backend | ||
|
|
||
| The Openstack backend is easy to setup and use, and allows distributing | ||
| your tasks to an openstack environment. | ||
|
|
||
| _$PROJECT/spread.yaml_ | ||
| ``` | ||
| (...) | ||
|
|
||
| backends: | ||
| openstack: | ||
| plan: cpu2-ram4-disk10 | ||
| halt-timeout: 2h | ||
| systems: | ||
| - ubuntu-20.04: | ||
mvo5 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| image: ubuntu-focal-daily-amd64 | ||
| workers: 5 | ||
|
|
||
| # Extended syntax: | ||
| - another-system: | ||
| image: some-other-image | ||
| networks: | ||
| - network_external | ||
| - network_pvn | ||
| groups: | ||
| - group_external | ||
|
|
||
| ``` | ||
|
|
||
| The Openstack backend gets all the information to authenticate from the | ||
| environment variables. The following variables have to be set: | ||
|
There was a problem hiding this comment. This does not look like a great practice, as it's sending real authentication data to every single test run. It also disagrees with what we do with the Google backend, and every other backend maybe? I'm almost certainly not the first one to point this out, so what is the actual alternative practice inside the OpenStack community? There was a problem hiding this comment. The most common mechanism to connect a client through the Openstack API is by sourcing a file with teh environment variables (like the one we get to use openstack client in canonistack). For example in PS5, in the environment I also see the openstack env vars in my environment (they are managed by vault tool), so I presume juju is using those to connect to openstack. I agree with you this is not a good practice because this means we need to have an env var with the user and password. I'll research which workaround we could use. There was a problem hiding this comment. I updated the documentation to explain better which env vars need to be defined. It is also supported to authenticate by using a key (similar to what we have in google). The key has to be stored in an env var to be loaded. There was a problem hiding this comment. An improvement could be to load the vars from a file (as we hav in google) instead of the env. @niemeyer What do you think? There was a problem hiding this comment. Finally I added a similar approach that the used in google. |
||
| ``` | ||
| OS_AUTH_URL | ||
| OS_PROJECT_ID | ||
| OS_PROJECT_NAME | ||
| OS_USER_DOMAIN_NAME | ||
| OS_PROJECT_DOMAIN_ID (optional) | ||
| OS_TENANT_ID | ||
| OS_TENANT_NAME | ||
| OS_USERNAME | ||
| OS_PASSWORD | ||
| OS_REGION_NAME | ||
| OS_INTERFACE | ||
| OS_IDENTITY_API_VERSION | ||
|
There was a problem hiding this comment. In addition to the above, which of those variables have a direct equivalent in the Google backend setup? There was a problem hiding this comment. In google we use the env var SPREAD_GOOGLE_KEY which has a link to the file with the following data: I updated the list of environment variables in the docs and I understand that the equivalent are: |
||
| ``` | ||
|
|
||
| You can set up those variables by sourcing the Openstack RC file. | ||
| For more information about the environment setup please read | ||
| https://docs.openstack.org/ocata/user-guide/common/cli-set-environment-variables-using-openstack-rc.html | ||
|
|
||
| Images are located by first attempting to match the provided value exactly | ||
| against the image name, if there is not exact match the most recent image | ||
mvo5 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| with partial match will be selected. | ||
|
|
||
| When these machines terminate running, they will be removed. If anything | ||
| happens that prevents the immediate removal, they will remain in the account | ||
| and need to be removed by hand. | ||
|
|
||
|
|
||
| <a name="linode"/> | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| package spread | ||
|
|
||
| import ( | ||
| "context" | ||
| "time" | ||
|
|
||
| gooseClient "github.com/go-goose/goose/v5/client" | ||
|
There was a problem hiding this comment. s/gooseClient/gooseclient/ Package names in Go are not typically cammel-cased. |
||
| "github.com/go-goose/goose/v5/glance" | ||
| ) | ||
|
|
||
| var ( | ||
| OpenstackName = openstackName | ||
| ) | ||
|
|
||
| type ( | ||
| OpenstackProvider = openstackProvider | ||
|
There was a problem hiding this comment. Do we need the types themselves being exposed? There was a problem hiding this comment. Good point, Thank you! I refactored the tests to avoid the need to export internal types. |
||
| GlanceImageClient = glanceImageClient | ||
| OpenstackServerData = openstackServerData | ||
| ) | ||
|
|
||
| func MockOpenstackImageClient(opst *OpenstackProvider, newIC glanceImageClient) (restore func()) { | ||
|
There was a problem hiding this comment. newIC, newIC, but then "timeout" and "retry" shortly below. Let's stick to the latter convention please and have "imageClient" and "computeClient" instead as these are more readable. |
||
| oldGlanceImageClient := opst.imageClient | ||
| opst.imageClient = newIC | ||
| return func() { | ||
| opst.imageClient = oldGlanceImageClient | ||
| } | ||
| } | ||
|
|
||
| func MockOpenstackComputeClient(opst *OpenstackProvider, newIC novaComputeClient) (restore func()) { | ||
| oldNovaImageClient := opst.computeClient | ||
| opst.computeClient = newIC | ||
| return func() { | ||
| opst.computeClient = oldNovaImageClient | ||
| } | ||
| } | ||
|
|
||
| func MockOpenstackGooseClient(opst *OpenstackProvider, newC gooseClient.Client) (restore func()) { | ||
| oldOsClient := opst.osClient | ||
| opst.osClient = newC | ||
| return func() { | ||
| opst.osClient = oldOsClient | ||
| } | ||
| } | ||
|
|
||
| func MockOpenstackProvisionTimeout(timeout, retry time.Duration) (restore func()) { | ||
| oldTimeout := openstackProvisionTimeout | ||
| oldRetry := openstackProvisionRetry | ||
| openstackProvisionTimeout = timeout | ||
| openstackProvisionRetry = retry | ||
| return func() { | ||
| openstackProvisionTimeout = oldTimeout | ||
| openstackProvisionRetry = oldRetry | ||
| } | ||
| } | ||
|
|
||
| func MockOpenstackServerBootTimeout(timeout, retry time.Duration) (restore func()) { | ||
| oldTimeout := openstackServerBootTimeout | ||
| oldRetry := openstackServerBootRetry | ||
| openstackServerBootTimeout = timeout | ||
| openstackServerBootRetry = retry | ||
| return func() { | ||
| openstackServerBootTimeout = oldTimeout | ||
| openstackServerBootRetry = oldRetry | ||
| } | ||
| } | ||
|
|
||
| func (opst *OpenstackProvider) FindImage(name string) (*glance.ImageDetail, error) { | ||
| return opst.findImage(name) | ||
| } | ||
|
|
||
| func (opst *openstackProvider) WaitProvision(ctx context.Context, serverData OpenstackServerData) error { | ||
| server := &openstackServer{ | ||
| p: opst, | ||
| d: serverData, | ||
| } | ||
| return opst.waitProvision(ctx, server) | ||
|
There was a problem hiding this comment. This is adding a fake method, with a real internal type that has been renamed to be public, and then before calling the internal type it actually changes the call signature by cooking another internal type over with a fake object as well. To close it all, some methods are defined in the type that has been renamed, and some methods are defined in the original internal type name. It would be nice to have a cleaner API here. There was a problem hiding this comment. Good point, Thank you! I updated to use a simpler signature only exposing params relevant to the tests:
|
||
| } | ||
|
|
||
| func (opst *openstackProvider) WaitServerBoot(ctx context.Context, serverData OpenstackServerData) error { | ||
| server := &openstackServer{ | ||
| p: opst, | ||
| d: serverData, | ||
| } | ||
| return opst.waitServerBoot(ctx, server) | ||
| } | ||
|
|
||
| func NewOpenstackError(gooseError error) error { | ||
| return &openstackError{gooseError} | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What happened here? If this makes sense (unclear for now), it should be documented so it's more obvious what was disabled and what was enabled here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, thanks for the review,
In order to test openstack backend against a real openstack interface, we pre-configured a new image with devstack already installed. The main raeson was to speed up and simplify the test (so that image is only used to test openstack). Should I add this explanation in the workflow?