provider/google: detach disks before deleting them.

When a `google_compute_disk` is attached to a `google_compute_instance`,
deleting can be tricky. GCP doesn't allow disks that are attached to
instances to be deleted. Normally, this is fine; the instance depends on
the disk, so by the time the disk is deleted, the instance should
already be gone.

However, some reports have cropped up (#8667) that deleting disks is
failing because they're still attached to instances. Though this
shouldn't happen, it appears it can happen under some unknown
conditions.

This PR adds logic that will attempt to detach disks from any instances
they're attached to before deleting the disks, adding another safeguard
that should prevent this behaviour.

builtin/providers/google/resource_compute_disk.go

@@ -3,12 +3,21 @@ package google
 import (
 	"fmt"
 	"log"
+	"regexp"
 
 	"github.com/hashicorp/terraform/helper/schema"
 	"google.golang.org/api/compute/v1"
 	"google.golang.org/api/googleapi"
 )
 
+const (
+	computeDiskUserRegexString = "^(?:https://www.googleapis.com/compute/v1/projects/)?([-_a-zA-Z0-9]*)/zones/([-_a-zA-Z0-9]*)/instances/([-_a-zA-Z0-9]*)$"
+)
+
+var (
+	computeDiskUserRegex = regexp.MustCompile(computeDiskUserRegexString)
+)
+
 func resourceComputeDisk() *schema.Resource {
 	return &schema.Resource{
 		Create: resourceComputeDiskCreate,
@@ -74,6 +83,11 @@ func resourceComputeDisk() *schema.Resource {
 				Optional: true,
 				ForceNew: true,
 			},
+			"users": &schema.Schema{
+				Type:     schema.TypeList,
+				Computed: true,
+				Elem:     &schema.Schema{Type: schema.TypeString},
+			},
 		},
 	}
 }
@@ -181,6 +195,7 @@ func resourceComputeDiskRead(d *schema.ResourceData, meta interface{}) error {
 	if disk.DiskEncryptionKey != nil && disk.DiskEncryptionKey.Sha256 != "" {
 		d.Set("disk_encryption_key_sha256", disk.DiskEncryptionKey.Sha256)
 	}
+	d.Set("users", disk.Users)
 
 	return nil
 }
@@ -193,6 +208,52 @@ func resourceComputeDiskDelete(d *schema.ResourceData, meta interface{}) error {
 		return err
 	}
 
+	// if disks are attached, they must be detached before the disk can be deleted
+	if instances, ok := d.Get("users").([]interface{}); ok {
+		type detachArgs struct{ project, zone, instance, deviceName string }
+		var detachCalls []detachArgs
+		self := d.Get("self_link").(string)
+		for _, instance := range instances {
+			if !computeDiskUserRegex.MatchString(instance.(string)) {
+				return fmt.Errorf("Unknown user %q of disk %q", instance, self)
+			}
+			matches := computeDiskUserRegex.FindStringSubmatch(instance.(string))
+			instanceProject := matches[1]
+			instanceZone := matches[2]
+			instanceName := matches[3]
+			i, err := config.clientCompute.Instances.Get(instanceProject, instanceZone, instanceName).Do()
+			if err != nil {
+				if gerr, ok := err.(*googleapi.Error); ok && gerr.Code == 404 {
+					log.Printf("[WARN] instance %q not found, not bothering to detach disks", instance.(string))
+					continue
+				}
+				return fmt.Errorf("Error retrieving instance %s: %s", instance.(string), err.Error())
+			}
+			for _, disk := range i.Disks {
+				if disk.Source == self {
+					detachCalls = append(detachCalls, detachArgs{
+						project:    project,
+						zone:       i.Zone,
+						instance:   i.Name,
+						deviceName: disk.DeviceName,
+					})
+				}
+			}
+		}
+		for _, call := range detachCalls {
+			op, err := config.clientCompute.Instances.DetachDisk(call.project, call.zone, call.instance, call.deviceName).Do()
+			if err != nil {
+				return fmt.Errorf("Error detaching disk %s from instance %s/%s/%s: %s", call.deviceName, call.project,
+					call.zone, call.instance, err.Error())
+			}
+			err = computeOperationWaitZone(config, op, call.project, call.zone,
+				fmt.Sprintf("Detaching disk from %s/%s/%s", call.project, call.zone, call.instance))
+			if err != nil {
+				return err
+			}
+		}
+	}
+
 	// Delete the disk
 	op, err := config.clientCompute.Disks.Delete(
 		project, d.Get("zone").(string), d.Id()).Do()

builtin/providers/google/resource_compute_disk_test.go

@@ -2,6 +2,7 @@ package google
 
 import (
 	"fmt"
+	"strconv"
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/acctest"
@@ -52,6 +53,40 @@ func TestAccComputeDisk_encryption(t *testing.T) {
 	})
 }
 
+func TestAccComputeDisk_deleteDetach(t *testing.T) {
+	diskName := fmt.Sprintf("tf-test-%s", acctest.RandString(10))
+	instanceName := fmt.Sprintf("tf-test-%s", acctest.RandString(10))
+	var disk compute.Disk
+
+	resource.Test(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckComputeDiskDestroy,
+		Steps: []resource.TestStep{
+			resource.TestStep{
+				Config: testAccComputeDisk_deleteDetach(instanceName, diskName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeDiskExists(
+						"google_compute_disk.foo", &disk),
+				),
+			},
+			// this needs to be a second step so we refresh and see the instance
+			// listed as attached to the disk; the instance is created after the
+			// disk. and the disk's properties aren't refreshed unless there's
+			// another step
+			resource.TestStep{
+				Config: testAccComputeDisk_deleteDetach(instanceName, diskName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckComputeDiskExists(
+						"google_compute_disk.foo", &disk),
+					testAccCheckComputeDiskInstances(
+						"google_compute_disk.foo", &disk),
+				),
+			},
+		},
+	})
+}
+
 func testAccCheckComputeDiskDestroy(s *terraform.State) error {
 	config := testAccProvider.Meta().(*Config)
 
@@ -119,6 +154,28 @@ func testAccCheckEncryptionKey(n string, disk *compute.Disk) resource.TestCheckF
 	}
 }
 
+func testAccCheckComputeDiskInstances(n string, disk *compute.Disk) resource.TestCheckFunc {
+	return func(s *terraform.State) error {
+		rs, ok := s.RootModule().Resources[n]
+		if !ok {
+			return fmt.Errorf("Not found: %s", n)
+		}
+
+		attr := rs.Primary.Attributes["users.#"]
+		if strconv.Itoa(len(disk.Users)) != attr {
+			return fmt.Errorf("Disk %s has mismatched users.\nTF State: %+v\nGCP State: %+v", n, rs.Primary.Attributes["users"], disk.Users)
+		}
+
+		for pos, user := range disk.Users {
+			if rs.Primary.Attributes["users."+strconv.Itoa(pos)] != user {
+				return fmt.Errorf("Disk %s has mismatched users.\nTF State: %+v.\nGCP State: %+v",
+					n, rs.Primary.Attributes["users"], disk.Users)
+			}
+		}
+		return nil
+	}
+}
+
 func testAccComputeDisk_basic(diskName string) string {
 	return fmt.Sprintf(`
 resource "google_compute_disk" "foobar" {
@@ -141,3 +198,33 @@ resource "google_compute_disk" "foobar" {
 	disk_encryption_key_raw = "SGVsbG8gZnJvbSBHb29nbGUgQ2xvdWQgUGxhdGZvcm0="
 }`, diskName)
 }
+
+func testAccComputeDisk_deleteDetach(instanceName, diskName string) string {
+	return fmt.Sprintf(`
+resource "google_compute_disk" "foo" {
+	name = "%s"
+	image = "debian-8"
+	size = 50
+	type = "pd-ssd"
+	zone = "us-central1-a"
+}
+
+resource "google_compute_instance" "bar" {
+	name = "%s"
+	machine_type = "n1-standard-1"
+	zone = "us-central1-a"
+
+	disk {
+		image = "debian-8"
+	}
+
+	disk {
+		disk = "${google_compute_disk.foo.name}"
+		auto_delete = false
+	}
+
+	network_interface {
+		network = "default"
+	}
+}`, diskName, instanceName)
+}