unused outputs in a destroy should be pruned

During a full destroy when outputs are removed, the
NodeDestroyableOutput was preventing it's sibling output from being
destroyed. Prune the output node if it only has its destroy node as a
dependent.

The destroy output test is simply run a second time with no state, which
would cause the output interpolation to fail if it remained in the
graph.
This commit is contained in:
James Bardin 2018-04-03 13:19:04 -04:00
parent db5ed51703
commit 620f1985a1
2 changed files with 52 additions and 18 deletions

View File

@ -6582,13 +6582,11 @@ module.child.child2:
func TestContext2Apply_destroyOutputs(t *testing.T) { func TestContext2Apply_destroyOutputs(t *testing.T) {
m := testModule(t, "apply-destroy-outputs") m := testModule(t, "apply-destroy-outputs")
h := new(HookRecordApplyOrder)
p := testProvider("aws") p := testProvider("aws")
p.ApplyFn = testApplyFn p.ApplyFn = testApplyFn
p.DiffFn = testDiffFn p.DiffFn = testDiffFn
ctx := testContext2(t, &ContextOpts{ ctx := testContext2(t, &ContextOpts{
Module: m, Module: m,
Hooks: []Hook{h},
ProviderResolver: ResourceProviderResolverFixed( ProviderResolver: ResourceProviderResolverFixed(
map[string]ResourceProviderFactory{ map[string]ResourceProviderFactory{
"aws": testProviderFuncFixed(p), "aws": testProviderFuncFixed(p),
@ -6608,12 +6606,10 @@ func TestContext2Apply_destroyOutputs(t *testing.T) {
} }
// Next, plan and apply a destroy operation // Next, plan and apply a destroy operation
h.Active = true
ctx = testContext2(t, &ContextOpts{ ctx = testContext2(t, &ContextOpts{
Destroy: true, Destroy: true,
State: state, State: state,
Module: m, Module: m,
Hooks: []Hook{h},
ProviderResolver: ResourceProviderResolverFixed( ProviderResolver: ResourceProviderResolverFixed(
map[string]ResourceProviderFactory{ map[string]ResourceProviderFactory{
"aws": testProviderFuncFixed(p), "aws": testProviderFuncFixed(p),
@ -6622,17 +6618,36 @@ func TestContext2Apply_destroyOutputs(t *testing.T) {
}) })
if _, err := ctx.Plan(); err != nil { if _, err := ctx.Plan(); err != nil {
t.Fatalf("err: %s", err) t.Fatal(err)
} }
state, err = ctx.Apply() state, err = ctx.Apply()
if err != nil { if err != nil {
t.Fatalf("err: %s", err) t.Fatal(err)
} }
mod := state.RootModule() mod := state.RootModule()
if len(mod.Resources) > 0 { if len(mod.Resources) > 0 {
t.Fatalf("bad: %#v", mod) t.Fatalf("expected no resources, got: %#v", mod)
}
// destroying again should produce no errors
ctx = testContext2(t, &ContextOpts{
Destroy: true,
State: state,
Module: m,
ProviderResolver: ResourceProviderResolverFixed(
map[string]ResourceProviderFactory{
"aws": testProviderFuncFixed(p),
},
),
})
if _, err := ctx.Plan(); err != nil {
t.Fatal(err)
}
if _, err = ctx.Apply(); err != nil {
t.Fatal(err)
} }
} }
@ -7766,7 +7781,7 @@ func TestContext2Apply_destroyProvisionerWithOutput(t *testing.T) {
}, },
Destroy: true, Destroy: true,
// targeting the source of the value used by all resources shoudl still // targeting the source of the value used by all resources should still
// destroy them all. // destroy them all.
Targets: []string{"module.mod.aws_instance.baz"}, Targets: []string{"module.mod.aws_instance.baz"},
}) })

View File

@ -119,8 +119,10 @@ func (t *DestroyValueReferenceTransformer) Transform(g *Graph) error {
type PruneUnusedValuesTransformer struct{} type PruneUnusedValuesTransformer struct{}
func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error { func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
vs := g.Vertices() // this might need multiple runs in order to ensure that pruning a value
for _, v := range vs { // doesn't effect a previously checked value.
for removed := 0; ; removed = 0 {
for _, v := range g.Vertices() {
switch v.(type) { switch v.(type) {
case *NodeApplyableOutput, *NodeLocal: case *NodeApplyableOutput, *NodeLocal:
// OK // OK
@ -128,8 +130,25 @@ func (t *PruneUnusedValuesTransformer) Transform(g *Graph) error {
continue continue
} }
if len(g.EdgesTo(v)) == 0 { dependants := g.UpEdges(v)
switch dependants.Len() {
case 0:
// nothing at all depends on this
g.Remove(v) g.Remove(v)
removed++
case 1:
// because an output's destroy node always depends on the output,
// we need to check for the case of a single destroy node.
d := dependants.List()[0]
if _, ok := d.(*NodeDestroyableOutput); ok {
g.Remove(v)
removed++
}
}
}
if removed == 0 {
break
} }
} }