State migration for google_compute_instance.[boot|scratch|attached]_disk

[danawillow]

Right now, users have a deprecation message for disk but no way to move to boot/scratch/attached disks without recreating their instances. This should be solved using a state migration.

Because of the way disks is implemented currently, this will unfortunately require several steps.

Issue 1: We don't have the device_name.
Disks that were creating using disk.image have very little information about them in state. A state file for such a disk might look like this:

  disk.0.auto_delete = true
  disk.0.device_name =
  disk.0.disk =
  disk.0.disk_encryption_key_raw =
  disk.0.disk_encryption_key_sha256 =
  disk.0.image = debian-8-jessie-v20160803
  disk.0.scratch = false
  disk.0.size = 0
  disk.0.type =

For attached_disk we need the device_name in order to allow for detaching. All disks have a device_name, but the current implementation of disk does not store it in state.

Issue 2: We can't store disk information in state from the API as-is because the API does not return disks in the order they were sent.
See https://gist.github.com/danawillow/060a2975f7d6e4d7b6ef0c4ee4d4947a. Note that setting index on the request does not make a difference for the order.
Since the returned disks won't necessarily match up with the order in state, this will lead to diffs. Since we have so little information about the disk stored in state, we have no way to match them in order to reorder.

Issue 3: Not all data that we store in state is returned from the API.
We currently store the disk_encryption_key_raw in state, but this data is not returned from the API- we just copy it over from what was already in state.

Issue 4: We rely on the ordering of elements in the config.
We assume that, unless a different disk is specified to be the boot disk via the boot_disk attribute, that the first disk in the list of disks is the boot disk. If we were to use a Set instead of a List, we would no longer be able to know which disk was set first in the config.

There is a way around all of these issues, but it requires several steps:

1. Add a boot field to the existing disk schema. Make this field Computed so users can't change it, and set it to whether it is currently a boot disk in a migration function.
2. Change disks from a List to a Set. Keep track of all the disk_encryption_key_raws that were set in the config so they can be matched up to the correct disk by hashing it and seeing if it matches the disk_encryption_key_sha256 that was returned by the API.
3. Set disk and device_name on each disk based on what was returned from the API. This can be done at the same time as the previous step (and probably needs to be so we can use the device_name for our set hash function).
4. Write a new migration function to map disks into the other three types of disks based on what is set in state.

@paddycarver mind looking this over to make sure this seems like it will work and see if there's anything you can find that I missed?

[paddycarver]

At first glance, my main concern is step 2 of that proposed action plan. List -> Set explicitly drops order, but right now we can't uniquely identify disks. But Terraform must be distinguishing between them somehow (my guess is probably order) just because otherwise users would be seeing perpetual diffs.

So changing things from a List to a Set without having a concrete way to identify them makes me very, very nervous.

[paddycarver]

(That being said, the rest of this approach sounds reasonable and well-thought out, but I'll do some experiments to confirm it. Ideally, before releasing this, I'd like to just build up a body of examples and test cases and try running them through the migrations, to make sure we end up in the right spot. But whether that level of work is necessary or not, I'll leave to you.)

[danawillow]

Yeah, I think steps 2 and 3 have to be done at the same time, that way we can use the device name to uniquely identify disks.

[ghost]

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. If you feel I made an error 🤖 🙉 , please reach out to my human friends 👉 [email protected]. Thanks!