Merge pull request #7468 from jtopjian/openstack-secgroup-eof

provider/openstack: Fix Security Group EOF Error
hashicorp · Jul 2, 2016 · b5ca75f · b5ca75f
2 parents c965d9b + bc6be73
commit b5ca75f
Show file tree

Hide file tree

Showing 4 changed files with 164 additions and 18 deletions.
diff --git a/builtin/providers/openstack/resource_openstack_compute_instance_v2.go b/builtin/providers/openstack/resource_openstack_compute_instance_v2.go
@@ -622,28 +622,28 @@ func resourceComputeInstanceV2Update(d *schema.ResourceData, meta interface{}) e
 
 		for _, g := range secgroupsToRemove.List() {
 			err := secgroups.RemoveServerFromGroup(computeClient, d.Id(), g.(string)).ExtractErr()
-			if err != nil {
+			if err != nil && err.Error() != "EOF" {
 				errCode, ok := err.(*gophercloud.UnexpectedResponseCodeError)
 				if !ok {
-					return fmt.Errorf("Error removing security group from OpenStack server (%s): %s", d.Id(), err)
+					return fmt.Errorf("Error removing security group (%s) from OpenStack server (%s): %s", g, d.Id(), err)
 				}
 				if errCode.Actual == 404 {
 					continue
 				} else {
-					return fmt.Errorf("Error removing security group from OpenStack server (%s): %s", d.Id(), err)
+					return fmt.Errorf("Error removing security group (%s) from OpenStack server (%s): %s", g, d.Id(), err)
 				}
 			} else {
-				log.Printf("[DEBUG] Removed security group (%s) from instance (%s)", g.(string), d.Id())
+				log.Printf("[DEBUG] Removed security group (%s) from instance (%s)", g, d.Id())
 			}
 		}
+
 		for _, g := range secgroupsToAdd.List() {
 			err := secgroups.AddServerToGroup(computeClient, d.Id(), g.(string)).ExtractErr()
-			if err != nil {
-				return fmt.Errorf("Error adding security group to OpenStack server (%s): %s", d.Id(), err)
+			if err != nil && err.Error() != "EOF" {
+				return fmt.Errorf("Error adding security group (%s) to OpenStack server (%s): %s", g, d.Id(), err)
 			}
-			log.Printf("[DEBUG] Added security group (%s) to instance (%s)", g.(string), d.Id())
+			log.Printf("[DEBUG] Added security group (%s) to instance (%s)", g, d.Id())
 		}
-
 	}
 
 	if d.HasChange("admin_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
@@ -316,11 +316,11 @@ func TestAccComputeV2Instance_floatingIPAttachAndChange(t *testing.T) {
 }
 
 func TestAccComputeV2Instance_multi_secgroups(t *testing.T) {
-	var instance servers.Server
-	var secgroup secgroups.SecurityGroup
+	var instance_1 servers.Server
+	var secgroup_1 secgroups.SecurityGroup
 	var testAccComputeV2Instance_multi_secgroups = fmt.Sprintf(`
-		resource "openstack_compute_secgroup_v2" "foo" {
-			name = "terraform-test"
+		resource "openstack_compute_secgroup_v2" "secgroup_1" {
+			name = "secgroup_1"
 			description = "a security group"
 			rule {
 				from_port = 22
@@ -330,9 +330,9 @@ func TestAccComputeV2Instance_multi_secgroups(t *testing.T) {
 			}
 		}
 
-		resource "openstack_compute_instance_v2" "foo" {
-			name = "terraform-test"
-			security_groups = ["default", "${openstack_compute_secgroup_v2.foo.name}"]
+		resource "openstack_compute_instance_v2" "instance_1" {
+			name = "instance_1"
+			security_groups = ["default", "${openstack_compute_secgroup_v2.secgroup_1.name}"]
 			network {
 				uuid = "%s"
 			}
@@ -347,8 +347,92 @@ func TestAccComputeV2Instance_multi_secgroups(t *testing.T) {
 			resource.TestStep{
 				Config: testAccComputeV2Instance_multi_secgroups,
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckComputeV2SecGroupExists(t, "openstack_compute_secgroup_v2.foo", &secgroup),
-					testAccCheckComputeV2InstanceExists(t, "openstack_compute_instance_v2.foo", &instance),
+					testAccCheckComputeV2SecGroupExists(t, "openstack_compute_secgroup_v2.secgroup_1", &secgroup_1),
+					testAccCheckComputeV2InstanceExists(t, "openstack_compute_instance_v2.instance_1", &instance_1),
+				),
+			},
+		},
+	})
+}
+
+func TestAccComputeV2Instance_multi_secgroups_update(t *testing.T) {
+	var instance_1 servers.Server
+	var secgroup_1, secgroup_2 secgroups.SecurityGroup
+	var testAccComputeV2Instance_multi_secgroups_update_1 = fmt.Sprintf(`
+		resource "openstack_compute_secgroup_v2" "secgroup_1" {
+			name = "secgroup_1"
+			description = "a security group"
+			rule {
+				from_port = 22
+				to_port = 22
+				ip_protocol = "tcp"
+				cidr = "0.0.0.0/0"
+			}
+		}
+
+		resource "openstack_compute_secgroup_v2" "secgroup_2" {
+			name = "secgroup_2"
+			description = "another security group"
+			rule {
+				from_port = 80
+				to_port = 80
+				ip_protocol = "tcp"
+				cidr = "0.0.0.0/0"
+			}
+		}
+
+		resource "openstack_compute_instance_v2" "instance_1" {
+			name = "instance_1"
+			security_groups = ["default"]
+		}`)
+
+	var testAccComputeV2Instance_multi_secgroups_update_2 = fmt.Sprintf(`
+		resource "openstack_compute_secgroup_v2" "secgroup_1" {
+			name = "secgroup_1"
+			description = "a security group"
+			rule {
+				from_port = 22
+				to_port = 22
+				ip_protocol = "tcp"
+				cidr = "0.0.0.0/0"
+			}
+		}
+
+		resource "openstack_compute_secgroup_v2" "secgroup_2" {
+			name = "secgroup_2"
+			description = "another security group"
+			rule {
+				from_port = 80
+				to_port = 80
+				ip_protocol = "tcp"
+				cidr = "0.0.0.0/0"
+			}
+		}
+
+		resource "openstack_compute_instance_v2" "instance_1" {
+			name = "instance_1"
+			security_groups = ["default", "${openstack_compute_secgroup_v2.secgroup_1.name}", "${openstack_compute_secgroup_v2.secgroup_2.name}"]
+		}`)
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeV2InstanceDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccComputeV2Instance_multi_secgroups_update_1,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeV2SecGroupExists(t, "openstack_compute_secgroup_v2.secgroup_1", &secgroup_1),
+					testAccCheckComputeV2SecGroupExists(t, "openstack_compute_secgroup_v2.secgroup_2", &secgroup_2),
+					testAccCheckComputeV2InstanceExists(t, "openstack_compute_instance_v2.instance_1", &instance_1),
+				),
+			},
+			resource.TestStep{
+				Config: testAccComputeV2Instance_multi_secgroups_update_2,
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeV2SecGroupExists(t, "openstack_compute_secgroup_v2.secgroup_1", &secgroup_1),
+					testAccCheckComputeV2SecGroupExists(t, "openstack_compute_secgroup_v2.secgroup_2", &secgroup_2),
+					testAccCheckComputeV2InstanceExists(t, "openstack_compute_instance_v2.instance_1", &instance_1),
 				),
 			},
 		},

diff --git a/website/source/docs/providers/openstack/r/compute_instance_v2.html.markdown b/website/source/docs/providers/openstack/r/compute_instance_v2.html.markdown
@@ -227,7 +227,9 @@ The following arguments are supported:
 
 * `security_groups` - (Optional) An array of one or more security group names
     to associate with the server. Changing this results in adding/removing
-    security groups from the existing server.
+    security groups from the existing server. *Note*: When attaching the
+    instance to networks using Ports, place the security groups on the Port
+    and not the instance.
 
 * `availability_zone` - (Optional) The availability zone in which to create
     the server. Changing this creates a new server.
@@ -430,3 +432,55 @@ resource "openstack_compute_instance_v2" "foo" {
   }
 }
 ```
+
+### Instances and Ports
+
+Neutron Ports are a great feature and provide a lot of functionality. However,
+there are some notes to be aware of when mixing Instances and Ports:
+
+* When attaching an Instance to one or more networks using Ports, place the
+security groups on the Port and not the Instance. If you place the security
+groups on the Instance, the security groups will not be applied upon creation,
+but they will be applied upon a refresh. This is a known OpenStack bug.
+
+* Network IP information is not available within an instance for networks that
+are attached with Ports. This is mostly due to the flexibility Neutron Ports
+provide when it comes to IP addresses. For example, a Neutron Port can have
+multiple Fixed IP addresses associated with it. It's not possible to know which
+single IP address the user would want returned to the Instance's state
+information. Therefore, in order for a Provisioner to connect to an Instance
+via it's network Port, customize the `connection` information:
+
+```
+resource "openstack_networking_port_v2" "port_1" {
+  name = "port_1"
+  admin_state_up = "true"
+
+  network_id = "0a1d0a27-cffa-4de3-92c5-9d3fd3f2e74d"
+  security_group_ids = [
+    "2f02d20a-8dca-49b7-b26f-b6ce9fddaf4f",
+    "ca1e5ed7-dae8-4605-987b-fadaeeb30461",
+  ]
+
+}
+
+resource "openstack_compute_instance_v2" "instance_1" {
+  name        = "instance_1"
+
+  network {
+    port = "${openstack_networking_port_v2.port_1.id}"
+  }
+
+  connection {
+    user = "root"
+    host = "${openstack_networking_port_v2.port_1.fixed_ip.0.ip_address}"
+    private_key = "~/path/to/key"
+  }
+
+  provisioner "remote-exec" {
+    inline = [
+      "echo terraform executed > /tmp/foo"
+    ]
+  }
+}
+```
diff --git a/website/source/docs/providers/openstack/r/networking_port_v2.html.markdown b/website/source/docs/providers/openstack/r/networking_port_v2.html.markdown
@@ -85,3 +85,11 @@ The following attributes are exported:
 * `security_groups` - See Argument Reference above.
 * `device_id` - See Argument Reference above.
 * `fixed_ip/ip_address` - See Argument Reference above.
+
+## Notes
+
+### Ports and Instances
+
+There are some notes to consider when connecting Instances to networks using
+Ports. Please see the `openstack_compute_instance_v2` documentation for further
+documentation.