From 65abe9804727e1fbc63a78c526d19e0b9bbecf45 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 7 Feb 2017 18:00:09 -0500 Subject: [PATCH] Remove lock command and rename lock/force-unlock Remove the lock command for now to avoid confusion about the behavior of locks. Rename lock to force-unlock to make it more aparent what it does. Add a success message, and chose red because it can be a dangerous operation. Add confirmation akin to `destroy`, and a `-force` option for automation and testing. --- command/lock.go | 78 --------------------------------- command/lock_test.go | 97 ------------------------------------------ command/unlock.go | 59 +++++++++++++++++++++++-- command/unlock_test.go | 2 +- commands.go | 6 +++ 5 files changed, 63 insertions(+), 179 deletions(-) delete mode 100644 command/lock.go delete mode 100644 command/lock_test.go diff --git a/command/lock.go b/command/lock.go deleted file mode 100644 index e6b2d211bd..0000000000 --- a/command/lock.go +++ /dev/null @@ -1,78 +0,0 @@ -package command - -import ( - "fmt" - "strings" - - "github.com/hashicorp/terraform/state" -) - -// LockCommand is a cli.Command implementation that manually locks -// the state. -type LockCommand struct { - Meta -} - -func (c *LockCommand) Run(args []string) int { - args = c.Meta.process(args, false) - - cmdFlags := c.Meta.flagSet("lock") - cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } - if err := cmdFlags.Parse(args); err != nil { - return 1 - } - - // assume everything is initialized. The user can manually init if this is - // required. - configPath, err := ModulePath(cmdFlags.Args()) - if err != nil { - c.Ui.Error(err.Error()) - return 1 - } - - // Load the backend - b, err := c.Backend(&BackendOpts{ - ConfigPath: configPath, - }) - if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load backend: %s", err)) - return 1 - } - - st, err := b.State() - if err != nil { - c.Ui.Error(fmt.Sprintf("Failed to load state: %s", err)) - return 1 - } - - s, ok := st.(state.Locker) - if !ok { - c.Ui.Error("Current state does not support locking") - return 1 - } - - if err := s.Lock("lock"); err != nil { - c.Ui.Error(fmt.Sprintf("Failed to lock state: %s", err)) - return 1 - } - - return 0 -} - -func (c *LockCommand) Help() string { - helpText := ` -Usage: terraform lock [DIR] - - Manually lock the state for the defined configuration. - - This will not modify your infrastructure. This command obtains a lock on the - state for the current configuration. The behavior of this lock is dependent - on the backend being used. A lock on a local state file only lasts for the - duration of the calling process. -` - return strings.TrimSpace(helpText) -} - -func (c *LockCommand) Synopsis() string { - return "Manually lock the terraform state" -} diff --git a/command/lock_test.go b/command/lock_test.go deleted file mode 100644 index 0e5331133e..0000000000 --- a/command/lock_test.go +++ /dev/null @@ -1,97 +0,0 @@ -package command - -import ( - "os" - "path/filepath" - "strings" - "testing" - - "github.com/hashicorp/terraform/terraform" - "github.com/mitchellh/cli" -) - -func TestLock(t *testing.T) { - testData, _ := filepath.Abs("./testdata") - - td := tempDir(t) - os.MkdirAll(td, 0755) - defer os.RemoveAll(td) - defer testChdir(t, td)() - - // Write the legacy state - statePath := DefaultStateFilename - { - f, err := os.Create(statePath) - if err != nil { - t.Fatalf("err: %s", err) - } - err = terraform.WriteState(testState(), f) - f.Close() - if err != nil { - t.Fatalf("err: %s", err) - } - } - - p := testProvider() - ui := new(cli.MockUi) - c := &LockCommand{ - Meta: Meta{ - ContextOpts: testCtxConfig(p), - Ui: ui, - }, - } - - if code := c.Run(nil); code != 0 { - t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) - } - - unlock, err := testLockState(testData, statePath) - if err == nil { - unlock() - t.Fatal("expected error locking state") - } else if !strings.Contains(err.Error(), "locked") { - t.Fatal("does not appear to be a lock error:", err) - } -} - -func TestLock_lockedState(t *testing.T) { - testData, _ := filepath.Abs("./testdata") - - td := tempDir(t) - os.MkdirAll(td, 0755) - defer os.RemoveAll(td) - defer testChdir(t, td)() - - // Write the legacy state - statePath := DefaultStateFilename - { - f, err := os.Create(statePath) - if err != nil { - t.Fatalf("err: %s", err) - } - err = terraform.WriteState(testState(), f) - f.Close() - if err != nil { - t.Fatalf("err: %s", err) - } - } - - p := testProvider() - ui := new(cli.MockUi) - c := &LockCommand{ - Meta: Meta{ - ContextOpts: testCtxConfig(p), - Ui: ui, - }, - } - - unlock, err := testLockState(testData, statePath) - if err != nil { - t.Fatal(err) - } - defer unlock() - - if code := c.Run(nil); code == 0 { - t.Fatal("expected error when locking a locked state") - } -} diff --git a/command/unlock.go b/command/unlock.go index c31e82ad97..f277df6938 100644 --- a/command/unlock.go +++ b/command/unlock.go @@ -5,6 +5,7 @@ import ( "strings" "github.com/hashicorp/terraform/state" + "github.com/hashicorp/terraform/terraform" ) // UnlockCommand is a cli.Command implementation that manually unlocks @@ -16,7 +17,9 @@ type UnlockCommand struct { func (c *UnlockCommand) Run(args []string) int { args = c.Meta.process(args, false) - cmdFlags := c.Meta.flagSet("unlock") + force := false + cmdFlags := c.Meta.flagSet("force-unlock") + cmdFlags.BoolVar(&force, "force", false, "force") cmdFlags.Usage = func() { c.Ui.Error(c.Help()) } if err := cmdFlags.Parse(args); err != nil { return 1 @@ -47,21 +50,60 @@ func (c *UnlockCommand) Run(args []string) int { s, ok := st.(state.Locker) if !ok { - c.Ui.Error("Current state does not support locking") + c.Ui.Error("The remote state backend in use does not support locking, and therefor\n" + + "cannot be unlocked.") return 1 } + isLocal := false + switch s := st.(type) { + case *state.BackupState: + if _, ok := s.Real.(*state.LocalState); ok { + isLocal = true + } + case *state.LocalState: + isLocal = true + } + + if !force { + // Forcing this doesn't do anything, but doesn't break anything either, + // and allows us to run the basic command test too. + if isLocal { + c.Ui.Error("Local state cannot be unlocked by another process") + return 1 + } + + desc := "Terraform will remove the lock on the remote state.\n" + + "This will allow local Terraform commands to modify this state, even though it\n" + + "may be still be in use. Only 'yes' will be accepted to confirm." + + v, err := c.UIInput().Input(&terraform.InputOpts{ + Id: "force-unlock", + Query: "Do you really want to force-unlock?", + Description: desc, + }) + if err != nil { + c.Ui.Error(fmt.Sprintf("Error asking for confirmation: %s", err)) + return 1 + } + if v != "yes" { + c.Ui.Output("force-unlock cancelled.") + return 1 + } + } + if err := s.Unlock(); err != nil { c.Ui.Error(fmt.Sprintf("Failed to unlock state: %s", err)) return 1 } + c.Ui.Output(c.Colorize().Color(strings.TrimSpace(outputUnlockSuccess))) return 0 } func (c *UnlockCommand) Help() string { helpText := ` -Usage: terraform unlock [DIR] +Usage: terraform force-unlock [DIR] Manually unlock the state for the defined configuration. @@ -69,6 +111,10 @@ Usage: terraform unlock [DIR] state for the current configuration. The behavior of this lock is dependent on the backend being used. Local state files cannot be unlocked by another process. + +Options: + + -force Don't ask for input for unlock confirmation. ` return strings.TrimSpace(helpText) } @@ -76,3 +122,10 @@ Usage: terraform unlock [DIR] func (c *UnlockCommand) Synopsis() string { return "Manually unlock the terraform state" } + +const outputUnlockSuccess = ` +[reset][bold][red]Terraform state has been successfully unlocked![reset][red] + +The state has been unlocked, and Terraform commands should now be able to +obtain a new lock on the remote state. +` diff --git a/command/unlock_test.go b/command/unlock_test.go index b133dcb187..2496f3fe60 100644 --- a/command/unlock_test.go +++ b/command/unlock_test.go @@ -40,7 +40,7 @@ func TestUnlock(t *testing.T) { }, } - if code := c.Run(nil); code != 0 { + if code := c.Run([]string{"-force"}); code != 0 { t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) } } diff --git a/commands.go b/commands.go index ec4d037932..20e2ff8921 100644 --- a/commands.go +++ b/commands.go @@ -75,6 +75,12 @@ func init() { }, nil }, + "force-unlock": func() (cli.Command, error) { + return &command.UnlockCommand{ + Meta: meta, + }, nil + }, + "get": func() (cli.Command, error) { return &command.GetCommand{ Meta: meta,