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.
This commit is contained in:
Paddy 2017-05-18 17:28:16 -07:00
parent 36aff9a2c1
commit 127283a858
2 changed files with 148 additions and 0 deletions

View File

@ -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()

View File

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