OpenStack Provider

[jrperritt]

UPDATE: 2/11/2015

To Do:

• FWaaS
• Security Groups Update Issue
• Volume detachment from volume resource
• os-floating-ip/ neutron floating IP issue
• Refactor Security Group Rules and LB Members to their own files

This PR is to create an OpenStack Provider. It uses the Gophercloud v1.0 library and currently supports the following resources:

Compute v2

• Server
• Key Pair
• Security Group
• Boot From Volume
• Metadata
• Resizing (on flavor_id change)

Networking v2

• Network
• Subnet

Load Balancer v1

• Pool (with members)
• Virtual IP
• Monitor

Block Storage v1

• Volume

Object Storage v1

• Container

The PR includes acceptance tests for all the above resources (tested against DevStack), as well as documentation. In addition, the resources are versioned and region-based. Hopefully, this PR includes enough resources to close #51

[julienvey]

Hi @jrperritt

Really nice on work on this provider 👍

@ggiamarchi @haklop and I where also working on the same stuff this past two weeks :-( https://github.com/haklop/terraform/tree/openstack-provider-rebased

Since you're the first to submit the PR, we'd like to contribute some additional resources we implemented on our side by doing some sub pull requests:

• Routers
• Floating IPs
• Firewalls (depends on OpenStack Firewall as a Service rackspace/gophercloud#342)

We will also review and test your code on the OpenStack clouds we were testing our own code.

Expect some pull requests in the next few days :)
And again, great work!

[jrperritt]

@julienvey Oh, I'm sorry :( I was just working on it intermittently in my spare time until recently and didn't know anyone else was pursuing it heavily. After looking at y'alls branch, our codes look very similar, which I think is good :) The additional resources would be great (and, to be sure, any PRs are welcome), and I'd love to here if y'all have different ideas about the implementation.

[devcamcar]

I also just put up a pull request for adding support for using Swift as a Terraform remote: #942

[devcamcar]

Great job! I have been testing this a bit today and one thing I noticed is that currently 404s are not handled well. For instance, I deployed a block volume with your OpenStack provider and then deleted it manually. Subsequent calls to terraform apply result in:

openstack_compute_secgroup_v2.test: Refreshing state... (ID: 21405)
Error refreshing state: Error retrieving OpenStack volume: Expected HTTP response code [200] when accessing URL(https://proxy.env10.nebula.com:8776/v1/d168e07f763e45989a62d460d76a2ae2/volumes/f6087828-1917-4fa6-b38e-9287ad8f403f); got 404 instead with the following body:
{"itemNotFound": {"message": "The resource could not be found.", "code": 404}}

[devcamcar]

Similarly, if a security group exists prior to defining it in Terraform, a 400 bad request is generated:

openstack_compute_secgroup_v2.test: Error: Error creating OpenStack security group: Expected HTTP response code [200] when accessing URL(https://proxy.env10.nebula.com:8774/v2/d168e07f763e45989a62d460d76a2ae2/os-security-groups); got 400 instead with the following body:
{"badRequest": {"message": "Security group terraform-test already exists", "code": 400}}

[devcamcar]

And I noticed that access_ip_v4 is not showing up when I try to use it as an output:

output "address" {
  value = "${openstack_compute_instance_v2.terraform-test.access_ip_v4}"
}

Results in:

Outputs:

  address =

[jtopjian]

Hello,

Similarly, I've been working on a standalone plugin for an OpenStack provider rather than trying to get it merged into the main tree:

https://github.com/jtopjian/terraform-provider-openstack

It's currently working with nova-network, and Neutron was next on my list to get working, but now perhaps we could just merge efforts?

I'm really interested in the tests you created as I've been putting that off due to not entirely understanding how it works in Terraform (I was playing around with it, but could not get tests to fail when they should be failing...).

Thanks,
Joe

[jtopjian]

@jrperritt I'm browsing through this now and see a lot of stuff I can learn from (both the provider and Go in general :).

One question I have is more of a design opinion: I notice that the providers are hard-named based on the API version that creates them. Did you go this route to make the providers more parallel with gophercloud? Are there other reasons? I'm genuinely curious from a code design perspective.

As an end-user, I personally wouldn't want to have to distinguish a resource based on the API. Intuitively, if I want a "volume", then I'll choose a "volume". As a user, I'd hope that was taken care of internally (detect which version is available?) and perhaps allow me to choose the API version as a parameter.

This is the route I'm trying to pursue with the floating IP resource I made. Of course, I'm totally aware that I might hit a wall and this might not be possible to do. :)

[jrperritt]

@devcamcar Ah, I see what you mean about the 404s. I'll add some logic to handle that. As for the access_ipv4 variable, are you setting it in your config? Your OpenStack implementation may not automatically assign it when an instance is created.

[jrperritt]

@jtopjian The API-version-in-the-resource-name decision was driven by the OpenStack API and code organization. If you don't have a separate file per resource-version, then you have to delegate all the version logic to a single file, which could become unruly. I actually like forcing users to explicitly choose which resource-version combination they want, so that (hopefully) they don't end up surprised with what they get.

[jrperritt]

@jtopjian You're certainly welcome to make PRs against this branch :) One thing I noticed while looking at your terraform branch was that you aren't letting Gophercloud do the lifting for you. Any particular reason for that? For example, for HTTP requests you're using the perigee library, but that's what Gophercloud (currently) uses internally.

[jtopjian]

@jrperritt no reason besides I'm learning as I go. A lot of what you see, especially the perigee stuff, is just from looking at other examples. I'm starting to see how i can do things better and I'm quite excited to go through your code to learn more.

Hm, I think I disagree about the api thing. I get what you're saying and see the benefit, but I feel the user shouldn't have to know details like that to work with OpenStack. I'll definitely see what I can do to contribute to your branch, though.

[jrperritt]

@jtopjian Great :) I see you've added the 'Attach Volume' operation to your terraform branch. I think that's an OpenStack extension, and one we don't yet have in Gophercloud, so, if you like, you can make a PR to Gophercloud for that operation.

As for the API version issue, preference is probably user-dependent. You'll certainly need a version at some point in order to get the correct URL endpoint (or ServiceClient if you're using Gophercloud). If a user doesn't care which version of a resource they end up with, great; they can pick one at random. But for users who do care, I think explicit versioning is clearer, and the resulting file and code layout is easier to read. Having that been said, it's by no means set in stone. I like to hear others opinions about it, and hopefully others will chime in with their opinions as well.

[jtopjian]

@jrperritt All sounds good. I will definitely look into moving the volume attach stuff into gophercloud.

Thank you for your input and view as well - I hope I didn't offend you by disagreeing!

[jrperritt]

@jtopjian Not at all :) For me, different points of view (with regard to software design) lead to progress. I certainly don't have all the right answers, so I like discussing things like this to get other's perspectives. Please continue to question and/or challenge design decisions (mine and others). In my experience, hearing several different options can only improve the code.

[devcamcar]

@jrperritt The IP is allocated automatically at runtime via DHCP so I was expecting the value to end of in access_ip_v4. What network configuration are you testing against with your OpenStack install?

[jrperritt]

@devcamcar ah, yes. That will be fixed with this PR. If not directly set in the accessIPv4 variable, then we'll have to dig the IP addresses out of the server.Addresses variable.

[jtopjian]

@jrperritt @devcamcar That looks to be Neutron-based, right? I believe @devcamcar is running a nova-network based environment (from what I've pulled from other comments).

I have working nova-network floating IP support which I can clean up and send to gophercloud:

https://github.com/jtopjian/terraform-provider-openstack/blob/master/openstack/nova_network.go

(which would be really ironic since that's where I originally discovered the code :)

As well, here's my attempt at trudging around server.Addresses:

https://github.com/jtopjian/terraform-provider-openstack/blob/c1e72d61144acf4f04d2c3ec34dfc63c843b2ed2/openstack/resource_openstack_instance.go#L476-L491

Looping back around to the AccessIPv4 and AccessIPv6 variables, I've been wondering what the best way to determine these is for situations with multiple networks? Is there something in the API that says "use this network" or is it automatically chosen from the first network?

If the latter, I have some Neutron setups where the accessible IP is actually the last network in an instance's list. In that case, I'm considering not relying on AccessIPv* and instead push the responsibility to the end-user like this:

https://github.com/jtopjian/terraform-provider-openstack/blob/master/examples/basic/main.tf#L38

[devcamcar]

@jrperritt @jtopjian Here is how we solved it when adding OpenStack support to Packer:

https://github.com/mitchellh/packer/blob/master/builder/openstack/ssh.go#L15

Basically, multiple networks are supported in both Neutron and Nova-Networks. This approach just finds the first V4 address it can and uses it. This is a bit brittle for things attached to multiple networks (not really an issue when using Packer) since an instance could have multiple V4 IPs.

The most reliably implementation would be to do the following:

1. For the Terraform output, somehow allow specifying the output v4 address by name,

output "address" {
  value = "${openstack_compute_instance_v2.myserver.access_ip_v4.mynetwork}"
}

2. If no name specified, use first v4 address found (the standard example):

output "address" {
  value = "${openstack_compute_instance_v2.myserver.access_ip_v4}"
}

Does that make sense?

I haven't built Terraform providers yet so I'm not sure if this is the right approach, but conceptually it seems right.

[jtopjian]

@devcamcar @jrperritt It does, yes. :)

And in addition, I'm quite sure this can all be done via the nova api regardless of Neutron or nova-network. Everything can/should be accessible via server.Addresses.

[julienvey]

About the "_v2" at the end of the resource names, I agree with @jtopjian that it's not needed and shouldn't be visible from the user.

We could only rename the resource name and keep the .go files the same

ResourcesMap: map[string]*schema.Resource{
  "openstack_blockstorage_volume": resourceBlockStorageVolumeV1(),
  "openstack_compute_instance": resourceComputeInstanceV2(),
  "openstack_compute_keypair": resourceComputeKeypairV2(),
  ...
},

When we have multiple resources with different versions, then the resource without a version number should point to the latest stable version.

[ggiamarchi]

We should maybe always keep both. The name without the version would point to the "mainstream" version

compute_resource_v2 := resourceComputeInstanceV2()

ResourcesMap: map[string]*schema.Resource{
  "openstack_compute_instance_v2": compute_resource_v2,
  "openstack_compute_instance": compute_resource_v2,
  ...
},

[julienvey]

For this case, I'm wondering if terraform would consider the resources are the same.

For instance, if I start with a terraform file like this one

"openstack_compute_instance_v2" "test" {
   ...
}

and then change it to

"openstack_compute_instance" "test" {
   ...
}

Would terraform be able to understand the two resources are the same ?

[jtopjian]

@julienvey I think it might be able to if the new resource was able to detect an existing resource. This is something I'm going to work on this week and semi-related to what @devcamcar mentioned about errors being returned on existing security groups.

But ultimately I think Terraform is going to see them as two distinct resources, so it's probably going to mess up the state. If detection of existing resources works soundly, then one should be able to delete the local state and re-run an apply, I would think.

Another note about the versioned resources: The OpenStack command-line tools have always supported an environment variable or flag to specify what api version you want to use (such as OS_COMPUTE_API_VERSION. Everything else is abstracted from the user (they don't have to run nova_v1 or nova_v2 and there isn't a nova symlink that points to nova_v2). I think the provider should act similarly.

[devcamcar]

@julienvey I think the case of creating a template and then removing or adding "_v2" is a fairly insigificant corner case. I think people that would build the templates knowing they needed a specific API version would make use of that, and then the other 99% of folks would not need to bother.

[jtopjian]

@jrperritt I'm running into an issue where changes to the instance are removing the security groups.

How to reproduce:

1. Launch an instance
2. Add / Remove / Change metadata
3. Run terraform apply

Before:

$ nova show tf-test | grep default
| security_groups                      | [{u'name': u'default'}]                                  |

During:

2015/02/10 14:59:25 terraform-provider-openstack: 2015/02/10 14:59:25 [DEBUG] Security groups to add: &{0x472ff0 map[] {{0 0} 1}}
2015/02/10 14:59:25 terraform-provider-openstack: 2015/02/10 14:59:25 [DEBUG] Security groups to remove: &{0x472ff0 map[3814588639:default] {{0 0} 1}}
2015/02/10 14:59:26 terraform-provider-openstack: 2015/02/10 14:59:26 [DEBUG] Removed security group (default) from instance (4cc70d52-b941-4573-b866-d678b418bfdb)

After:

$ nova show tf-test | grep default

[jrperritt]

@jtopjian Ah, quite right. It took me a while to figure it out, but I think it's a bug in the Terraform schema.Set logic (possibly related to #873). It wasn't always broken, so it must've been a fairly recent (within the last month) change to the master branch. After changing it to a TypeList, it is working again (for me). I'll push the change to this branch after I fix merge conflicts from other PRs. It should be done some time tomorrow.

[jtopjian]

@jrperritt Do you have any items in mind that you need assistance with? Once the nova-network floating ip and volume attach/detach support is taken care of, this provider will be at parity with the work I was doing.

Also, does anyone know the status of this being merged into terraform proper? Would it make sense to enable "issues" on the jrperritt/terraform repo for further discussions? Or move this to a standalone plugin?

[jrperritt]

Not really :) I think we've exhausted the resources that are implemented in Gophercloud. As more get added to Gophercloud, we can continue to add the on here. I think the Terraform team had expressed interest in having this merged into Terraform proper. Is that still correct, @mitchellh ?

[mitchellh]

We'd like for OpenStack to be in Terraform core ASAP. We'll do a proper review of this when it is ready, i was just waiting for this issue to stabilize a bit. What would you all consider the status now of this PR?

[jtopjian]

@allomov @julienvey alternatively, I'm using the latest pre-compiled version of Terraform and just compiling the openstack provider. See my comments here.

[allomov]

@julienvey I appreciate it, it will be useful.

@jtopjian great tip, thank you.

[jrperritt]

I just rebased on upstream/master.

[phinze]

Alright I think we're ready to land this! Feel free to speak up in the next 20m if you disagree. :)

[jtopjian]

Per my previous comments, I disagree.

[metahertz]

Land it!

Sent from a mobile device.

On 31 Mar 2015, at 22:40, "Paul Hinze" <notifications@github.commailto:notifications@github.com> wrote:

Alright I think we're ready to land this! Feel free to speak up in the next 20m if you disagree. :)

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/924#issuecomment-88258781.

[ggiamarchi]

@jtopjian I don't understand why you still want to block the merge or this PR. Even if there are some bugs to fix or enhancements to do, most of the features are working well. Once it will be merged it will be easier for everybody to work on it.

[phinze]

@jtopjian you're referring to the floating ip comment you made here? #924 (comment)

Since this is initial implementation, the merge here would not prevent backwards incompatible changes from being landed afterwards.

I think it'd be valuable to get this change merged so we can split up the remaining work into smaller chunks.

We're also shipping 0.4 this week, so I'd really like to get OpenStack support in there, even just as an "initial implementation" or "beta" state.

[jtopjian]

@phinze Fair enough.

[phinze]

Alright here we go!

[rgbkrk]

Awesome!

[metahertz]

BOOM!!

Sent from a mobile device.

On 31 Mar 2015, at 22:59, "Paul Hinze" <notifications@github.commailto:notifications@github.com> wrote:

Merged #924#924.

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/924#event-270061994.

[nibalizer]

\o/

[jrperritt]

@allomov I don't plan on adding anything else to the OpenStack provider. However, since this PR has been merged into the master branch, you (or anyone else) can just clone the main Terraform repo and start making PRs to the OpenStack provider.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.