ResiLien
/
terraform
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

states/statemgr: In Filesystem, back up output file, not input file 

The filesystem backend has the option of using a different file for its
initial read.

Previously we were incorrectly writing the contents of that file out into
the backup file, rather than the prior contents of the output file. Now
we will always read the output file in RefreshState in order to decide
what we will back up but then we will optionally additionally read the
input file and prefer its content as the "current" state snapshot.

This is verified by command.TestMetaBackend_planLocalStatePath and
TestMetaBackend_configureNew, which are both now passing.
This commit is contained in:
Martin Atkins 2018-11-15 17:01:19 -08:00
parent 27abd9c6b8
commit 48601d261d
3 changed files with 185 additions and 48 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
command/meta_backend_test.go
View File

@ -238,7 +238,6 @@ func TestMetaBackend_configureInterpolation(t *testing.T) {
// Newly configured backend
func TestMetaBackend_configureNew(t *testing.T) {
// Create a temporary working directory that is empty
td := tempDir(t)
copy.CopyDir(testFixturePath("backend-new"), td)
defer os.RemoveAll(td)
@ -1532,7 +1531,6 @@ func TestMetaBackend_planLocal(t *testing.T) {
// A plan with a custom state save path
func TestMetaBackend_planLocalStatePath(t *testing.T) {
// Create a temporary working directory that is empty
td := tempDir(t)
copy.CopyDir(testFixturePath("backend-plan-local"), td)
defer os.RemoveAll(td)

115
states/statemgr/filesystem.go
View File

@ -56,7 +56,6 @@ type Filesystem struct {
file *statefile.File
readFile *statefile.File
backupFile *statefile.File
written bool
writtenBackup bool
}
@ -115,9 +114,6 @@ func (s *Filesystem) State() *states.State {
if s.file == nil {
return nil
}
if s.backupPath != "" && s.backupFile == nil {
s.backupFile = s.file.DeepCopy()
}
return s.file.DeepCopy().State
}
@ -128,36 +124,19 @@ func (s *Filesystem) WriteState(state *states.State) error {
// writing to a temp file on the same filesystem, and renaming the file over
// the original.
defer s.mutex()()
if s.readFile == nil {
err := s.RefreshState()
err := s.refreshState()
if err != nil {
return err
}
}
defer s.mutex()()
return s.writeState(state, nil)
}
func (s *Filesystem) writeState(state *states.State, meta *SnapshotMeta) error {
// 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) {
log.Printf("[TRACE] statemgr.Filesystem: creating backup snapshot at %s", s.backupPath)
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
@ -165,13 +144,46 @@ func (s *Filesystem) writeState(state *states.State, meta *SnapshotMeta) error {
}
defer s.stateFileOut.Sync()
// 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 != "" {
if !statefile.StatesMarshalEqual(state, s.backupFile.State) {
log.Printf("[TRACE] statemgr.Filesystem: creating backup snapshot at %s", s.backupPath)
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
} else {
log.Print("[TRACE] statemgr.Filesystem: not making a backup, because the new snapshot is identical to the old")
}
} else {
// This branch is all just logging, to help understand why we didn't make a backup.
switch {
case s.backupPath == "":
log.Print("[TRACE] statemgr.Filesystem: state file backups are disabled")
case s.writtenBackup:
log.Printf("[TRACE] statemgr.Filesystem: have already backed up original %s to %s on a previous write", s.path, s.backupPath)
case s.backupFile == nil:
log.Printf("[TRACE] statemgr.Filesystem: no original state snapshot to back up")
default:
log.Printf("[TRACE] statemgr.Filesystem: not creating a backup for an unknown reason")
}
}
s.file = s.file.DeepCopy()
if s.file == nil {
s.file = NewStateFile()
}
s.file.State = state.DeepCopy()
log.Print("[TRACE] statemgr.Filesystem: truncating the state file")
if _, err := s.stateFileOut.Seek(0, os.SEEK_SET); err != nil {
return err
}
@ -204,7 +216,8 @@ func (s *Filesystem) writeState(state *states.State, meta *SnapshotMeta) error {
return err
}
s.written = true
// Any future reads must come from the file we've now updated
s.readPath = s.path
return nil
}
@ -217,7 +230,10 @@ func (s *Filesystem) PersistState() error {
// RefreshState is an implementation of Refresher.
func (s *Filesystem) RefreshState() error {
defer s.mutex()()
return s.refreshState()
}
func (s *Filesystem) refreshState() error {
var reader io.Reader
// The s.readPath file is only OK to read if we have not written any state out
@ -227,10 +243,9 @@ func (s *Filesystem) RefreshState() error {
// This is important for Windows, as if the input file is the same as the
// output file, and the output file has been locked already, we can't open
// the file again.
if !s.written && (s.stateFileOut == nil || s.readPath != s.path) {
log.Printf("[TRACE] statemgr.Filesystem: reading initial snapshot from %s", s.readPath)
if s.stateFileOut == nil || s.readPath != s.path {
// we haven't written a state file yet, so load from readPath
log.Printf("[TRACE] statemgr.Filesystem: reading initial snapshot from %s", s.readPath)
f, err := os.Open(s.readPath)
if err != nil {
// It is okay if the file doesn't exist; we'll treat that as a nil state.
@ -247,11 +262,9 @@ func (s *Filesystem) RefreshState() error {
reader = f
}
} else {
log.Printf("[TRACE] statemgr.Filesystem: reading snapshot from %s", s.path)
log.Printf("[TRACE] statemgr.Filesystem: reading latest snapshot from %s", s.path)
// no state to refresh
if s.stateFileOut == nil {
log.Printf("[TRACE] statemgr.Filesystem: no state snapshot has been written yet")
return nil
}
@ -261,23 +274,20 @@ func (s *Filesystem) RefreshState() error {
}
f, err := statefile.Read(reader)
// nothing to backup if there's no initial state
if f == nil {
log.Printf("[TRACE] statemgr.Filesystem: no initial state, so will skip writing a backup")
s.writtenBackup = true
}
// if there's no state we just assign the nil return value
if err != nil && err != statefile.ErrNoState {
log.Printf("[TRACE] statemgr.Filesystem: state snapshot is nil")
return err
// if there's no state then a nil file is fine
if err != nil {
if err != statefile.ErrNoState {
return err
}
log.Printf("[TRACE] statemgr.Filesystem: snapshot file has nil snapshot, but that's okay")
}
s.file = f
s.readFile = s.file.DeepCopy()
if s.file != nil {
log.Printf("[TRACE] statemgr.Filesystem: read snapshot with lineage %q serial %d", s.file.Lineage, s.file.Serial)
} else {
log.Print("[TRACE] statemgr.Filesystem: read nil snapshot")
}
return nil
}
@ -378,13 +388,14 @@ func (s *Filesystem) StateForMigration() *statefile.File {
// WriteStateForMigration is part of our implementation of Migrator.
func (s *Filesystem) WriteStateForMigration(f *statefile.File, force bool) error {
defer s.mutex()()
if s.readFile == nil {
err := s.RefreshState()
err := s.refreshState()
if err != nil {
return err
}
}
defer s.mutex()()
if !force {
err := CheckValidImport(f, s.readFile)
@ -436,6 +447,20 @@ func (s *Filesystem) createStateFiles() error {
}
s.stateFileOut = f
// If the file already existed with content then that'll be the content
// of our backup file if we write a change later.
s.backupFile, err = statefile.Read(s.stateFileOut)
if err != nil {
if err != statefile.ErrNoState {
return err
}
log.Printf("[TRACE] statemgr.Filesystem: no previously-stored snapshot exists")
} else {
log.Printf("[TRACE] statemgr.Filesystem: existing snapshot has lineage %q serial %d", s.backupFile.Lineage, s.backupFile.Serial)
}
// Refresh now, to load in the snapshot if the file already existed
return nil
}

116
states/statemgr/filesystem_test.go
View File

@ -4,13 +4,17 @@ import (
"io/ioutil"
"os"
"os/exec"
"path/filepath"
"strings"
"sync"
"testing"
"github.com/go-test/deep"
version "github.com/hashicorp/go-version"
"github.com/zclconf/go-cty/cty"
"github.com/hashicorp/terraform/addrs"
"github.com/hashicorp/terraform/states"
"github.com/hashicorp/terraform/states/statefile"
tfversion "github.com/hashicorp/terraform/version"
)
@ -57,7 +61,7 @@ func TestFilesystemLocks(t *testing.T) {
t.Fatal("unexpected lock failure", err, string(out))
}
if string(out) != "lock failed" {
if !strings.Contains(string(out), "lock failed") {
t.Fatal("expected 'locked failed', got", string(out))
}
@ -172,6 +176,116 @@ func TestFilesystem_backup(t *testing.T) {
}
}
// This test verifies a particularly tricky behavior where the input file
// is overridden and backups are enabled at the same time. This combination
// requires special care because we must ensure that when we create a backup
// it is of the original contents of the output file (which we're overwriting),
// not the contents of the input file (which is left unchanged).
func TestFilesystem_backupAndReadPath(t *testing.T) {
defer testOverrideVersion(t, "1.2.3")()
workDir, err := ioutil.TempDir("", "tf")
if err != nil {
t.Fatalf("failed to create temporary directory: %s", err)
}
defer os.RemoveAll(workDir)
markerOutput := addrs.OutputValue{Name: "foo"}.Absolute(addrs.RootModuleInstance)
outState := states.BuildState(func(ss *states.SyncState) {
ss.SetOutputValue(
markerOutput,
cty.StringVal("from-output-state"),
false, // not sensitive
)
})
outFile, err := os.Create(filepath.Join(workDir, "output.tfstate"))
if err != nil {
t.Fatalf("failed to create temporary outFile %s", err)
}
defer outFile.Close()
err = statefile.Write(&statefile.File{
Lineage: "-",
Serial: 0,
TerraformVersion: version.Must(version.NewVersion("1.2.3")),
State: outState,
}, outFile)
if err != nil {
t.Fatalf("failed to write initial outfile state to %s: %s", outFile.Name(), err)
}
inState := states.BuildState(func(ss *states.SyncState) {
ss.SetOutputValue(
markerOutput,
cty.StringVal("from-input-state"),
false, // not sensitive
)
})
inFile, err := os.Create(filepath.Join(workDir, "input.tfstate"))
if err != nil {
t.Fatalf("failed to create temporary inFile %s", err)
}
defer inFile.Close()
err = statefile.Write(&statefile.File{
Lineage: "-",
Serial: 0,
TerraformVersion: version.Must(version.NewVersion("1.2.3")),
State: inState,
}, inFile)
if err != nil {
t.Fatalf("failed to write initial infile state to %s: %s", inFile.Name(), err)
}
backupPath := outFile.Name() + ".backup"
ls := NewFilesystemBetweenPaths(inFile.Name(), outFile.Name())
ls.SetBackupPath(backupPath)
newState := states.BuildState(func(ss *states.SyncState) {
ss.SetOutputValue(
markerOutput,
cty.StringVal("from-new-state"),
false, // not sensitive
)
})
err = ls.WriteState(newState)
if err != nil {
t.Fatalf("failed to write new state: %s", err)
}
// The backup functionality should've saved a copy of the original contents
// of the _output_ file, even though the first snapshot was read from
// the _input_ file.
t.Run("backup file", func (t *testing.T) {
bfh, err := os.Open(backupPath)
if err != nil {
t.Fatal(err)
}
bf, err := statefile.Read(bfh)
if err != nil {
t.Fatal(err)
}
os := bf.State.OutputValue(markerOutput)
if got, want := os.Value, cty.StringVal("from-output-state"); !want.RawEquals(got) {
t.Errorf("wrong marker value in backup state file\ngot: %#v\nwant: %#v", got, want)
}
})
t.Run("output file", func (t *testing.T) {
ofh, err := os.Open(outFile.Name())
if err != nil {
t.Fatal(err)
}
of, err := statefile.Read(ofh)
if err != nil {
t.Fatal(err)
}
os := of.State.OutputValue(markerOutput)
if got, want := os.Value, cty.StringVal("from-new-state"); !want.RawEquals(got) {
t.Errorf("wrong marker value in backup state file\ngot: %#v\nwant: %#v", got, want)
}
})
}
func TestFilesystem_nonExist(t *testing.T) {
defer testOverrideVersion(t, "1.2.3")()
ls := NewFilesystem("ishouldntexist")