From fb4a365d12079ffb940db517b9bf9ee1d264bbde Mon Sep 17 00:00:00 2001 From: James Bardin Date: Thu, 30 Mar 2017 15:53:21 -0400 Subject: [PATCH] noop migrate copy, add -lock and -input A couple commits got rebased together here, and it's easier to enumerate them in a single commit. Skip copying of states during migration if they are the same state. This can happen when trying to reconfigure a backend's options, or if the state was manually transferred. This can fail unexpectedly with locking enabled. Honor the `-input` flag for all confirmations (the new test hit some more). Also unify where we reference the Meta.forceInitCopy and transfer the value to the existing backendMigrateOpts.force field. --- command/init.go | 1 - command/meta_backend_migrate.go | 48 ++++++++++++++++++++++--------- command/meta_backend_test.go | 51 +++++++++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 14 deletions(-) diff --git a/command/init.go b/command/init.go index 6721b87a1..6b7fbaf21 100644 --- a/command/init.go +++ b/command/init.go @@ -237,7 +237,6 @@ Options: -force-copy Suppress prompts about copying state data. This is equivalent to providing a "yes" to all confirmation prompts. - ` return strings.TrimSpace(helpText) } diff --git a/command/meta_backend_migrate.go b/command/meta_backend_migrate.go index 9a0b05285..40d0d12cb 100644 --- a/command/meta_backend_migrate.go +++ b/command/meta_backend_migrate.go @@ -2,6 +2,7 @@ package command import ( "context" + "errors" "fmt" "io/ioutil" "os" @@ -53,7 +54,7 @@ func (m *Meta) backendMigrateState(opts *backendMigrateOpts) error { // Setup defaults opts.oneEnv = backend.DefaultStateName opts.twoEnv = backend.DefaultStateName - opts.force = false + opts.force = m.forceInitCopy // Determine migration behavior based on whether the source/destionation // supports multi-state. @@ -163,7 +164,7 @@ func (m *Meta) backendMigrateState_S_S(opts *backendMigrateOpts) error { func (m *Meta) backendMigrateState_S_s(opts *backendMigrateOpts) error { currentEnv := m.Env() - migrate := m.forceInitCopy + migrate := opts.force if !migrate { var err error // Ask the user if they want to migrate their existing remote state @@ -218,6 +219,19 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { errMigrateSingleLoadDefault), opts.TwoType, err) } + // Check if we need migration at all. + // This is before taking a lock, because they may also correspond to the same lock. + one := stateOne.State() + two := stateTwo.State() + + // no reason to migrate if the state is already there + if one.Equal(two) { + // Equal isn't identical; it doesn't check lineage. + if one != nil && two != nil && one.Lineage == two.Lineage { + return nil + } + } + if m.stateLock { lockCtx, cancel := context.WithTimeout(context.Background(), m.stateLockTimeout) defer cancel() @@ -241,10 +255,21 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { return fmt.Errorf("Error locking destination state: %s", err) } defer clistate.Unlock(stateTwo, lockIDTwo, m.Ui, m.Colorize()) - } - one := stateOne.State() - two := stateTwo.State() + // We now own a lock, so double check that we have the version + // corresponding to the lock. + if err := stateOne.RefreshState(); err != nil { + return fmt.Errorf(strings.TrimSpace( + errMigrateSingleLoadDefault), opts.OneType, err) + } + if err := stateTwo.RefreshState(); err != nil { + return fmt.Errorf(strings.TrimSpace( + errMigrateSingleLoadDefault), opts.OneType, err) + } + + one = stateOne.State() + two = stateTwo.State() + } // Clear the legacy remote state in both cases. If we're at the migration // step then this won't be used anymore. @@ -281,6 +306,11 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { } if !opts.force { + // Abort if we can't ask for input. + if !m.input { + return errors.New("error asking for state migration action: inptut disabled") + } + // Confirm with the user whether we want to copy state over confirm, err := confirmFunc(stateOne, stateTwo, opts) if err != nil { @@ -306,10 +336,6 @@ func (m *Meta) backendMigrateState_s_s(opts *backendMigrateOpts) error { } func (m *Meta) backendMigrateEmptyConfirm(one, two state.State, opts *backendMigrateOpts) (bool, error) { - if m.forceInitCopy { - return true, nil - } - inputOpts := &terraform.InputOpts{ Id: "backend-migrate-copy-to-empty", Query: fmt.Sprintf( @@ -372,10 +398,6 @@ func (m *Meta) backendMigrateNonEmptyConfirm( return false, fmt.Errorf("Error saving temporary state: %s", err) } - if m.forceInitCopy { - return true, nil - } - // Ask for confirmation inputOpts := &terraform.InputOpts{ Id: "backend-migrate-to-backend", diff --git a/command/meta_backend_test.go b/command/meta_backend_test.go index ea0085dba..ffb6e3b61 100644 --- a/command/meta_backend_test.go +++ b/command/meta_backend_test.go @@ -426,6 +426,57 @@ func TestMetaBackend_configureNewWithState(t *testing.T) { } } +// Newly configured backend with matching local and remote state doesn't prompt +// for copy. +func TestMetaBackend_configureNewWithoutCopy(t *testing.T) { + // Create a temporary working directory that is empty + td := tempDir(t) + copy.CopyDir(testFixturePath("backend-new-migrate"), td) + defer os.RemoveAll(td) + defer testChdir(t, td)() + + if err := copy.CopyFile(DefaultStateFilename, "local-state.tfstate"); err != nil { + t.Fatal(err) + } + + // Setup the meta + m := testMetaBackend(t, nil) + m.input = false + + // init the backend + _, err := m.Backend(&BackendOpts{Init: true}) + if err != nil { + t.Fatalf("bad: %s", err) + } + + // Verify the state is where we expect + f, err := os.Open("local-state.tfstate") + if err != nil { + t.Fatalf("err: %s", err) + } + actual, err := terraform.ReadState(f) + f.Close() + if err != nil { + t.Fatalf("err: %s", err) + } + + if actual.Lineage != "backend-new-migrate" { + t.Fatalf("incorrect state lineage: %q", actual.Lineage) + } + + // Verify the default paths don't exist + if !isEmptyState(DefaultStateFilename) { + data, _ := ioutil.ReadFile(DefaultStateFilename) + + t.Fatal("state should not exist, but contains:\n", string(data)) + } + + // Verify a backup does exist + if isEmptyState(DefaultStateFilename + DefaultBackupExtension) { + t.Fatal("backup state is empty or missing") + } +} + // Newly configured backend with prior local state and no remote state, // but opting to not migrate. func TestMetaBackend_configureNewWithStateNoMigrate(t *testing.T) {