Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cloud: when saving state, utilize new 'pending' state version #33336

Conversation

brandonc
Copy link
Contributor

@brandonc brandonc commented Jun 8, 2023

When using a cloud block and saving state, terraform cloud will no longer close the connection and fail to save with a gateway timeout response due to exceedingly large state or slow network operations. This utilizes a new platform feature to create a pending state version followed by uploading state content directly to the object store which has no gateway timeout. If the new feature is unavailable (during pre-release, or when using Terraform Enterprise) the legacy fallback is used.

Target Release

1.5.1

Important! ⚠️

Depends on the release of hashicorp/go-tfe#717

Draft CHANGELOG entry

ENHANCEMENTS

When using a cloud block and saving state, terraform cloud will no longer close the connection and fail to save with a gateway timeout response due to exceedingly large or slow network operations.

@brandonc brandonc force-pushed the TF-7056-uploading-state-directly-to-hosted-state-upload-url-when-available branch from afafd00 to 58c1cb3 Compare June 9, 2023 17:19
Copy link
Contributor

@sebasslash sebasslash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR has my approval 👍 Did not directly approve since you may address some of the items below.

@brandonc brandonc force-pushed the TF-7056-uploading-state-directly-to-hosted-state-upload-url-when-available branch from 58c1cb3 to ff9179a Compare June 13, 2023 23:34
@brandonc brandonc force-pushed the TF-7056-uploading-state-directly-to-hosted-state-upload-url-when-available branch from ff9179a to cd018f8 Compare June 13, 2023 23:43
@brandonc brandonc force-pushed the TF-7056-uploading-state-directly-to-hosted-state-upload-url-when-available branch from cd018f8 to b3890b1 Compare June 14, 2023 16:35
@nfagerlund
Copy link
Member

Gonna smoke test this and spy on atlas/archivist logs

Copy link
Member

@nfagerlund nfagerlund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, this checks out! I ran it while spying on the atlas and archivist logs, and the sequence seems right:

  • atlas: GET current-state-version: 404
  • atlas: POST state-versions 201
  • archivist: create-object (VERSION)
  • archivist: starting upload request
  • archivist: PUT callback
  • atlas: callback received
  • atlas: setting state version as current
  • atlas: stateparsing controller: parsed successfully

etc. etc.

Code also seems straightforward and good.

I'm assuming you'll want to delay this until the go-tfe version is shipped, but I'll pre-approve anyway since it's working fine; ping me for an auto- ✅ if you fixup the go.mod later.

_, err := s.tfeClient.StateVersions.Upload(ctx, s.workspace.ID, options)
if errors.Is(err, tfe.ErrStateVersionUploadNotSupported) {
// Create the new state with content included in the request (Terraform Enterprise v202306-1 and below)
log.Println("[WARN] Trying compatibility state upload")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 Is WARN justified for this, given that we expect compatibility mode to usually be fine? Or would INFO suffice?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

INFO or even TRACE would probably suffice.

@brandonc brandonc force-pushed the TF-7056-uploading-state-directly-to-hosted-state-upload-url-when-available branch from b3890b1 to 7544cc3 Compare June 16, 2023 17:29
@brandonc brandonc force-pushed the TF-7056-uploading-state-directly-to-hosted-state-upload-url-when-available branch from 7544cc3 to 00b0d78 Compare June 16, 2023 22:36
brandonc added 2 commits June 21, 2023 12:30
Create a pending state version followed by a separate state upload

When this version of the endpoint fails (It is not yet generally available, or when using with Terraform Enterprise) Fall back to the original call with state content included in the request.

This strategy will reduce the amount of save failures due to network latency and gateway timeouts.
@brandonc brandonc force-pushed the TF-7056-uploading-state-directly-to-hosted-state-upload-url-when-available branch from 00b0d78 to 19b17ad Compare June 21, 2023 18:30
nfagerlund added a commit that referenced this pull request Jun 21, 2023
… PR #33336

Updating go-tfe made the mocks no longer compile, due to the new Upload method on state versions.
@brandonc
Copy link
Contributor Author

(Smoke tested both backends with the platform feature enabled and disabled using go-tfe 1.28)

@brandonc brandonc merged commit 63124e0 into main Jun 22, 2023
@github-actions
Copy link
Contributor

Reminder for the merging maintainer: if this is a user-visible change, please update the changelog on the appropriate release branch.

@brandonc brandonc added the 1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged label Jun 22, 2023
Copy link
Contributor

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
1.5-backport If you add this label to a PR before merging, backport-assistant will open a new PR once merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants