Previously we would write to the backend for every call to PersistState,
even if nothing changed since the last write, but update the serial only
if the state had changed.
The Terraform Cloud & Enterprise state storage have a simple safety check
that any future write with an already-used lineage and serial must be
byte-for-byte identical. StatesMarshalEqual is intended to detect that,
but it only actually detects changes the state itself, and not changes
to the snapshot metadata.
Because we write the current Terraform version into the snapshot metadata
during serialization, we'd previously have an issue where if the first
state write after upgrading Terraform to a new version happened to change
nothing about the state content then we'd write a new snapshot that
differed only by Terraform version, and Terraform Cloud/Enterprise would
then reject it.
The snapshot header is discarded immediately after decoding, so we can't
use information from it when deciding whether to increment the serial.
The next best thing is to skip sending no-op snapshot updates to the state
client in the first place.
These writes are unnecessary anyway, and state storage owners have asked
us in the past to elide these to avoid generating noise in their version
logs, so we'll also finally meet those requests as a nice side-effect of
this change.
We didn't previously have tests for the full flow of retrieving and then
successively updating persisted state snapshots, so this includes a test
which covers that logic and includes an assertion that a no-op update does
not get written to the state client.
In our recent refactoring of the state manager interfaces we made serial
and lineage management the responsibility of the state managers
themselves, not exposing them at all to most callers, and allowing for
simple state managers that don't implement them at all.
However, we do have some specific cases where we need to preserve these
properly when available, such as migration between backends, and the
"terraform state push" and "terraform state pull" commands.
These new functions and their associated optional interface allow the
logic here to be captured in one place and access via some simple
calls. Separating this from the main interface leaves things simple for
the normal uses of state managers.
Since these functions are mostly just thin wrappers around other
functionality, they are not yet well-tested directly, but will be
indirectly tested through the tests of their callers. A subsequent commit
will add more unit tests here.
We were calling from PersistState into RefreshState, but RefreshState is
protected by the same lock as PersistState and so the call would deadlock.
Instead, we introduce a new entry point refreshState which can be used
when already holding the lock.
The state manager refactoring in an earlier commit was reflected in the
implementations of these backends, but not in their tests. This gets us
back to a state where the backend tests will compile, and gets _most_ of
them passing again, with a few exceptions that will be addressed in a
subsequent commit.
Due to how often the state and plan types are referenced throughout
Terraform, there isn't a great way to switch them out gradually. As a
consequence, this huge commit gets us from the old world to a _compilable_
new world, but still has a large number of known test failures due to
key functionality being stubbed out.
The stubs here are for anything that interacts with providers, since we
now need to do the follow-up work to similarly replace the old
terraform.ResourceProvider interface with its replacement in the new
"providers" package. That work, along with work to fix the remaining
failing tests, will follow in subsequent commits.
The aim here was to replace all references to terraform.State and its
downstream types with states.State, terraform.Plan with plans.Plan,
state.State with statemgr.State, and switch to the new implementations of
the state and plan file formats. However, due to the number of times those
types are used, this also ended up affecting numerous other parts of core
such as terraform.Hook, the backend.Backend interface, and most of the CLI
commands.
Just as with 5861dbf3fc49b19587a31816eb06f511ab861bb4 before, I apologize
in advance to the person who inevitably just found this huge commit while
spelunking through the commit history.
We can't check lineage in the remote state instance, because we may need
to overwrite a state with a new lineage. Whil it's tempting to add an
optional interface for this, like OverwriteState(), optional interfaces
are never _really_ optional, and will have to be implemented by any
wrapper types as well.
Another solution may be to add a State.Supersedes field to indicate that
we intend to replace an existing state, but that may not be worth the
extra check either.
Previously we relied on a constellation of coincidences for everything to
work out correctly with state serials. In particular, callers needed to
be very careful about mutating states (or not) because many different bits
of code shared pointers to the same objects.
Here we move to a model where all of the state managers always use
distinct instances of state, copied when WriteState is called. This means
that they are truly a snapshot of the state as it was at that call, even
if the caller goes on mutating the state that was passed.
We also adjust the handling of serials so that the state managers ignore
any serials in incoming states and instead just treat each Persist as
the next version after what was most recently Refreshed.
(An exception exists for when nothing has been refreshed, e.g. because
we are writing a state to a location for the first time. In that case
we _do_ trust the caller, since the given state is either a new state
or it's a copy of something we're migrating from elsewhere with its
state and lineage intact.)
The intent here is to allow the rest of Terraform to not worry about
serials and state identity, and instead just treat the state as a mutable
structure. We'll just snapshot it occasionally, when WriteState is called,
and deal with serials _only_ at persist time.
This is intended as a more robust version of #15423, which was a quick
hotfix to an issue that resulted from our previous slopping handling
of state serials but arguably makes the problem worse by depending on
an additional coincidental behavior of the local backend's apply
implementation.
In order to provide lockign for remote states, the Cache state,
remote.State need to expose Lock and Unlock methods. The actual locking
will be done by the remote.Client, which can implement the same
state.Locker methods.
When refreshing remote state, indicate when no state file was found with
an ErrRemoteStateNotFound error. This prevents us from inadvertantly
getting a nil state into a terraform.State where we assume there's
always a root module.
Martin Atkins