search

displague / deprecated-terraform-provider-linode

[OLD] Terraform provider plugin for Linode Cloud resources.. See
https://github.com/terraform-providers/terraform-provider-linode
Mozilla Public License 2.0
15 stars 5 forks source link

fix disk diffing necessary for disk resizes #23

Closed displague closed 6 years ago

displague commented 6 years ago

Fix disk diffing which is necessary for updates to disk detection.

The terraform disk value that we get from d.Get("disk") (a schema.TypeSet) during an Update shows the original disk size instead of the new disk size in the updated test config.

The debug output showing that size is not updated:

RESIZE
 *Set(map[string]interface {}{"208985346":map[string]interface {}{"label":"disk", "read_only":false, "size":3000, "stackscript_id":0, "filesystem":"ext4", "authorized_keys":[]interface {}{}, "root_pass":"", "image":"", "stackscript_data":map[string]interface {}{}}})

The disk TypeSet uses a custom hashing function which only considers the label, acting as a surrogate Id within the provider.

https://github.com/displague/terraform-provider-linode/blob/f3058c0585f9094bf732e73b3591f81980abd639/linode/linode_instance_helpers.go#L379-L395

I have a hunch that the default Differ for a TypeSet compares this hash, the Set identifier (https://github.com/hashicorp/terraform/pull/2336#issuecomment-115744243) which would never reflect a change in size. I think I'll need a custom differ. I would have expected a diff to only determine if Update/Create/Delete needs to be called, not to have an effect on the values in the d.Get("disk") returned schema data.

This appears to be similar to the problems encountered in: https://github.com/DimensionDataResearch/dd-cloud-compute-terraform/issues/84#issuecomment-294599939 resolved in https://github.com/DimensionDataResearch/dd-cloud-compute-terraform/pull/86

displague commented 6 years ago

Merging with debug comments left in place. I'll remove those after adding more Live Volume handling tests as these debugging lines will be useful for that.

displague commented 6 years ago

Ultimately, configs and disks were set to TypeLists instead of TypeSets.

This makes testing and user configs easier as the list index is predictable. Accessing Set indexes demanded convoluted function composition during tests, and would be very difficult for user-configs.

Moving a config or disk to a different list order in the terraform config file is not a supported behavior. I will create an issue for this as I believe it can be remedied by having the Read function append configs and disks fetched from the API to the TF state in an order that matches the current TF state (for any API returned config or disk whose label already exists in TF state).

There were a number of issues with how Computed was being used. When the TypeSet Set function was being used, the state diffs were being ignored because the Set function returned no change in state.

Removing the Set functions revealed that the diffs were not consistent with the state.

Disks are now including their id in their schema, which may be useful for output. (Configs should do this too .. I'll add an issue.)

(image) vs (disk and config) TF configurations have been made usable without lifecycle ignore_change stanzas by adding DiffSupressFunc.

Additional tests may be needed to verify that a resize on a live / running disk results in a shutdown and before the resize and a reboot afterwards.

Testing during this PR shows that a Shredder would be helpful for acceptance test usage.