diff --git a/internal/cloud/state.go b/internal/cloud/state.go index 32e6b0a7ee..dd464e56d0 100644 --- a/internal/cloud/state.go +++ b/internal/cloud/state.go @@ -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 diff --git a/internal/cloud/state_test.go b/internal/cloud/state_test.go index 8ffb64c832..a65a846098 100644 --- a/internal/cloud/state_test.go +++ b/internal/cloud/state_test.go @@ -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) } } }