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 support for retrying identity entity reads #1263

Merged
merged 8 commits into from
Dec 20, 2021

Conversation

benashz
Copy link
Contributor

@benashz benashz commented Dec 16, 2021

This fix contains a mitigation that retries stale reads from a
performance standby on new resource creation.

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccXXX'


$ make dev testacc TESTARGS='-v -test.run TestAccIdentit'                                                    
==> Checking that code complies with gofmt requirements...
go build -o terraform-provider-vault
mv terraform-provider-vault ~/.terraform.d/plugins/
TF_ACC=1 go test $(go list ./...) -v -v -test.run TestAccIdentit -timeout 20m

[...]

ok      github.com/hashicorp/terraform-provider-vault/util      1.200s [no tests to run]
=== RUN   TestAccIdentityEntityAlias
--- PASS: TestAccIdentityEntityAlias (3.48s)
=== RUN   TestAccIdentityEntityAlias_Update
--- PASS: TestAccIdentityEntityAlias_Update (3.38s)
=== RUN   TestAccIdentityEntityAlias_Metadata
--- PASS: TestAccIdentityEntityAlias_Metadata (3.40s)
=== RUN   TestAccIdentityEntityPoliciesExclusive
--- PASS: TestAccIdentityEntityPoliciesExclusive (3.23s)
=== RUN   TestAccIdentityEntityPoliciesNonExclusive
--- PASS: TestAccIdentityEntityPoliciesNonExclusive (4.06s)
=== RUN   TestAccIdentityEntity
--- PASS: TestAccIdentityEntity (2.16s)
=== RUN   TestAccIdentityEntityUpdate
--- PASS: TestAccIdentityEntityUpdate (3.21s)
=== RUN   TestAccIdentityEntityUpdateRemoveValues
--- PASS: TestAccIdentityEntityUpdateRemoveValues (3.16s)
=== RUN   TestAccIdentityEntityUpdateRemovePolicies
--- PASS: TestAccIdentityEntityUpdateRemovePolicies (7.63s)
=== RUN   TestAccIdentityGroupAlias
--- PASS: TestAccIdentityGroupAlias (1.92s)
=== RUN   TestAccIdentityGroupAliasUpdate
--- PASS: TestAccIdentityGroupAliasUpdate (3.36s)
=== RUN   TestAccIdentityGroupMemberEntityIdsExclusiveEmpty
--- PASS: TestAccIdentityGroupMemberEntityIdsExclusiveEmpty (4.53s)
=== RUN   TestAccIdentityGroupMemberEntityIdsExclusive
    resource_identity_group_member_entity_ids_test.go:52: &{{{{0 0} 0 0 0 0} [] {0xc001239040} false false false false map[] map[] []  [] false 0xc0002ada70 false 0 0 testing.tRunner 0xc000682ea0 1 [17888078 17877730 17887743 17882973 28984395 17012103 17213889] TestAccIdentityGroupMemberEntityIdsExclusive {13864046867648191152 43551867250 0x2877960} 0 0xc000f91ec0 0xc000f3c380 [] {0 0}  <nil> 0} false false 0xc00014cb90}
--- SKIP: TestAccIdentityGroupMemberEntityIdsExclusive (0.00s)
=== RUN   TestAccIdentityGroupMemberEntityIdsNonExclusiveEmpty
--- PASS: TestAccIdentityGroupMemberEntityIdsNonExclusiveEmpty (19.15s)
=== RUN   TestAccIdentityGroupMemberEntityIdsNonExclusive
    resource_identity_group_member_entity_ids_test.go:122: &{{{{0 0} 0 0 0 0} [] {0xc001831040} false false false false map[] map[] []  [] false 0xc0002ada70 false 0 0 testing.tRunner 0xc000682ea0 1 [17888078 17877730 17887743 17882973 28984395 17012103 17213889] TestAccIdentityGroupMemberEntityIdsNonExclusive {13864046888195704808 62697963013 0x2877960} 0 0xc0004e9b00 0xc00057c070 [] {0 0}  <nil> 0} false false 0xc00014cb90}
--- SKIP: TestAccIdentityGroupMemberEntityIdsNonExclusive (0.00s)
=== RUN   TestAccIdentityGroupPoliciesExclusive
--- PASS: TestAccIdentityGroupPoliciesExclusive (3.50s)
=== RUN   TestAccIdentityGroupPoliciesNonExclusive
--- PASS: TestAccIdentityGroupPoliciesNonExclusive (3.42s)
=== RUN   TestAccIdentityGroup
--- PASS: TestAccIdentityGroup (1.85s)
=== RUN   TestAccIdentityGroupUpdate
--- PASS: TestAccIdentityGroupUpdate (7.83s)
=== RUN   TestAccIdentityGroupExternal
--- PASS: TestAccIdentityGroupExternal (1.87s)
=== RUN   TestAccIdentityOidcKeyAllowedClientId
--- PASS: TestAccIdentityOidcKeyAllowedClientId (5.45s)
=== RUN   TestAccIdentityOidcKey
--- PASS: TestAccIdentityOidcKey (3.24s)
=== RUN   TestAccIdentityOidcKeyUpdate
--- PASS: TestAccIdentityOidcKeyUpdate (10.92s)
=== RUN   TestAccIdentityOidcRole
--- PASS: TestAccIdentityOidcRole (2.85s)
=== RUN   TestAccIdentityOidcRoleWithClientId
--- PASS: TestAccIdentityOidcRoleWithClientId (11.51s)
=== RUN   TestAccIdentityOidcRoleUpdate
--- PASS: TestAccIdentityOidcRoleUpdate (5.00s)
=== RUN   TestAccIdentityOidc
--- PASS: TestAccIdentityOidc (3.14s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     123.995s

$ make dev testacc TESTARGS='-v -test.run TestReadEntity'
==> Checking that code complies with gofmt requirements...
go build -o terraform-provider-vault
mv terraform-provider-vault ~/.terraform.d/plugins/
TF_ACC=1 go test $(go list ./...) -v -v -test.run TestReadEntity -timeout 20m

=== RUN   TestReadEntity
=== RUN   TestReadEntity/retry-none
2021/12/16 12:57:27 [DEBUG] Reading Entity from "retry-none"
=== RUN   TestReadEntity/retry-ok-404
2021/12/16 12:57:27 [DEBUG] Reading Entity from "retry-ok-404"
=== RUN   TestReadEntity/retry-ok-412
2021/12/16 12:57:27 [DEBUG] Reading Entity from "retry-ok-412"
=== RUN   TestReadEntity/retry-exhausted-default-max-404
2021/12/16 12:57:27 [DEBUG] Reading Entity from "/identity/entity/id/retry-exhausted-default-max-404"
=== RUN   TestReadEntity/retry-exhausted-default-max-412
2021/12/16 12:57:27 [DEBUG] Reading Entity from "/identity/entity/id/retry-exhausted-default-max-412"
=== RUN   TestReadEntity/retry-exhausted-custom-max-404
2021/12/16 12:57:27 [DEBUG] Reading Entity from "/identity/entity/id/retry-exhausted-custom-max-404"
=== RUN   TestReadEntity/retry-exhausted-custom-max-412
2021/12/16 12:57:27 [DEBUG] Reading Entity from "/identity/entity/id/retry-exhausted-custom-max-412"
--- PASS: TestReadEntity (0.98s)
    --- PASS: TestReadEntity/retry-none (0.00s)
    --- PASS: TestReadEntity/retry-ok-404 (0.04s)
    --- PASS: TestReadEntity/retry-ok-412 (0.03s)
    --- PASS: TestReadEntity/retry-exhausted-default-max-404 (0.07s)
    --- PASS: TestReadEntity/retry-exhausted-default-max-412 (0.07s)
    --- PASS: TestReadEntity/retry-exhausted-custom-max-404 (0.38s)
    --- PASS: TestReadEntity/retry-exhausted-custom-max-412 (0.38s)
PASS
ok      github.com/hashicorp/terraform-provider-vault/vault     2.579s


...

// DefaultMaxHTTPRetries is used for configuring the api.Client's MaxRetries.
DefaultMaxHTTPRetries = 2

// MaxGetRetriesMultiplier is used
Copy link
Contributor

Choose a reason for hiding this comment

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

Why a multipler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we think that the default or whatever is provided by the user is good enough, then I can drop it. The docs claim the retry is used for 5xx type errors.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dropped the multiplier in d70cc26

@benashz benashz added this to the 3.1.0 milestone Dec 17, 2021
This fix contains a mitigation that retries stale reads from a
performance standby.
- compute connection timeout from configured backoff function.
- when setting ReadYourRights() on client instance that already has this
  feature set, the client's replicationStore is re-initialized to nil.
@benashz benashz merged commit 26c9eba into main Dec 20, 2021
@benashz benashz deleted the VAULT-4527/retry-ccc-reads branch December 21, 2021 19:29
@benashz
Copy link
Contributor Author

benashz commented Dec 22, 2021

Introduced #1269

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

Successfully merging this pull request may close these issues.

3 participants