From 6efd0640ece1bfa689508ce8ef7666b018b9fc68 Mon Sep 17 00:00:00 2001 From: Joe Topjian Date: Thu, 13 Apr 2017 05:37:15 -0600 Subject: [PATCH] Openstack port update fixes (#13604) * provider/openstack: Handle 409 Errors Upon Security Group Deletion If a security group is currently in use, it will throw a 409 error. This commit catches the 409, allowing other resources to finish deleting. * Update openstack_networking_port_v2 resource to pass empty arrays for AllowedAddressPairs and SecurityGroups if not specified. Fixes #13531 * provider/openstack: Port Update comment --- .../resource_openstack_networking_port_v2.go | 21 +-- ...ource_openstack_networking_port_v2_test.go | 151 ++++++++++++++++++ ...source_openstack_networking_secgroup_v2.go | 5 + 3 files changed, 164 insertions(+), 13 deletions(-) diff --git a/builtin/providers/openstack/resource_openstack_networking_port_v2.go b/builtin/providers/openstack/resource_openstack_networking_port_v2.go index 508ebc813..4be432935 100644 --- a/builtin/providers/openstack/resource_openstack_networking_port_v2.go +++ b/builtin/providers/openstack/resource_openstack_networking_port_v2.go @@ -236,7 +236,14 @@ func resourceNetworkingPortV2Update(d *schema.ResourceData, meta interface{}) er return fmt.Errorf("Error creating OpenStack networking client: %s", err) } - var updateOpts ports.UpdateOpts + // security_group_ids and allowed_address_pairs are able to send empty arrays + // to denote the removal of each. But their default zero-value is translated + // to "null", which has been reported to cause problems in vendor-modified + // OpenStack clouds. Therefore, we must set them in each request update. + updateOpts := ports.UpdateOpts{ + AllowedAddressPairs: resourceAllowedAddressPairsV2(d), + SecurityGroups: resourcePortSecurityGroupsV2(d), + } if d.HasChange("name") { updateOpts.Name = d.Get("name").(string) @@ -250,10 +257,6 @@ func resourceNetworkingPortV2Update(d *schema.ResourceData, meta interface{}) er updateOpts.DeviceOwner = d.Get("device_owner").(string) } - if d.HasChange("security_group_ids") { - updateOpts.SecurityGroups = resourcePortSecurityGroupsV2(d) - } - if d.HasChange("device_id") { updateOpts.DeviceID = d.Get("device_id").(string) } @@ -262,10 +265,6 @@ func resourceNetworkingPortV2Update(d *schema.ResourceData, meta interface{}) er updateOpts.FixedIPs = resourcePortFixedIpsV2(d) } - if d.HasChange("allowed_address_pairs") { - updateOpts.AllowedAddressPairs = resourceAllowedAddressPairsV2(d) - } - log.Printf("[DEBUG] Updating Port %s with options: %+v", d.Id(), updateOpts) _, err = ports.Update(networkingClient, d.Id(), updateOpts).Extract() @@ -332,10 +331,6 @@ func resourceAllowedAddressPairsV2(d *schema.ResourceData) []ports.AddressPair { // ports.AddressPair rawPairs := d.Get("allowed_address_pairs").(*schema.Set).List() - if len(rawPairs) == 0 { - return nil - } - pairs := make([]ports.AddressPair, len(rawPairs)) for i, raw := range rawPairs { rawMap := raw.(map[string]interface{}) diff --git a/builtin/providers/openstack/resource_openstack_networking_port_v2_test.go b/builtin/providers/openstack/resource_openstack_networking_port_v2_test.go index 28e08bebd..a9d7281af 100644 --- a/builtin/providers/openstack/resource_openstack_networking_port_v2_test.go +++ b/builtin/providers/openstack/resource_openstack_networking_port_v2_test.go @@ -7,6 +7,7 @@ import ( "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" + "github.com/gophercloud/gophercloud/openstack/networking/v2/extensions/security/groups" "github.com/gophercloud/gophercloud/openstack/networking/v2/networks" "github.com/gophercloud/gophercloud/openstack/networking/v2/ports" "github.com/gophercloud/gophercloud/openstack/networking/v2/subnets" @@ -168,6 +169,54 @@ func TestAccNetworkingV2Port_fixedIPs(t *testing.T) { }) } +func TestAccNetworkingV2Port_updateSecurityGroups(t *testing.T) { + var network networks.Network + var port ports.Port + var security_group groups.SecGroup + var subnet subnets.Subnet + + resource.Test(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckNetworkingV2PortDestroy, + Steps: []resource.TestStep{ + resource.TestStep{ + Config: testAccNetworkingV2Port_updateSecurityGroups_1, + Check: resource.ComposeTestCheckFunc( + testAccCheckNetworkingV2SubnetExists("openstack_networking_subnet_v2.subnet_1", &subnet), + testAccCheckNetworkingV2NetworkExists("openstack_networking_network_v2.network_1", &network), + testAccCheckNetworkingV2PortExists("openstack_networking_port_v2.port_1", &port), + testAccCheckNetworkingV2SecGroupExists( + "openstack_networking_secgroup_v2.secgroup_1", &security_group), + testAccCheckNetworkingV2PortCountSecurityGroups(&port, 1), + ), + }, + resource.TestStep{ + Config: testAccNetworkingV2Port_updateSecurityGroups_2, + Check: resource.ComposeTestCheckFunc( + testAccCheckNetworkingV2SubnetExists("openstack_networking_subnet_v2.subnet_1", &subnet), + testAccCheckNetworkingV2NetworkExists("openstack_networking_network_v2.network_1", &network), + testAccCheckNetworkingV2PortExists("openstack_networking_port_v2.port_1", &port), + testAccCheckNetworkingV2SecGroupExists( + "openstack_networking_secgroup_v2.secgroup_1", &security_group), + testAccCheckNetworkingV2PortCountSecurityGroups(&port, 1), + ), + }, + resource.TestStep{ + Config: testAccNetworkingV2Port_updateSecurityGroups_3, + Check: resource.ComposeTestCheckFunc( + testAccCheckNetworkingV2SubnetExists("openstack_networking_subnet_v2.subnet_1", &subnet), + testAccCheckNetworkingV2NetworkExists("openstack_networking_network_v2.network_1", &network), + testAccCheckNetworkingV2PortExists("openstack_networking_port_v2.port_1", &port), + testAccCheckNetworkingV2SecGroupExists( + "openstack_networking_secgroup_v2.secgroup_1", &security_group), + testAccCheckNetworkingV2PortCountSecurityGroups(&port, 0), + ), + }, + }, + }) +} + func testAccCheckNetworkingV2PortDestroy(s *terraform.State) error { config := testAccProvider.Meta().(*Config) networkingClient, err := config.networkingV2Client(OS_REGION_NAME) @@ -231,6 +280,16 @@ func testAccCheckNetworkingV2PortCountFixedIPs(port *ports.Port, expected int) r } } +func testAccCheckNetworkingV2PortCountSecurityGroups(port *ports.Port, expected int) resource.TestCheckFunc { + return func(s *terraform.State) error { + if len(port.SecurityGroups) != expected { + return fmt.Errorf("Expected %d Security Groups, got %d", expected, len(port.SecurityGroups)) + } + + return nil + } +} + const testAccNetworkingV2Port_basic = ` resource "openstack_networking_network_v2" "network_1" { name = "network_1" @@ -472,3 +531,95 @@ resource "openstack_networking_port_v2" "port_1" { } } ` + +const testAccNetworkingV2Port_updateSecurityGroups_1 = ` +resource "openstack_networking_network_v2" "network_1" { + name = "network_1" + admin_state_up = "true" +} + +resource "openstack_networking_subnet_v2" "subnet_1" { + name = "subnet_1" + cidr = "192.168.199.0/24" + ip_version = 4 + network_id = "${openstack_networking_network_v2.network_1.id}" +} + +resource "openstack_networking_secgroup_v2" "secgroup_1" { + name = "security_group" + description = "terraform security group acceptance test" +} + +resource "openstack_networking_port_v2" "port_1" { + name = "port_1" + admin_state_up = "true" + network_id = "${openstack_networking_network_v2.network_1.id}" + + fixed_ip { + subnet_id = "${openstack_networking_subnet_v2.subnet_1.id}" + ip_address = "192.168.199.23" + } +} +` + +const testAccNetworkingV2Port_updateSecurityGroups_2 = ` +resource "openstack_networking_network_v2" "network_1" { + name = "network_1" + admin_state_up = "true" +} + +resource "openstack_networking_subnet_v2" "subnet_1" { + name = "subnet_1" + cidr = "192.168.199.0/24" + ip_version = 4 + network_id = "${openstack_networking_network_v2.network_1.id}" +} + +resource "openstack_networking_secgroup_v2" "secgroup_1" { + name = "security_group" + description = "terraform security group acceptance test" +} + +resource "openstack_networking_port_v2" "port_1" { + name = "port_1" + admin_state_up = "true" + network_id = "${openstack_networking_network_v2.network_1.id}" + security_group_ids = ["${openstack_networking_secgroup_v2.secgroup_1.id}"] + + fixed_ip { + subnet_id = "${openstack_networking_subnet_v2.subnet_1.id}" + ip_address = "192.168.199.23" + } +} +` + +const testAccNetworkingV2Port_updateSecurityGroups_3 = ` +resource "openstack_networking_network_v2" "network_1" { + name = "network_1" + admin_state_up = "true" +} + +resource "openstack_networking_subnet_v2" "subnet_1" { + name = "subnet_1" + cidr = "192.168.199.0/24" + ip_version = 4 + network_id = "${openstack_networking_network_v2.network_1.id}" +} + +resource "openstack_networking_secgroup_v2" "secgroup_1" { + name = "security_group" + description = "terraform security group acceptance test" +} + +resource "openstack_networking_port_v2" "port_1" { + name = "port_1" + admin_state_up = "true" + network_id = "${openstack_networking_network_v2.network_1.id}" + security_group_ids = [] + + fixed_ip { + subnet_id = "${openstack_networking_subnet_v2.subnet_1.id}" + ip_address = "192.168.199.23" + } +} +` diff --git a/builtin/providers/openstack/resource_openstack_networking_secgroup_v2.go b/builtin/providers/openstack/resource_openstack_networking_secgroup_v2.go index f76d24c57..8dad6fad8 100644 --- a/builtin/providers/openstack/resource_openstack_networking_secgroup_v2.go +++ b/builtin/providers/openstack/resource_openstack_networking_secgroup_v2.go @@ -167,6 +167,11 @@ func waitForSecGroupDelete(networkingClient *gophercloud.ServiceClient, secGroup log.Printf("[DEBUG] Successfully deleted OpenStack Neutron Security Group %s", secGroupId) return r, "DELETED", nil } + if errCode, ok := err.(gophercloud.ErrUnexpectedResponseCode); ok { + if errCode.Actual == 409 { + return r, "ACTIVE", nil + } + } return r, "ACTIVE", err }