From ec2e6cb06f2f481c7cb9fca4bc8d4c9ed024501c Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Sun, 30 Sep 2018 08:51:47 -0700 Subject: [PATCH] terraform: Prune resource husks at the end of "terraform destroy" When we're being asked to destroy everything, we ideally want to end up with a totally empty state. Normally we will conservatively keep around the "husks" of resources (what's left after all of the instances have been destroyed) unless they are configured without count or for_each, but in this special case we'll prune those out. The implication of this is that in "weird" expression contexts that happen before the next "terraform plan", such as evaluation in "terraform console" or expressions in data resources and provider blocks that get evaluated during the refresh walk, we will see these results as unknown rather than as empty lists of objects. We accept that weirdness for now because in a future release we are likely to remove "refresh" as a separate walk anyway, doing all of that work during the plan walk where we can ensure that these values are properly re-populated before trying to use them. --- states/module.go | 14 ++++++++++++++ states/state.go | 23 ++++++++++++++++++++++- terraform/context.go | 19 +++++++++++++++++++ terraform/context_apply_test.go | 5 +---- 4 files changed, 56 insertions(+), 5 deletions(-) diff --git a/states/module.go b/states/module.go index 2938a087f..99cd4b03f 100644 --- a/states/module.go +++ b/states/module.go @@ -234,6 +234,20 @@ func (ms *Module) RemoveLocalValue(name string) { delete(ms.LocalValues, name) } +// PruneResourceHusks is a specialized method that will remove any Resource +// objects that do not contain any instances, even if they have an EachMode. +// +// You probably shouldn't call this! See the method of the same name on +// type State for more information on what this is for and the rare situations +// where it is safe to use. +func (ms *Module) PruneResourceHusks() { + for _, rs := range ms.Resources { + if len(rs.Instances) == 0 { + ms.RemoveResource(rs.Addr) + } + } +} + // empty returns true if the receving module state is contributing nothing // to the state. In other words, it returns true if the module could be // removed from the state altogether without changing the meaning of the state. diff --git a/states/state.go b/states/state.go index 67dfe891d..c6d005a11 100644 --- a/states/state.go +++ b/states/state.go @@ -78,7 +78,7 @@ func (s *State) Module(addr addrs.ModuleInstance) *Module { // elements is removed. func (s *State) RemoveModule(addr addrs.ModuleInstance) { if addr.IsRoot() { - panic("attempted to remote root module") + panic("attempted to remove root module") } delete(s.Modules, addr.String()) @@ -194,6 +194,27 @@ func (s *State) ProviderAddrs() []addrs.AbsProviderConfig { return ret } +// PruneResourceHusks is a specialized method that will remove any Resource +// objects that do not contain any instances, even if they have an EachMode. +// +// This should generally be used only after a "terraform destroy" operation, +// to finalize the cleanup of the state. It is not correct to use this after +// other operations because if a resource has "count = 0" or "for_each" over +// an empty collection then we want to retain it in the state so that references +// to it, particularly in "strange" contexts like "terraform console", can be +// properly resolved. +// +// This method MUST NOT be called concurrently with other readers and writers +// of the receiving state. +func (s *State) PruneResourceHusks() { + for _, m := range s.Modules { + m.PruneResourceHusks() + if len(m.Resources) == 0 && !m.Addr.IsRoot() { + s.RemoveModule(m.Addr) + } + } +} + // SyncWrapper returns a SyncState object wrapping the receiver. func (s *State) SyncWrapper() *SyncState { return &SyncState{ diff --git a/terraform/context.go b/terraform/context.go index 0c99f893f..ff3679aa1 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -451,6 +451,25 @@ func (c *Context) Apply() (*states.State, tfdiags.Diagnostics) { diags = diags.Append(walker.NonFatalDiagnostics) diags = diags.Append(walkDiags) + if c.destroy && !diags.HasErrors() { + // If we know we were trying to destroy objects anyway, and we + // completed without any errors, then we'll also prune out any + // leftover empty resource husks (left after all of the instances + // of a resource with "count" or "for_each" are destroyed) to + // help ensure we end up with an _actually_ empty state, assuming + // we weren't destroying with -target here. + // + // (This doesn't actually take into account -target, but that should + // be okay because it doesn't throw away anything we can't recompute + // on a subsequent "terraform plan" run, if the resources are still + // present in the configuration. However, this _will_ cause "count = 0" + // resources to read as unknown during the next refresh walk, which + // may cause some additional churn if used in a data resource or + // provider block, until we remove refreshing as a separate walk and + // just do it as part of the plan walk.) + c.state.PruneResourceHusks() + } + return c.state, diags } diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index a052328d1..4a3b9c1f5 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -6662,10 +6662,7 @@ func TestContext2Apply_destroyTargetWithModuleVariableAndCount(t *testing.T) { //Test that things were destroyed actual := strings.TrimSpace(state.String()) - expected := strings.TrimSpace(` - -module.child: - `) + expected := strings.TrimSpace(``) if actual != expected { t.Fatalf("expected: \n%s\n\nbad: \n%s", expected, actual) }