r/virtual_machine: Remove clone config restriction for import/legacy #460
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 adds in commits that are needed for a pending PR that adjusts how ForceNew works in ResourceDiff. These are needed by some of our new diff customization bits to make clone config pass-through work. Also adds in copystructure/reflectwalk as there has been some long-standing bug fixes that have been finally merged, which affect some updates to helper/schema.
This removes the restriction on the presence of the clone sub-resource block for imported and legacy resources. Previously, this was blocked as it forced a new resource, which is obviously not what people would want during this process. ForceNew was (naturally) controlled in schema, so there was not much we could do here. Some recent changes to diff customization has allowed us to change this process in a way that has been relatively non-invasive. This has resulted in the changes below: * Rather than using schema to force new resources for clone attributes, we now go through changed keys in the clone namespace and ForceNew these keys through diff customization. This happens for both new and existing resources to prevent diff mismatches. * We leverage imported as a conditional to provide a one-time exemption from this behavior. We now also transition imported from true to false after the first apply post-import/migration. These two changes have the effect in that on the first apply operation after a resource has been imported or transitioned from a legacy version of the provider, all clone configuration settings are simply persisted to state, without forcing a new resource. This, like other post-import operations like flagging keep_on_remove for disks, is a no-op on the vSphere API side and does not trigger a reconfigure or a rebuild of the VM. The new behavior has been documented. It should be noted that if configuration changes to clone are made in the first post-apply run they will be ignored permanently as well (as they will persist to state without any action carried out on them), however this should be avoidable when import/migration instructions are followed. This has been documented as well.
vancluever
force-pushed
the
f-import-legacy-allow-clone
branch
from
35d393a
to
e857920
Compare
ImportStateVerify does not fully simulate an import from scratch. This means that we can't properly test if imported is being properly checked, reverted, and passing the clone sub-resource through unless we hack the state ourselves, which we do by adding another test step.
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 removes the restriction on the presence of the
clonesub-resourceblock for imported and legacy resources.
Previously, this was blocked as it forced a new resource, which is
obviously not what people would want during this process.
ForceNewwas(naturally) controlled in schema, so there was not much we could do
here.
Some recent changes to diff customization has allowed us to change this
process in a way that has been relatively non-invasive. This has
resulted in the changes below:
we now go through changed keys in the
clonenamespace andForceNewthesekeys through diff customization. This happens for both new and existing
resources to prevent diff mismatches.
importedas a conditional to provide a one-time exemptionfrom this behavior. We now also transition
importedfromtruetofalseafter the first apply post-import/migration.
These two changes have the effect in that on the first apply operation
after a resource has been imported or transitioned from a legacy version
of the provider, all
cloneconfiguration settings are simply persistedto state, without forcing a new resource. This, like other post-import
operations like flagging
keep_on_removefor disks, is a no-op on thevSphere API side and does not trigger a reconfigure or a rebuild of the
VM.
The new behavior has been documented. It should be noted that if
configuration changes to
cloneare made in the first post-apply run theywill be ignored permanently as well (as they will persist to state
without any action carried out on them), however this should be
avoidable when import/migration instructions are followed. This has been
documented as well.
This PR also includes updates to TF and
mapstructure/copystructure(the latter as dependencies of changes in 0.11.6) that were needed to fix some bugs withForceNewthat was preventing this from working. The commits are still in PR andvendorshould be updated when the fixes are fully upstream.