From f7f1f17b49bd326cf42c7d71b7af792b9d755b4a Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 13 Feb 2015 15:57:37 -0800 Subject: [PATCH] terraform: create before destroy --- terraform/context_test.go | 1 - terraform/eval_state.go | 96 +++++++++++++++++++ terraform/graph_builder.go | 2 +- terraform/graph_config_node.go | 36 ++++++- terraform/graph_old.go | 2 +- .../main.tf | 9 ++ terraform/transform_destroy.go | 64 ++++++++++++- terraform/transform_destroy_test.go | 44 +++++++++ terraform/transform_resource.go | 26 ++++- terraform/transform_tainted.go | 21 ++-- terraform/transform_tainted_test.go | 1 - 11 files changed, 277 insertions(+), 25 deletions(-) create mode 100644 terraform/test-fixtures/transform-create-before-destroy-basic/main.tf diff --git a/terraform/context_test.go b/terraform/context_test.go index 6c308cf0c..e880bb499 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -3369,7 +3369,6 @@ func TestContext2Apply_provisionerFail(t *testing.T) { } } -/* func TestContext2Apply_provisionerFail_createBeforeDestroy(t *testing.T) { m := testModule(t, "apply-provisioner-fail-create-before") p := testProvider("aws") diff --git a/terraform/eval_state.go b/terraform/eval_state.go index 61e12662f..93225e2f9 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -126,3 +126,99 @@ func (n *EvalWriteState) Eval( func (n *EvalWriteState) Type() EvalType { return EvalTypeNull } + +// EvalDeposeState is an EvalNode implementation that reads the +// InstanceState for a specific resource out of the state. +type EvalDeposeState struct { + Name string +} + +func (n *EvalDeposeState) Args() ([]EvalNode, []EvalType) { + return nil, nil +} + +// TODO: test +func (n *EvalDeposeState) Eval( + ctx EvalContext, args []interface{}) (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 + } + + // If we don't have a primary, we have nothing to depose + if rs.Primary == nil { + return nil, nil + } + + // Depose to the tainted + rs.Tainted = append(rs.Tainted, rs.Primary) + rs.Primary = nil + + return nil, nil +} + +func (n *EvalDeposeState) Type() EvalType { + return EvalTypeNull +} + +// EvalUndeposeState is an EvalNode implementation that reads the +// InstanceState for a specific resource out of the state. +type EvalUndeposeState struct { + Name string +} + +func (n *EvalUndeposeState) Args() ([]EvalNode, []EvalType) { + return nil, nil +} + +// TODO: test +func (n *EvalUndeposeState) Eval( + ctx EvalContext, args []interface{}) (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 + } + + // If we don't have any tainted, then we don't have anything to do + if len(rs.Tainted) == 0 { + return nil, nil + } + + // Undepose to the tainted + idx := len(rs.Tainted) - 1 + rs.Primary = rs.Tainted[idx] + rs.Tainted[idx] = nil + rs.Tainted = rs.Tainted[:idx] + + return nil, nil +} + +func (n *EvalUndeposeState) Type() EvalType { + return EvalTypeNull +} diff --git a/terraform/graph_builder.go b/terraform/graph_builder.go index bc9482b2e..aec167f93 100644 --- a/terraform/graph_builder.go +++ b/terraform/graph_builder.go @@ -81,7 +81,6 @@ func (b *BuiltinGraphBuilder) Steps() []GraphTransformer { // Create all our resources from the configuration and state &ConfigTransformer{Module: b.Root}, &OrphanTransformer{State: b.State, Module: b.Root}, - &TaintedTransformer{State: b.State}, // Provider-related transformations &MissingProviderTransformer{Providers: b.Providers}, @@ -105,6 +104,7 @@ func (b *BuiltinGraphBuilder) Steps() []GraphTransformer { // Create the destruction nodes &DestroyTransformer{}, + &CreateBeforeDestroyTransformer{}, &PruneDestroyTransformer{Diff: b.Diff}, // Make sure we create one root diff --git a/terraform/graph_config_node.go b/terraform/graph_config_node.go index ee625349f..84ac9eef0 100644 --- a/terraform/graph_config_node.go +++ b/terraform/graph_config_node.go @@ -177,7 +177,7 @@ type GraphNodeConfigResource struct { // that logically this node is where it would happen. Destroy bool - destroyNode dag.Vertex + destroyNode GraphNodeDestroy } func (n *GraphNodeConfigResource) DependableName() []string { @@ -248,6 +248,11 @@ func (n *GraphNodeConfigResource) DynamicExpand(ctx EvalContext) (*Graph, error) State: state, View: n.Resource.Id(), }) + + steps = append(steps, &TaintedTransformer{ + State: state, + View: n.Resource.Id(), + }) } // Always end with the root being added @@ -288,7 +293,7 @@ func (n *GraphNodeConfigResource) ProvisionedBy() []string { } // GraphNodeDestroyable -func (n *GraphNodeConfigResource) DestroyNode() dag.Vertex { +func (n *GraphNodeConfigResource) DestroyNode() GraphNodeDestroy { // If we're already a destroy node, then don't do anything if n.Destroy { return nil @@ -300,13 +305,36 @@ func (n *GraphNodeConfigResource) DestroyNode() dag.Vertex { } // Just make a copy that is set to destroy - result := *n + result := &graphNodeResourceDestroy{ + GraphNodeConfigResource: *n, + Original: n, + } result.Destroy = true - n.destroyNode = &result + n.destroyNode = result return n.destroyNode } +// graphNodeResourceDestroy represents the logical destruction of a +// resource. This node doesn't mean it will be destroyed for sure, but +// instead that if a destroy were to happen, it must happen at this point. +type graphNodeResourceDestroy struct { + GraphNodeConfigResource + Original *GraphNodeConfigResource +} + +func (n *graphNodeResourceDestroy) CreateBeforeDestroy() bool { + return n.Original.Resource.Lifecycle.CreateBeforeDestroy +} + +func (n *graphNodeResourceDestroy) CreateNode() dag.Vertex { + return n.Original +} + +func (n *graphNodeResourceDestroy) DiffId() string { + return "" +} + // graphNodeModuleExpanded represents a module where the graph has // been expanded. It stores the graph of the module as well as a reference // to the map of variables. diff --git a/terraform/graph_old.go b/terraform/graph_old.go index 2cb8a8e44..f174bbe41 100644 --- a/terraform/graph_old.go +++ b/terraform/graph_old.go @@ -663,7 +663,7 @@ func graphAddDiff(g *depgraph.Graph, gDiff *Diff, d *ModuleDiff) error { } // Set the ReplacePrimary flag on the new instance so that - // it will become the new primary, and Diposed flag on the + // it will become the new primary, and Deposed flag on the // existing instance so that it will step down rn.Resource.Flags |= FlagReplacePrimary newNode.Resource.Flags |= FlagDeposed diff --git a/terraform/test-fixtures/transform-create-before-destroy-basic/main.tf b/terraform/test-fixtures/transform-create-before-destroy-basic/main.tf new file mode 100644 index 000000000..478c911c0 --- /dev/null +++ b/terraform/test-fixtures/transform-create-before-destroy-basic/main.tf @@ -0,0 +1,9 @@ +resource "aws_instance" "web" { + lifecycle { + create_before_destroy = true + } +} + +resource "aws_load_balancer" "lb" { + member = "${aws_instance.web.id}" +} diff --git a/terraform/transform_destroy.go b/terraform/transform_destroy.go index e9bd86430..87c98e901 100644 --- a/terraform/transform_destroy.go +++ b/terraform/transform_destroy.go @@ -14,14 +14,24 @@ type GraphNodeDestroyable interface { // DestroyNode returns the node used for the destroy. This should // return the same node every time so that it can be used later for // lookups as well. - DestroyNode() dag.Vertex + DestroyNode() GraphNodeDestroy } -// GraphNodeDestroyer is the interface that must implemented by +// GraphNodeDestroy is the interface that must implemented by // nodes that destroy. -type GraphNodeDestroyer interface { +type GraphNodeDestroy interface { dag.Vertex + // CreateBeforeDestroy is called to check whether this node + // should be created before it is destroyed. The CreateBeforeDestroy + // transformer uses this information to setup the graph. + CreateBeforeDestroy() bool + + // CreateNode returns the node used for the create side of this + // destroy. This must already exist within the graph. + CreateNode() dag.Vertex + + // Not used right now DiffId() string } @@ -94,6 +104,52 @@ func (t *DestroyTransformer) Transform(g *Graph) error { return nil } +// CreateBeforeDestroyTransformer is a GraphTransformer that modifies +// the destroys of some nodes so that the creation happens before the +// destroy. +type CreateBeforeDestroyTransformer struct{} + +func (t *CreateBeforeDestroyTransformer) Transform(g *Graph) error { + for _, v := range g.Vertices() { + // We only care to use the destroy nodes + dn, ok := v.(GraphNodeDestroy) + if !ok { + continue + } + + // If the node doesn't need to create before destroy, then continue + if !dn.CreateBeforeDestroy() { + continue + } + + // Get the creation side of this node + cn := dn.CreateNode() + + // Take all the things which depend on the web creation and + // make them dependencies on the destruction. Clarifying this + // with an example: if you have a web server and a load balancer + // and the load balancer depends on the web server, then when we + // do a create before destroy, we want to make sure the steps are: + // + // 1.) Create new web server + // 2.) Update load balancer + // 3.) Delete old web server + // + // This ensures that. + for _, sourceRaw := range g.UpEdges(cn).List() { + source := sourceRaw.(dag.Vertex) + g.Connect(dag.BasicEdge(dn, source)) + } + + // Swap the edge so that the destroy depends on the creation + // happening... + g.Connect(dag.BasicEdge(dn, cn)) + g.RemoveEdge(dag.BasicEdge(cn, dn)) + } + + return nil +} + // PruneDestroyTransformer is a GraphTransformer that removes the destroy // nodes that aren't in the diff. type PruneDestroyTransformer struct { @@ -108,7 +164,7 @@ func (t *PruneDestroyTransformer) Transform(g *Graph) error { for _, v := range g.Vertices() { // If it is not a destroyer, we don't care - dn, ok := v.(GraphNodeDestroyer) + dn, ok := v.(GraphNodeDestroy) if !ok { continue } diff --git a/terraform/transform_destroy_test.go b/terraform/transform_destroy_test.go index 5ca2b9b3e..97341e8e7 100644 --- a/terraform/transform_destroy_test.go +++ b/terraform/transform_destroy_test.go @@ -30,6 +30,38 @@ func TestDestroyTransformer(t *testing.T) { } } +func TestCreateBeforeDestroyTransformer(t *testing.T) { + mod := testModule(t, "transform-create-before-destroy-basic") + + g := Graph{Path: RootModulePath} + { + tf := &ConfigTransformer{Module: mod} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &DestroyTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + { + tf := &CreateBeforeDestroyTransformer{} + if err := tf.Transform(&g); err != nil { + t.Fatalf("err: %s", err) + } + } + + actual := strings.TrimSpace(g.String()) + expected := strings.TrimSpace(testTransformCreateBeforeDestroyBasicStr) + if actual != expected { + t.Fatalf("bad:\n\n%s", actual) + } +} + const testTransformDestroyBasicStr = ` aws_instance.bar aws_instance.bar (destroy) @@ -40,3 +72,15 @@ aws_instance.foo aws_instance.foo (destroy) aws_instance.bar (destroy) ` + +const testTransformCreateBeforeDestroyBasicStr = ` +aws_instance.web +aws_instance.web (destroy) + aws_instance.web + aws_load_balancer.lb + aws_load_balancer.lb (destroy) +aws_load_balancer.lb + aws_instance.web + aws_load_balancer.lb (destroy) +aws_load_balancer.lb (destroy) +` diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 1a183b9dc..940a53fa1 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -251,6 +251,15 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { Node: EvalNoop{}, }, + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + return n.Resource.Lifecycle.CreateBeforeDestroy, nil + }, + Node: &EvalDeposeState{ + Name: n.stateId(), + }, + }, + &EvalDiff{ Info: info, Config: interpolateNode, @@ -304,6 +313,15 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { Tainted: &tainted, Error: &err, }, + &EvalIf{ + If: func(ctx EvalContext) (bool, error) { + return n.Resource.Lifecycle.CreateBeforeDestroy && + tainted, nil + }, + Node: &EvalUndeposeState{ + Name: n.stateId(), + }, + }, &EvalWriteState{ Name: n.stateId(), ResourceType: n.Resource.Type, @@ -311,7 +329,7 @@ func (n *graphNodeExpandedResource) EvalTree() EvalNode { State: &state, Tainted: &tainted, TaintedIndex: -1, - TaintedClearPrimary: true, + TaintedClearPrimary: !n.Resource.Lifecycle.CreateBeforeDestroy, }, &EvalApplyPost{ Info: info, @@ -389,8 +407,10 @@ func (n *graphNodeExpandedResourceDestroy) EvalTree() EvalNode { Output: &provider, }, &EvalReadState{ - Name: n.stateId(), - Output: &state, + Name: n.stateId(), + Output: &state, + Tainted: n.Resource.Lifecycle.CreateBeforeDestroy, + TaintedIndex: -1, }, &EvalApply{ Info: info, diff --git a/terraform/transform_tainted.go b/terraform/transform_tainted.go index 2e1936fd3..74e15cb45 100644 --- a/terraform/transform_tainted.go +++ b/terraform/transform_tainted.go @@ -10,6 +10,10 @@ type TaintedTransformer struct { // State is the global state. We'll automatically find the correct // ModuleState based on the Graph.Path that is being transformed. State *State + + // View, if non-empty, is the ModuleState.View used around the state + // to find tainted resources. + View string } func (t *TaintedTransformer) Transform(g *Graph) error { @@ -20,6 +24,11 @@ func (t *TaintedTransformer) Transform(g *Graph) error { return nil } + // If we have a view, apply it now + if t.View != "" { + state = state.View(t.View) + } + // Go through all the resources in our state to look for tainted resources for k, rs := range state.Resources { // If we have no tainted resources, then move on @@ -31,16 +40,14 @@ func (t *TaintedTransformer) Transform(g *Graph) error { // Add the graph node and make the connection from any untainted // resources with this name to the tainted resource, so that // the tainted resource gets destroyed first. - g.ConnectFrom(k, g.Add(&graphNodeTaintedResource{ + g.Add(&graphNodeTaintedResource{ Index: i, ResourceName: k, ResourceType: rs.Type, - })) + }) } } - // TODO: Any other dependencies? - return nil } @@ -51,10 +58,6 @@ type graphNodeTaintedResource struct { ResourceType string } -func (n *graphNodeTaintedResource) DependentOn() []string { - return []string{n.ResourceName} -} - func (n *graphNodeTaintedResource) Name() string { return fmt.Sprintf("%s (tainted #%d)", n.ResourceName, n.Index+1) } @@ -94,7 +97,6 @@ func (n *graphNodeTaintedResource) EvalTree() EvalNode { &EvalWriteState{ Name: n.ResourceName, ResourceType: n.ResourceType, - Dependencies: n.DependentOn(), State: &state, Tainted: &tainted, TaintedIndex: n.Index, @@ -139,7 +141,6 @@ func (n *graphNodeTaintedResource) EvalTree() EvalNode { &EvalWriteState{ Name: n.ResourceName, ResourceType: n.ResourceType, - Dependencies: n.DependentOn(), State: &state, Tainted: &tainted, TaintedIndex: n.Index, diff --git a/terraform/transform_tainted_test.go b/terraform/transform_tainted_test.go index 5ce20ff4b..f78e86b5c 100644 --- a/terraform/transform_tainted_test.go +++ b/terraform/transform_tainted_test.go @@ -60,6 +60,5 @@ func TestGraphNodeTaintedResource_ProvidedBy(t *testing.T) { const testTransformTaintedBasicStr = ` aws_instance.web - aws_instance.web (tainted #1) aws_instance.web (tainted #1) `