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

provider/openstack: Add support provider network #10265

Merged
merged 7 commits into from
May 30, 2017
Merged

provider/openstack: Add support provider network #10265

merged 7 commits into from
May 30, 2017

Conversation

takaishi
Copy link
Contributor

@takaishi takaishi changed the title [WIP] provider/openstack: Add support provider network provider/openstack: Add support provider network Nov 21, 2016
@jtopjian
Copy link
Contributor

@takaishi Thanks for this! I will review this ASAP.

@jtopjian
Copy link
Contributor

jtopjian commented Dec 2, 2016

@takaishi I'm so sorry for the delay on this.

I noticed that you manually patched the vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/networks/requests.go file. This file is part of the https://github.com/gophercloud/gophercloud repository, so the addition of Segments will first need to be submitted as a PR to Gophercloud.

Once that patch has been merged, you can use govendor to pull in the upstream patch.

Unfortunately patching the vendor files like this isn't recommended because then we would diverge from the upstream Gophercloud, essentially creating our own version.

Again, I do apologize for not getting to this sooner. Please let me know if you have any questions.

@takaishi
Copy link
Contributor Author

takaishi commented Dec 3, 2016

@jtopjian
Thank you for your comemnt!

I noticed that you manually patched the vendor/github.com/gophercloud/gophercloud/openstack/networking/v2/networks/requests.go file. This file is part of the https://github.com/gophercloud/gophercloud repository, so the addition of Segments will first need to be submitted as a PR to Gophercloud.

OK, I will to submmit PR to gopercloud.

@stack72
Copy link
Contributor

stack72 commented Mar 18, 2017

ping on this PR :)

@takaishi
Copy link
Contributor Author

@stack72
I created new issue and PR on gophercloud.
So, when it merged I will fix this PR.

@jtopjian
Copy link
Contributor

@takaishi Is this ready for a review or are you still working on it?

@takaishi
Copy link
Contributor Author

@jtopjian Yes, ready for review.

@jtopjian
Copy link
Contributor

@takaishi Thanks for putting this together.

There were some areas that needed to be cleaned up. It was easier to do a code diff than try to explain them with inline comments. You can see it here. Let me know if you have any questions about any of the changes.

In addition, the commit history needs cleaned up. There shouldn't be revert and "merge" commits. Do you mind making a clean commit history and then doing a force push?

@takaishi
Copy link
Contributor Author

@jtopjian Thank you for review.
I'll clean up code and commit history.

@takaishi
Copy link
Contributor Author

 @jtopjian Thank you for review.
I'll clean up code and commit history.

I finished fix and rebase.

@jtopjian
Copy link
Contributor

@takaishi This looks good to me - thank you :)

==> Checking that code complies with gofmt requirements...
go generate $(go list ./... | grep -v /terraform/vendor/)
2017/05/29 21:23:59 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/openstack -v -run=TestAccNetworkingV2Network -timeout 120m
=== RUN   TestAccNetworkingV2Network_importBasic
--- PASS: TestAccNetworkingV2Network_importBasic (25.55s)
=== RUN   TestAccNetworkingV2Network_basic
--- PASS: TestAccNetworkingV2Network_basic (32.71s)
=== RUN   TestAccNetworkingV2Network_netstack
--- PASS: TestAccNetworkingV2Network_netstack (70.00s)
=== RUN   TestAccNetworkingV2Network_fullstack
--- PASS: TestAccNetworkingV2Network_fullstack (116.16s)
=== RUN   TestAccNetworkingV2Network_timeout
--- PASS: TestAccNetworkingV2Network_timeout (24.05s)
=== RUN   TestAccNetworkingV2Network_with_multiple_segment_mappings
--- PASS: TestAccNetworkingV2Network_with_multiple_segment_mappings (24.68s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/openstack      293.188s

@jtopjian jtopjian merged commit c501cb1 into hashicorp:master May 30, 2017
@takaishi takaishi deleted the support-provider-network branch May 30, 2017 03:42
ewilde pushed a commit to ewilde/terraform that referenced this pull request May 30, 2017
* provider:openstack Add support provider network

* revert vendor file changes

* vendor: Updating Gophercloud for OpenStack Provider

* create provider network if parameter has segments

* segments is not computed resource

* extract to generate []provider.Segment

* change segmentstion id type
@ghost
Copy link

ghost commented Apr 12, 2020

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.

@ghost ghost locked and limited conversation to collaborators Apr 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants