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

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{})

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"
+  }
+}
+`

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
 		}