From 6087c35ddab6e6c568f2c79e65313f6c3c3c8886 Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Tue, 8 Aug 2017 11:28:05 -0700 Subject: [PATCH 01/11] Add state migration from disk to boot_disk/scratch_disk/attached_disk --- google/resource_compute_instance.go | 2 +- google/resource_compute_instance_migrate.go | 292 ++++++++++++++++++ .../resource_compute_instance_migrate_test.go | 50 ++- 3 files changed, 342 insertions(+), 2 deletions(-) diff --git a/google/resource_compute_instance.go b/google/resource_compute_instance.go index e0674888c49..ad0f94f51fd 100644 --- a/google/resource_compute_instance.go +++ b/google/resource_compute_instance.go @@ -40,7 +40,7 @@ func resourceComputeInstance() *schema.Resource { Update: resourceComputeInstanceUpdate, Delete: resourceComputeInstanceDelete, - SchemaVersion: 2, + SchemaVersion: 3, MigrateState: resourceComputeInstanceMigrateState, Schema: map[string]*schema.Schema{ diff --git a/google/resource_compute_instance_migrate.go b/google/resource_compute_instance_migrate.go index 2b463f9a802..d5f4fe8b983 100644 --- a/google/resource_compute_instance_migrate.go +++ b/google/resource_compute_instance_migrate.go @@ -1,11 +1,15 @@ package google import ( + "crypto/sha256" + "encoding/base64" "fmt" "log" "strconv" "strings" + compute "google.golang.org/api/compute/v1" + "github.com/hashicorp/terraform/helper/hashcode" "github.com/hashicorp/terraform/terraform" ) @@ -39,6 +43,13 @@ func resourceComputeInstanceMigrateState( return is, err } return is, nil + case 3: + log.Println("[INFO] Found Compute Instance State v3; migrating to v4") + is, err := migrateStateV3toV4(is, meta) + if err != nil { + return is, err + } + return is, nil default: return is, fmt.Errorf("Unexpected schema version: %d", v) } @@ -152,3 +163,284 @@ func migrateStateV2toV3(is *terraform.InstanceState) (*terraform.InstanceState, log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes) return is, nil } + +func migrateStateV3toV4(is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { + log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes) + + // Read instance from GCP. Since disks are not necessarily returned from the API in the order they were set, + // we have no other way to know which source belongs to which attached disk. + // Also note that the following code modifies the returned instance- if you need immutability, please change + // this to make a copy of the needed data. + config := meta.(*Config) + instance, err := getInstanceFromInstanceState(config, is) + if err != nil { + return is, fmt.Errorf("migration error: %s", err) + } + diskList, err := getAllDisksFromInstanceState(config, is) + if err != nil { + return is, fmt.Errorf("migration error: %s", err) + } + allDisks := make(map[string]*compute.Disk) + for _, disk := range diskList { + allDisks[disk.Name] = disk + } + + hasBootDisk := is.Attributes["boot_disk.#"] == "1" + + scratchDisks := 0 + if v := is.Attributes["scratch_disk.#"]; v != "" { + scratchDisks, err = strconv.Atoi(v) + if err != nil { + return is, fmt.Errorf("migration error: found scratch_disk.# value in unexpected format", err) + } + } + + attachedDisks := 0 + if v := is.Attributes["attached_disk.#"]; v != "" { + attachedDisks, err = strconv.Atoi(v) + if err != nil { + return is, fmt.Errorf("migration error: found attached_disk.# value in unexpected format", err) + } + } + + disks, err := strconv.Atoi(is.Attributes["disk.#"]) + if err != nil { + return is, fmt.Errorf("migration error: found disk.# value in unexpected format", err) + } + + for i := 0; i < disks; i++ { + if !hasBootDisk && i == 0 { + is.Attributes["boot_disk.#"] = "1" + + // Note: the GCP API does not allow for scratch disks to be boot disks, so this situation + // should never occur. + if is.Attributes["disk.0.scratch_disk"] == "true" { + return is, fmt.Errorf("migration error: found scratch disk at index 0") + } + + for _, disk := range instance.Disks { + if disk.Boot { + sourceUrl := strings.Split(disk.Source, "/") + is.Attributes["boot_disk.0.source"] = sourceUrl[len(sourceUrl)-1] + is.Attributes["boot_disk.0.device_name"] = disk.DeviceName + break + } + } + is.Attributes["boot_disk.0.auto_delete"] = is.Attributes["disk.0.auto_delete"] + is.Attributes["boot_disk.0.disk_encryption_key_raw"] = is.Attributes["disk.0.disk_encryption_key_raw"] + is.Attributes["boot_disk.0.disk_encryption_key_sha256"] = is.Attributes["disk.0.disk_encryption_key_sha256"] + + // Don't worry about initialize_params, since the disk has already been created. + } else if is.Attributes[fmt.Sprintf("disk.%d.scratch", i)] == "true" { + // Note: the GCP API does not allow for scratch disks without auto_delete, so this situation + // should never occur. + if is.Attributes[fmt.Sprintf("disk.%d.auto_delete", i)] != "true" { + return is, fmt.Errorf("migration error: attempted to migrate scratch disk where auto_delete is not true") + } + + is.Attributes[fmt.Sprintf("scratch_disk.%d.interface", scratchDisks)] = "SCSI" + + scratchDisks++ + } else { + // If disk is neither boot nor scratch, then it is attached. + + disk, err := getDiskFromAttributes(config, instance, allDisks, is.Attributes, i) + if err != nil { + return is, fmt.Errorf("migration error: %s", err) + } + + is.Attributes[fmt.Sprintf("attached_disk.%d.source", attachedDisks)] = disk.Source + is.Attributes[fmt.Sprintf("attached_disk.%d.device_name", attachedDisks)] = disk.DeviceName + is.Attributes[fmt.Sprintf("attached_disk.%d.disk_encryption_key_raw", attachedDisks)] = is.Attributes[fmt.Sprintf("disk.%d.disk_encryption_key_raw", i)] + is.Attributes[fmt.Sprintf("attached_disk.%d.disk_encryption_key_sha256", attachedDisks)] = is.Attributes[fmt.Sprintf("disk.%d.disk_encryption_key_sha256", i)] + + attachedDisks++ + } + } + + for k, _ := range is.Attributes { + if !strings.HasPrefix(k, "disk.") { + continue + } + + delete(is.Attributes, k) + } + if scratchDisks > 0 { + is.Attributes["scratch_disk.#"] = strconv.Itoa(scratchDisks) + } + if attachedDisks > 0 { + is.Attributes["attached_disk.#"] = strconv.Itoa(attachedDisks) + } + + log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes) + return is, nil +} + +func getInstanceFromInstanceState(config *Config, is *terraform.InstanceState) (*compute.Instance, error) { + project, ok := is.Attributes["project"] + if !ok && config.Project == "" { + return nil, fmt.Errorf("could not determine 'project'") + } + project = config.Project + + zone, ok := is.Attributes["zone"] + if !ok { + return nil, fmt.Errorf("could not determine 'zone'") + } + + instance, err := config.clientCompute.Instances.Get( + project, zone, is.ID).Do() + if err != nil { + return nil, fmt.Errorf("error reading instance: %s", err) + } + + return instance, nil +} + +func getAllDisksFromInstanceState(config *Config, is *terraform.InstanceState) ([]*compute.Disk, error) { + project, ok := is.Attributes["project"] + if !ok && config.Project == "" { + return nil, fmt.Errorf("could not determine 'project'") + } + project = config.Project + + zone, ok := is.Attributes["zone"] + if !ok { + return nil, fmt.Errorf("could not determine 'zone'") + } + + diskList := []*compute.Disk{} + token := "" + for { + disks, err := config.clientCompute.Disks.List(project, zone).PageToken(token).Do() + if err != nil { + return nil, fmt.Errorf("error reading disks: %s", err) + } + diskList = append(diskList, disks.Items...) + token = disks.NextPageToken + if token == "" { + break + } + } + + return diskList, nil +} + +func getDiskFromAttributes(config *Config, instance *compute.Instance, allDisks map[string]*compute.Disk, attributes map[string]string, i int) (*compute.AttachedDisk, error) { + if diskSource := attributes[fmt.Sprintf("disk.%d.disk", i)]; diskSource != "" { + return getDiskFromSource(instance, diskSource) + } + + if deviceName := attributes[fmt.Sprintf("disk.%d.device_name", i)]; deviceName != "" { + return getDiskFromDeviceName(instance, deviceName) + } + + if encryptionKey := attributes[fmt.Sprintf("disk.%d.disk_encryption_key_raw", i)]; encryptionKey != "" { + return getDiskFromEncryptionKey(instance, encryptionKey) + } + + autoDelete, err := strconv.ParseBool(attributes[fmt.Sprintf("disk.%d.auto_delete", i)]) + if err != nil { + return nil, fmt.Errorf("error parsing auto_delete attribute of disk %d", i) + } + image := attributes[fmt.Sprintf("disk.%d.image", i)] + + // We know project and zone are set because we used them to read the instance + project := attributes["project"] + zone := attributes["zone"] + return getDiskFromAutoDeleteAndImage(config, instance, allDisks, autoDelete, image, project, zone) +} + +func getDiskFromSource(instance *compute.Instance, source string) (*compute.AttachedDisk, error) { + for _, disk := range instance.Disks { + if disk.Boot == true || disk.Type == "SCRATCH" { + // Ignore boot/scratch disks since this is just for finding attached disks + continue + } + // we can just compare suffixes because terraform only allows setting "disk" by name and uses + // the zone of the instance so we know there can be no duplicate names. + if strings.HasSuffix(disk.Source, "/"+source) { + return disk, nil + } + } + return nil, fmt.Errorf("could not find attached disk with source %q", source) +} + +func getDiskFromDeviceName(instance *compute.Instance, deviceName string) (*compute.AttachedDisk, error) { + for _, disk := range instance.Disks { + if disk.Boot == true || disk.Type == "SCRATCH" { + // Ignore boot/scratch disks since this is just for finding attached disks + continue + } + if disk.DeviceName == deviceName { + return disk, nil + } + } + return nil, fmt.Errorf("could not find attached disk with deviceName %q", deviceName) +} + +func getDiskFromEncryptionKey(instance *compute.Instance, encryptionKey string) (*compute.AttachedDisk, error) { + sha := sha256.Sum256([]byte(encryptionKey)) + encryptionSha := base64.StdEncoding.EncodeToString(sha[:]) + for _, disk := range instance.Disks { + if disk.Boot == true || disk.Type == "SCRATCH" { + // Ignore boot/scratch disks since this is just for finding attached disks + continue + } + if disk.DiskEncryptionKey.Sha256 == encryptionSha { + return disk, nil + } + } + return nil, fmt.Errorf("could not find attached disk with encryption hash %q", encryptionSha) +} + +func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, allDisks map[string]*compute.Disk, autoDelete bool, image, project, zone string) (*compute.AttachedDisk, error) { + + log.Printf("[DEBUG] AllDisks is %+v", allDisks) + for i, disk := range instance.Disks { + if disk.Boot == true || disk.Type == "SCRATCH" { + // Ignore boot/scratch disks since this is just for finding attached disks + continue + } + if disk.AutoDelete == autoDelete { + log.Printf("[DEBUG] Checking disk %+v", disk) + // Read the disk to check if its image matches + sourceUrl := strings.Split(disk.Source, "/") + fullDisk := allDisks[sourceUrl[len(sourceUrl)-1]] + fullDiskImageUrl := strings.Split(fullDisk.SourceImage, "/") + fullDiskImage := fullDiskImageUrl[len(fullDiskImageUrl)-1] + + if fullDiskImage == image { + // Delete this disk because there might be multiple that match + instance.Disks = append(instance.Disks[:i], instance.Disks[i+1:]...) + return disk, nil + } + } + } + + // We're not done! It's possible the disk was created with an image family rather than the image itself. + // Now, do the exact same iteration but do some prefix matching to check if the families match. + // This assumes that all disks with a given family are of the format family-name-{diskname}, because + // there's not much else we can do at this point. + for i, disk := range instance.Disks { + if disk.Boot == true || disk.Type == "SCRATCH" { + // Ignore boot/scratch disks since this is just for finding attached disks + continue + } + if disk.AutoDelete == autoDelete { + // Read the disk to check if its image matches + sourceUrl := strings.Split(disk.Source, "/") + fullDisk := allDisks[sourceUrl[len(sourceUrl)-1]] + fullDiskImageUrl := strings.Split(fullDisk.SourceImage, "/") + fullDiskImage := fullDiskImageUrl[len(fullDiskImageUrl)-1] + + if strings.HasPrefix(fullDiskImage, image+"-") { + // Delete this disk because there might be multiple that match + instance.Disks = append(instance.Disks[:i], instance.Disks[i+1:]...) + return disk, nil + } + } + } + + return nil, fmt.Errorf("could not find attached disk with image %q", image) +} diff --git a/google/resource_compute_instance_migrate_test.go b/google/resource_compute_instance_migrate_test.go index bce44e63545..1197b1ed975 100644 --- a/google/resource_compute_instance_migrate_test.go +++ b/google/resource_compute_instance_migrate_test.go @@ -55,6 +55,46 @@ func TestComputeInstanceMigrateState(t *testing.T) { "create_timeout": "4", }, }, + "replace disk with boot disk": { + StateVersion: 3, + Attributes: map[string]string{ + "disk.#": "1", + "disk.0.disk": "disk-1", + "disk.0.type": "pd-ssd", + "disk.0.auto_delete": "false", + "disk.0.size": "12", + "disk.0.device_name": "device-name", + "disk.0.disk_encryption_key_raw": "encrypt-key", + "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + }, + Expected: map[string]string{ + "boot_disk.#": "1", + "boot_disk.0.auto_delete": "false", + "boot_disk.0.device_name": "device-name", + "boot_disk.0.disk_encryption_key_raw": "encrypt-key", + "boot_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + "boot_disk.0.source": "disk-1", + }, + }, + "replace disk with attached disk": { + StateVersion: 3, + Attributes: map[string]string{ + "boot_disk.#": "1", + "disk.#": "1", + "disk.0.disk": "path/to/disk", + "disk.0.device_name": "device-name", + "disk.0.disk_encryption_key_raw": "encrypt-key", + "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + }, + Expected: map[string]string{ + "boot_disk.#": "1", + "attached_disk.#": "1", + "attached_disk.0.source": "path/to/disk", + "attached_disk.0.device_name": "device-name", + "attached_disk.0.disk_encryption_key_raw": "encrypt-key", + "attached_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + }, + }, } for tn, tc := range cases { @@ -73,7 +113,15 @@ func TestComputeInstanceMigrateState(t *testing.T) { if is.Attributes[k] != v { t.Fatalf( "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", - tn, k, v, k, is.Attributes[k], is.Attributes) + tn, k, tc.Expected[k], k, is.Attributes[k], is.Attributes) + } + } + + for k, v := range is.Attributes { + if tc.Expected[k] != v { + t.Fatalf( + "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", + tn, k, tc.Expected[k], k, is.Attributes[k], is.Attributes) } } } From 820a04cd745b2ca9eab0ff7e1186b3e640910041 Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Wed, 16 Aug 2017 10:15:20 -0700 Subject: [PATCH 02/11] get rid of test for now --- .../resource_compute_instance_migrate_test.go | 78 +++++++++---------- 1 file changed, 39 insertions(+), 39 deletions(-) diff --git a/google/resource_compute_instance_migrate_test.go b/google/resource_compute_instance_migrate_test.go index 1197b1ed975..7460b5db898 100644 --- a/google/resource_compute_instance_migrate_test.go +++ b/google/resource_compute_instance_migrate_test.go @@ -55,45 +55,45 @@ func TestComputeInstanceMigrateState(t *testing.T) { "create_timeout": "4", }, }, - "replace disk with boot disk": { - StateVersion: 3, - Attributes: map[string]string{ - "disk.#": "1", - "disk.0.disk": "disk-1", - "disk.0.type": "pd-ssd", - "disk.0.auto_delete": "false", - "disk.0.size": "12", - "disk.0.device_name": "device-name", - "disk.0.disk_encryption_key_raw": "encrypt-key", - "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", - }, - Expected: map[string]string{ - "boot_disk.#": "1", - "boot_disk.0.auto_delete": "false", - "boot_disk.0.device_name": "device-name", - "boot_disk.0.disk_encryption_key_raw": "encrypt-key", - "boot_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", - "boot_disk.0.source": "disk-1", - }, - }, - "replace disk with attached disk": { - StateVersion: 3, - Attributes: map[string]string{ - "boot_disk.#": "1", - "disk.#": "1", - "disk.0.disk": "path/to/disk", - "disk.0.device_name": "device-name", - "disk.0.disk_encryption_key_raw": "encrypt-key", - "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", - }, - Expected: map[string]string{ - "boot_disk.#": "1", - "attached_disk.#": "1", - "attached_disk.0.source": "path/to/disk", - "attached_disk.0.device_name": "device-name", - "attached_disk.0.disk_encryption_key_raw": "encrypt-key", - "attached_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", - }, + // "replace disk with boot disk": { + // StateVersion: 3, + // Attributes: map[string]string{ + // "disk.#": "1", + // "disk.0.disk": "disk-1", + // "disk.0.type": "pd-ssd", + // "disk.0.auto_delete": "false", + // "disk.0.size": "12", + // "disk.0.device_name": "device-name", + // "disk.0.disk_encryption_key_raw": "encrypt-key", + // "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + // }, + // Expected: map[string]string{ + // "boot_disk.#": "1", + // "boot_disk.0.auto_delete": "false", + // "boot_disk.0.device_name": "device-name", + // "boot_disk.0.disk_encryption_key_raw": "encrypt-key", + // "boot_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + // "boot_disk.0.source": "disk-1", + // }, + // }, + // "replace disk with attached disk": { + // StateVersion: 3, + // Attributes: map[string]string{ + // "boot_disk.#": "1", + // "disk.#": "1", + // "disk.0.disk": "path/to/disk", + // "disk.0.device_name": "device-name", + // "disk.0.disk_encryption_key_raw": "encrypt-key", + // "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + // }, + // Expected: map[string]string{ + // "boot_disk.#": "1", + // "attached_disk.#": "1", + // "attached_disk.0.source": "path/to/disk", + // "attached_disk.0.device_name": "device-name", + // "attached_disk.0.disk_encryption_key_raw": "encrypt-key", + // "attached_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + // }, }, } From a662479a4daf13e5afe8b1206335d711cd036e1e Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Wed, 16 Aug 2017 12:17:07 -0700 Subject: [PATCH 03/11] update schema version --- google/resource_compute_instance.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/resource_compute_instance.go b/google/resource_compute_instance.go index ad0f94f51fd..269c801eafd 100644 --- a/google/resource_compute_instance.go +++ b/google/resource_compute_instance.go @@ -40,7 +40,7 @@ func resourceComputeInstance() *schema.Resource { Update: resourceComputeInstanceUpdate, Delete: resourceComputeInstanceDelete, - SchemaVersion: 3, + SchemaVersion: 4, MigrateState: resourceComputeInstanceMigrateState, Schema: map[string]*schema.Schema{ From 443c161b10d262dea9ad5c2d2af82abc26690ac7 Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Mon, 11 Sep 2017 18:33:15 +0800 Subject: [PATCH 04/11] add tests for migration --- .../resource_compute_instance_migrate_test.go | 346 ++++++++++++++---- 1 file changed, 280 insertions(+), 66 deletions(-) diff --git a/google/resource_compute_instance_migrate_test.go b/google/resource_compute_instance_migrate_test.go index 7460b5db898..9bbf470f29b 100644 --- a/google/resource_compute_instance_migrate_test.go +++ b/google/resource_compute_instance_migrate_test.go @@ -1,8 +1,13 @@ package google import ( + "fmt" + "log" "testing" + compute "google.golang.org/api/compute/v1" + + "github.com/hashicorp/terraform/helper/acctest" "github.com/hashicorp/terraform/terraform" ) @@ -55,75 +60,10 @@ func TestComputeInstanceMigrateState(t *testing.T) { "create_timeout": "4", }, }, - // "replace disk with boot disk": { - // StateVersion: 3, - // Attributes: map[string]string{ - // "disk.#": "1", - // "disk.0.disk": "disk-1", - // "disk.0.type": "pd-ssd", - // "disk.0.auto_delete": "false", - // "disk.0.size": "12", - // "disk.0.device_name": "device-name", - // "disk.0.disk_encryption_key_raw": "encrypt-key", - // "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", - // }, - // Expected: map[string]string{ - // "boot_disk.#": "1", - // "boot_disk.0.auto_delete": "false", - // "boot_disk.0.device_name": "device-name", - // "boot_disk.0.disk_encryption_key_raw": "encrypt-key", - // "boot_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", - // "boot_disk.0.source": "disk-1", - // }, - // }, - // "replace disk with attached disk": { - // StateVersion: 3, - // Attributes: map[string]string{ - // "boot_disk.#": "1", - // "disk.#": "1", - // "disk.0.disk": "path/to/disk", - // "disk.0.device_name": "device-name", - // "disk.0.disk_encryption_key_raw": "encrypt-key", - // "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", - // }, - // Expected: map[string]string{ - // "boot_disk.#": "1", - // "attached_disk.#": "1", - // "attached_disk.0.source": "path/to/disk", - // "attached_disk.0.device_name": "device-name", - // "attached_disk.0.disk_encryption_key_raw": "encrypt-key", - // "attached_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", - // }, - }, } for tn, tc := range cases { - is := &terraform.InstanceState{ - ID: "i-abc123", - Attributes: tc.Attributes, - } - is, err := resourceComputeInstanceMigrateState( - tc.StateVersion, is, tc.Meta) - - if err != nil { - t.Fatalf("bad: %s, err: %#v", tn, err) - } - - for k, v := range tc.Expected { - if is.Attributes[k] != v { - t.Fatalf( - "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", - tn, k, tc.Expected[k], k, is.Attributes[k], is.Attributes) - } - } - - for k, v := range is.Attributes { - if tc.Expected[k] != v { - t.Fatalf( - "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", - tn, k, tc.Expected[k], k, is.Attributes[k], is.Attributes) - } - } + runInstanceMigrateTest(t, "i-abc123", tn, tc.StateVersion, tc.Attributes, tc.Expected, tc.Meta) } } @@ -149,3 +89,277 @@ func TestComputeInstanceMigrateState_empty(t *testing.T) { t.Fatalf("err: %#v", err) } } + +func TestComputeInstanceMigrateState_bootDisk(t *testing.T) { + config := getInitializedConfig(t) + zone := "us-central1-f" + + // Seed test data + instanceName := fmt.Sprintf("instance-test-%s", acctest.RandString(10)) + instance := &compute.Instance{ + Name: instanceName, + Disks: []*compute.AttachedDisk{ + { + Boot: true, + InitializeParams: &compute.AttachedDiskInitializeParams{ + SourceImage: "projects/debian-cloud/global/images/family/debian-8", + }, + }, + }, + MachineType: "zones/" + zone + "/machineTypes/n1-standard-1", + NetworkInterfaces: []*compute.NetworkInterface{ + { + Network: "global/networks/default", + }, + }, + } + op, err := config.clientCompute.Instances.Insert(config.Project, zone, instance).Do() + + if err != nil { + t.Fatalf("Error creating instance: %s", err) + } + waitErr := computeSharedOperationWait(config, op, config.Project, "instance to create") + if waitErr != nil { + t.Fatal(waitErr) + } + defer cleanUpInstance(config, instanceName, zone) + + attributes := map[string]string{ + "disk.#": "1", + "disk.0.disk": "disk-1", + "disk.0.type": "pd-ssd", + "disk.0.auto_delete": "false", + "disk.0.size": "12", + "disk.0.device_name": "persistent-disk-0", + "disk.0.disk_encryption_key_raw": "encrypt-key", + "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + "zone": zone, + } + expected := map[string]string{ + "boot_disk.#": "1", + "boot_disk.0.auto_delete": "false", + "boot_disk.0.device_name": "persistent-disk-0", + "boot_disk.0.disk_encryption_key_raw": "encrypt-key", + "boot_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + "boot_disk.0.source": instanceName, + "zone": zone, + } + + runInstanceMigrateTest(t, instanceName, "migrate disk to boot disk", 3 /* state version */, attributes, expected, config) +} + +func TestComputeInstanceMigrateState_attachedDisk(t *testing.T) { + config := getInitializedConfig(t) + zone := "us-central1-f" + + // Seed test data + diskName := fmt.Sprintf("instance-test-%s", acctest.RandString(10)) + disk := &compute.Disk{ + Name: diskName, + SourceImage: "projects/debian-cloud/global/images/family/debian-8", + Zone: zone, + } + op, err := config.clientCompute.Disks.Insert(config.Project, zone, disk).Do() + if err != nil { + t.Fatalf("Error creating disk: %s", err) + } + waitErr := computeSharedOperationWait(config, op, config.Project, "disk to create") + if waitErr != nil { + t.Fatal(waitErr) + } + defer cleanUpDisk(config, diskName, zone) + + instanceName := fmt.Sprintf("instance-test-%s", acctest.RandString(10)) + instance := &compute.Instance{ + Name: instanceName, + Disks: []*compute.AttachedDisk{ + { + Boot: true, + InitializeParams: &compute.AttachedDiskInitializeParams{ + SourceImage: "projects/debian-cloud/global/images/family/debian-8", + }, + }, + { + Source: "projects/" + config.Project + "/zones/" + zone + "/disks/" + diskName, + }, + }, + MachineType: "zones/" + zone + "/machineTypes/n1-standard-1", + NetworkInterfaces: []*compute.NetworkInterface{ + { + Network: "global/networks/default", + }, + }, + } + op, err = config.clientCompute.Instances.Insert(config.Project, zone, instance).Do() + if err != nil { + t.Fatalf("Error creating instance: %s", err) + } + waitErr = computeSharedOperationWait(config, op, config.Project, "instance to create") + if waitErr != nil { + t.Fatal(waitErr) + } + defer cleanUpInstance(config, instanceName, zone) + + attributes := map[string]string{ + "boot_disk.#": "1", + "disk.#": "1", + "disk.0.disk": diskName, + "disk.0.device_name": "persistent-disk-1", + "disk.0.disk_encryption_key_raw": "encrypt-key", + "disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + "zone": zone, + } + expected := map[string]string{ + "boot_disk.#": "1", + "attached_disk.#": "1", + "attached_disk.0.source": "https://www.googleapis.com/compute/v1/projects/" + config.Project + "/zones/" + zone + "/disks/" + diskName, + "attached_disk.0.device_name": "persistent-disk-1", + "attached_disk.0.disk_encryption_key_raw": "encrypt-key", + "attached_disk.0.disk_encryption_key_sha256": "encrypt-key-sha", + "zone": zone, + } + + runInstanceMigrateTest(t, instanceName, "migrate disk to attached disk", 3 /* state version */, attributes, expected, config) +} + +func TestComputeInstanceMigrateState_scratchDisk(t *testing.T) { + config := getInitializedConfig(t) + zone := "us-central1-f" + + // Seed test data + instanceName := fmt.Sprintf("instance-test-%s", acctest.RandString(10)) + instance := &compute.Instance{ + Name: instanceName, + Disks: []*compute.AttachedDisk{ + { + Boot: true, + InitializeParams: &compute.AttachedDiskInitializeParams{ + SourceImage: "projects/debian-cloud/global/images/family/debian-8", + }, + }, + { + AutoDelete: true, + Type: "SCRATCH", + InitializeParams: &compute.AttachedDiskInitializeParams{ + DiskType: "zones/" + zone + "/diskTypes/local-ssd", + }, + }, + }, + MachineType: "zones/" + zone + "/machineTypes/n1-standard-1", + NetworkInterfaces: []*compute.NetworkInterface{ + { + Network: "global/networks/default", + }, + }, + } + op, err := config.clientCompute.Instances.Insert(config.Project, zone, instance).Do() + if err != nil { + t.Fatalf("Error creating instance: %s", err) + } + waitErr := computeSharedOperationWait(config, op, config.Project, "instance to create") + if waitErr != nil { + t.Fatal(waitErr) + } + defer cleanUpInstance(config, instanceName, zone) + + attributes := map[string]string{ + "boot_disk.#": "1", + "disk.#": "1", + "disk.0.auto_delete": "true", + "disk.0.type": "local-ssd", + "disk.0.scratch": "true", + "zone": zone, + } + expected := map[string]string{ + "boot_disk.#": "1", + "scratch_disk.#": "1", + "scratch_disk.0.interface": "SCSI", + "zone": zone, + } + + runInstanceMigrateTest(t, instanceName, "migrate disk to scratch disk", 3 /* state version */, attributes, expected, config) +} + +func runInstanceMigrateTest(t *testing.T, id, testName string, version int, attributes, expected map[string]string, meta interface{}) { + is := &terraform.InstanceState{ + ID: id, + Attributes: attributes, + } + is, err = resourceComputeInstanceMigrateState(version, is, meta) + if err != nil { + t.Fatal(err) + } + + for k, v := range expected { + if attributes[k] != v { + t.Fatalf( + "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", + testName, k, expected[k], k, attributes[k], attributes) + } + } + + for k, v := range attributes { + if expected[k] != v { + t.Fatalf( + "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", + testName, k, expected[k], k, attributes[k], attributes) + } + } +} + +func cleanUpInstance(config *Config, instanceName, zone string) { + op, err := config.clientCompute.Instances.Delete(config.Project, zone, instanceName).Do() + if err != nil { + log.Printf("[WARNING] Error deleting instance %q, dangling resources may exist: %s", instanceName, err) + return + } + + // Wait for the operation to complete + opErr := computeOperationWait(config, op, config.Project, "instance to delete") + if opErr != nil { + log.Printf("[WARNING] Error deleting instance %q, dangling resources may exist: %s", instanceName, opErr) + } +} + +func cleanUpDisk(config *Config, diskName, zone string) { + op, err := config.clientCompute.Disks.Delete(config.Project, zone, diskName).Do() + if err != nil { + log.Printf("[WARNING] Error deleting disk %q, dangling resources may exist: %s", diskName, err) + return + } + + // Wait for the operation to complete + opErr := computeOperationWait(config, op, config.Project, "disk to delete") + if opErr != nil { + log.Printf("[WARNING] Error deleting disk %q, dangling resources may exist: %s", diskName, opErr) + } +} + +func getInitializedConfig(t *testing.T) *Config { + // Check that all required environment variables are set + testAccPreCheck(t) + + project := multiEnvSearch([]string{"GOOGLE_PROJECT", "GCLOUD_PROJECT", "CLOUDSDK_CORE_PROJECT"}) + creds := multiEnvSearch([]string{ + "GOOGLE_CREDENTIALS", + "GOOGLE_CLOUD_KEYFILE_JSON", + "GCLOUD_KEYFILE_JSON", + "GOOGLE_USE_DEFAULT_CREDENTIALS", + }) + region := multiEnvSearch([]string{ + "GOOGLE_REGION", + "GCLOUD_REGION", + "CLOUDSDK_COMPUTE_REGION", + }) + + config := &Config{ + Project: project, + Credentials: creds, + Region: region, + } + err := config.loadAndValidate() + if err != nil { + t.Fatal(err) + } + return config +} From 693661563501d075e28e448d79db407e46c758a5 Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Tue, 12 Sep 2017 17:17:39 +0800 Subject: [PATCH 05/11] fix travis errors --- google/resource_compute_instance_migrate.go | 6 +++--- .../resource_compute_instance_migrate_test.go | 17 ++++++++++++++--- 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/google/resource_compute_instance_migrate.go b/google/resource_compute_instance_migrate.go index d5f4fe8b983..b835b7f8117 100644 --- a/google/resource_compute_instance_migrate.go +++ b/google/resource_compute_instance_migrate.go @@ -191,7 +191,7 @@ func migrateStateV3toV4(is *terraform.InstanceState, meta interface{}) (*terrafo if v := is.Attributes["scratch_disk.#"]; v != "" { scratchDisks, err = strconv.Atoi(v) if err != nil { - return is, fmt.Errorf("migration error: found scratch_disk.# value in unexpected format", err) + return is, fmt.Error("migration error: found scratch_disk.# value in unexpected format", err) } } @@ -199,13 +199,13 @@ func migrateStateV3toV4(is *terraform.InstanceState, meta interface{}) (*terrafo if v := is.Attributes["attached_disk.#"]; v != "" { attachedDisks, err = strconv.Atoi(v) if err != nil { - return is, fmt.Errorf("migration error: found attached_disk.# value in unexpected format", err) + return is, fmt.Error("migration error: found attached_disk.# value in unexpected format", err) } } disks, err := strconv.Atoi(is.Attributes["disk.#"]) if err != nil { - return is, fmt.Errorf("migration error: found disk.# value in unexpected format", err) + return is, fmt.Error("migration error: found disk.# value in unexpected format", err) } for i := 0; i < disks; i++ { diff --git a/google/resource_compute_instance_migrate_test.go b/google/resource_compute_instance_migrate_test.go index 9bbf470f29b..392f1cb90ff 100644 --- a/google/resource_compute_instance_migrate_test.go +++ b/google/resource_compute_instance_migrate_test.go @@ -3,11 +3,13 @@ package google import ( "fmt" "log" + "os" "testing" compute "google.golang.org/api/compute/v1" "github.com/hashicorp/terraform/helper/acctest" + "github.com/hashicorp/terraform/helper/resource" "github.com/hashicorp/terraform/terraform" ) @@ -90,7 +92,10 @@ func TestComputeInstanceMigrateState_empty(t *testing.T) { } } -func TestComputeInstanceMigrateState_bootDisk(t *testing.T) { +func TestAccComputeInstanceMigrateState_bootDisk(t *testing.T) { + if os.Getenv(resource.TestEnvVar) == "" { + t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", resource.TestEnvVar)) + } config := getInitializedConfig(t) zone := "us-central1-f" @@ -148,7 +153,10 @@ func TestComputeInstanceMigrateState_bootDisk(t *testing.T) { runInstanceMigrateTest(t, instanceName, "migrate disk to boot disk", 3 /* state version */, attributes, expected, config) } -func TestComputeInstanceMigrateState_attachedDisk(t *testing.T) { +func TestAccComputeInstanceMigrateState_attachedDisk(t *testing.T) { + if os.Getenv(resource.TestEnvVar) == "" { + t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", resource.TestEnvVar)) + } config := getInitializedConfig(t) zone := "us-central1-f" @@ -222,7 +230,10 @@ func TestComputeInstanceMigrateState_attachedDisk(t *testing.T) { runInstanceMigrateTest(t, instanceName, "migrate disk to attached disk", 3 /* state version */, attributes, expected, config) } -func TestComputeInstanceMigrateState_scratchDisk(t *testing.T) { +func TestAccComputeInstanceMigrateState_scratchDisk(t *testing.T) { + if os.Getenv(resource.TestEnvVar) == "" { + t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", resource.TestEnvVar)) + } config := getInitializedConfig(t) zone := "us-central1-f" From 8a23b6ad3ecad833704993358ed2f66fb3e1181d Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Tue, 12 Sep 2017 17:20:47 +0800 Subject: [PATCH 06/11] actually fix travis errors --- google/resource_compute_instance_migrate.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/google/resource_compute_instance_migrate.go b/google/resource_compute_instance_migrate.go index b835b7f8117..9dd07ae7c78 100644 --- a/google/resource_compute_instance_migrate.go +++ b/google/resource_compute_instance_migrate.go @@ -191,7 +191,7 @@ func migrateStateV3toV4(is *terraform.InstanceState, meta interface{}) (*terrafo if v := is.Attributes["scratch_disk.#"]; v != "" { scratchDisks, err = strconv.Atoi(v) if err != nil { - return is, fmt.Error("migration error: found scratch_disk.# value in unexpected format", err) + return is, fmt.Errorf("migration error: found scratch_disk.# value in unexpected format: %s", err) } } @@ -199,13 +199,13 @@ func migrateStateV3toV4(is *terraform.InstanceState, meta interface{}) (*terrafo if v := is.Attributes["attached_disk.#"]; v != "" { attachedDisks, err = strconv.Atoi(v) if err != nil { - return is, fmt.Error("migration error: found attached_disk.# value in unexpected format", err) + return is, fmt.Errorf("migration error: found attached_disk.# value in unexpected format: %s", err) } } disks, err := strconv.Atoi(is.Attributes["disk.#"]) if err != nil { - return is, fmt.Error("migration error: found disk.# value in unexpected format", err) + return is, fmt.Errorf("migration error: found disk.# value in unexpected format: %s", err) } for i := 0; i < disks; i++ { From acb721f95cccfa111c7f32b2d991f7b1b2200290 Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Thu, 14 Sep 2017 13:15:25 +0800 Subject: [PATCH 07/11] fix logic when project is set, also remove some log statements --- google/resource_compute_instance_migrate.go | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/google/resource_compute_instance_migrate.go b/google/resource_compute_instance_migrate.go index 9dd07ae7c78..24cc8c47449 100644 --- a/google/resource_compute_instance_migrate.go +++ b/google/resource_compute_instance_migrate.go @@ -278,10 +278,13 @@ func migrateStateV3toV4(is *terraform.InstanceState, meta interface{}) (*terrafo func getInstanceFromInstanceState(config *Config, is *terraform.InstanceState) (*compute.Instance, error) { project, ok := is.Attributes["project"] - if !ok && config.Project == "" { - return nil, fmt.Errorf("could not determine 'project'") + if !ok { + if config.Project == "" { + return nil, fmt.Errorf("could not determine 'project'") + } else { + project = config.Project + } } - project = config.Project zone, ok := is.Attributes["zone"] if !ok { @@ -299,10 +302,13 @@ func getInstanceFromInstanceState(config *Config, is *terraform.InstanceState) ( func getAllDisksFromInstanceState(config *Config, is *terraform.InstanceState) ([]*compute.Disk, error) { project, ok := is.Attributes["project"] - if !ok && config.Project == "" { - return nil, fmt.Errorf("could not determine 'project'") + if !ok { + if config.Project == "" { + return nil, fmt.Errorf("could not determine 'project'") + } else { + project = config.Project + } } - project = config.Project zone, ok := is.Attributes["zone"] if !ok { @@ -395,15 +401,12 @@ func getDiskFromEncryptionKey(instance *compute.Instance, encryptionKey string) } func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, allDisks map[string]*compute.Disk, autoDelete bool, image, project, zone string) (*compute.AttachedDisk, error) { - - log.Printf("[DEBUG] AllDisks is %+v", allDisks) for i, disk := range instance.Disks { if disk.Boot == true || disk.Type == "SCRATCH" { // Ignore boot/scratch disks since this is just for finding attached disks continue } if disk.AutoDelete == autoDelete { - log.Printf("[DEBUG] Checking disk %+v", disk) // Read the disk to check if its image matches sourceUrl := strings.Split(disk.Source, "/") fullDisk := allDisks[sourceUrl[len(sourceUrl)-1]] From f0d911f9b840a8d1da41f7fec8da08b319df671b Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Thu, 14 Sep 2017 15:45:45 +0800 Subject: [PATCH 08/11] add tests for reading based on encryption key and image --- google/resource_compute_instance_migrate.go | 15 +- .../resource_compute_instance_migrate_test.go | 136 +++++++++++++++++- 2 files changed, 143 insertions(+), 8 deletions(-) diff --git a/google/resource_compute_instance_migrate.go b/google/resource_compute_instance_migrate.go index 24cc8c47449..32e1179f80c 100644 --- a/google/resource_compute_instance_migrate.go +++ b/google/resource_compute_instance_migrate.go @@ -1,8 +1,6 @@ package google import ( - "crypto/sha256" - "encoding/base64" "fmt" "log" "strconv" @@ -386,8 +384,10 @@ func getDiskFromDeviceName(instance *compute.Instance, deviceName string) (*comp } func getDiskFromEncryptionKey(instance *compute.Instance, encryptionKey string) (*compute.AttachedDisk, error) { - sha := sha256.Sum256([]byte(encryptionKey)) - encryptionSha := base64.StdEncoding.EncodeToString(sha[:]) + encryptionSha, err := hash256(encryptionKey) + if err != nil { + return nil, err + } for _, disk := range instance.Disks { if disk.Boot == true || disk.Type == "SCRATCH" { // Ignore boot/scratch disks since this is just for finding attached disks @@ -401,6 +401,8 @@ func getDiskFromEncryptionKey(instance *compute.Instance, encryptionKey string) } func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, allDisks map[string]*compute.Disk, autoDelete bool, image, project, zone string) (*compute.AttachedDisk, error) { + imageUrl := strings.Split(image, "/") + imageName := imageUrl[len(imageUrl)-1] for i, disk := range instance.Disks { if disk.Boot == true || disk.Type == "SCRATCH" { // Ignore boot/scratch disks since this is just for finding attached disks @@ -412,8 +414,7 @@ func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, a fullDisk := allDisks[sourceUrl[len(sourceUrl)-1]] fullDiskImageUrl := strings.Split(fullDisk.SourceImage, "/") fullDiskImage := fullDiskImageUrl[len(fullDiskImageUrl)-1] - - if fullDiskImage == image { + if fullDiskImage == imageName { // Delete this disk because there might be multiple that match instance.Disks = append(instance.Disks[:i], instance.Disks[i+1:]...) return disk, nil @@ -437,7 +438,7 @@ func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, a fullDiskImageUrl := strings.Split(fullDisk.SourceImage, "/") fullDiskImage := fullDiskImageUrl[len(fullDiskImageUrl)-1] - if strings.HasPrefix(fullDiskImage, image+"-") { + if strings.HasPrefix(fullDiskImage, imageName+"-") { // Delete this disk because there might be multiple that match instance.Disks = append(instance.Disks[:i], instance.Disks[i+1:]...) return disk, nil diff --git a/google/resource_compute_instance_migrate_test.go b/google/resource_compute_instance_migrate_test.go index 392f1cb90ff..11b00d23082 100644 --- a/google/resource_compute_instance_migrate_test.go +++ b/google/resource_compute_instance_migrate_test.go @@ -153,7 +153,7 @@ func TestAccComputeInstanceMigrateState_bootDisk(t *testing.T) { runInstanceMigrateTest(t, instanceName, "migrate disk to boot disk", 3 /* state version */, attributes, expected, config) } -func TestAccComputeInstanceMigrateState_attachedDisk(t *testing.T) { +func TestAccComputeInstanceMigrateState_attachedDiskFromSource(t *testing.T) { if os.Getenv(resource.TestEnvVar) == "" { t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", resource.TestEnvVar)) } @@ -230,6 +230,140 @@ func TestAccComputeInstanceMigrateState_attachedDisk(t *testing.T) { runInstanceMigrateTest(t, instanceName, "migrate disk to attached disk", 3 /* state version */, attributes, expected, config) } +func TestAccComputeInstanceMigrateState_attachedDiskFromEncryptionKey(t *testing.T) { + if os.Getenv(resource.TestEnvVar) == "" { + t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", resource.TestEnvVar)) + } + config := getInitializedConfig(t) + zone := "us-central1-f" + + instanceName := fmt.Sprintf("instance-test-%s", acctest.RandString(10)) + instance := &compute.Instance{ + Name: instanceName, + Disks: []*compute.AttachedDisk{ + { + Boot: true, + InitializeParams: &compute.AttachedDiskInitializeParams{ + SourceImage: "projects/debian-cloud/global/images/family/debian-8", + }, + }, + { + AutoDelete: true, + InitializeParams: &compute.AttachedDiskInitializeParams{ + SourceImage: "projects/debian-cloud/global/images/family/debian-8", + }, + DiskEncryptionKey: &compute.CustomerEncryptionKey{ + RawKey: "SGVsbG8gZnJvbSBHb29nbGUgQ2xvdWQgUGxhdGZvcm0=", + }, + }, + }, + MachineType: "zones/" + zone + "/machineTypes/n1-standard-1", + NetworkInterfaces: []*compute.NetworkInterface{ + { + Network: "global/networks/default", + }, + }, + } + op, err := config.clientCompute.Instances.Insert(config.Project, zone, instance).Do() + if err != nil { + t.Fatalf("Error creating instance: %s", err) + } + waitErr := computeSharedOperationWait(config, op, config.Project, "instance to create") + if waitErr != nil { + t.Fatal(waitErr) + } + defer cleanUpInstance(config, instanceName, zone) + + attributes := map[string]string{ + "boot_disk.#": "1", + "disk.#": "1", + "disk.0.image": "projects/debian-cloud/global/images/family/debian-8", + "disk.0.disk_encryption_key_raw": "SGVsbG8gZnJvbSBHb29nbGUgQ2xvdWQgUGxhdGZvcm0=", + "disk.0.disk_encryption_key_sha256": "esTuF7d4eatX4cnc4JsiEiaI+Rff78JgPhA/v1zxX9E=", + "zone": zone, + } + expected := map[string]string{ + "boot_disk.#": "1", + "attached_disk.#": "1", + "attached_disk.0.source": "https://www.googleapis.com/compute/v1/projects/" + config.Project + "/zones/" + zone + "/disks/" + instanceName + "-1", + "attached_disk.0.device_name": "persistent-disk-1", + "attached_disk.0.disk_encryption_key_raw": "SGVsbG8gZnJvbSBHb29nbGUgQ2xvdWQgUGxhdGZvcm0=", + "attached_disk.0.disk_encryption_key_sha256": "esTuF7d4eatX4cnc4JsiEiaI+Rff78JgPhA/v1zxX9E=", + "zone": zone, + } + + runInstanceMigrateTest(t, instanceName, "migrate disk to attached disk", 3 /* state version */, attributes, expected, config) +} + +func TestAccComputeInstanceMigrateState_attachedDiskFromAutoDeleteAndImage(t *testing.T) { + if os.Getenv(resource.TestEnvVar) == "" { + t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", resource.TestEnvVar)) + } + config := getInitializedConfig(t) + zone := "us-central1-f" + + instanceName := fmt.Sprintf("instance-test-%s", acctest.RandString(10)) + instance := &compute.Instance{ + Name: instanceName, + Disks: []*compute.AttachedDisk{ + { + Boot: true, + InitializeParams: &compute.AttachedDiskInitializeParams{ + SourceImage: "projects/debian-cloud/global/images/family/debian-8", + }, + }, + { + AutoDelete: true, + InitializeParams: &compute.AttachedDiskInitializeParams{ + SourceImage: "projects/debian-cloud/global/images/family/debian-8", + }, + }, + { + AutoDelete: true, + InitializeParams: &compute.AttachedDiskInitializeParams{ + SourceImage: "projects/debian-cloud/global/images/debian-8-jessie-v20170110", + }, + }, + }, + MachineType: "zones/" + zone + "/machineTypes/n1-standard-1", + NetworkInterfaces: []*compute.NetworkInterface{ + { + Network: "global/networks/default", + }, + }, + } + op, err := config.clientCompute.Instances.Insert(config.Project, zone, instance).Do() + if err != nil { + t.Fatalf("Error creating instance: %s", err) + } + waitErr := computeSharedOperationWait(config, op, config.Project, "instance to create") + if waitErr != nil { + t.Fatal(waitErr) + } + defer cleanUpInstance(config, instanceName, zone) + + attributes := map[string]string{ + "boot_disk.#": "1", + "disk.#": "2", + "disk.0.image": "projects/debian-cloud/global/images/debian-8-jessie-v20170110", + "disk.0.auto_delete": "true", + "disk.1.image": "projects/debian-cloud/global/images/family/debian-8", + "disk.1.auto_delete": "true", + "zone": zone, + } + expected := map[string]string{ + "boot_disk.#": "1", + "attached_disk.#": "2", + "attached_disk.0.source": "https://www.googleapis.com/compute/v1/projects/" + config.Project + "/zones/" + zone + "/disks/" + instanceName + "-2", + "attached_disk.0.device_name": "persistent-disk-2", + "attached_disk.1.source": "https://www.googleapis.com/compute/v1/projects/" + config.Project + "/zones/" + zone + "/disks/" + instanceName + "-1", + "attached_disk.1.device_name": "persistent-disk-1", + "zone": zone, + } + + runInstanceMigrateTest(t, instanceName, "migrate disk to attached disk", 3 /* state version */, attributes, expected, config) +} + func TestAccComputeInstanceMigrateState_scratchDisk(t *testing.T) { if os.Getenv(resource.TestEnvVar) == "" { t.Skip(fmt.Sprintf("Network access not allowed; use %s=1 to enable", resource.TestEnvVar)) From f47af05d475c5c0e25d55c2350d94f922bed283d Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Wed, 20 Sep 2017 23:57:08 -0400 Subject: [PATCH 09/11] use as much of the image URL as we can for matching on image --- google/resource_compute_instance_migrate.go | 30 ++++++++++++------- .../resource_compute_instance_migrate_test.go | 2 +- 2 files changed, 21 insertions(+), 11 deletions(-) diff --git a/google/resource_compute_instance_migrate.go b/google/resource_compute_instance_migrate.go index 32e1179f80c..a9084a2c79b 100644 --- a/google/resource_compute_instance_migrate.go +++ b/google/resource_compute_instance_migrate.go @@ -401,8 +401,13 @@ func getDiskFromEncryptionKey(instance *compute.Instance, encryptionKey string) } func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, allDisks map[string]*compute.Disk, autoDelete bool, image, project, zone string) (*compute.AttachedDisk, error) { - imageUrl := strings.Split(image, "/") - imageName := imageUrl[len(imageUrl)-1] + img, err := resolveImage(config, image) + if err != nil { + return nil, err + } + imgParts := strings.Split(img, "/projects/") + canonicalImage := imgParts[len(imgParts)-1] + for i, disk := range instance.Disks { if disk.Boot == true || disk.Type == "SCRATCH" { // Ignore boot/scratch disks since this is just for finding attached disks @@ -412,9 +417,11 @@ func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, a // Read the disk to check if its image matches sourceUrl := strings.Split(disk.Source, "/") fullDisk := allDisks[sourceUrl[len(sourceUrl)-1]] - fullDiskImageUrl := strings.Split(fullDisk.SourceImage, "/") - fullDiskImage := fullDiskImageUrl[len(fullDiskImageUrl)-1] - if fullDiskImage == imageName { + sourceImage, err := getRelativePath(fullDisk.SourceImage) + if err != nil { + return nil, err + } + if canonicalImage == sourceImage { // Delete this disk because there might be multiple that match instance.Disks = append(instance.Disks[:i], instance.Disks[i+1:]...) return disk, nil @@ -424,8 +431,9 @@ func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, a // We're not done! It's possible the disk was created with an image family rather than the image itself. // Now, do the exact same iteration but do some prefix matching to check if the families match. - // This assumes that all disks with a given family are of the format family-name-{diskname}, because - // there's not much else we can do at this point. + // This assumes that all disks with a given family have a sourceImage whose name starts with the name of + // the image family. + canonicalImage = strings.Replace(canonicalImage, "/family/", "/", -1) for i, disk := range instance.Disks { if disk.Boot == true || disk.Type == "SCRATCH" { // Ignore boot/scratch disks since this is just for finding attached disks @@ -435,10 +443,12 @@ func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, a // Read the disk to check if its image matches sourceUrl := strings.Split(disk.Source, "/") fullDisk := allDisks[sourceUrl[len(sourceUrl)-1]] - fullDiskImageUrl := strings.Split(fullDisk.SourceImage, "/") - fullDiskImage := fullDiskImageUrl[len(fullDiskImageUrl)-1] + sourceImage, err := getRelativePath(fullDisk.SourceImage) + if err != nil { + return nil, err + } - if strings.HasPrefix(fullDiskImage, imageName+"-") { + if strings.Contains(sourceImage, "/"+canonicalImage+"-") { // Delete this disk because there might be multiple that match instance.Disks = append(instance.Disks[:i], instance.Disks[i+1:]...) return disk, nil diff --git a/google/resource_compute_instance_migrate_test.go b/google/resource_compute_instance_migrate_test.go index 11b00d23082..125bc6a1c8a 100644 --- a/google/resource_compute_instance_migrate_test.go +++ b/google/resource_compute_instance_migrate_test.go @@ -347,7 +347,7 @@ func TestAccComputeInstanceMigrateState_attachedDiskFromAutoDeleteAndImage(t *te "disk.#": "2", "disk.0.image": "projects/debian-cloud/global/images/debian-8-jessie-v20170110", "disk.0.auto_delete": "true", - "disk.1.image": "projects/debian-cloud/global/images/family/debian-8", + "disk.1.image": "global/images/family/debian-8", "disk.1.auto_delete": "true", "zone": zone, } From d234c1d28298e25c10b659274a48ac8907dfa4d5 Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Thu, 28 Sep 2017 10:42:23 -0700 Subject: [PATCH 10/11] read project from config if it wasn't set in the attribute --- google/resource_compute_instance_migrate.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/google/resource_compute_instance_migrate.go b/google/resource_compute_instance_migrate.go index a9084a2c79b..0b23fd3ad07 100644 --- a/google/resource_compute_instance_migrate.go +++ b/google/resource_compute_instance_migrate.go @@ -350,7 +350,10 @@ func getDiskFromAttributes(config *Config, instance *compute.Instance, allDisks image := attributes[fmt.Sprintf("disk.%d.image", i)] // We know project and zone are set because we used them to read the instance - project := attributes["project"] + project, ok := attributes["project"] + if !ok { + project = config.Project + } zone := attributes["zone"] return getDiskFromAutoDeleteAndImage(config, instance, allDisks, autoDelete, image, project, zone) } From a14d598e4dc825bd059efcd3437da3d67036c3a1 Mon Sep 17 00:00:00 2001 From: Dana Hoffman Date: Thu, 28 Sep 2017 13:32:08 -0700 Subject: [PATCH 11/11] update resolveImage call --- google/resource_compute_instance_migrate.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/google/resource_compute_instance_migrate.go b/google/resource_compute_instance_migrate.go index 0b23fd3ad07..f4e58603a82 100644 --- a/google/resource_compute_instance_migrate.go +++ b/google/resource_compute_instance_migrate.go @@ -404,7 +404,7 @@ func getDiskFromEncryptionKey(instance *compute.Instance, encryptionKey string) } func getDiskFromAutoDeleteAndImage(config *Config, instance *compute.Instance, allDisks map[string]*compute.Disk, autoDelete bool, image, project, zone string) (*compute.AttachedDisk, error) { - img, err := resolveImage(config, image) + img, err := resolveImage(config, project, image) if err != nil { return nil, err }