provider/openstack: Redesign openstack_blockstorage_volume_attach_v2 (#12071)

* provider/openstack: Redesign openstack_blockstorage_volume_attach_v2

The current design of openstack_blockstorage_volume_attach_v2 does
not correctly implement the Block Storage API attachment call. It
was only partially implemented, only marking volumes as being
attached, while never actually attaching them.

This redesign is a closer alignment to how creating attachments
to a standalone Block Storage service works.

For creating attachments specifically in the case of OpenStack
Compute instances, the openstack_compute_volume_attach_v2 resource
is required.

* provider/openstack: re-adding instance_id for backwards compatibility

builtin/providers/openstack/import_openstack_blockstorage_volume_attach_v2_test.go

@@ -1,28 +0,0 @@
-package openstack
-
-import (
-	"testing"
-
-	"github.com/hashicorp/terraform/helper/resource"
-)
-
-func TestAccBlockStorageVolumeAttachV2_importBasic(t *testing.T) {
-	resourceName := "openstack_blockstorage_volume_attach_v2.va_1"
-
-	resource.Test(t, resource.TestCase{
-		PreCheck:     func() { testAccPreCheck(t) },
-		Providers:    testAccProviders,
-		CheckDestroy: testAccCheckBlockStorageVolumeAttachV2Destroy,
-		Steps: []resource.TestStep{
-			resource.TestStep{
-				Config: testAccBlockStorageVolumeAttachV2_basic,
-			},
-
-			resource.TestStep{
-				ResourceName:      resourceName,
-				ImportState:       true,
-				ImportStateVerify: true,
-			},
-		},
-	})
-}

builtin/providers/openstack/resource_openstack_blockstorage_volume_attach_v2.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/gophercloud/gophercloud/openstack/blockstorage/extensions/volumeactions"
 	"github.com/gophercloud/gophercloud/openstack/blockstorage/v2/volumes"
+
 	"github.com/hashicorp/terraform/helper/resource"
 	"github.com/hashicorp/terraform/helper/schema"
 )
@@ -17,9 +18,6 @@ func resourceBlockStorageVolumeAttachV2() *schema.Resource {
 		Create: resourceBlockStorageVolumeAttachV2Create,
 		Read:   resourceBlockStorageVolumeAttachV2Read,
 		Delete: resourceBlockStorageVolumeAttachV2Delete,
-		Importer: &schema.ResourceImporter{
-			State: schema.ImportStatePassthrough,
-		},
 
 		Schema: map[string]*schema.Schema{
 			"region": &schema.Schema{
@@ -36,23 +34,22 @@ func resourceBlockStorageVolumeAttachV2() *schema.Resource {
 			},
 
 			"instance_id": &schema.Schema{
-				Type:          schema.TypeString,
+				Type:       schema.TypeString,
-				Optional:      true,
+				Optional:   true,
-				ForceNew:      true,
+				ForceNew:   true,
-				ConflictsWith: []string{"host_name"},
+				Deprecated: "instance_id is no longer used in this resource",
 			},
 
 			"host_name": &schema.Schema{
-				Type:          schema.TypeString,
+				Type:     schema.TypeString,
-				Optional:      true,
+				Required: true,
-				ForceNew:      true,
+				ForceNew: true,
-				ConflictsWith: []string{"instance_id"},
 			},
 
 			"device": &schema.Schema{
 				Type:     schema.TypeString,
 				Optional: true,
-				Computed: true,
+				ForceNew: true,
 			},
 
 			"attach_mode": &schema.Schema{
@@ -68,6 +65,66 @@ func resourceBlockStorageVolumeAttachV2() *schema.Resource {
 					return
 				},
 			},
+
+			"initiator": &schema.Schema{
+				Type:     schema.TypeString,
+				Optional: true,
+				ForceNew: true,
+			},
+
+			"ip_address": &schema.Schema{
+				Type:     schema.TypeString,
+				Optional: true,
+				ForceNew: true,
+			},
+
+			"multipath": &schema.Schema{
+				Type:     schema.TypeBool,
+				Optional: true,
+				ForceNew: true,
+			},
+
+			"os_type": &schema.Schema{
+				Type:     schema.TypeString,
+				Optional: true,
+				ForceNew: true,
+			},
+
+			"platform": &schema.Schema{
+				Type:     schema.TypeString,
+				Optional: true,
+				ForceNew: true,
+			},
+
+			"wwpn": &schema.Schema{
+				Type:     schema.TypeList,
+				Optional: true,
+				ForceNew: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},
+			},
+
+			"wwnn": &schema.Schema{
+				Type:     schema.TypeString,
+				Optional: true,
+				ForceNew: true,
+			},
+
+			// Volume attachment information
+			"data": &schema.Schema{
+				Type:      schema.TypeMap,
+				Computed:  true,
+				Sensitive: true,
+			},
+
+			"driver_volume_type": &schema.Schema{
+				Type:     schema.TypeString,
+				Computed: true,
+			},
+
+			"mount_point_base": &schema.Schema{
+				Type:     schema.TypeString,
+				Computed: true,
+			},
 		},
 	}
 }
@@ -79,25 +136,86 @@ func resourceBlockStorageVolumeAttachV2Create(d *schema.ResourceData, meta inter
 		return fmt.Errorf("Error creating OpenStack block storage client: %s", err)
 	}
 
-	// Check if either instance_id or host_name was set.
+	// initialize the connection
-	instanceId := d.Get("instance_id").(string)
+	volumeId := d.Get("volume_id").(string)
-	hostName := d.Get("host_name").(string)
+	connOpts := &volumeactions.InitializeConnectionOpts{}
-	if instanceId == "" && hostName == "" {
+	if v, ok := d.GetOk("host_name"); ok {
-		return fmt.Errorf("One of 'instance_id' or 'host_name' must be set.")
+		connOpts.Host = v.(string)
 	}
 
-	volumeId := d.Get("volume_id").(string)
+	if v, ok := d.GetOk("multipath"); ok {
+		multipath := v.(bool)
+		connOpts.Multipath = &multipath
+	}
 
+	if v, ok := d.GetOk("ip_address"); ok {
+		connOpts.IP = v.(string)
+	}
+
+	if v, ok := d.GetOk("initiator"); ok {
+		connOpts.Initiator = v.(string)
+	}
+
+	if v, ok := d.GetOk("os_type"); ok {
+		connOpts.OSType = v.(string)
+	}
+
+	if v, ok := d.GetOk("platform"); ok {
+		connOpts.Platform = v.(string)
+	}
+
+	if v, ok := d.GetOk("wwnns"); ok {
+		connOpts.Wwnns = v.(string)
+	}
+
+	if v, ok := d.GetOk("wwpns"); ok {
+		var wwpns []string
+		for _, i := range v.([]string) {
+			wwpns = append(wwpns, i)
+		}
+
+		connOpts.Wwpns = wwpns
+	}
+
+	connInfo, err := volumeactions.InitializeConnection(client, volumeId, connOpts).Extract()
+	if err != nil {
+		return fmt.Errorf("Unable to create connection: %s", err)
+	}
+
+	// Only uncomment this when debugging since connInfo contains sensitive information.
+	// log.Printf("[DEBUG] Volume Connection for %s: %#v", volumeId, connInfo)
+
+	// Because this information is only returned upon creation,
+	// it must be set in Create.
+	if v, ok := connInfo["data"]; ok {
+		data := make(map[string]string)
+		for key, value := range v.(map[string]interface{}) {
+			if v, ok := value.(string); ok {
+				data[key] = v
+			}
+		}
+
+		d.Set("data", data)
+	}
+
+	if v, ok := connInfo["driver_volume_type"]; ok {
+		d.Set("driver_volume_type", v)
+	}
+
+	if v, ok := connInfo["mount_point_base"]; ok {
+		d.Set("mount_point_base", v)
+	}
+
+	// Once the connection has been made, tell Cinder to mark the volume as attached.
 	attachMode, err := blockStorageVolumeAttachV2AttachMode(d.Get("attach_mode").(string))
 	if err != nil {
 		return nil
 	}
 
 	attachOpts := &volumeactions.AttachOpts{
-		InstanceUUID: d.Get("instance_id").(string),
+		HostName:   d.Get("host_name").(string),
-		HostName:     d.Get("host_name").(string),
+		MountPoint: d.Get("device").(string),
-		MountPoint:   d.Get("device").(string),
+		Mode:       attachMode,
-		Mode:         attachMode,
 	}
 
 	log.Printf("[DEBUG] Attachment Options: %#v", attachOpts)
@@ -123,17 +241,17 @@ func resourceBlockStorageVolumeAttachV2Create(d *schema.ResourceData, meta inter
 		return fmt.Errorf("Error waiting for volume (%s) to become ready: %s", volumeId, err)
 	}
 
+	// Once the volume has been marked as attached,
+	// retrieve a fresh copy of it with all information now available.
 	volume, err := volumes.Get(client, volumeId).Extract()
 	if err != nil {
 		return err
 	}
 
+	// Search for the attachmentId
 	var attachmentId string
+	hostName := d.Get("host_name").(string)
 	for _, attachment := range volume.Attachments {
-		if instanceId != "" && instanceId == attachment.ServerID {
-			attachmentId = attachment.AttachmentID
-		}
-
 		if hostName != "" && hostName == attachment.HostName {
 			attachmentId = attachment.AttachmentID
 		}
@@ -144,7 +262,7 @@ func resourceBlockStorageVolumeAttachV2Create(d *schema.ResourceData, meta inter
 	}
 
 	// The ID must be a combination of the volume and attachment ID
-	// in order to import attachments.
+	// since a volume ID is required to retrieve an attachment ID.
 	id := fmt.Sprintf("%s/%s", volumeId, attachmentId)
 	d.SetId(id)
 
@@ -179,13 +297,6 @@ func resourceBlockStorageVolumeAttachV2Read(d *schema.ResourceData, meta interfa
 
 	log.Printf("[DEBUG] Retrieved volume attachment: %#v", attachment)
 
-	d.Set("volume_id", volumeId)
-	d.Set("attachment_id", attachmentId)
-	d.Set("device", attachment.Device)
-	d.Set("instance_id", attachment.ServerID)
-	d.Set("host_name", attachment.HostName)
-	d.Set("region", GetRegion(d))
-
 	return nil
 }
 
@@ -197,10 +308,53 @@ func resourceBlockStorageVolumeAttachV2Delete(d *schema.ResourceData, meta inter
 	}
 
 	volumeId, attachmentId, err := blockStorageVolumeAttachV2ParseId(d.Id())
-	if err != nil {
+
-		return err
+	// Terminate the connection
+	termOpts := &volumeactions.TerminateConnectionOpts{}
+	if v, ok := d.GetOk("host_name"); ok {
+		termOpts.Host = v.(string)
 	}
 
+	if v, ok := d.GetOk("multipath"); ok {
+		multipath := v.(bool)
+		termOpts.Multipath = &multipath
+	}
+
+	if v, ok := d.GetOk("ip_address"); ok {
+		termOpts.IP = v.(string)
+	}
+
+	if v, ok := d.GetOk("initiator"); ok {
+		termOpts.Initiator = v.(string)
+	}
+
+	if v, ok := d.GetOk("os_type"); ok {
+		termOpts.OSType = v.(string)
+	}
+
+	if v, ok := d.GetOk("platform"); ok {
+		termOpts.Platform = v.(string)
+	}
+
+	if v, ok := d.GetOk("wwnns"); ok {
+		termOpts.Wwnns = v.(string)
+	}
+
+	if v, ok := d.GetOk("wwpns"); ok {
+		var wwpns []string
+		for _, i := range v.([]string) {
+			wwpns = append(wwpns, i)
+		}
+
+		termOpts.Wwpns = wwpns
+	}
+
+	err = volumeactions.TerminateConnection(client, volumeId, termOpts).ExtractErr()
+	if err != nil {
+		return fmt.Errorf("Error terminating volume connection %s: %s", volumeId, err)
+	}
+
+	// Detach the volume
 	detachOpts := volumeactions.DetachOpts{
 		AttachmentID: attachmentId,
 	}

builtin/providers/openstack/resource_openstack_blockstorage_volume_attach_v2_test.go

@@ -113,14 +113,14 @@ resource "openstack_blockstorage_volume_v2" "volume_1" {
   size = 1
 }
 
-resource "openstack_compute_instance_v2" "instance_1" {
-  name = "instance_1"
-  security_groups = ["default"]
-}
-
 resource "openstack_blockstorage_volume_attach_v2" "va_1" {
-  instance_id = "${openstack_compute_instance_v2.instance_1.id}"
   volume_id = "${openstack_blockstorage_volume_v2.volume_1.id}"
   device = "auto"
+
+  host_name = "devstack"
+  ip_address = "192.168.255.10"
+  initiator = "iqn.1993-08.org.debian:01:e9861fb1859"
+  os_type = "linux2"
+  platform = "x86_64"
 }
 `

website/source/docs/providers/openstack/r/blockstorage_volume_attach_v2.html.markdown

@@ -3,13 +3,24 @@ layout: "openstack"
 page_title: "OpenStack: openstack_blockstorage_volume_attach_v2"
 sidebar_current: "docs-openstack-resource-blockstorage-volume-attach-v2"
 description: |-
-  Attaches a Block Storage Volume to an Instance.
+  Creates an attachment connection to a Block Storage volume
 ---
 
-# openstack\_blockstorage\_volume_attach_v2
+# openstack\_blockstorage\_volume\_attach\_v2
 
-Attaches a Block Storage Volume to an Instance using the OpenStack
+This resource is experimental and may be removed in the future! Feedback
-Block Storage (Cinder) v2 API.
+is requested if you find this resource useful or if you find any problems
+with it.
+
+Creates a general purpose attachment connection to a Block
+Storage volume using the OpenStack Block Storage (Cinder) v2 API.
+Depending on your Block Storage service configuration, this
+resource can assist in attaching a volume to a non-OpenStack resource
+such as a bare-metal server or a remote virtual machine in a
+different cloud provider.
+
+This does not actually attach a volume to an instance. Please use
+the `openstack_compute_volume_attach_v2` resource for that.
 
 ## Example Usage
 
@@ -19,16 +30,14 @@ resource "openstack_blockstorage_volume_v2" "volume_1" {
   size = 1
 }
 
-resource "openstack_compute_instance_v2" "instance_1" {
-  name            = "instance_1"
-  security_groups = ["default"]
-}
-
 resource "openstack_blockstorage_volume_attach_v2" "va_1" {
-  instance_id = "${openstack_compute_instance_v2.instance_1.id}"
+  volume_id = "${openstack_blockstorage_volume_v2.volume_1.id}"
-  volume_id   = "${openstack_blockstorage_volume_v2.volume_1.id}"
+  device = "auto"
-  device      = "auto"
+  host_name = "devstack"
-  attach_mode = "rw"
+  ip_address = "192.168.255.10"
+  initiator = "iqn.1993-08.org.debian:01:e9861fb1859"
+  os_type = "linux2"
+  platform = "x86_64"
 }
 ```
 
@@ -41,40 +50,82 @@ The following arguments are supported:
     If omitted, the `OS_REGION_NAME` environment variable is used. Changing
     this creates a new volume attachment.
 
-* `volume_id` - (Required) The ID of the Volume to attach to an Instance.
-
-* `instance_id` - (Required if `host_name` is not used) The ID of the Instance
-  to attach the Volume to.
-
-* `host_name` - (Required if `instance_id` is not used) The host to attach the
-  volume to.
-
-* `device` - (Optional) The device to attach the volume as.
-
 * `attach_mode` - (Optional) Specify whether to attach the volume as Read-Only
   (`ro`) or Read-Write (`rw`). Only values of `ro` and `rw` are accepted.
   If left unspecified, the Block Storage API will apply a default of `rw`.
 
+* `device` - (Optional) The device to tell the Block Storage service this
+  volume will be attached as. This is purely for informational purposes.
+  You can specify `auto` or a device such as `/dev/vdc`.
+
+* `host_name` - (Required) The host to attach the volume to.
+
+* `initiator` - (Optional) The iSCSI initiator string to make the connection.
+
+* `ip_address` - (Optional) The IP address of the `host_name` above.
+
+* `multipath` - (Optional) Whether to connect to this volume via multipath.
+
+* `os_type` - (Optional) The iSCSI initiator OS type.
+
+* `platform` - (Optional) The iSCSI initiator platform.
+
+* `volume_id` - (Required) The ID of the Volume to attach to an Instance.
+
+* `wwpn` - (Optional) An array of wwpn strings. Used for Fibre Channel
+  connections.
+
+* `wwnn` - (Optional) A wwnn name. Used for Fibre Channel connections.
+
 ## Attributes Reference
 
-The following attributes are exported:
+In addition to the above, the following attributes are exported:
 
-* `region` - See Argument Reference above.
+* `data` - This is a map of key/value pairs that contain the connection
-* `volume_id` - See Argument Reference above.
+  information. You will want to pass this information to a provisioner
-* `instance_id` - See Argument Reference above.
+  script to finalize the connection. See below for more information.
-* `host_name` - See Argument Reference above.
+
-* `attach_mode` - See Argument Reference above.
+* `driver_volume_type` - The storage driver that the volume is based on.
-* `device` - See Argument Reference above.
+
-  _NOTE_: Whether or not this is really the device the volume was attached
+* `mount_point_base` - A mount point base name for shared storage.
-  as depends on the hypervisor being used in the OpenStack cloud. Do not
+
-  consider this an authoritative piece of information.
+## Volume Connection Data
+
+Upon creation of this resource, a `data` exported attribute will be available.
+This attribute is a set of key/value pairs that contains the information
+required to complete the block storage connection.
+
+As an example, creating an iSCSI-based volume will return the following:
+
+```
+data.access_mode = rw
+data.auth_method = CHAP
+data.auth_password = xUhbGKQ8QCwKmHQ2
+data.auth_username = Sphn5X4EoyFUUMYVYSA4
+data.target_iqn = iqn.2010-10.org.openstack:volume-2d87ed25-c312-4f42-be1d-3b36b014561d
+data.target_portal = 192.168.255.10:3260
+data.volume_id = 2d87ed25-c312-4f42-be1d-3b36b014561d
+```
+
+This information can then be fed into a provisioner or a template shell script,
+where the final result would look something like:
+
+```
+iscsiadm -m node -T ${self.data.target_iqn} -p ${self.data.target_portal} --interface default --op new
+iscsiadm -m node -T ${self.data.target_iqn} -p ${self.data.target_portal} --op update -n node.session.auth.authmethod -v ${self.data.auth_method}
+iscsiadm -m node -T ${self.data.target_iqn} -p ${self.data.target_portal} --op update -n node.session.auth.username -v ${self.data.auth_username}
+iscsiadm -m node -T ${self.data.target_iqn} -p ${self.data.target_portal} --op update -n node.session.auth.password -v ${self.data.auth_password}
+iscsiadm -m node -T ${self.data.target_iqn} -p ${self.data.target_portal} --login
+iscsiadm -m node -T ${self.data.target_iqn} -p ${self.data.target_portal} --op update -n node.startup -v automatic
+iscsiadm -m node -T ${self.data.target_iqn} -p ${self.data.target_portal} --rescan
+```
+
+The contents of `data` will vary from each Block Storage service. You must have
+a good understanding of how the service is configured and how to make the
+appropriate final connection. However, if used correctly, this has the
+flexibility to be able to attach OpenStack Block Storage volumes to
+non-OpenStack resources.
 
 ## Import
 
-Volume Attachments can be imported using the Volume and Attachment ID
+It is not possible to import this resource.
-separated by a slash, e.g.
-
-```
-$ terraform import openstack_blockstorage_volume_attach_v2.va_1 89c60255-9bd6-460c-822a-e2b959ede9d2/45670584-225f-46c3-b33e-6707b589b666
-```
-