From 05936df0e78bcac111ed70dc69b8966e8807c3fb Mon Sep 17 00:00:00 2001 From: Martin Atkins Date: Thu, 12 Jul 2018 17:06:05 -0700 Subject: [PATCH] statemgr: Backup file support for statemgr.Filesystem In the old state package we had this as a separate manager state.BackupState, but that doesn't work with our new interfaces because we handle lineage and serial within the state managers themselves and don't expose them to callers anymore. In practice it being built in to the filesystem manager is not a problem because we only use the backup functionality for local state anyway. This also slightly adjusts the behavior to be more intuitive. The old BackupState relied on the implementation detail that Terraform re-persists the original state early in an apply operation, which meant that by coincidence it would back up the right snapshot. In this new approach, we instead take an in-memory copy during State and then write _that_ to disk in WriteState if the new state seems different, so we're guaranteed that we'll always write out what we read before any changes were made. In future we may improve this further, such as keeping multiple generations of backups, etc. But for now this is intended to preserve the goals of the original implementation while making its behavior self-contained and not dependent on coincidences. --- states/statemgr/filesystem.go | 48 ++++++++++++++++++++++++++++-- states/statemgr/filesystem_test.go | 34 +++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/states/statemgr/filesystem.go b/states/statemgr/filesystem.go index 6cfdb2a30..e44f85db5 100644 --- a/states/statemgr/filesystem.go +++ b/states/statemgr/filesystem.go @@ -34,6 +34,11 @@ type Filesystem struct { // The file at readPath must never be written to by this manager. readPath string + // backupPath is an optional extra path which, if non-empty, will be + // created or overwritten with the first state snapshot we read if there + // is a subsequent call to write a different state. + backupPath string + // the file handle corresponding to PathOut stateFileOut *os.File @@ -47,9 +52,11 @@ type Filesystem struct { // hurt to remove file we never wrote to. created bool - file *statefile.File - readFile *statefile.File - written bool + file *statefile.File + readFile *statefile.File + backupFile *statefile.File + written bool + writtenBackup bool } var ( @@ -79,12 +86,30 @@ func NewFilesystemBetweenPaths(readPath, writePath string) *Filesystem { } } +// SetBackupPath configures the receiever so that it will create a local +// backup file of the next state snapshot it reads (in State) if a different +// snapshot is subsequently written (in WriteState). Only one backup is +// written for the lifetime of the object, unless reset as described below. +// +// For correct operation, this must be called before any other state methods +// are called. If called multiple times, each call resets the backup +// function so that the next read will become the backup snapshot and a +// following write will save a backup of it. +func (s *Filesystem) SetBackupPath(path string) { + s.backupPath = path + s.backupFile = nil + s.writtenBackup = false +} + // State is an implementation of Reader. func (s *Filesystem) State() *states.State { defer s.mutex()() if s.file == nil { return nil } + if s.backupPath != "" && s.backupFile == nil { + s.backupFile = s.file.DeepCopy() + } return s.file.DeepCopy().State } @@ -100,6 +125,23 @@ func (s *Filesystem) WriteState(state *states.State) error { defer s.mutex()() + // We'll try to write our backup first, so we can be sure we've created + // it successfully before clobbering the original file it came from. + if !s.writtenBackup && s.backupFile != nil && s.backupPath != "" && !statefile.StatesMarshalEqual(state, s.backupFile.State) { + bfh, err := os.Create(s.backupPath) + if err != nil { + return fmt.Errorf("failed to create local state backup file: %s", err) + } + defer bfh.Close() + + err = statefile.Write(s.backupFile, bfh) + if err != nil { + return fmt.Errorf("failed to write to local state backup file: %s", err) + } + + s.writtenBackup = true + } + if s.stateFileOut == nil { if err := s.createStateFiles(); err != nil { return nil diff --git a/states/statemgr/filesystem_test.go b/states/statemgr/filesystem_test.go index ae7868db3..4e50b4a43 100644 --- a/states/statemgr/filesystem_test.go +++ b/states/statemgr/filesystem_test.go @@ -7,6 +7,8 @@ import ( "sync" "testing" + "github.com/go-test/deep" + version "github.com/hashicorp/go-version" "github.com/hashicorp/terraform/states/statefile" @@ -131,6 +133,38 @@ func TestFilesystem_pathOut(t *testing.T) { TestFull(t, ls) } +func TestFilesystem_backup(t *testing.T) { + f, err := ioutil.TempFile("", "tf") + if err != nil { + t.Fatalf("err: %s", err) + } + f.Close() + defer os.Remove(f.Name()) + + ls := testFilesystem(t) + backupPath := f.Name() + ls.SetBackupPath(backupPath) + + TestFull(t, ls) + + // The backup functionality should've saved a copy of the original state + // prior to all of the modifications that TestFull does. + bfh, err := os.Open(backupPath) + if err != nil { + t.Fatal(err) + } + bf, err := statefile.Read(bfh) + if err != nil { + t.Fatal(err) + } + origState := TestFullInitialState() + if !bf.State.Equal(origState) { + for _, problem := range deep.Equal(origState, bf.State) { + t.Error(problem) + } + } +} + func TestFilesystem_nonExist(t *testing.T) { ls := NewFilesystem("ishouldntexist") if err := ls.RefreshState(); err != nil {