provider/openstack Add value_specs for routers

[Fodoj]

There is a "hidden" parameter - router type. Either exclusive or shared (so exclusive will get a separate VM for sure, while shared will be shared with other routers on same tenants). The only actual reference (except testing it on live system) I could find is this blog post: https://blogs.vmware.com/openstack/openstack-networking-with-vmware-nsx-part-2/ It could be that this feature is VIO only.

Depends on rackspace/gophercloud#526

[Fodoj]

@jtopjian ping. Added driver_opts also here.

[jtopjian]

Does this throw an error if driver_opts is empty? I'm just being overly cautious from the bug I caused last week by doing an unsafe type cast.

I wasn't able to trigger the bug until I actually compiled Terraform and ran it with a new configuration. Acceptance tests and existing configurations failed to trigger the bug.

[Fodoj]

It doesnt, verified.

[jtopjian]

Thank you for double-checking :)

[jtopjian]

Does this supersede #4878?

[Fodoj]

No. Distributed router is official open stack feature

On 9 Feb 2016 04:10 +0100, Joe Topjiannotifications@github.com, wrote:

Does this supersede#4878(#4878)?

—
Reply to this email directly orview it on GitHub(#4898 (comment)).

[jtopjian]

My mistake! Sorry, I got your two features tangled up.

[jtopjian]

@phinze Is the WIP label not appropriate here? Is there a better way to label something as "waiting on the underlying library to have support added first"?

Edit: Nevermind, I've seen the label has been removed in other PRs, so it looks like you're doing a cleanup 😄

[phinze]

@jtopjian yep these changes are to move us towards the new pull request lifecycle guidelines. We're removing the label in favor of a PR title prefix, as that is author-modifiable. Since we're asking authors to use [WIP] to indicate whether they believe the PR is complete or not, the label becomes redundant. 👍

[Fodoj]

I've updated PR quite a bit, after implementing proposals from this PR rackspace/gophercloud#526

[jtopjian]

@Fodoj Nice! A few thoughts:

1. I think CreateOpts and such should be renamed to either routerCreateOpts or RouterCreateOpts. I'm leaning on the former since I don't think anything outside of the Terraform OpenStack package would use it (unless there's another reason it needs to be RouterCreateOpts that I'm overlooking). Either way, making the name specific to "router" would ensure another OpenStack resource could do something similar in the future.
2. I'm getting more Google results for values_specs than value_specs. I wasn't able to find either string in the stock Neutron code... do you know where that name came from?
3. Is there a generic values_spec that can be used on the default Devstack Open vSwitch environment? That would allow an acceptance test to be created.

Also, you'll have to update the vendor deps before we can get this merged in. That should also make Travis happy.

[Fodoj]

1. Will rename
2. Name comes from Heat API. I will rename it to value_specs, you are right
3. I am not aware of one. Only about the VMware router type that I tested.

[Fodoj]

Re value specs: http://docs.openstack.org/developer/heat/template_guide/openstack.html#OS::Neutron::Firewall-prop-value_specs

[Fodoj]

@jtopjian I updated deps and added some fixes. Tests still fail on Travis despite the fact that dependency is there. Locally I can compile and run tests just fine. Any ideas what went wrong here?

[jtopjian]

It looks like a lot more dependencies got pulled in than just gophercloud -- or are they all dependent on the changes you made?

Can you try reverting all dependencies and just doing:

godep update github.com/rackspace/gophercloud/...

Then git status should only show gophercloud as being updated.

Let's start there and see what happens. If there are still errors triggered, I'll download this PR as-is and see if I can spot the error. 😄

[jtopjian]

BTW: everything else sounds good! Thank you for addressing it all.

[jtopjian]

@Fodoj I had a gophercloud PR merged today so I just did a dedicated PR to update the gophercloud repo: #6079. This includes your router changes 😄

[Fodoj]

@jtopjian thanks, I updated PR, specs are green now!

[jtopjian]

@Fodoj There's still a very high number of vendor files being changed. I would imagine now that the Gophercloud dep have been updated to the latest Gophercloud commit, then only builtin/providers/openstack/resource_openstack_networking_router_v2.go should be committed, right?

Maybe starting with a clean branch based off of master, applying the changes to builtin/providers/openstack/resource_openstack_networking_router_v2.go, and then forcing a commit to your add-router-type branch will clean things up? The result of that should be a single commit.

Sorry for the troubles with getting a clean commit... let me know if you need a hand.

[Fodoj]

@jtopjian ping :)

[jtopjian]

Nice! Looks like all acceptance tests are passing, so I'm going to go ahead and merge this. 🎉

[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.