From 2019a44f044b91f7f17a50154f06b2d4b30d2b74 Mon Sep 17 00:00:00 2001 From: Mitchell Hashimoto Date: Fri, 28 Oct 2016 20:51:05 -0400 Subject: [PATCH] command/apply: apply from plan respects -backup and -state-out Fixes #5409 I didn't expect this to be such a rabbit hole! Based on git history, it appears that for "historical reasons"(tm), setting up the various `state.State` structures for a plan were _completely different logic_ than a normal `terraform apply`. This meant that it was skipping things like disabling backups with `-backup="-"`. This PR unifies loading from a plan to the normal state setup mechanism. A few tests that were failing prior to this PR were added, no existing tests were changed. --- command/apply_test.go | 73 +++++++++++++++++++++++++ command/command_test.go | 14 +++++ command/meta.go | 14 +++-- command/state.go | 118 +++++++++++++++++++++------------------- 4 files changed, 158 insertions(+), 61 deletions(-) diff --git a/command/apply_test.go b/command/apply_test.go index 23e1f3f6e..150c6d014 100644 --- a/command/apply_test.go +++ b/command/apply_test.go @@ -508,6 +508,79 @@ func TestApply_plan(t *testing.T) { } } +func TestApply_plan_backup(t *testing.T) { + planPath := testPlanFile(t, testPlan(t)) + statePath := testTempFile(t) + backupPath := testTempFile(t) + + p := testProvider() + ui := new(cli.MockUi) + c := &ApplyCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(p), + Ui: ui, + }, + } + + args := []string{ + "-state", statePath, + "-backup", backupPath, + planPath, + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + { + // Should have a backup file + f, err := os.Open(backupPath) + if err != nil { + t.Fatalf("err: %s", err) + } + + _, err = terraform.ReadState(f) + f.Close() + if err != nil { + t.Fatalf("err: %s", err) + } + } +} + +func TestApply_plan_noBackup(t *testing.T) { + planPath := testPlanFile(t, testPlan(t)) + statePath := testTempFile(t) + + p := testProvider() + ui := new(cli.MockUi) + c := &ApplyCommand{ + Meta: Meta{ + ContextOpts: testCtxConfig(p), + Ui: ui, + }, + } + + args := []string{ + "-state", statePath, + "-backup", "-", + planPath, + } + if code := c.Run(args); code != 0 { + t.Fatalf("bad: %d\n\n%s", code, ui.ErrorWriter.String()) + } + + // Ensure there is no backup + _, err := os.Stat(statePath + DefaultBackupExtension) + if err == nil || !os.IsNotExist(err) { + t.Fatalf("backup should not exist") + } + + // Ensure there is no literal "-" + _, err = os.Stat("-") + if err == nil || !os.IsNotExist(err) { + t.Fatalf("backup should not exist") + } +} + func TestApply_plan_remoteState(t *testing.T) { // Disable test mode so input would be asked test = false diff --git a/command/command_test.go b/command/command_test.go index 2104d33a9..098d2eae0 100644 --- a/command/command_test.go +++ b/command/command_test.go @@ -98,6 +98,20 @@ func testModule(t *testing.T, name string) *module.Tree { return mod } +// testPlan returns a non-nil noop plan. +func testPlan(t *testing.T) *terraform.Plan { + state := terraform.NewState() + state.RootModule().Outputs["foo"] = &terraform.OutputState{ + Type: "string", + Value: "foo", + } + + return &terraform.Plan{ + Module: testModule(t, "apply"), + State: state, + } +} + func testPlanFile(t *testing.T, plan *terraform.Plan) string { path := testTempFile(t) diff --git a/command/meta.go b/command/meta.go index bd2016564..92f25db1e 100644 --- a/command/meta.go +++ b/command/meta.go @@ -112,18 +112,24 @@ func (m *Meta) Context(copts contextOpts) (*terraform.Context, bool, error) { plan, err := terraform.ReadPlan(f) f.Close() if err == nil { - // Setup our state - state, statePath, err := StateFromPlan(m.statePath, m.stateOutPath, plan) + // Setup our state, force it to use our plan's state + stateOpts := m.StateOpts() + if plan != nil { + stateOpts.ForceState = plan.State + } + + // Get the state + result, err := State(stateOpts) if err != nil { return nil, false, fmt.Errorf("Error loading plan: %s", err) } // Set our state - m.state = state + m.state = result.State // this is used for printing the saved location later if m.stateOutPath == "" { - m.stateOutPath = statePath + m.stateOutPath = result.StatePath } if len(m.variables) > 0 { diff --git a/command/state.go b/command/state.go index b4ff0e583..f1b5e2e0b 100644 --- a/command/state.go +++ b/command/state.go @@ -3,7 +3,6 @@ package command import ( "fmt" "os" - "path/filepath" "strings" "github.com/hashicorp/errwrap" @@ -33,6 +32,10 @@ type StateOpts struct { // it is assumed to be the path where the state is stored locally // plus the DefaultBackupExtension. BackupPath string + + // ForceState is a state structure to force the value to be. This + // is used by Terraform plans (which contain their state). + ForceState *terraform.State } // StateResult is the result of calling State and holds various different @@ -75,6 +78,12 @@ func State(opts *StateOpts) (*StateResult, error) { if err := ls.RefreshState(); err != nil { return nil, err } + + // If we have a forced state, set it + if opts.ForceState != nil { + ls.SetState(opts.ForceState) + } + is := &state.InmemState{} is.WriteState(ls.State()) @@ -88,14 +97,30 @@ func State(opts *StateOpts) (*StateResult, error) { return nil, err } } else { - if _, err := os.Stat(opts.RemotePath); err == nil { - // We have a remote state, initialize that. - remote, err = remoteStateFromPath( + // If we have a forced state that is remote, then we load that + if opts.ForceState != nil && + opts.ForceState.Remote != nil && + opts.ForceState.Remote.Type != "" { + var err error + remote, err = remoteState( + opts.ForceState, opts.RemotePath, - opts.RemoteRefresh) + false) if err != nil { return nil, err } + } else { + // Only if we have no forced state, we check our normal + // remote path. + if _, err := os.Stat(opts.RemotePath); err == nil { + // We have a remote state, initialize that. + remote, err = remoteStateFromPath( + opts.RemotePath, + opts.RemoteRefresh) + if err != nil { + return nil, err + } + } } } @@ -106,6 +131,15 @@ func State(opts *StateOpts) (*StateResult, error) { } } + // If we have a forced state and we were able to initialize that + // into a remote state, we don't do any local state stuff. This is + // because normally we're able to test whether we should do local vs. + // remote by checking file existence. With ForceState, file existence + // doesn't work because neither may exist, so we use state attributes. + if opts.ForceState != nil && result.Remote != nil { + opts.LocalPath = "" + } + // Do we have a local state? if opts.LocalPath != "" { local := &state.LocalState{ @@ -120,23 +154,30 @@ func State(opts *StateOpts) (*StateResult, error) { result.LocalPath = local.PathOut } - err := local.RefreshState() - if err == nil { - if result.State != nil && !result.State.State().Empty() { - if !local.State().Empty() { - // We already have a remote state... that is an error. - return nil, fmt.Errorf( - "Remote state found, but state file '%s' also present.", - opts.LocalPath) - } + // If we're forcing, then set it + if opts.ForceState != nil { + local.SetState(opts.ForceState) + } else { + // If we're not forcing, then we load the state directly + // from disk. + err := local.RefreshState() + if err == nil { + if result.State != nil && !result.State.State().Empty() { + if !local.State().Empty() { + // We already have a remote state... that is an error. + return nil, fmt.Errorf( + "Remote state found, but state file '%s' also present.", + opts.LocalPath) + } - // Empty state - local = nil + // Empty state + local = nil + } + } + if err != nil { + return nil, errwrap.Wrapf( + "Error reading local state: {{err}}", err) } - } - if err != nil { - return nil, errwrap.Wrapf( - "Error reading local state: {{err}}", err) } if local != nil { @@ -167,43 +208,6 @@ func State(opts *StateOpts) (*StateResult, error) { return result, nil } -// StateFromPlan gets our state from the plan. -func StateFromPlan( - localPath, outPath string, - plan *terraform.Plan) (state.State, string, error) { - var result state.State - resultPath := localPath - if plan != nil && plan.State != nil && - plan.State.Remote != nil && plan.State.Remote.Type != "" { - var err error - - // It looks like we have a remote state in the plan, so - // we have to initialize that. - resultPath = filepath.Join(DefaultDataDir, DefaultStateFilename) - result, err = remoteState(plan.State, resultPath, false) - if err != nil { - return nil, "", err - } - } - - if result == nil { - local := &state.LocalState{ - Path: resultPath, - PathOut: outPath, - } - local.SetState(plan.State) - result = local - } - - // If we have a result, make sure to back it up - result = &state.BackupState{ - Real: result, - Path: resultPath + DefaultBackupExtension, - } - - return result, resultPath, nil -} - func remoteState( local *terraform.State, localPath string, refresh bool) (*state.CacheState, error) {