jprdonnelly/opentf
1
0
Fork 0
mirror of https://github.com/opentffoundation/opentf.git synced 2025-12-25 10:00:44 -05:00
Code Issues Packages Projects Releases Wiki Activity

Consider missing x-terraform-snapshot-interval for State Snapshot (#696)

Browse Source
This commit is contained in:
Elbaz
2023-10-12 14:02:45 +03:00
committed by GitHub
parent 0b66f6296a
commit 00165570e2
4 changed files with 76 additions and 107 deletions
Download Patch File Download Diff File Expand all files Collapse all files

21
internal/cloud/state.go
View File

@@ -34,6 +34,11 @@ import (
"github.com/opentofu/opentofu/internal/tofu"
)
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,14 @@ func clamp(val, min, max int64) int64 {
}
func (s *State) readSnapshotIntervalHeader(status int, header http.Header) {
intervalStr := header.Get("x-terraform-snapshot-interval")
// Only proceed if this came from tfe.v2 API
contentType := header.Get("Content-Type")
if !strings.Contains(contentType, tfe.ContentTypeJSONAPI) {
log.Printf("[TRACE] Skipping intermediate state interval because Content-Type was %q", contentType)
return
}
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
@@ -588,11 +602,12 @@ func (s *State) readSnapshotIntervalHeader(status int, header http.Header) {
// If the header field is either absent or invalid then we'll
// just choose zero, which effectively means that we'll just use
// the caller's requested interval instead. If the caller has no
// requested interval or it is zero, then we will disable snapshots.
// requested interval, or it is zero, then we will disable snapshots.
s.stateSnapshotInterval = time.Duration(0)
}
// We will only enable snapshots for intervals greater than zero
log.Printf("[TRACE] Intermediate state interval is set by header to %v", s.stateSnapshotInterval)
s.enableIntermediateSnapshots = s.stateSnapshotInterval > 0
}

159
internal/cloud/state_test.go
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")
}
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",
},
}
// 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
for _, testCase := range testCases {
cloudState.enableIntermediateSnapshots = testCase.Enabled
cloudState.stateSnapshotInterval = testCase.Interval
// 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),
actual := cloudState.ShouldPersistIntermediateState(&local.IntermediateStatePersistInfo{
LastPersist: testCase.LastPersist,
ForcePersist: testCase.Force,
})
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")
}
}
// 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")
}
}
// 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,
})
if !should {
t.Errorf("ignored ForcePersist")
if actual != testCase.Expected {
t.Errorf("%s: expected %v but got %v", testCase.Description, testCase.Expected, actual)
}
}
}

2
internal/cloud/testing.go
View File

@@ -437,7 +437,7 @@ func testServerWithSnapshotsEnabled(t *testing.T, enabled bool) *httptest.Server
t.Fatal(err)
}
w.Header().Set("content-type", "application/json")
w.Header().Set("content-type", tfe.ContentTypeJSONAPI)
w.Header().Set("content-length", strconv.FormatInt(int64(len(fakeBodyRaw)), 10))
switch r.Method {