-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add state migration from disk
to boot_disk/scratch_disk/attached_disk
#329
Changes from 9 commits
6087c35
820a04c
a662479
443c161
6936615
8a23b6a
acb721f
f0d911f
f47af05
d234c1d
1dbeafa
a14d598
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,8 @@ import ( | |
"strconv" | ||
"strings" | ||
|
||
compute "google.golang.org/api/compute/v1" | ||
|
||
"github.com/hashicorp/terraform/helper/hashcode" | ||
"github.com/hashicorp/terraform/terraform" | ||
) | ||
|
@@ -39,6 +41,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 +161,300 @@ 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: %s", 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: %s", err) | ||
} | ||
} | ||
|
||
disks, err := strconv.Atoi(is.Attributes["disk.#"]) | ||
if err != nil { | ||
return is, fmt.Errorf("migration error: found disk.# value in unexpected format: %s", 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 { | ||
if config.Project == "" { | ||
return nil, fmt.Errorf("could not determine 'project'") | ||
} else { | ||
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 { | ||
if config.Project == "" { | ||
return nil, fmt.Errorf("could not determine 'project'") | ||
} else { | ||
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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we though? From what I saw, the instance attributes were never updated with the project if we inferred the project from the config. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, fixed. |
||
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) { | ||
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 | ||
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) { | ||
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 | ||
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]] | ||
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 | ||
} | ||
} | ||
} | ||
|
||
// 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 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 | ||
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]] | ||
sourceImage, err := getRelativePath(fullDisk.SourceImage) | ||
if err != nil { | ||
return nil, err | ||
} | ||
|
||
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 | ||
} | ||
} | ||
} | ||
|
||
return nil, fmt.Errorf("could not find attached disk with image %q", image) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'm missing a migration here--where does this get set to 3?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It never got set to 3, but there's a migration fn hanging out that migrates from 2 to 3 so the next unclaimed value is 4.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
That sounds terrifying, only because it means there's dead code that we'll be turning on. But that also, that's not this PR's fault, and from what I can see, the code doesn't change that much.
Sounds good to me, let's just do some heavy manual testing prior to cutting the release to make sure we don't end up in a weird place.