r/virtual_machine: Ensure datastore_id will always have a diff on new disk sub-resources #469
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is something that has been an issue for while but is now even more prominent now that datastores don't necessarily need to be specified at all. Owing in part that datastore_id is not populated during the initial TF apply when using the default datastore for disks, changing both the default datastore (or now, possibly, datastore clusters) and a key that forces a new resource at the same time will create a diff mismatch. Setting datastore_id in the disk sub-resource schema fixes this issue.
vancluever
force-pushed
the
b-vm-disk-subresource-datastore-diff-mismatch
branch
from
79458b5 to
9fe0b06
Compare
After discussing the change to datastore_id in disk sub-resources to computed, it was determined that there could possibly be scenarios where this would be damaging to a diff where someone was creating a new disk at a list location that contained a previous disk's state. This would result in the old datastore information being grafted into the new disk due to how Optional/Computed works. This reverts the schema to the old non-computed form, opting instead to replace a blank datastore_id with "<computed>" in disk diff when datastore_id is not supplied. This is then treated the same as an empty value on virtual disk creation.
vancluever
changed the title
|
Thanks @bill-rich! |
ghost
locked and limited conversation to collaborators
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is something that has been an issue for while but is now even more prominent now that datastores don't necessarily need to be specified at all. Owing in part that
datastore_idis not populated during the initial TF apply when using the default datastore for disks, changing both the default datastore (or now, possibly, datastore clusters) and a key that forces a new resource at the same time will create a diff mismatch.A couple of approaches can fix this issue:
datastore_idattribute of thedisksub-resource asComputed, but this method opens the resource up to other bugs when a new disk is taking the place of an old disk in state, and Optional/Computed ultimately results in the old datastore ID setting being grafted into the new sub-resource.The latter approach is what was ultimately taken as it fixes the issue without introducing the regression mentioned in the first.