a deleted orphan should have no plan

If when refreshing an orphaned instance the provider indicates it has
already been deleted, there is no reason to create a change for that
instance. A NoOp change should only represent an object that exists and
is not changing.

This was likely left in before in order to try and provide a record of
the change for external consumers of the plan, but newer plans also
contain all changes made outside of Terraform which better accounts for
the difference. The NoOp change now can cause problems, because it may
represent an instance with conditions to check even though that instance
does not exist.
This commit is contained in:
James Bardin 2022-11-18 08:48:15 -05:00
parent 62a8b9ef1d
commit 7946e4a88a
2 changed files with 31 additions and 36 deletions

View File

@ -121,37 +121,37 @@ func (n *NodePlannableResourceInstanceOrphan) managedResourceExecute(ctx EvalCon
oldState = refreshedState
}
if !n.skipPlanChanges {
var change *plans.ResourceInstanceChange
change, destroyPlanDiags := n.planDestroy(ctx, oldState, "")
diags = diags.Append(destroyPlanDiags)
if diags.HasErrors() {
return diags
}
diags = diags.Append(n.checkPreventDestroy(change))
if diags.HasErrors() {
return diags
}
// We might be able to offer an approximate reason for why we are
// planning to delete this object. (This is best-effort; we might
// sometimes not have a reason.)
change.ActionReason = n.deleteActionReason(ctx)
diags = diags.Append(n.writeChange(ctx, change, ""))
if diags.HasErrors() {
return diags
}
diags = diags.Append(n.writeResourceInstanceState(ctx, nil, workingState))
} else {
// The working state should at least be updated with the result
// of upgrading and refreshing from above.
diags = diags.Append(n.writeResourceInstanceState(ctx, oldState, workingState))
// If we're skipping planning, all we need to do is write the state. If the
// refresh indicates the instance no longer exists, there is also nothing
// to plan because there is no longer any state and it doesn't exist in the
// config.
if n.skipPlanChanges || oldState.Value.IsNull() {
return diags.Append(n.writeResourceInstanceState(ctx, oldState, workingState))
}
return diags
var change *plans.ResourceInstanceChange
change, destroyPlanDiags := n.planDestroy(ctx, oldState, "")
diags = diags.Append(destroyPlanDiags)
if diags.HasErrors() {
return diags
}
diags = diags.Append(n.checkPreventDestroy(change))
if diags.HasErrors() {
return diags
}
// We might be able to offer an approximate reason for why we are
// planning to delete this object. (This is best-effort; we might
// sometimes not have a reason.)
change.ActionReason = n.deleteActionReason(ctx)
diags = diags.Append(n.writeChange(ctx, change, ""))
if diags.HasErrors() {
return diags
}
return diags.Append(n.writeResourceInstanceState(ctx, nil, workingState))
}
func (n *NodePlannableResourceInstanceOrphan) deleteActionReason(ctx EvalContext) plans.ResourceInstanceChangeActionReason {

View File

@ -137,12 +137,7 @@ func TestNodeResourcePlanOrphanExecute_alreadyDeleted(t *testing.T) {
if got := refreshState.ResourceInstance(addr); got != nil {
t.Errorf("refresh state has entry for %s; should've been removed", addr)
}
if got := changes.ResourceInstance(addr); got == nil {
t.Errorf("no entry for %s in the planned changes; should have a NoOp change", addr)
} else {
if got, want := got.Action, plans.NoOp; got != want {
t.Errorf("planned change for %s has wrong action\ngot: %s\nwant: %s", addr, got, want)
}
if got := changes.ResourceInstance(addr); got != nil {
t.Errorf("there should be no change for the %s instance, got %s", addr, got.Action)
}
}