From 15ea04af8a70a80ebe1ea37815cf0784fc693760 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Wed, 8 Nov 2017 18:49:26 -0500 Subject: [PATCH] remove modules from state Remove the module entry from the state if a module is no longer in the configuration. Modules are not removed if there are any existing resources with the module path as a prefix. The only time this should be the case is if a module was removed in the config, but the apply didn't target that module. Create a NodeModuleRemoved and an associated EvalDeleteModule to track the module in the graph then remove it from the state. The NodeModuleRemoved dependencies are simply any other node which contains the module path as a prefix in its path. This could have probably been done much easier as a step in pruning the state, but modules are going to have to be promoted to full graph nodes anyway in order to support count. --- terraform/context_apply_test.go | 38 ++++---------- terraform/eval_state.go | 31 ----------- terraform/graph_builder_apply.go | 3 ++ terraform/node_module_removed.go | 71 ++++++++++++++++++++++++++ terraform/state.go | 4 ++ terraform/terraform_test.go | 5 -- terraform/transform_removed_modules.go | 55 ++++++++++++++++++++ 7 files changed, 142 insertions(+), 65 deletions(-) create mode 100644 terraform/node_module_removed.go create mode 100644 terraform/transform_removed_modules.go diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 04f8b1db2..4c9ec48f6 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -342,11 +342,7 @@ func TestContext2Apply_resourceDependsOnModuleStateOnly(t *testing.T) { t.Fatal("should check") } - checkStateString(t, state, ` - -module.child: - - `) + checkStateString(t, state, "") } } @@ -2698,10 +2694,7 @@ func TestContext2Apply_moduleOrphanInheritAlias(t *testing.T) { t.Fatal("must call configure") } - checkStateString(t, state, ` -module.child: - - `) + checkStateString(t, state, "") } func TestContext2Apply_moduleOrphanProvider(t *testing.T) { @@ -4075,10 +4068,9 @@ func TestContext2Apply_outputOrphanModule(t *testing.T) { t.Fatalf("err: %s", err) } - expected = "" actual = strings.TrimSpace(state.String()) - if actual != expected { - t.Fatalf("expected:\n%s\n\ngot:\n%s", expected, actual) + if actual != "" { + t.Fatalf("expected no state, got:\n%s", actual) } } @@ -6127,9 +6119,8 @@ func TestContext2Apply_destroyNestedModule(t *testing.T) { // Test that things were destroyed actual := strings.TrimSpace(state.String()) - expected := strings.TrimSpace(testTerraformApplyDestroyNestedModuleStr) - if actual != expected { - t.Fatalf("bad: \n%s", actual) + if actual != "" { + t.Fatalf("expected no state, got: %s", actual) } } @@ -6177,12 +6168,8 @@ func TestContext2Apply_destroyDeeplyNestedModule(t *testing.T) { // Test that things were destroyed actual := strings.TrimSpace(state.String()) - expected := strings.TrimSpace(` -module.child.subchild.subsubchild: - - `) - if actual != expected { - t.Fatalf("bad: \n%s", actual) + if actual != "" { + t.Fatalf("epected no state, got: %s", actual) } } @@ -9107,14 +9094,7 @@ func TestContext2Apply_destroyWithProviders(t *testing.T) { got := strings.TrimSpace(state.String()) - // This should fail once modules are removed from the state entirely. - want := strings.TrimSpace(` - -module.child: - -module.mod.removed: - `) - + want := strings.TrimSpace("") if got != want { t.Fatalf("wrong final state\ngot:\n%s\nwant:\n%s", got, want) } diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 1f67e3d86..11826907c 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -214,37 +214,6 @@ func writeInstanceToState( return nil, nil } -// EvalClearPrimaryState is an EvalNode implementation that clears the primary -// instance from a resource state. -type EvalClearPrimaryState struct { - Name string -} - -func (n *EvalClearPrimaryState) Eval(ctx EvalContext) (interface{}, error) { - state, lock := ctx.State() - - // Get a read lock so we can access this instance - lock.RLock() - defer lock.RUnlock() - - // Look for the module state. If we don't have one, then it doesn't matter. - mod := state.ModuleByPath(ctx.Path()) - if mod == nil { - return nil, nil - } - - // Look for the resource state. If we don't have one, then it is okay. - rs := mod.Resources[n.Name] - if rs == nil { - return nil, nil - } - - // Clear primary from the resource state - rs.Primary = nil - - return nil, nil -} - // EvalDeposeState is an EvalNode implementation that takes the primary // out of a state and makes it Deposed. This is done at the beginning of // create-before-destroy calls so that the create can create while preserving diff --git a/terraform/graph_builder_apply.go b/terraform/graph_builder_apply.go index 614da2c85..2f1a988be 100644 --- a/terraform/graph_builder_apply.go +++ b/terraform/graph_builder_apply.go @@ -133,6 +133,9 @@ func (b *ApplyGraphBuilder) Steps() []GraphTransformer { &CloseProviderTransformer{}, &CloseProvisionerTransformer{}, + // Remove modules no longer present in the config + &RemovedModuleTransformer{Module: b.Module, State: b.State}, + // Single root &RootTransformer{}, } diff --git a/terraform/node_module_removed.go b/terraform/node_module_removed.go new file mode 100644 index 000000000..36f36ef18 --- /dev/null +++ b/terraform/node_module_removed.go @@ -0,0 +1,71 @@ +package terraform + +import ( + "fmt" + "log" + "reflect" +) + +// NodeModuleRemoved represents a module that is no longer in the +// config. +type NodeModuleRemoved struct { + PathValue []string +} + +func (n *NodeModuleRemoved) Name() string { + return fmt.Sprintf("%s (removed)", modulePrefixStr(n.PathValue)) +} + +// GraphNodeSubPath +func (n *NodeModuleRemoved) Path() []string { + return n.PathValue +} + +// GraphNodeEvalable +func (n *NodeModuleRemoved) EvalTree() EvalNode { + return &EvalOpFilter{ + Ops: []walkOperation{walkRefresh, walkApply, walkDestroy}, + Node: &EvalDeleteModule{ + PathValue: n.PathValue, + }, + } +} + +// EvalDeleteModule is an EvalNode implementation that removes an empty module +// entry from the state. +type EvalDeleteModule struct { + PathValue []string +} + +func (n *EvalDeleteModule) Eval(ctx EvalContext) (interface{}, error) { + state, lock := ctx.State() + if state == nil { + return nil, nil + } + + // Get a write lock so we can access this instance + lock.Lock() + defer lock.Unlock() + + // Make sure we have a clean state + // Destroyed resources aren't deleted, they're written with an ID of "". + state.prune() + + // find the module and delete it + for i, m := range state.Modules { + if reflect.DeepEqual(m.Path, n.PathValue) { + if !m.Empty() { + // a targeted apply may leave module resources even without a config, + // so just log this and return. + log.Printf("[DEBUG] cannot remove module %s, not empty", modulePrefixStr(n.PathValue)) + break + } + tail := len(state.Modules) - 1 + state.Modules[i] = state.Modules[tail] + state.Modules = state.Modules[:tail] + break + } + } + + return nil, nil +} diff --git a/terraform/state.go b/terraform/state.go index 1818364b6..8773745fd 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -1339,6 +1339,10 @@ func (m *ModuleState) String() string { return buf.String() } +func (m *ModuleState) Empty() bool { + return len(m.Locals) == 0 && len(m.Outputs) == 0 && len(m.Resources) == 0 +} + // ResourceStateKey is a structured representation of the key used for the // ModuleState.Resources mapping type ResourceStateKey struct { diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 0a84e34e6..2deb44153 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -713,11 +713,6 @@ const testTerraformApplyDestroyStr = ` ` -const testTerraformApplyDestroyNestedModuleStr = ` -module.child.subchild: - -` - const testTerraformApplyErrorStr = ` aws_instance.bar: ID = bar diff --git a/terraform/transform_removed_modules.go b/terraform/transform_removed_modules.go new file mode 100644 index 000000000..08816f10f --- /dev/null +++ b/terraform/transform_removed_modules.go @@ -0,0 +1,55 @@ +package terraform + +import ( + "log" + "strings" + + "github.com/hashicorp/terraform/config/module" + "github.com/hashicorp/terraform/dag" +) + +type RemovedModuleTransformer struct { + Module *module.Tree // root module + State *State +} + +func (t *RemovedModuleTransformer) Transform(g *Graph) error { + // nothing to remove if there's no state! + if t.State == nil { + return nil + } + + // get a map of all nodes by path, so we can connect anything that might be + // in the module + refMap := map[string][]dag.Vertex{} + for _, v := range g.Vertices() { + if pn, ok := v.(GraphNodeSubPath); ok { + path := normalizeModulePath(pn.Path())[1:] + p := modulePrefixStr(path) + refMap[p] = append(refMap[p], v) + } + } + + for _, m := range t.State.Modules { + c := t.Module.Child(m.Path[1:]) + if c != nil { + continue + } + + log.Printf("[DEBUG] module %s no longer in config\n", modulePrefixStr(m.Path)) + + node := &NodeModuleRemoved{PathValue: m.Path} + g.Add(node) + + // connect this to anything that contains the module's path + refPath := modulePrefixStr(m.Path) + for p, nodes := range refMap { + if strings.HasPrefix(p, refPath) { + for _, parent := range nodes { + g.Connect(dag.BasicEdge(node, parent)) + } + } + } + } + return nil +}