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: Migrate to gophercloud/gophercloud #7926

Closed
jtopjian opened this issue Aug 3, 2016 · 20 comments · Fixed by #9407
Closed

provider/openstack: Migrate to gophercloud/gophercloud #7926

jtopjian opened this issue Aug 3, 2016 · 20 comments · Fixed by #9407

Comments

@jtopjian
Copy link
Contributor

jtopjian commented Aug 3, 2016

As a few users have pointed out, Gophercloud moved to its own organization and repo, gophercloud/gophercloud. It's in the best interest of the OpenStack provider to move to this repository -- and it will as soon as possible. This Issue will serve as confirmation that the move will happen, provide updates on a status, and answer any questions users might have.

First and foremost, it's unfortunately not possible to simply s/rackspace/gophercloud/g and call it a day. Gophercloud went through a large cleanup during the move and incompatibilities exist. I'm in the process of cleaning up the Gophercloud acceptance tests as a way to find any potential bugs. I'm adding these items to the MIGRATING doc, so keep an eye on it if you're interested.

@jtopjian
Copy link
Contributor Author

Quick update on this: I only have the Neutron/Networking component of Gophercloud left to review. Once that's done, it's on to Terraform. Rough steps will include:

  • Changing all packages to gophercloud/gophercloud.
  • Making changes were differences have been found via the MIGRATING doc.
  • Test, fix, repeat until all acceptance tests are working.
  • Note and fix any further Gophercloud changes along the way.

Conservatively, I'd estimate a month.

@fatmcgav
Copy link
Contributor

@jtopjian I'd be game for assisting with migration tasks if that would help...

Is it worth creating a long-lived branch somewhere to try out the changes?

@jtopjian
Copy link
Contributor Author

@fatmcgav Thanks for the offer. How about:

  1. I'll finish off the first sweep of Gophercloud. I should be able to have this done within a week.
  2. I'll clean up any outstanding or low-hanging Terraform/OpenStack pull requests.
  3. Consider Terraform/OpenStack to be in a soft feature freeze.
  4. Have a branch called something like gophercloud-migration at https://github.com/jtopjian/terraform be the central area to work on / merge changes.

The reason I'm hesitant to start, or recommend starting, migrating now is just to finish some housekeeping and getting ducks in a row. If anyone is really keen to start migrating right now, go for it, but there's probably going to be some rebasing required on any work done right now. :)

@fatmcgav
Copy link
Contributor

@jtopjian Sounds like a sensible plan to me... :)

@jtopjian
Copy link
Contributor Author

jtopjian commented Sep 4, 2016

It has begun!

https://github.com/jtopjian/terraform/tree/gophercloud-migration

Status: go test passes, but acceptance tests are failing.

@jtopjian
Copy link
Contributor Author

I wanted to provide a quick update:

Most acceptance tests are passing.

Unfortunately I've been tied up with some other work and haven't been able to spend a lot of time on this in the past few days. I hope to get back to work on this shortly.

@jtopjian
Copy link
Contributor Author

Quasi-weekly status update:

  • Issues with instance metadata have been resolved.
  • Currently working on "boot from volume" and some neutron subnet stuff.

The above work has involved submitting PRs to gophercloud, so all of my work has been over there lately.

Once the above is done, I need to look at error handling.

Unfortunately, I expect a few more weeks on this.

@fatmcgav
Copy link
Contributor

@jtopjian Is this at a point where I can hack away and try #7041?

As the lack of scoping is starting to cause us pain, and as we've already got to compile TF from source for an unrelated issue, no harm in adding in :)

@jtopjian
Copy link
Contributor Author

@fatmcgav Absolutely, go for it. :)

@fatmcgav
Copy link
Contributor

Cool, i'll report back any issues :)

@fatmcgav
Copy link
Contributor

@jtopjian Not sure if it's something I'm doing, but some of the unit tests fail on this branch :(

Making these tweaks seems to have enabled the tests to pass:

diff --git a/builtin/providers/openstack/resource_openstack_compute_instance_v2_test.go b/builtin/providers/openstack/resource_openstack_compute_instance_v2_test.go
index 42e6ec8..11185f1 100644
--- a/builtin/providers/openstack/resource_openstack_compute_instance_v2_test.go
+++ b/builtin/providers/openstack/resource_openstack_compute_instance_v2_test.go
@@ -1120,7 +1120,7 @@ func testAccCheckComputeV2InstanceMetadata(
                                continue
                        }

-                       if v == value.(string) {
+                       if v == value {
                                return nil
                        }

diff --git a/builtin/providers/openstack/resource_openstack_lb_vip_v1.go b/builtin/providers/openstack/resource_openstack_lb_vip_v1.go
index 9f64f75..3f7e3da 100644
--- a/builtin/providers/openstack/resource_openstack_lb_vip_v1.go
+++ b/builtin/providers/openstack/resource_openstack_lb_vip_v1.go
@@ -248,8 +248,9 @@ func resourceLBVipV1Update(d *schema.ResourceData, meta interface{}) error {

                // If a floating IP is found we unassign it
                if len(fips) == 1 {
+                       emptyPort := ""
                        updateOpts := floatingips.UpdateOpts{
-                               PortID: "",
+                               PortID: &emptyPort,
                        }
                        if err = floatingips.Update(networkingClient, fips[0].ID, updateOpts).Err; err != nil {
                                return err
@@ -339,7 +340,7 @@ func lbVipV1AssignFloatingIP(floatingIP, portID string, networkingClient *gopher
        }

        updateOpts := floatingips.UpdateOpts{
-               PortID: portID,
+               PortID: &portID,
        }
        if err = floatingips.Update(networkingClient, fips[0].ID, updateOpts).Err; err != nil {
                return err
diff --git a/builtin/providers/openstack/resource_openstack_networking_floatingip_v2.go b/builtin/providers/openstack/resource_openstack_networking_floatingip_v2.go
index 02c3f79..14db57e 100644
--- a/builtin/providers/openstack/resource_openstack_networking_floatingip_v2.go
+++ b/builtin/providers/openstack/resource_openstack_networking_floatingip_v2.go
@@ -139,7 +139,8 @@ func resourceNetworkFloatingIPV2Update(d *schema.ResourceData, meta interface{})
        var updateOpts floatingips.UpdateOpts

        if d.HasChange("port_id") {
-               updateOpts.PortID = d.Get("port_id").(string)
+               newPortId := d.Get("port_id").(string)
+               updateOpts.PortID = &newPortId
        }

        log.Printf("[DEBUG] Update Options: %#v", updateOpts)

To be expected?

@jtopjian
Copy link
Contributor Author

@fatmcgav Indeed expected - there are two things causing the issues you're seeing:

  1. Updates to Instance Metadata
  2. Updates to PortID

Both of which required changes to Gophercloud, but I haven't had a chance to modify Terraform appropriately.

@fatmcgav
Copy link
Contributor

Ah, fair enough :) Well the test's pass for the moment... ;)

@jtopjian
Copy link
Contributor Author

@fatmcgav I've pushed a new commit that takes care of the errors you're seeing.

@fatmcgav
Copy link
Contributor

@jtopjian Yeh, just spotted that... Cheers :)

@jtopjian
Copy link
Contributor Author

jtopjian commented Oct 16, 2016

I bring good news: All acceptance tests are passing!

A big Thank You to @fatmcgav for his help over the past few weeks! 😄

This brings a lot of confidence that the new Gophercloud repo is stable. There have been no changes to any resource attributes. If all goes well, this should be a drop-in replacement for the current OpenStack provider. However, there might still be some bugs hidden in cases that are not tested. I encourage anyone who has the time to test out the gophercloud-migration repo and report any issues.

Next steps: I just have to clean up the gophercloud-migration branch a bit and this should be good to go -- hopefully within the next week or two.

I apologize for the delay. There was more work than I anticipated, plus I had some scheduling issues.

@beddari
Copy link

beddari commented Oct 17, 2016

No need to apologize @jtopjian. You've done great babysitting this and related issues for a LONG time and also done most of the work. Thank you.

@jtopjian
Copy link
Contributor Author

#9407 is open. This can be considered an -rc1 release. Please feel free to test and report back any issues.

@jtopjian
Copy link
Contributor Author

This has been merged! Please let me know if you run into any issues.

@ghost
Copy link

ghost commented Apr 21, 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 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants