From d97b24e3c13e7c74bbe4f3be77e161bb91812cbf Mon Sep 17 00:00:00 2001 From: Sander van Harmelen Date: Thu, 21 Apr 2016 21:59:10 +0200 Subject: [PATCH] Add tests and fix last issues --- command/apply_test.go | 4 + command/plan_test.go | 2 + command/taint_test.go | 10 +- command/untaint_test.go | 155 +++++------------------- terraform/context_apply_test.go | 75 ++++++------ terraform/context_plan_test.go | 68 ++--------- terraform/context_refresh_test.go | 17 ++- terraform/context_test.go | 42 ++++--- terraform/context_validate_test.go | 7 +- terraform/eval_apply.go | 11 +- terraform/eval_diff.go | 10 +- terraform/eval_state.go | 5 +- terraform/eval_state_test.go | 41 ------- terraform/graph_builder_test.go | 5 - terraform/graph_config_node_resource.go | 6 +- terraform/resource.go | 2 - terraform/state.go | 34 +++--- terraform/state_filter.go | 13 -- terraform/state_test.go | 152 ++++++----------------- terraform/terraform_test.go | 19 ++- terraform/transform_destroy_test.go | 22 +--- terraform/transform_resource.go | 40 +++--- 22 files changed, 229 insertions(+), 511 deletions(-) diff --git a/command/apply_test.go b/command/apply_test.go index b847a769d..a01ff03a4 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -1361,16 +1361,20 @@ const applyVarFileJSON = ` const testApplyDisableBackupStr = ` ID = bar +Tainted = false ` const testApplyDisableBackupStateStr = ` ID = bar +Tainted = false ` const testApplyStateStr = ` ID = bar +Tainted = false ` const testApplyStateDiffStr = ` ID = bar +Tainted = false ` diff --git a/command/plan_test.go b/command/plan_test.go index e0bc6c41d..710d993a9 100644 --- a/command/plan_test.go +++ b/command/plan_test.go @@ -603,8 +603,10 @@ const testPlanNoStateStr = ` const testPlanStateStr = ` ID = bar +Tainted = false ` const testPlanStateDefaultStr = ` ID = bar +Tainted = false ` diff --git a/command/taint_test.go b/command/taint_test.go index 617555b00..c2ad652fa 100644 --- a/command/taint_test.go +++ b/command/taint_test.go @@ -347,9 +347,8 @@ func TestTaint_module(t *testing.T) { } const testTaintStr = ` -test_instance.foo: (1 tainted) - ID = - Tainted ID 1 = bar +test_instance.foo: (tainted) + ID = bar ` const testTaintDefaultStr = ` @@ -362,7 +361,6 @@ test_instance.foo: ID = bar module.child: - test_instance.blah: (1 tainted) - ID = - Tainted ID 1 = blah + test_instance.blah: (tainted) + ID = blah ` diff --git a/command/untaint_test.go b/command/untaint_test.go index 4627f9fa8..4923a6f7c 100644 --- a/command/untaint_test.go +++ b/command/untaint_test.go @@ -17,8 +17,9 @@ func TestUntaint(t *testing.T) { Resources: map[string]*terraform.ResourceState{ "test_instance.foo": &terraform.ResourceState{ Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, + Primary: &terraform.InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -49,101 +50,6 @@ test_instance.foo: testStateOutput(t, statePath, expected) } -func TestUntaint_indexRequired(t *testing.T) { - state := &terraform.State{ - Modules: []*terraform.ModuleState{ - &terraform.ModuleState{ - Path: []string{"root"}, - Resources: map[string]*terraform.ResourceState{ - "test_instance.foo": &terraform.ResourceState{ - Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, - &terraform.InstanceState{ID: "bar2"}, - }, - }, - }, - }, - }, - } - statePath := testStateFile(t, state) - - ui := new(cli.MockUi) - c := &UntaintCommand{ - Meta: Meta{ - Ui: ui, - }, - } - - args := []string{ - "-state", statePath, - "test_instance.foo", - } - if code := c.Run(args); code == 0 { - t.Fatalf("Expected non-zero exit. Output:\n\n%s", ui.OutputWriter.String()) - } - - // Nothing should have gotten untainted - expected := strings.TrimSpace(` -test_instance.foo: (2 tainted) - ID = - Tainted ID 1 = bar - Tainted ID 2 = bar2 - `) - testStateOutput(t, statePath, expected) - - // Should have gotten an error message mentioning index - errOut := ui.ErrorWriter.String() - errContains := "please specify an index" - if !strings.Contains(errOut, errContains) { - t.Fatalf("Expected err output: %s, to contain: %s", errOut, errContains) - } -} - -func TestUntaint_indexSpecified(t *testing.T) { - state := &terraform.State{ - Modules: []*terraform.ModuleState{ - &terraform.ModuleState{ - Path: []string{"root"}, - Resources: map[string]*terraform.ResourceState{ - "test_instance.foo": &terraform.ResourceState{ - Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, - &terraform.InstanceState{ID: "bar2"}, - }, - }, - }, - }, - }, - } - statePath := testStateFile(t, state) - - ui := new(cli.MockUi) - c := &UntaintCommand{ - Meta: Meta{ - Ui: ui, - }, - } - - args := []string{ - "-state", statePath, - "-index", "1", - "test_instance.foo", - } - if code := c.Run(args); code != 0 { - t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) - } - - // Nothing should have gotten untainted - expected := strings.TrimSpace(` -test_instance.foo: (1 tainted) - ID = bar2 - Tainted ID 1 = bar - `) - testStateOutput(t, statePath, expected) -} - func TestUntaint_backup(t *testing.T) { // Get a temp cwd tmp, cwd := testCwd(t) @@ -157,8 +63,9 @@ func TestUntaint_backup(t *testing.T) { Resources: map[string]*terraform.ResourceState{ "test_instance.foo": &terraform.ResourceState{ Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, + Primary: &terraform.InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -183,9 +90,8 @@ func TestUntaint_backup(t *testing.T) { // Backup is still tainted testStateOutput(t, path+".backup", strings.TrimSpace(` -test_instance.foo: (1 tainted) - ID = - Tainted ID 1 = bar +test_instance.foo: (tainted) + ID = bar `)) // State is untainted @@ -208,8 +114,9 @@ func TestUntaint_backupDisable(t *testing.T) { Resources: map[string]*terraform.ResourceState{ "test_instance.foo": &terraform.ResourceState{ Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, + Primary: &terraform.InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -273,8 +180,9 @@ func TestUntaint_defaultState(t *testing.T) { Resources: map[string]*terraform.ResourceState{ "test_instance.foo": &terraform.ResourceState{ Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, + Primary: &terraform.InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -311,8 +219,9 @@ func TestUntaint_missing(t *testing.T) { Resources: map[string]*terraform.ResourceState{ "test_instance.foo": &terraform.ResourceState{ Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, + Primary: &terraform.InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -345,8 +254,9 @@ func TestUntaint_missingAllow(t *testing.T) { Resources: map[string]*terraform.ResourceState{ "test_instance.foo": &terraform.ResourceState{ Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, + Primary: &terraform.InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -385,8 +295,9 @@ func TestUntaint_stateOut(t *testing.T) { Resources: map[string]*terraform.ResourceState{ "test_instance.foo": &terraform.ResourceState{ Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, + Primary: &terraform.InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -411,9 +322,8 @@ func TestUntaint_stateOut(t *testing.T) { } testStateOutput(t, path, strings.TrimSpace(` -test_instance.foo: (1 tainted) - ID = - Tainted ID 1 = bar +test_instance.foo: (tainted) + ID = bar `)) testStateOutput(t, "foo", strings.TrimSpace(` test_instance.foo: @@ -429,8 +339,9 @@ func TestUntaint_module(t *testing.T) { Resources: map[string]*terraform.ResourceState{ "test_instance.foo": &terraform.ResourceState{ Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, + Primary: &terraform.InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -440,8 +351,9 @@ func TestUntaint_module(t *testing.T) { Resources: map[string]*terraform.ResourceState{ "test_instance.blah": &terraform.ResourceState{ Type: "test_instance", - Tainted: []*terraform.InstanceState{ - &terraform.InstanceState{ID: "bar"}, + Primary: &terraform.InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -467,9 +379,8 @@ func TestUntaint_module(t *testing.T) { } testStateOutput(t, statePath, strings.TrimSpace(` -test_instance.foo: (1 tainted) - ID = - Tainted ID 1 = bar +test_instance.foo: (tainted) + ID = bar module.child: test_instance.blah: diff --git a/terraform/context_apply_test.go b/terraform/context_apply_test.go index 150ac13fe..99f2d805b 100644 --- a/terraform/context_apply_test.go +++ b/terraform/context_apply_test.go @@ -884,14 +884,13 @@ func TestContext2Apply_countTainted(t *testing.T) { Resources: map[string]*ResourceState{ "aws_instance.foo.0": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "bar", - Attributes: map[string]string{ - "foo": "foo", - "type": "aws_instance", - }, + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "foo": "foo", + "type": "aws_instance", }, + Tainted: true, }, }, }, @@ -2989,13 +2988,12 @@ func TestContext2Apply_destroyTaintedProvisioner(t *testing.T) { Resources: map[string]*ResourceState{ "aws_instance.foo": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "bar", - Attributes: map[string]string{ - "id": "bar", - }, + Primary: &InstanceState{ + ID: "bar", + Attributes: map[string]string{ + "id": "bar", }, + Tainted: true, }, }, }, @@ -3505,14 +3503,13 @@ func TestContext2Apply_taint(t *testing.T) { Resources: map[string]*ResourceState{ "aws_instance.bar": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "baz", - Attributes: map[string]string{ - "num": "2", - "type": "aws_instance", - }, + Primary: &InstanceState{ + ID: "baz", + Attributes: map[string]string{ + "num": "2", + "type": "aws_instance", }, + Tainted: true, }, }, }, @@ -3559,14 +3556,13 @@ func TestContext2Apply_taintDep(t *testing.T) { Resources: map[string]*ResourceState{ "aws_instance.foo": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "baz", - Attributes: map[string]string{ - "num": "2", - "type": "aws_instance", - }, + Primary: &InstanceState{ + ID: "baz", + Attributes: map[string]string{ + "num": "2", + "type": "aws_instance", }, + Tainted: true, }, }, "aws_instance.bar": &ResourceState{ @@ -3622,14 +3618,13 @@ func TestContext2Apply_taintDepRequiresNew(t *testing.T) { Resources: map[string]*ResourceState{ "aws_instance.foo": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "baz", - Attributes: map[string]string{ - "num": "2", - "type": "aws_instance", - }, + Primary: &InstanceState{ + ID: "baz", + Attributes: map[string]string{ + "num": "2", + "type": "aws_instance", }, + Tainted: true, }, }, "aws_instance.bar": &ResourceState{ @@ -4438,10 +4433,9 @@ func TestContext2Apply_targetedWithTaintedInState(t *testing.T) { Path: rootModulePath, Resources: map[string]*ResourceState{ "aws_instance.ifailedprovisioners": &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ - ID: "ifailedprovisioners", - }, + Primary: &InstanceState{ + ID: "ifailedprovisioners", + Tainted: true, }, }, }, @@ -4485,9 +4479,8 @@ func TestContext2Apply_targetedWithTaintedInState(t *testing.T) { expected := strings.TrimSpace(` aws_instance.iambeingadded: ID = foo -aws_instance.ifailedprovisioners: (1 tainted) - ID = - Tainted ID 1 = ifailedprovisioners +aws_instance.ifailedprovisioners: (tainted) + ID = ifailedprovisioners `) if actual != expected { t.Fatalf("expected state: \n%s\ngot: \n%s", expected, actual) diff --git a/terraform/context_plan_test.go b/terraform/context_plan_test.go index a0dd442ab..956065383 100644 --- a/terraform/context_plan_test.go +++ b/terraform/context_plan_test.go @@ -1718,10 +1718,9 @@ func TestContext2Plan_taint(t *testing.T) { }, "aws_instance.bar": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "baz", - }, + Primary: &InstanceState{ + ID: "baz", + Tainted: true, }, }, }, @@ -1748,57 +1747,6 @@ func TestContext2Plan_taint(t *testing.T) { } } -func TestContext2Plan_multiple_taint(t *testing.T) { - m := testModule(t, "plan-taint") - p := testProvider("aws") - p.DiffFn = testDiffFn - s := &State{ - Modules: []*ModuleState{ - &ModuleState{ - Path: rootModulePath, - Resources: map[string]*ResourceState{ - "aws_instance.foo": &ResourceState{ - Type: "aws_instance", - Primary: &InstanceState{ - ID: "bar", - Attributes: map[string]string{"num": "2"}, - }, - }, - "aws_instance.bar": &ResourceState{ - Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "baz", - }, - &InstanceState{ - ID: "zip", - }, - }, - }, - }, - }, - }, - } - ctx := testContext2(t, &ContextOpts{ - Module: m, - Providers: map[string]ResourceProviderFactory{ - "aws": testProviderFuncFixed(p), - }, - State: s, - }) - - plan, err := ctx.Plan() - if err != nil { - t.Fatalf("err: %s", err) - } - - actual := strings.TrimSpace(plan.String()) - expected := strings.TrimSpace(testTerraformPlanMultipleTaintStr) - if actual != expected { - t.Fatalf("bad:\n%s", actual) - } -} - // Fails about 50% of the time before the fix for GH-4982, covers the fix. func TestContext2Plan_taintDestroyInterpolatedCountRace(t *testing.T) { m := testModule(t, "plan-taint-interpolated-count") @@ -1811,8 +1759,9 @@ func TestContext2Plan_taintDestroyInterpolatedCountRace(t *testing.T) { Resources: map[string]*ResourceState{ "aws_instance.foo.0": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ID: "bar"}, + Primary: &InstanceState{ + ID: "bar", + Tainted: true, }, }, "aws_instance.foo.1": &ResourceState{ @@ -1849,9 +1798,8 @@ DESTROY/CREATE: aws_instance.foo.0 STATE: -aws_instance.foo.0: (1 tainted) - ID = - Tainted ID 1 = bar +aws_instance.foo.0: (tainted) + ID = bar aws_instance.foo.1: ID = bar aws_instance.foo.2: diff --git a/terraform/context_refresh_test.go b/terraform/context_refresh_test.go index 862028eee..9f4ee58f2 100644 --- a/terraform/context_refresh_test.go +++ b/terraform/context_refresh_test.go @@ -324,10 +324,9 @@ func TestContext2Refresh_modules(t *testing.T) { Resources: map[string]*ResourceState{ "aws_instance.web": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "bar", - }, + Primary: &InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -650,10 +649,9 @@ func TestContext2Refresh_tainted(t *testing.T) { Resources: map[string]*ResourceState{ "aws_instance.web": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "bar", - }, + Primary: &InstanceState{ + ID: "bar", + Tainted: true, }, }, }, @@ -670,7 +668,8 @@ func TestContext2Refresh_tainted(t *testing.T) { p.RefreshFn = nil p.RefreshReturn = &InstanceState{ - ID: "foo", + ID: "foo", + Tainted: true, } s, err := ctx.Refresh() diff --git a/terraform/context_test.go b/terraform/context_test.go index eee648cd2..897539f6d 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -107,9 +107,13 @@ func testDiffFn( info *InstanceInfo, s *InstanceState, c *ResourceConfig) (*InstanceDiff, error) { - var diff InstanceDiff + diff := new(InstanceDiff) diff.Attributes = make(map[string]*ResourceAttrDiff) + if s != nil { + diff.DestroyTainted = s.Tainted + } + for k, v := range c.Raw { if _, ok := v.(string); !ok { continue @@ -181,17 +185,21 @@ func testDiffFn( } } - for k, v := range diff.Attributes { - if v.NewComputed { - continue - } + // If we recreate this resource because it's tainted, we keep all attrs + if !diff.RequiresNew() { + for k, v := range diff.Attributes { + if v.NewComputed { + continue + } - old, ok := s.Attributes[k] - if !ok { - continue - } - if old == v.New { - delete(diff.Attributes, k) + old, ok := s.Attributes[k] + if !ok { + continue + } + + if old == v.New { + delete(diff.Attributes, k) + } } } @@ -202,7 +210,7 @@ func testDiffFn( } } - return &diff, nil + return diff, nil } func testProvider(prefix string) *MockResourceProvider { @@ -276,9 +284,8 @@ root ` const testContextRefreshModuleStr = ` -aws_instance.web: (1 tainted) - ID = - Tainted ID 1 = bar +aws_instance.web: (tainted) + ID = bar module.child: aws_instance.web: @@ -300,7 +307,6 @@ const testContextRefreshOutputPartialStr = ` ` const testContextRefreshTaintedStr = ` -aws_instance.web: (1 tainted) - ID = - Tainted ID 1 = foo +aws_instance.web: (tainted) + ID = foo ` diff --git a/terraform/context_validate_test.go b/terraform/context_validate_test.go index 56de23a36..8b47a17eb 100644 --- a/terraform/context_validate_test.go +++ b/terraform/context_validate_test.go @@ -651,10 +651,9 @@ func TestContext2Validate_tainted(t *testing.T) { Resources: map[string]*ResourceState{ "aws_instance.foo": &ResourceState{ Type: "aws_instance", - Tainted: []*InstanceState{ - &InstanceState{ - ID: "bar", - }, + Primary: &InstanceState{ + ID: "bar", + Tainted: true, }, }, }, diff --git a/terraform/eval_apply.go b/terraform/eval_apply.go index 6314baa86..fd687c5a1 100644 --- a/terraform/eval_apply.go +++ b/terraform/eval_apply.go @@ -139,7 +139,6 @@ type EvalApplyProvisioners struct { Resource *config.Resource InterpResource *Resource CreateNew *bool - Tainted *bool Error *error } @@ -159,9 +158,7 @@ func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) { if n.Error != nil && *n.Error != nil { // We're already errored creating, so mark as tainted and continue - if n.Tainted != nil { - *n.Tainted = true - } + state.Tainted = true // We're already tainted, so just return out return nil, nil @@ -180,10 +177,10 @@ func (n *EvalApplyProvisioners) Eval(ctx EvalContext) (interface{}, error) { // If there are no errors, then we append it to our output error // if we have one, otherwise we just output it. err := n.apply(ctx) - if n.Tainted != nil { - *n.Tainted = err != nil - } if err != nil { + // Provisioning failed, so mark the resource as tainted + state.Tainted = true + if n.Error != nil { *n.Error = multierror.Append(*n.Error, err) } else { diff --git a/terraform/eval_diff.go b/terraform/eval_diff.go index 10deacb26..6cbd20e03 100644 --- a/terraform/eval_diff.go +++ b/terraform/eval_diff.go @@ -69,8 +69,9 @@ type EvalDiff struct { Info *InstanceInfo Config **ResourceConfig Provider *ResourceProvider + Diff **InstanceDiff State **InstanceState - Output **InstanceDiff + OutputDiff **InstanceDiff OutputState **InstanceState } @@ -104,6 +105,11 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { diff = new(InstanceDiff) } + // Preserve the DestroyTainted flag + if n.Diff != nil { + diff.DestroyTainted = (*n.Diff).DestroyTainted + } + // Require a destroy if there is an ID and it requires new. if diff.RequiresNew() && state != nil && state.ID != "" { diff.Destroy = true @@ -135,7 +141,7 @@ func (n *EvalDiff) Eval(ctx EvalContext) (interface{}, error) { } // Update our output - *n.Output = diff + *n.OutputDiff = diff // Update the state if we care if n.OutputState != nil { diff --git a/terraform/eval_state.go b/terraform/eval_state.go index aa1d1e28e..35e7c2f26 100644 --- a/terraform/eval_state.go +++ b/terraform/eval_state.go @@ -285,7 +285,8 @@ func (n *EvalDeposeState) Eval(ctx EvalContext) (interface{}, error) { // EvalUndeposeState is an EvalNode implementation that reads the // InstanceState for a specific resource out of the state. type EvalUndeposeState struct { - Name string + Name string + State **InstanceState } // TODO: test @@ -316,7 +317,7 @@ func (n *EvalUndeposeState) Eval(ctx EvalContext) (interface{}, error) { // Undepose idx := len(rs.Deposed) - 1 rs.Primary = rs.Deposed[idx] - rs.Deposed[idx] = nil + rs.Deposed[idx] = *n.State return nil, nil } diff --git a/terraform/eval_state_test.go b/terraform/eval_state_test.go index 3e5265f4a..e702634e0 100644 --- a/terraform/eval_state_test.go +++ b/terraform/eval_state_test.go @@ -88,21 +88,6 @@ func TestEvalReadState(t *testing.T) { }, ExpectedInstanceId: "i-abc123", }, - "ReadStateTainted gets tainted instance": { - Resources: map[string]*ResourceState{ - "aws_instance.bar": &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "i-abc123"}, - }, - }, - }, - Node: &EvalReadStateTainted{ - Name: "aws_instance.bar", - Output: &output, - Index: 0, - }, - ExpectedInstanceId: "i-abc123", - }, "ReadStateDeposed gets deposed instance": { Resources: map[string]*ResourceState{ "aws_instance.bar": &ResourceState{ @@ -175,32 +160,6 @@ restype.resname: `) } -func TestEvalWriteStateTainted(t *testing.T) { - state := &State{} - ctx := new(MockEvalContext) - ctx.StateState = state - ctx.StateLock = new(sync.RWMutex) - ctx.PathPath = rootModulePath - - is := &InstanceState{ID: "i-abc123"} - node := &EvalWriteStateTainted{ - Name: "restype.resname", - ResourceType: "restype", - State: &is, - Index: -1, - } - _, err := node.Eval(ctx) - if err != nil { - t.Fatalf("Got err: %#v", err) - } - - checkStateString(t, state, ` -restype.resname: (1 tainted) - ID = - Tainted ID 1 = i-abc123 - `) -} - func TestEvalWriteStateDeposed(t *testing.T) { state := &State{} ctx := new(MockEvalContext) diff --git a/terraform/graph_builder_test.go b/terraform/graph_builder_test.go index b2ddd8d4e..2cb76757b 100644 --- a/terraform/graph_builder_test.go +++ b/terraform/graph_builder_test.go @@ -295,16 +295,11 @@ provider.aws (close) const testBuiltinGraphBuilderVerboseStr = ` aws_instance.db - aws_instance.db (destroy tainted) aws_instance.db (destroy) -aws_instance.db (destroy tainted) - aws_instance.web (destroy tainted) aws_instance.db (destroy) aws_instance.web (destroy) aws_instance.web aws_instance.db -aws_instance.web (destroy tainted) - provider.aws aws_instance.web (destroy) provider.aws provider.aws diff --git a/terraform/graph_config_node_resource.go b/terraform/graph_config_node_resource.go index dbe5a0ecd..36e1d9112 100644 --- a/terraform/graph_config_node_resource.go +++ b/terraform/graph_config_node_resource.go @@ -120,7 +120,11 @@ func (n *GraphNodeConfigResource) VarWalk(fn func(config.InterpolatedVariable)) } func (n *GraphNodeConfigResource) Name() string { - return n.Resource.Id() + " (destroy)" + result := n.Resource.Id() + if n.Destroy { + result += " (destroy)" + } + return result } // GraphNodeDotter impl. diff --git a/terraform/resource.go b/terraform/resource.go index 5b9fbf7d2..7d22a6e9b 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -42,7 +42,6 @@ type Resource struct { State *InstanceState Provisioners []*ResourceProvisionerConfig Flags ResourceFlag - TaintedIndex int } // ResourceKind specifies what kind of instance we're working with, whether @@ -53,7 +52,6 @@ const ( FlagPrimary ResourceFlag = 1 << iota FlagTainted FlagOrphan - FlagHasTainted FlagReplacePrimary FlagDeposed ) diff --git a/terraform/state.go b/terraform/state.go index 19b781b55..02ee9cf4f 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -327,7 +327,7 @@ func (s *State) removeInstance(path []string, r *ResourceState, v *InstanceState } // Check lists - lists := [][]*InstanceState{r.Tainted, r.Deposed} + lists := [][]*InstanceState{r.Deposed} for _, is := range lists { for i, instance := range is { if instance == v { @@ -861,7 +861,11 @@ func (m *ModuleState) String() string { } for idx, t := range rs.Deposed { - buf.WriteString(fmt.Sprintf(" Deposed ID %d = %s\n", idx+1, t.ID)) + taintStr := "" + if t.Tainted { + taintStr = " (tainted)" + } + buf.WriteString(fmt.Sprintf(" Deposed ID %d = %s%s\n", idx+1, t.ID, taintStr)) } if len(rs.Dependencies) > 0 { @@ -1030,11 +1034,14 @@ type ResourceState struct { // Deposed is used in the mechanics of CreateBeforeDestroy: the existing // Primary is Deposed to get it out of the way for the replacement Primary to // be created by Apply. If the replacement Primary creates successfully, the - // Deposed instance is cleaned up. If there were problems creating the - // replacement, the instance remains in the Deposed list so it can be - // destroyed in a future run. Functionally, Deposed instances are very - // similar to Tainted instances in that Terraform is only tracking them in - // order to remember to destroy them. + // Deposed instance is cleaned up. + // + // If there were problems creating the replacement Primary, the Deposed + // instance and the (now tainted) replacement Primary will be swapped so the + // tainted replacement will be cleaned up instead. + // + // An instance will remain in the Deposed list until it is successfully + // destroyed and purged. Deposed []*InstanceState `json:"deposed,omitempty"` // Provider is used when a resource is connected to a provider with an alias. @@ -1071,22 +1078,21 @@ func (s *ResourceState) Equal(other *ResourceState) bool { return false } - // Tainted - if s.Primary.Tainted != other.Primary.Tainted { - return false - } - return true } // Taint marks a resource as tainted. func (r *ResourceState) Taint() { - r.Primary.Tainted = true + if r.Primary != nil { + r.Primary.Tainted = true + } } // Untaint unmarks a resource as tainted. func (r *ResourceState) Untaint() { - r.Primary.Tainted = false + if r.Primary != nil { + r.Primary.Tainted = false + } } func (r *ResourceState) init() { diff --git a/terraform/state_filter.go b/terraform/state_filter.go index 7a2937123..89cf0d898 100644 --- a/terraform/state_filter.go +++ b/terraform/state_filter.go @@ -133,19 +133,6 @@ func (f *StateFilter) filterSingle(a *ResourceAddress) []*StateFilterResult { }) } - for _, instance := range r.Tainted { - if f.relevant(a, instance) { - addr.InstanceType = TypeTainted - addr.InstanceTypeSet = true - results = append(results, &StateFilterResult{ - Path: addr.Path, - Address: addr.String(), - Parent: resourceResult, - Value: instance, - }) - } - } - for _, instance := range r.Deposed { if f.relevant(a, instance) { addr.InstanceType = TypeDeposed diff --git a/terraform/state_test.go b/terraform/state_test.go index 7a7dd0f0a..e3ee69137 100644 --- a/terraform/state_test.go +++ b/terraform/state_test.go @@ -742,11 +742,14 @@ func TestResourceStateEqual(t *testing.T) { { false, &ResourceState{ - Tainted: nil, + Primary: &InstanceState{ + ID: "foo", + }, }, &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, + Primary: &InstanceState{ + ID: "foo", + Tainted: true, }, }, }, @@ -754,28 +757,15 @@ func TestResourceStateEqual(t *testing.T) { { true, &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, + Primary: &InstanceState{ + ID: "foo", + Tainted: true, }, }, &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, - }, - }, - }, - - { - true, - &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, - nil, - }, - }, - &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, + Primary: &InstanceState{ + ID: "foo", + Tainted: true, }, }, }, @@ -801,28 +791,29 @@ func TestResourceStateTaint(t *testing.T) { &ResourceState{}, }, - "primary, no tainted": { + "primary, not tainted": { &ResourceState{ Primary: &InstanceState{ID: "foo"}, }, &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, + Primary: &InstanceState{ + ID: "foo", + Tainted: true, }, }, }, - "primary, with tainted": { + "primary, tainted": { &ResourceState{ - Primary: &InstanceState{ID: "foo"}, - Tainted: []*InstanceState{ - &InstanceState{ID: "bar"}, + Primary: &InstanceState{ + ID: "foo", + Tainted: true, }, }, &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "bar"}, - &InstanceState{ID: "foo"}, + Primary: &InstanceState{ + ID: "foo", + Tainted: true, }, }, }, @@ -841,92 +832,36 @@ func TestResourceStateTaint(t *testing.T) { func TestResourceStateUntaint(t *testing.T) { cases := map[string]struct { Input *ResourceState - Index func() int ExpectedOutput *ResourceState - ExpectedErrMsg string }{ - "no primary, no tainted, err": { + "no primary, err": { Input: &ResourceState{}, ExpectedOutput: &ResourceState{}, - ExpectedErrMsg: "Nothing to untaint", }, - "one tainted, no primary": { + "primary, not tainted": { Input: &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, + Primary: &InstanceState{ID: "foo"}, + }, + ExpectedOutput: &ResourceState{ + Primary: &InstanceState{ID: "foo"}, + }, + }, + "primary, tainted": { + Input: &ResourceState{ + Primary: &InstanceState{ + ID: "foo", + Tainted: true, }, }, ExpectedOutput: &ResourceState{ Primary: &InstanceState{ID: "foo"}, - Tainted: []*InstanceState{}, }, }, - - "one tainted, existing primary error": { - Input: &ResourceState{ - Primary: &InstanceState{ID: "foo"}, - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, - }, - }, - ExpectedErrMsg: "Resource has a primary", - }, - - "multiple tainted, no index": { - Input: &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "bar"}, - &InstanceState{ID: "foo"}, - }, - }, - ExpectedErrMsg: "please specify an index", - }, - - "multiple tainted, with index": { - Input: &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "bar"}, - &InstanceState{ID: "foo"}, - }, - }, - Index: func() int { return 1 }, - ExpectedOutput: &ResourceState{ - Primary: &InstanceState{ID: "foo"}, - Tainted: []*InstanceState{ - &InstanceState{ID: "bar"}, - }, - }, - }, - - "index out of bounds error": { - Input: &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "bar"}, - &InstanceState{ID: "foo"}, - }, - }, - Index: func() int { return 2 }, - ExpectedErrMsg: "out of range", - }, } for k, tc := range cases { - index := -1 - if tc.Index != nil { - index = tc.Index() - } - err := tc.Input.Untaint(index) - if tc.ExpectedErrMsg == "" && err != nil { - t.Fatalf("[%s] unexpected err: %s", k, err) - } - if tc.ExpectedErrMsg != "" { - if strings.Contains(err.Error(), tc.ExpectedErrMsg) { - continue - } - t.Fatalf("[%s] expected err: %s to contain: %s", - k, err, tc.ExpectedErrMsg) - } + tc.Input.Untaint() if !reflect.DeepEqual(tc.Input, tc.ExpectedOutput) { t.Fatalf( "Failure: %s\n\nExpected: %#v\n\nGot: %#v", @@ -1152,7 +1087,7 @@ func TestInstanceState_MergeDiff(t *testing.T) { } func TestInstanceState_MergeDiff_nil(t *testing.T) { - var is *InstanceState = nil + var is *InstanceState diff := &InstanceDiff{ Attributes: map[string]*ResourceAttrDiff{ @@ -1504,7 +1439,7 @@ func TestUpgradeV0State(t *testing.T) { foo.Primary.Attributes["key"] != "val" { t.Fatalf("bad: %#v", foo) } - if len(foo.Tainted) > 0 { + if foo.Primary.Tainted { t.Fatalf("bad: %#v", foo) } @@ -1512,16 +1447,9 @@ func TestUpgradeV0State(t *testing.T) { if bar.Type != "test_resource" { t.Fatalf("bad: %#v", bar) } - if bar.Primary != nil { + if !bar.Primary.Tainted { t.Fatalf("bad: %#v", bar) } - if len(bar.Tainted) != 1 { - t.Fatalf("bad: %#v", bar) - } - bt := bar.Tainted[0] - if bt.ID != "1234" || bt.Attributes["a"] != "b" { - t.Fatalf("bad: %#v", bt) - } } func TestParseResourceStateKey(t *testing.T) { diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index ad19d7b1b..33412a8ce 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -399,9 +399,8 @@ aws_instance.foo: ` const testTerraformApplyProvisionerFailStr = ` -aws_instance.bar: (1 tainted) - ID = - Tainted ID 1 = foo +aws_instance.bar: (tainted) + ID = foo aws_instance.foo: ID = foo num = 2 @@ -409,9 +408,8 @@ aws_instance.foo: ` const testTerraformApplyProvisionerFailCreateStr = ` -aws_instance.bar: (1 tainted) - ID = - Tainted ID 1 = foo +aws_instance.bar: (tainted) + ID = foo ` const testTerraformApplyProvisionerFailCreateNoIdStr = ` @@ -419,10 +417,10 @@ const testTerraformApplyProvisionerFailCreateNoIdStr = ` ` const testTerraformApplyProvisionerFailCreateBeforeDestroyStr = ` -aws_instance.bar: (1 tainted) +aws_instance.bar: (1 deposed) ID = bar require_new = abc - Tainted ID 1 = foo + Deposed ID 1 = foo (tainted) ` const testTerraformApplyProvisionerResourceRefStr = ` @@ -1242,9 +1240,8 @@ DESTROY/CREATE: aws_instance.bar STATE: -aws_instance.bar: (1 tainted) - ID = - Tainted ID 1 = baz +aws_instance.bar: (tainted) + ID = baz aws_instance.foo: ID = bar num = 2 diff --git a/terraform/transform_destroy_test.go b/terraform/transform_destroy_test.go index 1a50ce053..23b994aed 100644 --- a/terraform/transform_destroy_test.go +++ b/terraform/transform_destroy_test.go @@ -359,8 +359,9 @@ func TestPruneDestroyTransformer_tainted(t *testing.T) { Path: RootModulePath, Resources: map[string]*ResourceState{ "aws_instance.bar": &ResourceState{ - Tainted: []*InstanceState{ - &InstanceState{ID: "foo"}, + Primary: &InstanceState{ + ID: "foo", + Tainted: true, }, }, }, @@ -399,16 +400,11 @@ func TestPruneDestroyTransformer_tainted(t *testing.T) { const testTransformDestroyBasicStr = ` aws_instance.bar - aws_instance.bar (destroy tainted) aws_instance.bar (destroy) aws_instance.foo -aws_instance.bar (destroy tainted) aws_instance.bar (destroy) aws_instance.foo - aws_instance.foo (destroy tainted) aws_instance.foo (destroy) -aws_instance.foo (destroy tainted) - aws_instance.bar (destroy tainted) aws_instance.foo (destroy) aws_instance.bar (destroy) ` @@ -456,40 +452,28 @@ aws_instance.foo-bar (destroy) const testTransformPruneDestroyTaintedStr = ` aws_instance.bar - aws_instance.bar (destroy tainted) aws_instance.foo -aws_instance.bar (destroy tainted) aws_instance.foo ` const testTransformCreateBeforeDestroyBasicStr = ` aws_instance.web - aws_instance.web (destroy tainted) -aws_instance.web (destroy tainted) - aws_load_balancer.lb (destroy tainted) 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 tainted) aws_load_balancer.lb (destroy) -aws_load_balancer.lb (destroy tainted) aws_load_balancer.lb (destroy) ` const testTransformCreateBeforeDestroyTwiceStr = ` aws_autoscale.bar - aws_autoscale.bar (destroy tainted) aws_lc.foo -aws_autoscale.bar (destroy tainted) aws_autoscale.bar (destroy) aws_autoscale.bar aws_lc.foo - aws_lc.foo (destroy tainted) -aws_lc.foo (destroy tainted) - aws_autoscale.bar (destroy tainted) aws_lc.foo (destroy) aws_autoscale.bar aws_autoscale.bar (destroy) diff --git a/terraform/transform_resource.go b/terraform/transform_resource.go index 07da19b77..0b2df921a 100644 --- a/terraform/transform_resource.go +++ b/terraform/transform_resource.go @@ -337,7 +337,7 @@ func (n *graphNodeExpandedResource) managedResourceEvalNodes(resource *Resource, Config: &resourceConfig, Provider: &provider, State: &state, - Output: &diff, + OutputDiff: &diff, OutputState: &state, }, &EvalCheckPreventDestroy{ @@ -392,7 +392,7 @@ func (n *graphNodeExpandedResource) managedResourceEvalNodes(resource *Resource, // Apply var diffApply *InstanceDiff var err error - var createNew, tainted bool + var createNew bool var createBeforeDestroyEnabled bool var wasChangeType DiffChangeType nodes = append(nodes, &EvalOpFilter{ @@ -456,11 +456,12 @@ func (n *graphNodeExpandedResource) managedResourceEvalNodes(resource *Resource, }, &EvalDiff{ - Info: info, - Config: &resourceConfig, - Provider: &provider, - State: &state, - Output: &diffApply, + Info: info, + Config: &resourceConfig, + Provider: &provider, + Diff: &diffApply, + State: &state, + OutputDiff: &diffApply, }, &EvalIgnoreChanges{ Resource: n.Resource, @@ -511,20 +512,22 @@ func (n *graphNodeExpandedResource) managedResourceEvalNodes(resource *Resource, Resource: n.Resource, InterpResource: resource, CreateNew: &createNew, - Tainted: &tainted, Error: &err, }, &EvalIf{ If: func(ctx EvalContext) (bool, error) { - if createBeforeDestroyEnabled { - tainted = err != nil - } - - failure := tainted || err != nil - return createBeforeDestroyEnabled && failure, nil + return createBeforeDestroyEnabled && err != nil, nil }, Then: &EvalUndeposeState{ - Name: n.stateId(), + Name: n.stateId(), + State: &state, + }, + Else: &EvalWriteState{ + Name: n.stateId(), + ResourceType: n.Resource.Type, + Provider: n.Resource.Provider, + Dependencies: n.StateDependencies(), + State: &state, }, }, @@ -536,13 +539,6 @@ func (n *graphNodeExpandedResource) managedResourceEvalNodes(resource *Resource, Diff: nil, }, - &EvalWriteState{ - Name: n.stateId(), - ResourceType: n.Resource.Type, - Provider: n.Resource.Provider, - Dependencies: n.StateDependencies(), - State: &state, - }, &EvalApplyPost{ Info: info, State: &state,