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

Use RetryContext to eliminate busy-waiting #2615

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

alyssaruth
Copy link

@alyssaruth alyssaruth commented Nov 4, 2024

Description

Fixes #2614

Update waiter.go to use retry.RetryContext, which brings a couple of benefits:

  • Avoid boilerplate loop/break logic
  • Better consistency with other code within this provider (e.g. resource_kubernetes_deployment_v1.go)
  • We get back-off retries for free, rather than busy-looping every second.

Acceptance tests

  • Have you added an acceptance test for the functionality being added?
  • Have you run the acceptance tests on this branch?

Behaviour isn't much different, but I added a couple of tests anyway as there were existing gaps (timeout cases for two of the wait types).

Output from acceptance testing:

I couldn't get the regex stuff to work on the command line, apologies - I ran the file through my IDE instead (IntelliJ):

$ /opt/go/1.21.13/bin/go tool test2json -t /home/alyssa/.cache/JetBrains/IntelliJIdea2024.1/tmp/GoLand/___wait_test_go.test -test.v -test.paniconexit0 -test.run ^\QTestKubernetesManifest_WaitFields_Pod\E|\QTestKubernetesManifest_WaitFields_PodInvalid\E|\QTestKubernetesManifest_WaitRollout_Deployment\E|\QTestKubernetesManifest_WaitRollout_FailingDeployment\E|\QTestKubernetesManifest_WaitCondition_Pod\E|\QTestKubernetesManifest_Wait_InvalidCondition\E|\QTestKubernetesManifest_WaitFields_Annotations\E$

2024/11/04 17:16:49 Testing against Kubernetes API version: v1.29.10
=== RUN   TestKubernetesManifest_WaitFields_Pod
2024/11/04 17:16:50 [DEBUG] Waiting for state to become: [success]
2024/11/04 17:16:50 [TRACE] Waiting 500ms before next try
2024/11/04 17:16:50 [TRACE] Waiting 1s before next try
2024/11/04 17:16:51 [TRACE] Waiting 2s before next try
2024/11/04 17:16:53 [TRACE] Waiting 4s before next try
2024/11/04 17:16:57 [TRACE] Waiting 8s before next try
--- PASS: TestKubernetesManifest_WaitFields_Pod (17.49s)
=== RUN   TestKubernetesManifest_WaitFields_PodInvalid
2024/11/04 17:17:07 [DEBUG] Waiting for state to become: [success]
2024/11/04 17:17:07 [TRACE] Waiting 500ms before next try
2024/11/04 17:17:08 [TRACE] Waiting 1s before next try
2024/11/04 17:17:09 [TRACE] Waiting 2s before next try
2024/11/04 17:17:11 [TRACE] Waiting 4s before next try
2024/11/04 17:17:12 [WARN] WaitForState timeout after 4.920920079s
2024/11/04 17:17:12 [WARN] WaitForState starting 30s refresh grace period
2024/11/04 17:17:12 [ERROR] Context cancelation detected, abandoning grace period
--- PASS: TestKubernetesManifest_WaitFields_PodInvalid (6.87s)
=== RUN   TestKubernetesManifest_WaitRollout_Deployment
2024/11/04 17:17:14 [DEBUG] Waiting for state to become: [success]
2024/11/04 17:17:14 [TRACE] Waiting 500ms before next try
2024/11/04 17:17:15 [TRACE] Waiting 1s before next try
2024/11/04 17:17:16 [TRACE] Waiting 2s before next try
2024/11/04 17:17:18 [TRACE] Waiting 4s before next try
2024/11/04 17:17:22 [TRACE] Waiting 8s before next try
2024/11/04 17:17:30 [TRACE] Waiting 10s before next try
--- PASS: TestKubernetesManifest_WaitRollout_Deployment (26.40s)
=== RUN   TestKubernetesManifest_WaitRollout_FailingDeployment
2024/11/04 17:17:41 [DEBUG] Waiting for state to become: [success]
2024/11/04 17:17:41 [TRACE] Waiting 500ms before next try
2024/11/04 17:17:41 [TRACE] Waiting 1s before next try
2024/11/04 17:17:42 [TRACE] Waiting 2s before next try
2024/11/04 17:17:44 [TRACE] Waiting 4s before next try
2024/11/04 17:17:46 [WARN] WaitForState timeout after 4.936336905s
2024/11/04 17:17:46 [WARN] WaitForState starting 30s refresh grace period
2024/11/04 17:17:46 [ERROR] Context cancelation detected, abandoning grace period
--- PASS: TestKubernetesManifest_WaitRollout_FailingDeployment (5.80s)
=== RUN   TestKubernetesManifest_WaitCondition_Pod
2024/11/04 17:17:46 [DEBUG] Waiting for state to become: [success]
2024/11/04 17:17:46 [TRACE] Waiting 500ms before next try
2024/11/04 17:17:47 [TRACE] Waiting 1s before next try
2024/11/04 17:17:48 [TRACE] Waiting 2s before next try
2024/11/04 17:17:50 [TRACE] Waiting 4s before next try
2024/11/04 17:17:54 [TRACE] Waiting 8s before next try
--- PASS: TestKubernetesManifest_WaitCondition_Pod (17.52s)
=== RUN   TestKubernetesManifest_Wait_InvalidCondition
2024/11/04 17:18:04 [DEBUG] Waiting for state to become: [success]
2024/11/04 17:18:04 [TRACE] Waiting 500ms before next try
2024/11/04 17:18:04 [TRACE] Waiting 1s before next try
2024/11/04 17:18:05 [TRACE] Waiting 2s before next try
--- PASS: TestKubernetesManifest_Wait_InvalidCondition (9.60s)
=== RUN   TestKubernetesManifest_WaitFields_Annotations
2024/11/04 17:18:13 [DEBUG] Waiting for state to become: [success]
--- PASS: TestKubernetesManifest_WaitFields_Annotations (0.70s)
PASS

Release Note

Release note for CHANGELOG:

`wait` blocks for the `kubernetes_manifest` resource-type will use a back-off pattern, rather than retrying every second

Community Note

  • Please vote on this issue by adding a 👍 reaction to the original issue to help the community and maintainers prioritize this request
  • If you are interested in working on this issue or have submitted a pull request, please leave a comment

also add a couple of tests to cover the other timeout cases. Fixes hashicorp#2614
@alyssaruth alyssaruth requested a review from a team as a code owner November 4, 2024 17:25
Copy link

hashicorp-cla-app bot commented Nov 4, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

CLA assistant check

Thank you for your submission! We require that all contributors sign our Contributor License Agreement ("CLA") before we can accept the contribution. Read and sign the agreement

Learn more about why HashiCorp requires a CLA and what the CLA includes

Have you signed the CLA already but the status is still pending? Recheck it.

@github-actions github-actions bot added the size/L label Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent busy-waiting when using kubernetes_manifest.wait block
1 participant