aherman
/
terraform
mirror of https://github.com/hashicorp/terraform
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

cloud: fix ShouldPersistIntermediateState should be false when header absent

Browse Source
pull/33820/head
Brandon Croft 2 years ago
parent cd99fac466
commit f5463ce77c
No known key found for this signature in database
GPG Key ID: B01E32423322EB9D
2 changed files with 65 additions and 105 deletions
Show Stats Download Patch File Download Diff File

11
internal/cloud/state.go
Unescape View File

@ -34,6 +34,11 @@ import (
"github.com/hashicorp/terraform/internal/terraform"
)
const (
// HeaderSnapshotInterval is the header key that controls the snapshot interval
HeaderSnapshotInterval = "x-terraform-snapshot-interval"
)
// State implements the State interfaces in the state package to handle
// reading and writing the remote state to TFC. This State on its own does no
// local caching so every persist will go to the remote storage and local
@ -248,7 +253,9 @@ func (s *State) ShouldPersistIntermediateState(info *local.IntermediateStatePers
return true
}
if !s.enableIntermediateSnapshots && info.RequestedPersistInterval == time.Duration(0) {
// This value is controlled by a x-terraform-snapshot-interval header intercepted during
// state-versions API responses
if !s.enableIntermediateSnapshots {
return false
}
@ -577,7 +584,7 @@ func clamp(val, min, max int64) int64 {
}
func (s *State) readSnapshotIntervalHeader(status int, header http.Header) {
intervalStr := header.Get("x-terraform-snapshot-interval")
intervalStr := header.Get(HeaderSnapshotInterval)
if intervalSecs, err := strconv.ParseInt(intervalStr, 10, 64); err == nil {
// More than an hour is an unreasonable delay, so we'll just

159
internal/cloud/state_test.go
Unescape View File

@ -360,113 +360,66 @@ func TestState_PersistState(t *testing.T) {
func TestState_ShouldPersistIntermediateState(t *testing.T) {
cloudState := testCloudState(t)
// We'll specify a normal interval and a server-supplied interval that
// have enough time between them that we can be confident that the
// fake timestamps we'll pass into ShouldPersistIntermediateState are
// either too soon for normal, long enough for normal but not for server,
// or too long for server.
shortServerInterval := 5 * time.Second
normalInterval := 60 * time.Second
longServerInterval := 120 * time.Second
beforeNormalInterval := 20 * time.Second
betweenInterval := 90 * time.Second
afterLongServerInterval := 200 * time.Second
// Before making any requests the state manager should just respect the
// normal interval, because it hasn't yet heard a request from the server.
{
should := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
RequestedPersistInterval: normalInterval,
LastPersist: time.Now().Add(-beforeNormalInterval),
})
if should {
t.Errorf("indicated that should persist before normal interval")
}
}
{
should := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
RequestedPersistInterval: normalInterval,
LastPersist: time.Now().Add(-betweenInterval),
})
if !should {
t.Errorf("indicated that should not persist after normal interval")
}
}
// After making a request to the "Create a State Version" operation, the
// server might return a header that causes us to set this field:
cloudState.stateSnapshotInterval = shortServerInterval
// The short server interval is shorter than the normal interval, so the
// normal interval takes priority here.
{
should := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
RequestedPersistInterval: normalInterval,
LastPersist: time.Now().Add(-beforeNormalInterval),
})
if should {
t.Errorf("indicated that should persist before normal interval")
}
}
{
should := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
RequestedPersistInterval: normalInterval,
LastPersist: time.Now().Add(-betweenInterval),
})
if !should {
t.Errorf("indicated that should not persist after normal interval")
}
testCases := []struct {
Enabled bool
LastPersist time.Time
Interval time.Duration
Expected bool
Force bool
Description string
}{
{
Interval: 20 * time.Second,
Enabled: true,
Expected: true,
Description: "Not persisted yet",
},
{
Interval: 20 * time.Second,
Enabled: false,
Expected: false,
Description: "Intermediate snapshots not enabled",
},
{
Interval: 20 * time.Second,
Enabled: false,
Force: true,
Expected: true,
Description: "Force persist",
},
{
Interval: 20 * time.Second,
LastPersist: time.Now().Add(-15 * time.Second),
Enabled: true,
Expected: false,
Description: "Last persisted 15s ago",
},
{
Interval: 20 * time.Second,
LastPersist: time.Now().Add(-25 * time.Second),
Enabled: true,
Expected: true,
Description: "Last persisted 25s ago",
},
{
Interval: 5 * time.Second,
LastPersist: time.Now().Add(-15 * time.Second),
Enabled: true,
Expected: true,
Description: "Last persisted 15s ago, but interval is 5s",
},
}
// The server might instead request a longer interval.
cloudState.stateSnapshotInterval = longServerInterval
{
should := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
RequestedPersistInterval: normalInterval,
LastPersist: time.Now().Add(-beforeNormalInterval),
})
if should {
t.Errorf("indicated that should persist before server interval")
}
}
{
should := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
RequestedPersistInterval: normalInterval,
LastPersist: time.Now().Add(-betweenInterval),
})
if should {
t.Errorf("indicated that should persist before server interval")
}
}
{
should := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
RequestedPersistInterval: normalInterval,
LastPersist: time.Now().Add(-afterLongServerInterval),
})
if !should {
t.Errorf("indicated that should not persist after server interval")
}
}
for _, testCase := range testCases {
cloudState.enableIntermediateSnapshots = testCase.Enabled
cloudState.stateSnapshotInterval = testCase.Interval
// The "force" mode should always win, regardless of how much time has passed.
{
should := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
RequestedPersistInterval: normalInterval,
LastPersist: time.Now().Add(-beforeNormalInterval),
ForcePersist: true,
})
if !should {
t.Errorf("ignored ForcePersist")
}
}
{
should := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
RequestedPersistInterval: normalInterval,
LastPersist: time.Now().Add(-betweenInterval),
ForcePersist: true,
actual := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
LastPersist: testCase.LastPersist,
ForcePersist: testCase.Force,
})
if !should {
t.Errorf("ignored ForcePersist")
if actual != testCase.Expected {
t.Errorf("%s: expected %v but got %v", testCase.Description, testCase.Expected, actual)
}
}
}

Write Preview
Loading…
Cancel
Save