From 200c8de4e9eadba52620e3ea9aef9e40c25fb086 Mon Sep 17 00:00:00 2001 From: James Bardin Date: Tue, 14 Feb 2017 17:43:42 -0500 Subject: [PATCH] Update the state.Locker interface Remove CacheState rather than update it, since it's no longer used. --- state/backup.go | 10 +- state/cache.go | 290 -------------------------------------------- state/cache_test.go | 118 ------------------ state/local.go | 21 ---- state/state.go | 60 ++++++++- 5 files changed, 61 insertions(+), 438 deletions(-) delete mode 100644 state/cache.go delete mode 100644 state/cache_test.go diff --git a/state/backup.go b/state/backup.go index bb935eb09..15d8f6f3e 100644 --- a/state/backup.go +++ b/state/backup.go @@ -42,16 +42,16 @@ func (s *BackupState) PersistState() error { } // all states get wrapped by BackupState, so it has to be a Locker -func (s *BackupState) Lock(reason string) error { +func (s *BackupState) Lock(info *LockInfo) (string, error) { if s, ok := s.Real.(Locker); ok { - return s.Lock(reason) + return s.Lock(info) } - return nil + return "", nil } -func (s *BackupState) Unlock() error { +func (s *BackupState) Unlock(id string) error { if s, ok := s.Real.(Locker); ok { - return s.Unlock() + return s.Unlock(id) } return nil } diff --git a/state/cache.go b/state/cache.go deleted file mode 100644 index 3b2b9739c..000000000 --- a/state/cache.go +++ /dev/null @@ -1,290 +0,0 @@ -package state - -import ( - "fmt" - - multierror "github.com/hashicorp/go-multierror" - "github.com/hashicorp/terraform/terraform" -) - -// CacheState is an implementation of the state interfaces that uses -// a StateReadWriter for a local cache. -type CacheState struct { - Cache CacheStateCache - Durable CacheStateDurable - - refreshResult CacheRefreshResult - state *terraform.State -} - -// Locker implementation. -// Since remote states are wrapped in a CacheState, we need to implement the -// Lock/Unlock methods here to delegate them to the remote client. -func (s *CacheState) Lock(reason string) error { - durable, durableIsLocker := s.Durable.(Locker) - cache, cacheIsLocker := s.Cache.(Locker) - - if durableIsLocker { - if err := durable.Lock(reason); err != nil { - return err - } - } - - // We try to lock the Cache too, which is usually a local file. This also - // protects against multiple local processes if the remote state doesn't - // support locking. - if cacheIsLocker { - if err := cache.Lock(reason); err != nil { - // try to unlock Durable if this failed - if unlockErr := durable.Unlock(); unlockErr != nil { - err = multierror.Append(err, unlockErr) - } - return err - } - } - - return nil -} - -// Unlock unlocks both the Durable and Cache states. -func (s *CacheState) Unlock() error { - durable, durableIsLocker := s.Durable.(Locker) - cache, cacheIsLocker := s.Cache.(Locker) - - var err error - if durableIsLocker { - if unlockErr := durable.Unlock(); unlockErr != nil { - err = multierror.Append(err, unlockErr) - } - } - - if cacheIsLocker { - if unlockErr := cache.Unlock(); unlockErr != nil { - err = multierror.Append(err, unlockErr) - } - } - - return err -} - -// StateReader impl. -func (s *CacheState) State() *terraform.State { - return s.state.DeepCopy() -} - -// WriteState will write and persist the state to the cache. -// -// StateWriter impl. -func (s *CacheState) WriteState(state *terraform.State) error { - if err := s.Cache.WriteState(state); err != nil { - return err - } - - s.state = state - return s.Cache.PersistState() -} - -// RefreshState will refresh both the cache and the durable states. It -// can return a myriad of errors (defined at the top of this file) depending -// on potential conflicts that can occur while doing this. -// -// If the durable state is newer than the local cache, then the local cache -// will be replaced with the durable. -// -// StateRefresher impl. -func (s *CacheState) RefreshState() error { - // Refresh the durable state - if err := s.Durable.RefreshState(); err != nil { - return err - } - - // Refresh the cached state - if err := s.Cache.RefreshState(); err != nil { - return err - } - - // Handle the matrix of cases that can happen when comparing these - // two states. - cached := s.Cache.State() - durable := s.Durable.State() - switch { - case cached == nil && durable == nil: - // Initialized - s.refreshResult = CacheRefreshInit - case cached != nil && durable == nil: - // Cache is newer than remote. Not a big deal, user can just - // persist to get correct state. - s.refreshResult = CacheRefreshLocalNewer - case !cached.HasResources() && durable != nil: - // Cache should be updated since the remote is set but cache isn't - // - // If local is empty then we'll treat it as missing so that - // it can be overwritten by an already-existing remote. This - // allows the user to activate remote state for the first time - // against an already-existing remote state. - s.refreshResult = CacheRefreshUpdateLocal - case durable.Serial < cached.Serial: - // Cache is newer than remote. Not a big deal, user can just - // persist to get correct state. - s.refreshResult = CacheRefreshLocalNewer - case durable.Serial > cached.Serial: - // Cache should be updated since the remote is newer - s.refreshResult = CacheRefreshUpdateLocal - case durable.Serial == cached.Serial: - // They're supposedly equal, verify. - if cached.Equal(durable) { - // Hashes are the same, everything is great - s.refreshResult = CacheRefreshNoop - break - } - - // This is very bad. This means we have two state files that - // have the same serial but have a different hash. We can't - // reconcile this. The most likely cause is parallel apply - // operations. - s.refreshResult = CacheRefreshConflict - - // Return early so we don't updtae the state - return nil - default: - panic("unhandled cache refresh state") - } - - if s.refreshResult == CacheRefreshUpdateLocal { - if err := s.Cache.WriteState(durable); err != nil { - s.refreshResult = CacheRefreshNoop - return err - } - if err := s.Cache.PersistState(); err != nil { - s.refreshResult = CacheRefreshNoop - return err - } - - cached = durable - } - - s.state = cached - - return nil -} - -// RefreshResult returns the result of the last refresh. -func (s *CacheState) RefreshResult() CacheRefreshResult { - return s.refreshResult -} - -// PersistState takes the local cache, assuming it is newer than the remote -// state, and persists it to the durable storage. If you want to challenge the -// assumption that the local state is the latest, call a RefreshState prior -// to this. -// -// StatePersister impl. -func (s *CacheState) PersistState() error { - if err := s.Durable.WriteState(s.state); err != nil { - return err - } - - return s.Durable.PersistState() -} - -// CacheStateCache is the meta-interface that must be implemented for -// the cache for the CacheState. -type CacheStateCache interface { - StateReader - StateWriter - StatePersister - StateRefresher -} - -// CacheStateDurable is the meta-interface that must be implemented for -// the durable storage for CacheState. -type CacheStateDurable interface { - StateReader - StateWriter - StatePersister - StateRefresher -} - -// CacheRefreshResult is used to explain the result of the previous -// RefreshState for a CacheState. -type CacheRefreshResult int - -const ( - // CacheRefreshNoop indicates nothing has happened, - // but that does not indicate an error. Everything is - // just up to date. (Push/Pull) - CacheRefreshNoop CacheRefreshResult = iota - - // CacheRefreshInit indicates that there is no local or - // remote state, and that the state was initialized - CacheRefreshInit - - // CacheRefreshUpdateLocal indicates the local state - // was updated. (Pull) - CacheRefreshUpdateLocal - - // CacheRefreshUpdateRemote indicates the remote state - // was updated. (Push) - CacheRefreshUpdateRemote - - // CacheRefreshLocalNewer means the pull was a no-op - // because the local state is newer than that of the - // server. This means a Push should take place. (Pull) - CacheRefreshLocalNewer - - // CacheRefreshRemoteNewer means the push was a no-op - // because the remote state is newer than that of the - // local state. This means a Pull should take place. - // (Push) - CacheRefreshRemoteNewer - - // CacheRefreshConflict means that the push or pull - // was a no-op because there is a conflict. This means - // there are multiple state definitions at the same - // serial number with different contents. This requires - // an operator to intervene and resolve the conflict. - // Shame on the user for doing concurrent apply. - // (Push/Pull) - CacheRefreshConflict -) - -func (sc CacheRefreshResult) String() string { - switch sc { - case CacheRefreshNoop: - return "Local and remote state in sync" - case CacheRefreshInit: - return "Local state initialized" - case CacheRefreshUpdateLocal: - return "Local state updated" - case CacheRefreshUpdateRemote: - return "Remote state updated" - case CacheRefreshLocalNewer: - return "Local state is newer than remote state, push required" - case CacheRefreshRemoteNewer: - return "Remote state is newer than local state, pull required" - case CacheRefreshConflict: - return "Local and remote state conflict, manual resolution required" - default: - return fmt.Sprintf("Unknown state change type: %d", sc) - } -} - -// SuccessfulPull is used to clasify the CacheRefreshResult for -// a refresh operation. This is different by operation, but can be used -// to determine a proper exit code. -func (sc CacheRefreshResult) SuccessfulPull() bool { - switch sc { - case CacheRefreshNoop: - return true - case CacheRefreshInit: - return true - case CacheRefreshUpdateLocal: - return true - case CacheRefreshLocalNewer: - return false - case CacheRefreshConflict: - return false - default: - return false - } -} diff --git a/state/cache_test.go b/state/cache_test.go deleted file mode 100644 index 7252637e0..000000000 --- a/state/cache_test.go +++ /dev/null @@ -1,118 +0,0 @@ -package state - -import ( - "fmt" - "os" - "reflect" - "testing" - - "github.com/hashicorp/terraform/terraform" -) - -func TestCacheState(t *testing.T) { - cache := testLocalState(t) - durable := testLocalState(t) - defer os.Remove(cache.Path) - defer os.Remove(durable.Path) - - TestState(t, &CacheState{ - Cache: cache, - Durable: durable, - }) -} - -func TestCacheState_persistDurable(t *testing.T) { - cache := testLocalState(t) - durable := testLocalState(t) - defer os.Remove(cache.Path) - defer os.Remove(durable.Path) - - cs := &CacheState{ - Cache: cache, - Durable: durable, - } - - state := cache.State() - state.Modules = nil - if err := cs.WriteState(state); err != nil { - t.Fatalf("err: %s", err) - } - - if reflect.DeepEqual(cache.State(), durable.State()) { - t.Fatal("cache and durable should not be the same") - } - - if err := cs.PersistState(); err != nil { - t.Fatalf("err: %s", err) - } - - if !reflect.DeepEqual(cache.State(), durable.State()) { - t.Fatalf( - "cache and durable should be the same\n\n%#v\n\n%#v", - cache.State(), durable.State()) - } -} - -func TestCacheState_RefreshState(t *testing.T) { - for i, test := range []struct { - cacheModules []*terraform.ModuleState - expected CacheRefreshResult - }{ - { - cacheModules: nil, - expected: CacheRefreshUpdateLocal, - }, - { - cacheModules: []*terraform.ModuleState{}, - expected: CacheRefreshUpdateLocal, - }, - { - cacheModules: []*terraform.ModuleState{ - &terraform.ModuleState{ - Path: terraform.RootModulePath, - Resources: map[string]*terraform.ResourceState{ - "foo.foo": &terraform.ResourceState{ - Primary: &terraform.InstanceState{ - ID: "ID", - }, - }, - }, - }, - }, - expected: CacheRefreshLocalNewer, - }, - } { - t.Run(fmt.Sprintf("%d", i), func(t *testing.T) { - cache := testLocalState(t) - durable := testLocalState(t) - defer os.Remove(cache.Path) - defer os.Remove(durable.Path) - - cs := &CacheState{ - Cache: cache, - Durable: durable, - } - - state := cache.State() - state.Modules = test.cacheModules - if err := cs.WriteState(state); err != nil { - t.Fatalf("err: %s", err) - } - - if err := cs.RefreshState(); err != nil { - t.Fatalf("err: %s", err) - } - - if cs.RefreshResult() != test.expected { - t.Fatalf("bad %d: %v", i, cs.RefreshResult()) - } - }) - } -} - -func TestCacheState_impl(t *testing.T) { - var _ StateReader = new(CacheState) - var _ StateWriter = new(CacheState) - var _ StatePersister = new(CacheState) - var _ StateRefresher = new(CacheState) -} diff --git a/state/local.go b/state/local.go index 0aae22bb3..01708af6f 100644 --- a/state/local.go +++ b/state/local.go @@ -34,27 +34,6 @@ type LocalState struct { written bool } -// LockInfo stores metadata for locks taken. -type LockInfo struct { - Path string // Path to the state file - Created time.Time // The time the lock was taken - Info string // Extra info passed to State.Lock -} - -// Err returns the lock info formatted in an error -func (l *LockInfo) Err() error { - return fmt.Errorf("state locked. path:%q, created:%s, info:%q", - l.Path, l.Created, l.Info) -} - -func (l *LockInfo) String() string { - js, err := json.Marshal(l) - if err != nil { - panic(err) - } - return string(js) -} - // SetState will force a specific state in-memory for this local state. func (s *LocalState) SetState(state *terraform.State) { s.state = state diff --git a/state/state.go b/state/state.go index 532e14872..6f394f46d 100644 --- a/state/state.go +++ b/state/state.go @@ -1,6 +1,11 @@ package state import ( + "encoding/json" + "fmt" + "strings" + "time" + "github.com/hashicorp/terraform/terraform" ) @@ -42,9 +47,56 @@ type StatePersister interface { } // Locker is implemented to lock state during command execution. -// The optional info parameter can be recorded with the lock, but the -// implementation should not depend in its value. +// The info parameter can be recorded with the lock, but the +// implementation should not depend in its value. The string returned by Lock +// is an ID corresponding to the lock acquired, and must be passed to Unlock to +// ensure that the correct lock is being released. +// +// Lock and Unlock may return an error value of type LockError which in turn +// can contain the LockInfo of a conflicting lock. type Locker interface { - Lock(info string) error - Unlock() error + Lock(info *LockInfo) (string, error) + Unlock(id string) error +} + +// LockInfo stores metadata for locks taken. +type LockInfo struct { + ID string // unique ID + Path string // Path to the state file + Created time.Time // The time the lock was taken + Version string // Terraform version + Operation string // Terraform operation + Who string // user@hostname when available + Info string // Extra info field +} + +// Err returns the lock info formatted in an error +func (l *LockInfo) Err() error { + return fmt.Errorf("state locked. path:%q, created:%s, info:%q", + l.Path, l.Created, l.Info) +} + +func (l *LockInfo) String() string { + js, err := json.Marshal(l) + if err != nil { + panic(err) + } + return string(js) +} + +type LockError struct { + Info *LockInfo + Err error +} + +func (e *LockError) Error() string { + var out []string + if e.Err != nil { + out = append(out, e.Err.Error()) + } + + if e.Info != nil { + out = append(out, e.Info.Err().Error()) + } + return strings.Join(out, "\n") }