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

add kv-v2 write retry #1579

Merged
merged 5 commits into from
Sep 30, 2022
Merged

Conversation

czembower
Copy link
Contributor

@czembower czembower commented Aug 11, 2022

An error condition can occur if attempting to create a kv-v2 mount and write secrets to it in the same terraform run, resulting in the error:

Upgrading from non-versioned to versioned data. This backend will be unavailable for a brief period and will resume service shortly

It appears that this error is common on Vault running in a cluster, and less so when standalone.

This PR adds retry with backoff on writes to vault_kv_secret_v2 as well as vault_generic_secret resources. Followed precedent set for retry in namespaceDelete().

Closes #677

Release note for CHANGELOG:

Fixed failed writes to newly-created kv-v2 mount by adding retry/backoff

Output from acceptance testing:

TESTARGS="--run TestAccKVSecretV2 --run TestResourceGenericSecret" make testacc-ent
make testacc TF_ACC_ENTERPRISE=1
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test -v --run TestAccKVSecretV2 --run TestResourceGenericSecret -timeout 30m ./...

2022/08/11 11:24:20 [INFO] Using Vault token with the following policies: root
=== RUN   TestResourceGenericSecret
--- PASS: TestResourceGenericSecret (2.41s)
=== RUN   TestResourceGenericSecretNS
--- PASS: TestResourceGenericSecretNS (3.14s)
=== RUN   TestResourceGenericSecret_deleted
--- PASS: TestResourceGenericSecret_deleted (1.99s)
=== RUN   TestResourceGenericSecret_deleteAllVersions
--- PASS: TestResourceGenericSecret_deleteAllVersions (2.03s)
PASS
ok  	github.com/hashicorp/terraform-provider-vault/vault	9.999s

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" comments, they generate extra noise for pull request followers and do not help prioritize the request

Copy link
Contributor

@benashz benashz left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! I added some initial comments/suggestions, also It be good to include some tests for this case.

vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_generic_secret.go Outdated Show resolved Hide resolved
@benashz benashz added this to the 3.9.0 milestone Aug 11, 2022
Copy link

@bfiics bfiics left a comment

Choose a reason for hiding this comment

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

It makes the code better for sure. Good luck

vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
vault/resource_kv_secret_v2.go Outdated Show resolved Hide resolved
@github-actions github-actions bot added size/XS and removed size/S labels Sep 30, 2022
Copy link
Contributor

@benashz benashz left a comment

Choose a reason for hiding this comment

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

LGTM!

Thank you for your contribution to HashiCorp!

@benashz benashz merged commit 91c938d into hashicorp:main Sep 30, 2022
@benashz benashz added the bug label Oct 4, 2022
marcboudreau pushed a commit to marcboudreau/terraform-provider-vault that referenced this pull request Nov 6, 2022
@czembower czembower deleted the 677/kv-v2-upgrade-retry branch March 1, 2023 17:18
@czembower
Copy link
Contributor Author

@benashz it looks like you reverted this PR's changes to kv_secret_v2. Could you explain this? Several people have informed me that this is still broken for that resource, and looking at the final merge, that is clearly the reason. The retry/back-off is necessary here.

@eldengates
Copy link

any updates on this ?

@benashz
Copy link
Contributor

benashz commented May 30, 2023

Hi @eldengates, we should the fix out in the next release 3.16.0

@edupo
Copy link

edupo commented Feb 14, 2024

I still see this in v3.24.0 (signed by HashiCorp).

│ * Waiting for the primary to upgrade from non-versioned to versioned data. This backend will be unavailable for a brief period and will resume service when the primary is finished.

Any idea why? Workaround? Help is appreciated :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants