Merge pull request #11686 from hashicorp/jbardin/state-locking

Enable state locking for plan/apply/destroy/refresh/taint/untaint

backend/backend.go

@@ -22,7 +22,8 @@ type Backend interface {
 
 	// State returns the current state for this environment. This state may
 	// not be loaded locally: the proper APIs should be called on state.State
-	// to load the state.
+	// to load the state. If the state.State is a state.Locker, it's up to the
+	// caller to call Lock and Unlock as needed.
 	State() (state.State, error)
 }
 
@@ -38,6 +39,9 @@ type Enhanced interface {
 	// It is up to the implementation to determine what "performing" means.
 	// This DOES NOT BLOCK. The context returned as part of RunningOperation
 	// should be used to block for completion.
+	// If the state used in the operation can be locked, it is the
+	// responsibility of the Backend to lock the state for the duration of the
+	// running operation.
 	Operation(context.Context, *Operation) (*RunningOperation, error)
 }
 
@@ -99,6 +103,10 @@ type Operation struct {
 	// Input/output/control options.
 	UIIn  terraform.UIInput
 	UIOut terraform.UIOutput
+
+	// If LockState is true, the Operation must Lock any
+	// state.Lockers for its duration, and Unlock when complete.
+	LockState bool
 }
 
 // RunningOperation is the result of starting an operation.

backend/local/backend.go

@@ -34,6 +34,9 @@ type Local struct {
 	StateOutPath    string
 	StateBackupPath string
 
+	// we only want to create a single instance of the local state
+	state state.State
+
 	// ContextOpts are the base context options to set when initializing a
 	// Terraform context. Many of these will be overridden or merged by
 	// Operation. See Operation for more details.
@@ -100,6 +103,10 @@ func (b *Local) State() (state.State, error) {
 		return b.Backend.State()
 	}
 
+	if b.state != nil {
+		return b.state, nil
+	}
+
 	// Otherwise, we need to load the state.
 	var s state.State = &state.LocalState{
 		Path:    b.StatePath,
@@ -119,6 +126,7 @@ func (b *Local) State() (state.State, error) {
 		}
 	}
 
+	b.state = s
 	return s, nil
 }
 

backend/local/backend_apply.go

@@ -8,6 +8,7 @@ import (
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/go-multierror"
 	"github.com/hashicorp/terraform/backend"
+	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -28,12 +29,29 @@ func (b *Local) opApply(
 	b.ContextOpts.Hooks = append(b.ContextOpts.Hooks, countHook, stateHook)
 
 	// Get our context
-	tfCtx, state, err := b.context(op)
+	tfCtx, opState, err := b.context(op)
 	if err != nil {
 		runningOp.Err = err
 		return
 	}
 
+	// context acquired the state, and therefor the lock.
+	// Unlock it when the operation is complete
+	defer func() {
+		if s, ok := opState.(state.Locker); op.LockState && ok {
+			if err := s.Unlock(); err != nil {
+				runningOp.Err = multierror.Append(runningOp.Err,
+					errwrap.Wrapf("Error unlocking state:\n\n"+
+						"{{err}}\n\n"+
+						"The Terraform operation completed but there was an error unlocking the state.\n"+
+						"This may require unlocking the state manually with the `terraform unlock` command\n",
+						err,
+					),
+				)
+			}
+		}
+	}()
+
 	// Setup the state
 	runningOp.State = tfCtx.State()
 
@@ -58,7 +76,7 @@ func (b *Local) opApply(
 	}
 
 	// Setup our hook for continuous state updates
-	stateHook.State = state
+	stateHook.State = opState
 
 	// Start the apply in a goroutine so that we can be interrupted.
 	var applyState *terraform.State
@@ -98,11 +116,11 @@ func (b *Local) opApply(
 	runningOp.State = applyState
 
 	// Persist the state
-	if err := state.WriteState(applyState); err != nil {
+	if err := opState.WriteState(applyState); err != nil {
 		runningOp.Err = fmt.Errorf("Failed to save state: %s", err)
 		return
 	}
-	if err := state.PersistState(); err != nil {
+	if err := opState.PersistState(); err != nil {
 		runningOp.Err = fmt.Errorf("Failed to save state: %s", err)
 		return
 	}

backend/local/backend_local.go

@@ -23,6 +23,13 @@ func (b *Local) context(op *backend.Operation) (*terraform.Context, state.State,
 	if err != nil {
 		return nil, nil, errwrap.Wrapf("Error loading state: {{err}}", err)
 	}
+
+	if s, ok := s.(state.Locker); op.LockState && ok {
+		if err := s.Lock(op.Type.String()); err != nil {
+			return nil, nil, errwrap.Wrapf("Error locking state: {{err}}", err)
+		}
+	}
+
 	if err := s.RefreshState(); err != nil {
 		return nil, nil, errwrap.Wrapf("Error loading state: {{err}}", err)
 	}

backend/local/backend_plan.go

@@ -11,6 +11,7 @@ import (
 	"github.com/hashicorp/terraform/backend"
 	"github.com/hashicorp/terraform/command/format"
 	"github.com/hashicorp/terraform/config/module"
+	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -51,12 +52,22 @@ func (b *Local) opPlan(
 	b.ContextOpts.Hooks = append(b.ContextOpts.Hooks, countHook)
 
 	// Get our context
-	tfCtx, _, err := b.context(op)
+	tfCtx, opState, err := b.context(op)
 	if err != nil {
 		runningOp.Err = err
 		return
 	}
 
+	// context acquired the state, and therefor the lock.
+	// Unlock it when the operation is complete
+	defer func() {
+		if s, ok := opState.(state.Locker); op.LockState && ok {
+			if err := s.Unlock(); err != nil {
+				log.Printf("[ERROR]: %s", err)
+			}
+		}
+	}()
+
 	// Setup the state
 	runningOp.State = tfCtx.State()
 

backend/local/backend_refresh.go

@@ -3,10 +3,12 @@ package local
 import (
 	"context"
 	"fmt"
+	"log"
 	"os"
 
 	"github.com/hashicorp/errwrap"
 	"github.com/hashicorp/terraform/backend"
+	"github.com/hashicorp/terraform/state"
 )
 
 func (b *Local) opRefresh(
@@ -40,14 +42,24 @@ func (b *Local) opRefresh(
 	}
 
 	// Get our context
-	tfCtx, state, err := b.context(op)
+	tfCtx, opState, err := b.context(op)
 	if err != nil {
 		runningOp.Err = err
 		return
 	}
 
+	// context acquired the state, and therefor the lock.
+	// Unlock it when the operation is complete
+	defer func() {
+		if s, ok := opState.(state.Locker); op.LockState && ok {
+			if err := s.Unlock(); err != nil {
+				log.Printf("[ERROR]: %s", err)
+			}
+		}
+	}()
+
 	// Set our state
-	runningOp.State = state.State()
+	runningOp.State = opState.State()
 
 	// Perform operation and write the resulting state to the running op
 	newState, err := tfCtx.Refresh()
@@ -58,11 +70,11 @@ func (b *Local) opRefresh(
 	}
 
 	// Write and persist the state
-	if err := state.WriteState(newState); err != nil {
+	if err := opState.WriteState(newState); err != nil {
 		runningOp.Err = errwrap.Wrapf("Error writing state: {{err}}", err)
 		return
 	}
-	if err := state.PersistState(); err != nil {
+	if err := opState.PersistState(); err != nil {
 		runningOp.Err = errwrap.Wrapf("Error saving state: {{err}}", err)
 		return
 	}

command/apply.go

@@ -47,6 +47,7 @@ func (c *ApplyCommand) Run(args []string) int {
 	cmdFlags.StringVar(&c.Meta.statePath, "state", "", "path")
 	cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path")
 	cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path")
+	cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
 	if err := cmdFlags.Parse(args); err != nil {
 		return 1
@@ -182,6 +183,7 @@ func (c *ApplyCommand) Run(args []string) int {
 	opReq.Plan = plan
 	opReq.PlanRefresh = refresh
 	opReq.Type = backend.OperationTypeApply
+	opReq.LockState = c.Meta.stateLock
 
 	// Perform the operation
 	ctx, ctxCancel := context.WithCancel(context.Background())
@@ -272,6 +274,8 @@ Options:
                          modifying. Defaults to the "-state-out" path with
                          ".backup" extension. Set to "-" to disable backup.
 
+  -lock=true             Lock the state file when locking is supported.
+
   -input=true            Ask for input for variables if not directly set.
 
   -no-color              If specified, output won't contain any color.
@@ -319,6 +323,8 @@ Options:
 
   -force                 Don't ask for input for destroy confirmation.
 
+  -lock=true             Lock the state file when locking is supported.
+
   -no-color              If specified, output won't contain any color.
 
   -parallelism=n         Limit the number of concurrent operations.

command/apply_destroy_test.go

@@ -92,6 +92,58 @@ func TestApply_destroy(t *testing.T) {
 	}
 }
 
+func TestApply_destroyLockedState(t *testing.T) {
+	originalState := &terraform.State{
+		Modules: []*terraform.ModuleState{
+			&terraform.ModuleState{
+				Path: []string{"root"},
+				Resources: map[string]*terraform.ResourceState{
+					"test_instance.foo": &terraform.ResourceState{
+						Type: "test_instance",
+						Primary: &terraform.InstanceState{
+							ID: "bar",
+						},
+					},
+				},
+			},
+		},
+	}
+
+	statePath := testStateFile(t, originalState)
+
+	unlock, err := testLockState(statePath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer unlock()
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &ApplyCommand{
+		Destroy: true,
+		Meta: Meta{
+			ContextOpts: testCtxConfig(p),
+			Ui:          ui,
+		},
+	}
+
+	// Run the apply command pointing to our existing state
+	args := []string{
+		"-force",
+		"-state", statePath,
+		testFixturePath("apply"),
+	}
+
+	if code := c.Run(args); code == 0 {
+		t.Fatal("expected error")
+	}
+
+	output := ui.ErrorWriter.String()
+	if !strings.Contains(output, "locked") {
+		t.Fatal("command output does not look like a lock error:", output)
+	}
+}
+
 func TestApply_destroyPlan(t *testing.T) {
 	planPath := testPlanFile(t, &terraform.Plan{
 		Module: testModule(t, "apply"),

command/apply_test.go

@@ -15,6 +15,7 @@ import (
 	"testing"
 	"time"
 
+	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/mitchellh/cli"
 )
@@ -58,6 +59,39 @@ func TestApply(t *testing.T) {
 	}
 }
 
+// test apply with locked state
+func TestApply_lockedState(t *testing.T) {
+	statePath := testTempFile(t)
+
+	unlock, err := testLockState(statePath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer unlock()
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &ApplyCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(p),
+			Ui:          ui,
+		},
+	}
+
+	args := []string{
+		"-state", statePath,
+		testFixturePath("apply"),
+	}
+	if code := c.Run(args); code == 0 {
+		t.Fatal("expected error")
+	}
+
+	output := ui.ErrorWriter.String()
+	if !strings.Contains(output, "locked") {
+		t.Fatal("command output does not look like a lock error:", output)
+	}
+}
+
 // high water mark counter
 type hwm struct {
 	sync.Mutex
@@ -550,7 +584,8 @@ func TestApply_plan(t *testing.T) {
 }
 
 func TestApply_plan_backup(t *testing.T) {
-	planPath := testPlanFile(t, testPlan(t))
+	plan := testPlan(t)
+	planPath := testPlanFile(t, plan)
 	statePath := testTempFile(t)
 	backupPath := testTempFile(t)
 
@@ -563,6 +598,12 @@ func TestApply_plan_backup(t *testing.T) {
 		},
 	}
 
+	// create a state file that needs to be backed up
+	err := (&state.LocalState{Path: statePath}).WriteState(plan.State)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	args := []string{
 		"-state-out", statePath,
 		"-backup", backupPath,

command/command_test.go

@@ -6,14 +6,17 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"flag"
+	"fmt"
 	"io"
 	"io/ioutil"
 	"log"
 	"net/http"
 	"net/http/httptest"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"strings"
+	"syscall"
 	"testing"
 
 	"github.com/hashicorp/go-getter"
@@ -529,3 +532,55 @@ func testRemoteState(t *testing.T, s *terraform.State, c int) (*terraform.Remote
 
 	return remote, srv
 }
+
+// testlockState calls a separate process to the lock the state file at path.
+// deferFunc should be called in the caller to properly unlock the file.
+func testLockState(path string) (func(), error) {
+	// build and run the binary ourselves so we can quickly terminate it for cleanup
+	buildDir, err := ioutil.TempDir("", "locker")
+	if err != nil {
+		return nil, err
+	}
+	cleanFunc := func() {
+		os.RemoveAll(buildDir)
+	}
+
+	lockBin := filepath.Join(buildDir, "statelocker")
+	out, err := exec.Command("go", "build", "-o", lockBin, "testdata/statelocker.go").CombinedOutput()
+	if err != nil {
+		cleanFunc()
+		return nil, fmt.Errorf("%s %s", err, out)
+	}
+
+	locker := exec.Command(lockBin, path)
+	pr, pw, err := os.Pipe()
+	if err != nil {
+		cleanFunc()
+		return nil, err
+	}
+	defer pr.Close()
+	defer pw.Close()
+	locker.Stderr = pw
+	locker.Stdout = pw
+
+	if err := locker.Start(); err != nil {
+		return nil, err
+	}
+	deferFunc := func() {
+		cleanFunc()
+		locker.Process.Signal(syscall.SIGTERM)
+		locker.Wait()
+	}
+
+	// wait for the process to lock
+	buf := make([]byte, 1024)
+	n, err := pr.Read(buf)
+	if err != nil {
+		return deferFunc, fmt.Errorf("read from statelocker returned: %s", err)
+	}
+
+	if string(buf[:n]) != "LOCKED" {
+		return deferFunc, fmt.Errorf("statelocker wrote: %s", string(buf[:n]))
+	}
+	return deferFunc, nil
+}

command/meta.go

@@ -83,12 +83,15 @@ type Meta struct {
 	// shadow is used to enable/disable the shadow graph
 	//
 	// provider is to specify specific resource providers
+	//
+	// lockState is set to false to disable state locking
 	statePath    string
 	stateOutPath string
 	backupPath   string
 	parallelism  int
 	shadow       bool
 	provider     string
+	stateLock    bool
 }
 
 // initStatePaths is used to initialize the default values for

command/meta_backend_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/hashicorp/terraform/helper/copy"
+	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/terraform"
 	"github.com/mitchellh/cli"
 )
@@ -2208,6 +2209,12 @@ func TestMetaBackend_planLocalStatePath(t *testing.T) {
 	// Create an alternate output path
 	statePath := "foo.tfstate"
 
+	// put a initial state there that needs to be backed up
+	err := (&state.LocalState{Path: statePath}).WriteState(original)
+	if err != nil {
+		t.Fatal(err)
+	}
+
 	// Setup the meta
 	m := testMetaBackend(t, nil)
 	m.stateOutPath = statePath

command/plan.go

@@ -31,6 +31,7 @@ func (c *PlanCommand) Run(args []string) int {
 		&c.Meta.parallelism, "parallelism", DefaultParallelism, "parallelism")
 	cmdFlags.StringVar(&c.Meta.statePath, "state", "", "path")
 	cmdFlags.BoolVar(&detailed, "detailed-exitcode", false, "detailed-exitcode")
+	cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
 	if err := cmdFlags.Parse(args); err != nil {
 		return 1
@@ -84,6 +85,7 @@ func (c *PlanCommand) Run(args []string) int {
 	opReq.PlanRefresh = refresh
 	opReq.PlanOutPath = outPath
 	opReq.Type = backend.OperationTypePlan
+	opReq.LockState = c.Meta.stateLock
 
 	// Perform the operation
 	op, err := b.Operation(context.Background(), opReq)
@@ -141,6 +143,8 @@ Options:
 
   -input=true         Ask for input for variables if not directly set.
 
+  -lock=true          Lock the state file when locking is supported.
+
   -module-depth=n     Specifies the depth of modules to show in the output.
                       This does not affect the plan itself, only the output
                       shown. By default, this is -1, which will expand all.

command/plan_test.go

@@ -39,6 +39,44 @@ func TestPlan(t *testing.T) {
 	}
 }
 
+func TestPlan_lockedState(t *testing.T) {
+	cwd, err := os.Getwd()
+	if err != nil {
+		t.Fatalf("err: %s", err)
+	}
+
+	testPath := testFixturePath("plan")
+	unlock, err := testLockState(filepath.Join(testPath, DefaultStateFilename))
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer unlock()
+
+	if err := os.Chdir(testPath); err != nil {
+		t.Fatalf("err: %s", err)
+	}
+	defer os.Chdir(cwd)
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &PlanCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(p),
+			Ui:          ui,
+		},
+	}
+
+	args := []string{}
+	if code := c.Run(args); code == 0 {
+		t.Fatal("expected error")
+	}
+
+	output := ui.ErrorWriter.String()
+	if !strings.Contains(output, "locked") {
+		t.Fatal("command output does not look like a lock error:", output)
+	}
+}
+
 func TestPlan_plan(t *testing.T) {
 	tmp, cwd := testCwd(t)
 	defer testFixCwd(t, tmp, cwd)

command/refresh.go

@@ -23,6 +23,7 @@ func (c *RefreshCommand) Run(args []string) int {
 	cmdFlags.IntVar(&c.Meta.parallelism, "parallelism", 0, "parallelism")
 	cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path")
 	cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path")
+	cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
 	if err := cmdFlags.Parse(args); err != nil {
 		return 1
@@ -52,6 +53,7 @@ func (c *RefreshCommand) Run(args []string) int {
 	opReq := c.Operation()
 	opReq.Type = backend.OperationTypeRefresh
 	opReq.Module = mod
+	opReq.LockState = c.Meta.stateLock
 
 	// Perform the operation
 	op, err := b.Operation(context.Background(), opReq)
@@ -94,6 +96,8 @@ Options:
 
   -input=true         Ask for input for variables if not directly set.
 
+  -lock=true          Lock the state file when locking is supported.
+
   -no-color           If specified, output won't contain any color.
 
   -state=path         Path to read and save state (unless state-out

command/refresh_test.go

@@ -59,6 +59,43 @@ func TestRefresh(t *testing.T) {
 	}
 }
 
+func TestRefresh_lockedState(t *testing.T) {
+	state := testState()
+	statePath := testStateFile(t, state)
+
+	unlock, err := testLockState(statePath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer unlock()
+
+	p := testProvider()
+	ui := new(cli.MockUi)
+	c := &RefreshCommand{
+		Meta: Meta{
+			ContextOpts: testCtxConfig(p),
+			Ui:          ui,
+		},
+	}
+
+	p.RefreshFn = nil
+	p.RefreshReturn = &terraform.InstanceState{ID: "yes"}
+
+	args := []string{
+		"-state", statePath,
+		testFixturePath("refresh"),
+	}
+
+	if code := c.Run(args); code == 0 {
+		t.Fatal("expected error")
+	}
+
+	output := ui.ErrorWriter.String()
+	if !strings.Contains(output, "locked") {
+		t.Fatal("command output does not look like a lock error:", output)
+	}
+}
+
 func TestRefresh_badState(t *testing.T) {
 	p := testProvider()
 	ui := new(cli.MockUi)

command/taint.go

@@ -5,6 +5,7 @@ import (
 	"log"
 	"strings"
 
+	"github.com/hashicorp/terraform/state"
 	"github.com/hashicorp/terraform/terraform"
 )
 
@@ -25,6 +26,7 @@ func (c *TaintCommand) Run(args []string) int {
 	cmdFlags.StringVar(&c.Meta.statePath, "state", DefaultStateFilename, "path")
 	cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path")
 	cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path")
+	cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
 	if err := cmdFlags.Parse(args); err != nil {
 		return 1
@@ -64,14 +66,23 @@ func (c *TaintCommand) Run(args []string) int {
 	}
 
 	// Get the state
-	state, err := b.State()
+	st, err := b.State()
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))
 		return 1
 	}
 
+	if s, ok := st.(state.Locker); c.Meta.stateLock && ok {
+		if err := s.Lock("taint"); err != nil {
+			c.Ui.Error(fmt.Sprintf("Failed to lock state: %s", err))
+			return 1
+		}
+
+		defer s.Unlock()
+	}
+
 	// Get the actual state structure
-	s := state.State()
+	s := st.State()
 	if s.Empty() {
 		if allowMissing {
 			return c.allowMissingExit(name, module)
@@ -129,11 +140,11 @@ func (c *TaintCommand) Run(args []string) int {
 	rs.Taint()
 
 	log.Printf("[INFO] Writing state output to: %s", c.Meta.StateOutPath())
-	if err := state.WriteState(s); err != nil {
+	if err := st.WriteState(s); err != nil {
 		c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err))
 		return 1
 	}
-	if err := state.PersistState(); err != nil {
+	if err := st.PersistState(); err != nil {
 		c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err))
 		return 1
 	}
@@ -166,6 +177,8 @@ Options:
                       modifying. Defaults to the "-state-out" path with
                       ".backup" extension. Set to "-" to disable backup.
 
+  -lock=true          Lock the state file when locking is supported.
+
   -module=path        The module path where the resource lives. By
                       default this will be root. Child modules can be specified
                       by names. Ex. "consul" or "consul.vpc" (nested modules).

command/taint_test.go

@@ -2,6 +2,7 @@ package command
 
 import (
 	"os"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/terraform/terraform"
@@ -44,6 +45,50 @@ func TestTaint(t *testing.T) {
 	testStateOutput(t, statePath, testTaintStr)
 }
 
+func TestTaint_lockedState(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",
+						Primary: &terraform.InstanceState{
+							ID: "bar",
+						},
+					},
+				},
+			},
+		},
+	}
+	statePath := testStateFile(t, state)
+
+	unlock, err := testLockState(statePath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer unlock()
+	ui := new(cli.MockUi)
+	c := &TaintCommand{
+		Meta: Meta{
+			Ui: ui,
+		},
+	}
+
+	args := []string{
+		"-state", statePath,
+		"test_instance.foo",
+	}
+	if code := c.Run(args); code == 0 {
+		t.Fatal("expected error")
+	}
+
+	output := ui.ErrorWriter.String()
+	if !strings.Contains(output, "locked") {
+		t.Fatal("command output does not look like a lock error:", output)
+	}
+}
+
 func TestTaint_backup(t *testing.T) {
 	// Get a temp cwd
 	tmp, cwd := testCwd(t)

command/testdata/statelocker.go

@@ -0,0 +1,49 @@
+// statelocker use used for testing command with a locked state.
+// This will lock the state file at a given path, then wait for a sigal. On
+// SIGINT and SIGTERM the state will be Unlocked before exit.
+package main
+
+import (
+	"io"
+	"log"
+	"os"
+	"os/signal"
+	"syscall"
+	"time"
+
+	"github.com/hashicorp/terraform/state"
+)
+
+func main() {
+	if len(os.Args) != 2 {
+		log.Fatal(os.Args[0], "statefile")
+	}
+
+	s := &state.LocalState{
+		Path: os.Args[1],
+	}
+
+	err := s.Lock("command test")
+	if err != nil {
+		io.WriteString(os.Stderr, err.Error())
+		return
+	}
+
+	// signal to the caller that we're locked
+	io.WriteString(os.Stdout, "LOCKED")
+
+	defer func() {
+		if err := s.Unlock(); err != nil {
+			io.WriteString(os.Stderr, err.Error())
+		}
+	}()
+
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, syscall.SIGINT, syscall.SIGTERM, syscall.SIGHUP)
+
+	// timeout after 10 second in case we don't get cleaned up by the test
+	select {
+	case <-time.After(10 * time.Second):
+	case <-c:
+	}
+}

command/untaint.go

@@ -4,6 +4,8 @@ import (
 	"fmt"
 	"log"
 	"strings"
+
+	"github.com/hashicorp/terraform/state"
 )
 
 // UntaintCommand is a cli.Command implementation that manually untaints
@@ -23,6 +25,7 @@ func (c *UntaintCommand) Run(args []string) int {
 	cmdFlags.StringVar(&c.Meta.statePath, "state", DefaultStateFilename, "path")
 	cmdFlags.StringVar(&c.Meta.stateOutPath, "state-out", "", "path")
 	cmdFlags.StringVar(&c.Meta.backupPath, "backup", "", "path")
+	cmdFlags.BoolVar(&c.Meta.stateLock, "lock", true, "lock state")
 	cmdFlags.Usage = func() { c.Ui.Error(c.Help()) }
 	if err := cmdFlags.Parse(args); err != nil {
 		return 1
@@ -51,14 +54,23 @@ func (c *UntaintCommand) Run(args []string) int {
 	}
 
 	// Get the state
-	state, err := b.State()
+	st, err := b.State()
 	if err != nil {
 		c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err))
 		return 1
 	}
 
+	if s, ok := st.(state.Locker); c.Meta.stateLock && ok {
+		if err := s.Lock("untaint"); err != nil {
+			c.Ui.Error(fmt.Sprintf("Failed to lock state: %s", err))
+			return 1
+		}
+
+		defer s.Unlock()
+	}
+
 	// Get the actual state structure
-	s := state.State()
+	s := st.State()
 	if s.Empty() {
 		if allowMissing {
 			return c.allowMissingExit(name, module)
@@ -116,11 +128,11 @@ func (c *UntaintCommand) Run(args []string) int {
 	rs.Untaint()
 
 	log.Printf("[INFO] Writing state output to: %s", c.Meta.StateOutPath())
-	if err := state.WriteState(s); err != nil {
+	if err := st.WriteState(s); err != nil {
 		c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err))
 		return 1
 	}
-	if err := state.PersistState(); err != nil {
+	if err := st.PersistState(); err != nil {
 		c.Ui.Error(fmt.Sprintf("Error writing state file: %s", err))
 		return 1
 	}
@@ -153,6 +165,8 @@ Options:
                       modifying. Defaults to the "-state-out" path with
                       ".backup" extension. Set to "-" to disable backup.
 
+  -lock=true          Lock the state file when locking is supported.
+
   -module=path        The module path where the resource lives. By
                       default this will be root. Child modules can be specified
                       by names. Ex. "consul" or "consul.vpc" (nested modules).

command/untaint_test.go

@@ -50,6 +50,51 @@ test_instance.foo:
 	testStateOutput(t, statePath, expected)
 }
 
+func TestUntaint_lockedState(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",
+						Primary: &terraform.InstanceState{
+							ID:      "bar",
+							Tainted: true,
+						},
+					},
+				},
+			},
+		},
+	}
+	statePath := testStateFile(t, state)
+	unlock, err := testLockState(statePath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer unlock()
+
+	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.Fatal("expected error")
+	}
+
+	output := ui.ErrorWriter.String()
+	if !strings.Contains(output, "locked") {
+		t.Fatal("command output does not look like a lock error:", output)
+	}
+}
+
 func TestUntaint_backup(t *testing.T) {
 	// Get a temp cwd
 	tmp, cwd := testCwd(t)

state/local.go

@@ -19,16 +19,14 @@ type lockInfo struct {
 	Path string
 	// The time the lock was taken
 	Created time.Time
-	// The time this lock expires
+	// Extra info passed to State.Lock
-	Expires time.Time
-	// The lock reason passed to State.Lock
 	Reason string
 }
 
 // return the lock info formatted in an error
 func (l *lockInfo) Err() error {
-	return fmt.Errorf("state file %q locked. created:%s, expires:%s, reason:%s",
+	return fmt.Errorf("state file %q locked. created:%s, reason:%s",
-		l.Path, l.Created, l.Expires, l.Reason)
+		l.Path, l.Created, l.Reason)
 }
 
 // LocalState manages a state storage that is local to the filesystem.
@@ -41,6 +39,10 @@ type LocalState struct {
 
 	// the file handle corresponding to PathOut
 	stateFileOut *os.File
+	// created is set to tru if stateFileOut didn't exist before we created it.
+	// This is mostly so we can clean up emtpy files during tests, but doesn't
+	// hurt to remove file we never wrote to.
+	created bool
 
 	state     *terraform.State
 	readState *terraform.State
@@ -77,8 +79,26 @@ func (s *LocalState) Lock(reason string) error {
 }
 
 func (s *LocalState) Unlock() error {
+	// we can't be locked if we don't have a file
+	if s.stateFileOut == nil {
+		return nil
+	}
+
 	os.Remove(s.lockInfoPath())
-	return s.unlock()
+
+	fileName := s.stateFileOut.Name()
+
+	unlockErr := s.unlock()
+	s.stateFileOut.Close()
+	s.stateFileOut = nil
+
+	// clean up the state file if we created it an never wrote to it
+	stat, err := os.Stat(fileName)
+	if err == nil && stat.Size() == 0 && s.created {
+		os.Remove(fileName)
+	}
+
+	return unlockErr
 }
 
 // Open the state file, creating the directories and file as needed.
@@ -87,28 +107,25 @@ func (s *LocalState) createStateFiles() error {
 		s.PathOut = s.Path
 	}
 
-	f, err := createFileAndDirs(s.PathOut)
+	// yes this could race, but we only use it to clean up empty files
+	if _, err := os.Stat(s.PathOut); os.IsNotExist(err) {
+		s.created = true
+	}
+
+	// Create all the directories
+	if err := os.MkdirAll(filepath.Dir(s.PathOut), 0755); err != nil {
+		return err
+	}
+
+	f, err := os.OpenFile(s.PathOut, os.O_RDWR|os.O_CREATE, 0666)
 	if err != nil {
 		return err
 	}
+
 	s.stateFileOut = f
 	return nil
 }
 
-func createFileAndDirs(path string) (*os.File, error) {
-	// Create all the directories
-	if err := os.MkdirAll(filepath.Dir(path), 0755); err != nil {
-		return nil, err
-	}
-
-	f, err := os.OpenFile(path, os.O_RDWR|os.O_CREATE, 0666)
-	if err != nil {
-		return nil, err
-	}
-
-	return f, nil
-}
-
 // WriteState for LocalState always persists the state as well.
 // TODO: this should use a more robust method of writing state, by first
 // writing to a temp file on the same filesystem, and renaming the file over
@@ -229,7 +246,6 @@ func (s *LocalState) writeLockInfo(reason string) error {
 	lockInfo := &lockInfo{
 		Path:    s.Path,
 		Created: time.Now().UTC(),
-		Expires: time.Now().Add(time.Hour).UTC(),
 		Reason:  reason,
 	}
 

state/remote/s3.go

@@ -207,7 +207,6 @@ func (c *S3Client) Lock(reason string) error {
 		Item: map[string]*dynamodb.AttributeValue{
 			"LockID":  {S: aws.String(stateName)},
 			"Created": {S: aws.String(time.Now().UTC().Format(time.RFC3339))},
-			"Expires": {S: aws.String(time.Now().Add(time.Hour).UTC().Format(time.RFC3339))},
 			"Info":    {S: aws.String(reason)},
 		},
 		TableName:           aws.String(c.lockTable),
@@ -220,7 +219,7 @@ func (c *S3Client) Lock(reason string) error {
 			Key: map[string]*dynamodb.AttributeValue{
 				"LockID": {S: aws.String(fmt.Sprintf("%s/%s", c.bucketName, c.keyName))},
 			},
-			ProjectionExpression: aws.String("LockID, Created, Expires, Info"),
+			ProjectionExpression: aws.String("LockID, Created, Info"),
 			TableName:            aws.String(c.lockTable),
 		}
 
@@ -229,19 +228,16 @@ func (c *S3Client) Lock(reason string) error {
 			return fmt.Errorf("s3 state file %q locked, cfailed to retrive info: %s", stateName, err)
 		}
 
-		var created, expires, info string
+		var created, info string
 		if v, ok := resp.Item["Created"]; ok && v.S != nil {
 			created = *v.S
 		}
-		if v, ok := resp.Item["Expires"]; ok && v.S != nil {
-			expires = *v.S
-		}
 		if v, ok := resp.Item["Info"]; ok && v.S != nil {
 			info = *v.S
 		}
 
-		return fmt.Errorf("state file %q locked. created:%s, expires:%s, reason:%s",
+		return fmt.Errorf("state file %q locked. created:%s, reason:%s",
-			stateName, created, expires, info)
+			stateName, created, info)
 
 	}
 	return nil

state/state.go

@@ -42,7 +42,9 @@ type StatePersister interface {
 }
 
 // Locker is implemented to lock state during command execution.
+// The optional info parameter can be recorded with the lock, but the
+// implementation should not depend in its value.
 type Locker interface {
-	Lock(reason string) error
+	Lock(info string) error
 	Unlock() error
 }