From a90b9db3971a3b27e05bbc2fcf9180bc750c641d Mon Sep 17 00:00:00 2001 From: Guillaume Giamarchi Date: Fri, 27 Mar 2015 15:47:28 +0100 Subject: [PATCH 1/3] Bugfix on floating IP assignment The `getFirstNetworkID` does not work correctly because the first network is not always the private network of the instance. As long as the `GET /networks` gives a list containing also public networks we don't have any guarantee that the first network is the one we want. Furthermore, with a loop over the network list we are not able to determine which network is the one we want. Instead of retrieving the network ID and then finding the port ID, it's better to basically take the first port ID of the instance. --- .../resource_openstack_compute_instance_v2.go | 44 +++++-------------- 1 file changed, 11 insertions(+), 33 deletions(-) diff --git a/builtin/providers/openstack/resource_openstack_compute_instance_v2.go b/builtin/providers/openstack/resource_openstack_compute_instance_v2.go index 5a4c16c7e..b5fe36a10 100644 --- a/builtin/providers/openstack/resource_openstack_compute_instance_v2.go +++ b/builtin/providers/openstack/resource_openstack_compute_instance_v2.go @@ -20,7 +20,6 @@ import ( "github.com/rackspace/gophercloud/openstack/compute/v2/images" "github.com/rackspace/gophercloud/openstack/compute/v2/servers" "github.com/rackspace/gophercloud/openstack/networking/v2/extensions/layer3/floatingips" - "github.com/rackspace/gophercloud/openstack/networking/v2/networks" "github.com/rackspace/gophercloud/openstack/networking/v2/ports" "github.com/rackspace/gophercloud/pagination" ) @@ -303,7 +302,7 @@ func resourceComputeInstanceV2Create(d *schema.ResourceData, meta interface{}) e } err = assignFloatingIP(networkingClient, extractFloatingIPFromIP(allFloatingIPs, floatingIP), server.ID) if err != nil { - fmt.Errorf("Error assigning floating IP to OpenStack compute instance: %s", err) + return fmt.Errorf("Error assigning floating IP to OpenStack compute instance: %s", err) } } @@ -770,44 +769,18 @@ func extractFloatingIPFromIP(ips []floatingips.FloatingIP, IP string) *floatingi } func assignFloatingIP(networkingClient *gophercloud.ServiceClient, floatingIP *floatingips.FloatingIP, instanceID string) error { - networkID, err := getFirstNetworkID(networkingClient, instanceID) + portID, err := getInstancePortID(networkingClient, instanceID) if err != nil { return err } - portID, err := getInstancePortID(networkingClient, instanceID, networkID) - _, err = floatingips.Update(networkingClient, floatingIP.ID, floatingips.UpdateOpts{ + return floatingips.Update(networkingClient, floatingIP.ID, floatingips.UpdateOpts{ PortID: portID, - }).Extract() - return err + }).Err } -func getFirstNetworkID(networkingClient *gophercloud.ServiceClient, instanceID string) (string, error) { - pager := networks.List(networkingClient, networks.ListOpts{}) - - var networkdID string - err := pager.EachPage(func(page pagination.Page) (bool, error) { - networkList, err := networks.ExtractNetworks(page) - if err != nil { - return false, err - } - - if len(networkList) > 0 { - networkdID = networkList[0].ID - return false, nil - } - return false, fmt.Errorf("No network found for the instance %s", instanceID) - }) - if err != nil { - return "", err - } - return networkdID, nil - -} - -func getInstancePortID(networkingClient *gophercloud.ServiceClient, instanceID, networkID string) (string, error) { +func getInstancePortID(networkingClient *gophercloud.ServiceClient, instanceID string) (string, error) { pager := ports.List(networkingClient, ports.ListOpts{ - DeviceID: instanceID, - NetworkID: networkID, + DeviceID: instanceID, }) var portID string @@ -826,6 +799,11 @@ func getInstancePortID(networkingClient *gophercloud.ServiceClient, instanceID, if err != nil { return "", err } + + if portID == "" { + return "", fmt.Errorf("Cannot find port for instance %s", instanceID) + } + return portID, nil } From c0b85d4939b05e74a1e878ad72f3d6101561d029 Mon Sep 17 00:00:00 2001 From: Guillaume Giamarchi Date: Wed, 1 Apr 2015 11:24:51 +0200 Subject: [PATCH 2/3] Use env var OS_POOL_NAME as default for pool attribute To have the same behaviour for openstack_networking_floatingip_v2 and openstack_compute_foatingip_v2. --- .../resource_openstack_networking_floatingip_v2.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/builtin/providers/openstack/resource_openstack_networking_floatingip_v2.go b/builtin/providers/openstack/resource_openstack_networking_floatingip_v2.go index cbc430764..fd8b3fc65 100644 --- a/builtin/providers/openstack/resource_openstack_networking_floatingip_v2.go +++ b/builtin/providers/openstack/resource_openstack_networking_floatingip_v2.go @@ -28,9 +28,10 @@ func resourceNetworkingFloatingIPV2() *schema.Resource { Computed: true, }, "pool": &schema.Schema{ - Type: schema.TypeString, - Required: true, - ForceNew: true, + Type: schema.TypeString, + Required: true, + ForceNew: true, + DefaultFunc: envDefaultFunc("OS_POOL_NAME"), }, }, } From 56aa764b947fac3a0bb94fedcc049b5aaeeb2e73 Mon Sep 17 00:00:00 2001 From: Guillaume Giamarchi Date: Wed, 1 Apr 2015 11:27:56 +0200 Subject: [PATCH 3/3] Add floating IP association in aceptance tests --- .../resource_openstack_compute_floatingip_v2_test.go | 9 +++++++-- ...source_openstack_networking_floatingip_v2_test.go | 12 +++++++----- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/builtin/providers/openstack/resource_openstack_compute_floatingip_v2_test.go b/builtin/providers/openstack/resource_openstack_compute_floatingip_v2_test.go index c246d1a51..a298a87d1 100644 --- a/builtin/providers/openstack/resource_openstack_compute_floatingip_v2_test.go +++ b/builtin/providers/openstack/resource_openstack_compute_floatingip_v2_test.go @@ -81,6 +81,11 @@ func testAccCheckComputeV2FloatingIPExists(t *testing.T, n string, kp *floatingi } } -var testAccComputeV2FloatingIP_basic = fmt.Sprintf(` +var testAccComputeV2FloatingIP_basic = ` resource "openstack_compute_floatingip_v2" "foo" { - }`) + } + + resource "openstack_compute_instance_v2" "bar" { + name = "terraform-acc-floating-ip-test" + floating_ip = "${openstack_compute_floatingip_v2.foo.address}" + }` diff --git a/builtin/providers/openstack/resource_openstack_networking_floatingip_v2_test.go b/builtin/providers/openstack/resource_openstack_networking_floatingip_v2_test.go index cd08ea512..5c8ae38e3 100644 --- a/builtin/providers/openstack/resource_openstack_networking_floatingip_v2_test.go +++ b/builtin/providers/openstack/resource_openstack_networking_floatingip_v2_test.go @@ -81,9 +81,11 @@ func testAccCheckNetworkingV2FloatingIPExists(t *testing.T, n string, kp *floati } } -var testAccNetworkingV2FloatingIP_basic = fmt.Sprintf(` +var testAccNetworkingV2FloatingIP_basic = ` resource "openstack_networking_floatingip_v2" "foo" { - region = "%s" - pool = "%s" - }`, - OS_REGION_NAME, OS_POOL_NAME) + } + + resource "openstack_compute_instance_v2" "bar" { + name = "terraform-acc-floating-ip-test" + floating_ip = "${openstack_networking_floatingip_v2.foo.address}" + }`