diff --git a/builtin/provisioners/file/resource_provisioner.go b/builtin/provisioners/file/resource_provisioner.go index 9eae86481..c81721e8d 100644 --- a/builtin/provisioners/file/resource_provisioner.go +++ b/builtin/provisioners/file/resource_provisioner.go @@ -14,31 +14,31 @@ import ( type ResourceProvisioner struct{} func (p *ResourceProvisioner) Apply(s *terraform.ResourceState, - c *terraform.ResourceConfig) (*terraform.ResourceState, error) { + c *terraform.ResourceConfig) error { // Ensure the connection type is SSH if err := helper.VerifySSH(s); err != nil { - return s, err + return err } // Get the SSH configuration conf, err := helper.ParseSSHConfig(s) if err != nil { - return s, err + return err } // Get the source and destination sRaw := c.Config["source"] src, ok := sRaw.(string) if !ok { - return s, fmt.Errorf("Unsupported 'source' type! Must be string.") + return fmt.Errorf("Unsupported 'source' type! Must be string.") } dRaw := c.Config["destination"] dst, ok := dRaw.(string) if !ok { - return s, fmt.Errorf("Unsupported 'destination' type! Must be string.") + return fmt.Errorf("Unsupported 'destination' type! Must be string.") } - return s, p.copyFiles(conf, src, dst) + return p.copyFiles(conf, src, dst) } func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []error) { diff --git a/builtin/provisioners/local-exec/resource_provisioner.go b/builtin/provisioners/local-exec/resource_provisioner.go index d8e157892..4c756d1d1 100644 --- a/builtin/provisioners/local-exec/resource_provisioner.go +++ b/builtin/provisioners/local-exec/resource_provisioner.go @@ -21,16 +21,16 @@ type ResourceProvisioner struct{} func (p *ResourceProvisioner) Apply( s *terraform.ResourceState, - c *terraform.ResourceConfig) (*terraform.ResourceState, error) { + c *terraform.ResourceConfig) error { // Get the command commandRaw, ok := c.Config["command"] if !ok { - return s, fmt.Errorf("local-exec provisioner missing 'command'") + return fmt.Errorf("local-exec provisioner missing 'command'") } command, ok := commandRaw.(string) if !ok { - return s, fmt.Errorf("local-exec provisioner command must be a string") + return fmt.Errorf("local-exec provisioner command must be a string") } // Execute the command using a shell @@ -51,10 +51,10 @@ func (p *ResourceProvisioner) Apply( // Run the command to completion if err := cmd.Run(); err != nil { - return s, fmt.Errorf("Error running command '%s': %v. Output: %s", + return fmt.Errorf("Error running command '%s': %v. Output: %s", command, err, output.Bytes()) } - return s, nil + return nil } func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) ([]string, []error) { diff --git a/builtin/provisioners/local-exec/resource_provisioner_test.go b/builtin/provisioners/local-exec/resource_provisioner_test.go index 2882e26e8..e31641561 100644 --- a/builtin/provisioners/local-exec/resource_provisioner_test.go +++ b/builtin/provisioners/local-exec/resource_provisioner_test.go @@ -21,8 +21,7 @@ func TestResourceProvider_Apply(t *testing.T) { }) p := new(ResourceProvisioner) - _, err := p.Apply(nil, c) - if err != nil { + if err := p.Apply(nil, c); err != nil { t.Fatalf("err: %v", err) } diff --git a/builtin/provisioners/remote-exec/resource_provisioner.go b/builtin/provisioners/remote-exec/resource_provisioner.go index 7b02bd7e3..6a5ce1405 100644 --- a/builtin/provisioners/remote-exec/resource_provisioner.go +++ b/builtin/provisioners/remote-exec/resource_provisioner.go @@ -23,22 +23,22 @@ const ( type ResourceProvisioner struct{} func (p *ResourceProvisioner) Apply(s *terraform.ResourceState, - c *terraform.ResourceConfig) (*terraform.ResourceState, error) { + c *terraform.ResourceConfig) error { // Ensure the connection type is SSH if err := helper.VerifySSH(s); err != nil { - return s, err + return err } // Get the SSH configuration conf, err := helper.ParseSSHConfig(s) if err != nil { - return s, err + return err } // Collect the scripts scripts, err := p.collectScripts(c) if err != nil { - return s, err + return err } for _, s := range scripts { defer s.Close() @@ -46,9 +46,9 @@ func (p *ResourceProvisioner) Apply(s *terraform.ResourceState, // Copy and execute each script if err := p.runScripts(conf, scripts); err != nil { - return s, err + return err } - return s, nil + return nil } func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) (ws []string, es []error) { diff --git a/rpc/resource_provisioner.go b/rpc/resource_provisioner.go index 82552cf4f..171cf9f0a 100644 --- a/rpc/resource_provisioner.go +++ b/rpc/resource_provisioner.go @@ -37,7 +37,7 @@ func (p *ResourceProvisioner) Validate(c *terraform.ResourceConfig) ([]string, [ func (p *ResourceProvisioner) Apply( s *terraform.ResourceState, - c *terraform.ResourceConfig) (*terraform.ResourceState, error) { + c *terraform.ResourceConfig) error { var resp ResourceProvisionerApplyResponse args := &ResourceProvisionerApplyArgs{ State: s, @@ -46,13 +46,13 @@ func (p *ResourceProvisioner) Apply( err := p.Client.Call(p.Name+".Apply", args, &resp) if err != nil { - return nil, err + return err } if resp.Error != nil { err = resp.Error } - return resp.State, err + return err } type ResourceProvisionerValidateArgs struct { @@ -70,7 +70,6 @@ type ResourceProvisionerApplyArgs struct { } type ResourceProvisionerApplyResponse struct { - State *terraform.ResourceState Error *BasicError } @@ -83,9 +82,8 @@ type ResourceProvisionerServer struct { func (s *ResourceProvisionerServer) Apply( args *ResourceProvisionerApplyArgs, result *ResourceProvisionerApplyResponse) error { - state, err := s.Provisioner.Apply(args.State, args.Config) + err := s.Provisioner.Apply(args.State, args.Config) *result = ResourceProvisionerApplyResponse{ - State: state, Error: NewBasicError(err), } return nil diff --git a/rpc/resource_provisioner_test.go b/rpc/resource_provisioner_test.go index 73f11eb9a..e31db8dc4 100644 --- a/rpc/resource_provisioner_test.go +++ b/rpc/resource_provisioner_test.go @@ -21,14 +21,10 @@ func TestResourceProvisioner_apply(t *testing.T) { } provisioner := &ResourceProvisioner{Client: client, Name: name} - p.ApplyReturn = &terraform.ResourceState{ - ID: "bob", - } - // Apply state := &terraform.ResourceState{} conf := &terraform.ResourceConfig{} - newState, err := provisioner.Apply(state, conf) + err = provisioner.Apply(state, conf) if !p.ApplyCalled { t.Fatal("apply should be called") } @@ -38,9 +34,6 @@ func TestResourceProvisioner_apply(t *testing.T) { if err != nil { t.Fatalf("bad: %#v", err) } - if !reflect.DeepEqual(p.ApplyReturn, newState) { - t.Fatalf("bad: %#v", newState) - } } func TestResourceProvisioner_validate(t *testing.T) { diff --git a/terraform/context.go b/terraform/context.go index 7674714ac..4411dea56 100644 --- a/terraform/context.go +++ b/terraform/context.go @@ -554,10 +554,11 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc { // // Additionally, we need to be careful to not run this if there // was an error during the provider apply. + tainted := false if applyerr == nil && r.State.ID == "" && len(r.Provisioners) > 0 { - rs, err = c.applyProvisioners(r, rs) - if err != nil { + if err := c.applyProvisioners(r, rs); err != nil { errs = append(errs, err) + tainted = true } } @@ -567,6 +568,10 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc { delete(c.state.Resources, r.Id) } else { c.state.Resources[r.Id] = rs + + if tainted { + c.state.Tainted[r.Id] = struct{}{} + } } c.sl.Unlock() @@ -591,9 +596,7 @@ func (c *Context) applyWalkFn() depgraph.WalkFunc { // applyProvisioners is used to run any provisioners a resource has // defined after the resource creation has already completed. -func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) (*ResourceState, error) { - var err error - +func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) error { // Store the original connection info, restore later origConnInfo := rs.ConnInfo defer func() { @@ -604,13 +607,13 @@ func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) (*ResourceSt // Interpolate since we may have variables that depend on the // local resource. if err := prov.Config.interpolate(c); err != nil { - return rs, err + return err } // Interpolate the conn info, since it may contain variables connInfo := NewResourceConfig(prov.ConnInfo) if err := connInfo.interpolate(c); err != nil { - return rs, err + return err } // Merge the connection information @@ -643,12 +646,12 @@ func (c *Context) applyProvisioners(r *Resource, rs *ResourceState) (*ResourceSt rs.ConnInfo = overlay // Invoke the Provisioner - rs, err = prov.Provisioner.Apply(rs, prov.Config) - if err != nil { - return rs, err + if err := prov.Provisioner.Apply(rs, prov.Config); err != nil { + return err } } - return rs, nil + + return nil } func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc { @@ -678,7 +681,13 @@ func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc { // Get a diff from the newest state log.Printf("[DEBUG] %s: Executing diff", r.Id) var err error - diff, err = r.Provider.Diff(r.State, r.Config) + state := r.State + if r.Tainted { + // If we're tainted, we pretend to create a new thing. + state = new(ResourceState) + state.Type = r.State.Type + } + diff, err = r.Provider.Diff(state, r.Config) if err != nil { return err } @@ -688,6 +697,11 @@ func (c *Context) planWalkFn(result *Plan) depgraph.WalkFunc { diff = new(ResourceDiff) } + if r.Tainted { + // Tainted resources must also be destroyed + diff.Destroy = true + } + if diff.RequiresNew() && r.State.ID != "" { // This will also require a destroy diff.Destroy = true diff --git a/terraform/context_test.go b/terraform/context_test.go index 9d161ea6b..9a79feed0 100644 --- a/terraform/context_test.go +++ b/terraform/context_test.go @@ -414,12 +414,13 @@ func TestContextApply_Provisioner_compute(t *testing.T) { pr := testProvisioner() p.ApplyFn = testApplyFn p.DiffFn = testDiffFn - pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) (*ResourceState, error) { + pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) error { val, ok := c.Config["foo"] if !ok || val != "computed_dynamical" { t.Fatalf("bad value for foo: %v %#v", val, c) } - return rs, nil + + return nil } ctx := testContext(t, &ContextOpts{ Config: c, @@ -455,6 +456,46 @@ func TestContextApply_Provisioner_compute(t *testing.T) { } } +func TestContextApply_provisionerFail(t *testing.T) { + c := testConfig(t, "apply-provisioner-fail") + p := testProvider("aws") + pr := testProvisioner() + p.ApplyFn = testApplyFn + p.DiffFn = testDiffFn + + pr.ApplyFn = func(*ResourceState, *ResourceConfig) error { + return fmt.Errorf("EXPLOSION") + } + + ctx := testContext(t, &ContextOpts{ + Config: c, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + Provisioners: map[string]ResourceProvisionerFactory{ + "shell": testProvisionerFuncFixed(pr), + }, + Variables: map[string]string{ + "value": "1", + }, + }) + + if _, err := ctx.Plan(nil); err != nil { + t.Fatalf("err: %s", err) + } + + state, err := ctx.Apply() + if err == nil { + t.Fatal("should error") + } + + actual := strings.TrimSpace(state.String()) + expected := strings.TrimSpace(testTerraformApplyProvisionerFailStr) + if actual != expected { + t.Fatalf("bad: \n%s", actual) + } +} + func TestContextApply_outputDiffVars(t *testing.T) { c := testConfig(t, "apply-good") p := testProvider("aws") @@ -527,7 +568,7 @@ func TestContextApply_Provisioner_ConnInfo(t *testing.T) { } p.DiffFn = testDiffFn - pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) (*ResourceState, error) { + pr.ApplyFn = func(rs *ResourceState, c *ResourceConfig) error { conn := rs.ConnInfo if conn["type"] != "telnet" { t.Fatalf("Bad: %#v", conn) @@ -544,7 +585,8 @@ func TestContextApply_Provisioner_ConnInfo(t *testing.T) { if conn["pass"] != "test" { t.Fatalf("Bad: %#v", conn) } - return rs, nil + + return nil } ctx := testContext(t, &ContextOpts{ @@ -1415,6 +1457,44 @@ func TestContextPlan_state(t *testing.T) { } } +func TestContextPlan_taint(t *testing.T) { + c := testConfig(t, "plan-taint") + p := testProvider("aws") + p.DiffFn = testDiffFn + s := &State{ + Resources: map[string]*ResourceState{ + "aws_instance.foo": &ResourceState{ + ID: "bar", + Type: "aws_instance", + Attributes: map[string]string{"num": "2"}, + }, + "aws_instance.bar": &ResourceState{ + ID: "baz", + Type: "aws_instance", + }, + }, + Tainted: map[string]struct{}{"aws_instance.bar": struct{}{}}, + } + ctx := testContext(t, &ContextOpts{ + Config: c, + Providers: map[string]ResourceProviderFactory{ + "aws": testProviderFuncFixed(p), + }, + State: s, + }) + + plan, err := ctx.Plan(nil) + if err != nil { + t.Fatalf("err: %s", err) + } + + actual := strings.TrimSpace(plan.String()) + expected := strings.TrimSpace(testTerraformPlanTaintStr) + if actual != expected { + t.Fatalf("bad:\n%s", actual) + } +} + func TestContextRefresh(t *testing.T) { p := testProvider("aws") c := testConfig(t, "refresh-basic") diff --git a/terraform/graph.go b/terraform/graph.go index 4b993d5a3..2d8bb92d5 100644 --- a/terraform/graph.go +++ b/terraform/graph.go @@ -178,6 +178,12 @@ func graphAddConfigResources( index = i } + // Determine if this resource is tainted + tainted := false + if s != nil && s.Tainted != nil { + _, tainted = s.Tainted[r.Id()] + } + var state *ResourceState if s != nil { state = s.Resources[name] @@ -209,9 +215,10 @@ func graphAddConfigResources( Type: r.Type, Config: r, Resource: &Resource{ - Id: name, - State: state, - Config: NewResourceConfig(r.RawConfig), + Id: name, + State: state, + Config: NewResourceConfig(r.RawConfig), + Tainted: tainted, }, }, } diff --git a/terraform/resource.go b/terraform/resource.go index d19949840..8eba49db4 100644 --- a/terraform/resource.go +++ b/terraform/resource.go @@ -31,6 +31,7 @@ type Resource struct { Provider ResourceProvider State *ResourceState Provisioners []*ResourceProvisionerConfig + Tainted bool } // Vars returns the mapping of variables that should be replaced in diff --git a/terraform/resource_provisioner.go b/terraform/resource_provisioner.go index 6c0c03f10..967e0e037 100644 --- a/terraform/resource_provisioner.go +++ b/terraform/resource_provisioner.go @@ -20,7 +20,7 @@ type ResourceProvisioner interface { // resource state along with an error. Instead of a diff, the ResourceConfig // is provided since provisioners only run after a resource has been // newly created. - Apply(*ResourceState, *ResourceConfig) (*ResourceState, error) + Apply(*ResourceState, *ResourceConfig) error } // ResourceProvisionerFactory is a function type that creates a new instance diff --git a/terraform/resource_provisioner_mock.go b/terraform/resource_provisioner_mock.go index 2f9a70b9f..a6e62a593 100644 --- a/terraform/resource_provisioner_mock.go +++ b/terraform/resource_provisioner_mock.go @@ -9,8 +9,7 @@ type MockResourceProvisioner struct { ApplyCalled bool ApplyState *ResourceState ApplyConfig *ResourceConfig - ApplyFn func(*ResourceState, *ResourceConfig) (*ResourceState, error) - ApplyReturn *ResourceState + ApplyFn func(*ResourceState, *ResourceConfig) error ApplyReturnError error ValidateCalled bool @@ -29,12 +28,12 @@ func (p *MockResourceProvisioner) Validate(c *ResourceConfig) ([]string, []error return p.ValidateReturnWarns, p.ValidateReturnErrors } -func (p *MockResourceProvisioner) Apply(state *ResourceState, c *ResourceConfig) (*ResourceState, error) { +func (p *MockResourceProvisioner) Apply(state *ResourceState, c *ResourceConfig) error { p.ApplyCalled = true p.ApplyState = state p.ApplyConfig = c if p.ApplyFn != nil { return p.ApplyFn(state, c) } - return p.ApplyReturn, p.ApplyReturnError + return p.ApplyReturnError } diff --git a/terraform/state.go b/terraform/state.go index 5683f95f6..bd2767d5c 100644 --- a/terraform/state.go +++ b/terraform/state.go @@ -18,13 +18,20 @@ import ( type State struct { Outputs map[string]string Resources map[string]*ResourceState + Tainted map[string]struct{} once sync.Once } func (s *State) init() { s.once.Do(func() { - s.Resources = make(map[string]*ResourceState) + if s.Resources == nil { + s.Resources = make(map[string]*ResourceState) + } + + if s.Tainted == nil { + s.Tainted = make(map[string]struct{}) + } }) } @@ -97,7 +104,12 @@ func (s *State) String() string { id = "" } - buf.WriteString(fmt.Sprintf("%s:\n", k)) + taintStr := "" + if _, ok := s.Tainted[k]; ok { + taintStr = " (tainted)" + } + + buf.WriteString(fmt.Sprintf("%s:%s\n", k, taintStr)) buf.WriteString(fmt.Sprintf(" ID = %s\n", id)) attrKeys := make([]string, 0, len(rs.Attributes)) diff --git a/terraform/terraform_test.go b/terraform/terraform_test.go index 24e780f9e..d9f9027fd 100644 --- a/terraform/terraform_test.go +++ b/terraform/terraform_test.go @@ -130,6 +130,15 @@ aws_instance.foo: type = aws_instance ` +const testTerraformApplyProvisionerFailStr = ` +aws_instance.bar: (tainted) + ID = foo +aws_instance.foo: + ID = foo + num = 2 + type = aws_instance +` + const testTerraformApplyDestroyStr = ` ` @@ -402,3 +411,19 @@ STATE: aws_instance.foo: ID = bar ` + +const testTerraformPlanTaintStr = ` +DIFF: + +DESTROY: aws_instance.bar + foo: "" => "2" + type: "" => "aws_instance" + +STATE: + +aws_instance.bar: (tainted) + ID = baz +aws_instance.foo: + ID = bar + num = 2 +` diff --git a/terraform/test-fixtures/apply-provisioner-fail/main.tf b/terraform/test-fixtures/apply-provisioner-fail/main.tf new file mode 100644 index 000000000..d27208d4f --- /dev/null +++ b/terraform/test-fixtures/apply-provisioner-fail/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "foo" { + num = "2" +} + +resource "aws_instance" "bar" { + provisioner "shell" {} +} diff --git a/terraform/test-fixtures/plan-taint/main.tf b/terraform/test-fixtures/plan-taint/main.tf new file mode 100644 index 000000000..94ed55478 --- /dev/null +++ b/terraform/test-fixtures/plan-taint/main.tf @@ -0,0 +1,7 @@ +resource "aws_instance" "foo" { + num = "2" +} + +resource "aws_instance" "bar" { + foo = "${aws_instance.foo.num}" +}