From ab62b330c16ce38ea453d925813b4521c65ce8a6 Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Mon, 5 Nov 2018 16:02:45 -0800 Subject: [PATCH] core: Allow planned output changes to be updated during apply If plan and apply are both run against the same context then we still have the planned output values in memory while we're doing the apply walk, so we need to make sure to update them along with the state as we learn the final known values of each output. There were actually two different bugs here: - We weren't removing any existing planned change for an output when setting a new one. In retrospect a map would've been a better data structure for the output changes, rather than a slice to mimic what we do for resource instance objects, but for now we'll leave the structures alone and clean up as needed. (The set of outputs should be small for any reasonable configuration, so the main impact of this is some ugly code in RemoveOutputChange.) - RemoveOutputChange itself had a bug where it was iterating over the resource changes rather than the output changes. This didn't matter before because we weren't actually using that function, but now we are. This fix is confirmed by restoring various existing context apply tests back to passing again. --- plans/changes_sync.go | 4 ++-- terraform/eval_output.go | 20 ++++++++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/plans/changes_sync.go b/plans/changes_sync.go index e9305eaf9..6b4ff98ff 100644 --- a/plans/changes_sync.go +++ b/plans/changes_sync.go @@ -133,8 +133,8 @@ func (cs *ChangesSync) RemoveOutputChange(addr addrs.AbsOutputValue) { defer cs.lock.Unlock() addrStr := addr.String() - for i, r := range cs.changes.Resources { - if r.Addr.String() != addrStr { + for i, o := range cs.changes.Outputs { + if o.Addr.String() != addrStr { continue } copy(cs.changes.Outputs[i:], cs.changes.Outputs[i+1:]) diff --git a/terraform/eval_output.go b/terraform/eval_output.go index 6829934f0..10573971f 100644 --- a/terraform/eval_output.go +++ b/terraform/eval_output.go @@ -9,6 +9,7 @@ import ( "github.com/hashicorp/terraform/addrs" "github.com/hashicorp/terraform/plans" + "github.com/hashicorp/terraform/states" ) // EvalDeleteOutput is an EvalNode implementation that deletes an output @@ -61,22 +62,33 @@ func (n *EvalWriteOutput) Eval(ctx EvalContext) (interface{}, error) { // if we're continuing, make sure the output is included, and // marked as unknown. If the evaluator was able to find a type // for the value in spite of the error then we'll use it. - state.SetOutputValue(addr, cty.UnknownVal(val.Type()), n.Sensitive) + n.setValue(addr, state, changes, cty.UnknownVal(val.Type())) return nil, EvalEarlyExitError{} } return nil, diags.Err() } + n.setValue(addr, state, changes, val) + + return nil, nil +} + +func (n *EvalWriteOutput) setValue(addr addrs.AbsOutputValue, state *states.SyncState, changes *plans.ChangesSync, val cty.Value) { if val.IsKnown() && !val.IsNull() { // The state itself doesn't represent unknown values, so we null them // out here and then we'll save the real unknown value in the planned // changeset below, if we have one on this graph walk. + log.Printf("[TRACE] EvalWriteOutput: Saving value for %s in state", addr) stateVal := cty.UnknownAsNull(val) state.SetOutputValue(addr, stateVal, n.Sensitive) } else { + log.Printf("[TRACE] EvalWriteOutput: Removing %s from state (it is now null)", addr) state.RemoveOutputValue(addr) } + // If we also have an active changeset then we'll replicate the value in + // there. This is used in preference to the state where present, since it + // *is* able to represent unknowns, while the state cannot. if changes != nil { // For the moment we are not properly tracking changes to output // values, and just marking them always as "Create" or "Destroy" @@ -116,8 +128,8 @@ func (n *EvalWriteOutput) Eval(ctx EvalContext) (interface{}, error) { // Should never happen, since we just constructed this right above panic(fmt.Sprintf("planned change for %s could not be encoded: %s", addr, err)) } - changes.AppendOutputChange(cs) + log.Printf("[TRACE] EvalWriteOutput: Saving %s change for %s in changeset", change.Action, addr) + changes.RemoveOutputChange(addr) // remove any existing planned change, if present + changes.AppendOutputChange(cs) // add the new planned change } - - return nil, nil }