diff --git a/states/module.go b/states/module.go index ea5093754..2938a087f 100644 --- a/states/module.go +++ b/states/module.go @@ -181,6 +181,28 @@ func (ms *Module) deposeResourceInstanceObject(addr addrs.ResourceInstance, forc return is.deposeCurrentObject(forceKey) } +// maybeRestoreResourceInstanceDeposed is the real implementation of +// SyncState.MaybeRestoreResourceInstanceDeposed. +func (ms *Module) maybeRestoreResourceInstanceDeposed(addr addrs.ResourceInstance, key DeposedKey) bool { + rs := ms.Resource(addr.Resource) + if rs == nil { + return false + } + is := rs.Instance(addr.Key) + if is == nil { + return false + } + if is.Current != nil { + return false + } + if len(is.Deposed) == 0 { + return false + } + is.Current = is.Deposed[key] + delete(is.Deposed, key) + return true +} + // SetOutputValue writes an output value into the state, overwriting any // existing value of the same name. func (ms *Module) SetOutputValue(name string, value cty.Value, sensitive bool) *OutputValue { diff --git a/states/sync.go b/states/sync.go index 00f2fd317..20ef6d123 100644 --- a/states/sync.go +++ b/states/sync.go @@ -361,6 +361,30 @@ func (s *SyncState) ForgetResourceInstanceDeposed(addr addrs.AbsResourceInstance s.maybePruneModule(addr.Module) } +// MaybeRestoreResourceInstanceDeposed will restore the deposed object with the +// given key on the specified resource as the current object for that instance +// if and only if that would not cause us to forget an existing current +// object for that instance. +// +// Returns true if the object was restored to current, or false if no change +// was made at all. +func (s *SyncState) MaybeRestoreResourceInstanceDeposed(addr addrs.AbsResourceInstance, key DeposedKey) bool { + s.lock.Lock() + defer s.lock.Unlock() + + if key == NotDeposed { + panic("MaybeRestoreResourceInstanceDeposed called without DeposedKey") + } + + ms := s.state.Module(addr.Module) + if ms == nil { + // Nothing to do, since the specified deposed object cannot exist. + return false + } + + return ms.maybeRestoreResourceInstanceDeposed(addr.Resource, key) +} + // RemovePlannedResourceInstanceObjects removes from the state any resource // instance objects that have the status ObjectPlanned, indiciating that they // are just transient placeholders created during planning. diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 7e957f2a2..334c285b0 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -4600,7 +4600,7 @@ func TestContext2Apply_error_createBeforeDestroy(t *testing.T) { actual := strings.TrimSpace(state.String()) expected := strings.TrimSpace(testTerraformApplyErrorCreateBeforeDestroyStr) if actual != expected { - t.Fatalf("bad: \n%s\n\nExpected:\n\n%s", actual, expected) + t.Fatalf("wrong final state\ngot:\n%s\n\nwant:\n%s", actual, expected) } } diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 1573f3d38..5c08fb16a 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -343,13 +343,24 @@ func (n *EvalDeposeState) Eval(ctx EvalContext) (interface{}, error) { return nil, nil } -// EvalUndeposeState is an EvalNode implementation that forgets a particular -// deposed object from the state, causing Terraform to no longer track it. +// EvalMaybeRestoreDeposedObject is an EvalNode implementation that will +// restore a particular deposed object of the specified resource instance +// to be the "current" object if and only if the instance doesn't currently +// have a current object. // -// Users of this must ensure that the upstream object that the object was -// tracking has been deleted in the remote system before this node is -// evaluated. -type EvalUndeposeState struct { +// This is intended for use when the create leg of a create before destroy +// fails with no partial new object: if we didn't take any action, the user +// would be left in the unfortunate situation of having no current object +// and the previously-workign object now deposed. This EvalNode causes a +// better outcome by restoring things to how they were before the replace +// operation began. +// +// The create operation may have produced a partial result even though it +// failed and it's important that we don't "forget" that state, so in that +// situation the prior object remains deposed and the partial new object +// remains the current object, allowing the situation to hopefully be +// improved in a subsequent run. +type EvalMaybeRestoreDeposedObject struct { Addr addrs.ResourceInstance // Key is a pointer to the deposed object key that should be forgotten @@ -358,12 +369,17 @@ type EvalUndeposeState struct { } // TODO: test -func (n *EvalUndeposeState) Eval(ctx EvalContext) (interface{}, error) { +func (n *EvalMaybeRestoreDeposedObject) Eval(ctx EvalContext) (interface{}, error) { absAddr := n.Addr.Absolute(ctx.Path()) + dk := *n.Key state := ctx.State() - state.ForgetResourceInstanceDeposed(absAddr, *n.Key) - log.Printf("[TRACE] EvalDeposeState: %s deposed object %s is forgotten", absAddr, *n.Key) + restored := state.MaybeRestoreResourceInstanceDeposed(absAddr, dk) + if restored { + log.Printf("[TRACE] EvalMaybeRestoreDeposedObject: %s deposed object %s was restored as the current object", absAddr, dk) + } else { + log.Printf("[TRACE] EvalMaybeRestoreDeposedObject: %s deposed object %s remains deposed", absAddr, dk) + } return nil, nil } diff --git a/terraform/node_resource_apply_instance.go b/terraform/node_resource_apply_instance.go index 7b4123c23..2a66eef99 100644 --- a/terraform/node_resource_apply_instance.go +++ b/terraform/node_resource_apply_instance.go @@ -384,7 +384,7 @@ func (n *NodeApplyableResourceInstance) evalTreeManagedResource(addr addrs.AbsRe If: func(ctx EvalContext) (bool, error) { return createBeforeDestroyEnabled && err != nil, nil }, - Then: &EvalUndeposeState{ + Then: &EvalMaybeRestoreDeposedObject{ Addr: addr.Resource, Key: &deposedKey, }, diff --git a/terraform/provider_mock.go b/terraform/provider_mock.go index 13a6be484..b38478281 100644 --- a/terraform/provider_mock.go +++ b/terraform/provider_mock.go @@ -388,20 +388,22 @@ func (p *MockProvider) ApplyResourceChange(r providers.ApplyResourceChangeReques if err != nil { resp.Diagnostics = resp.Diagnostics.Append(err) } - var newVal cty.Value if newState != nil { - var err error - newVal, err = newState.AttrsAsObjectValue(schema.Block.ImpliedType()) - if err != nil { - resp.Diagnostics = resp.Diagnostics.Append(err) + var newVal cty.Value + if newState != nil { + var err error + newVal, err = newState.AttrsAsObjectValue(schema.Block.ImpliedType()) + if err != nil { + resp.Diagnostics = resp.Diagnostics.Append(err) + } + } else { + // If apply returned a nil new state then that's the old way to + // indicate that the object was destroyed. Our new interface calls + // for that to be signalled as a null value. + newVal = cty.NullVal(schema.Block.ImpliedType()) } - } else { - // If apply returned a nil new state then that's the old way to - // indicate that the object was destroyed. Our new interface calls - // for that to be signalled as a null value. - newVal = cty.NullVal(schema.Block.ImpliedType()) + resp.NewState = newVal } - resp.NewState = newVal return resp }