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

registry: retryable discovery requests #24260

Merged
merged 4 commits into from Mar 5, 2020

Conversation

findkim
Copy link
Contributor

@findkim findkim commented Mar 3, 2020

Adds retry request wrapping to the remote registry client during discovery for modules and providers. The default retry is 1 additional attempt for 500-range responses (except 501), and is configurable with the environment variable TF_REGISTRY_DISCOVERY_RETRY.

This PR adds logging for the retried attempts and also improves the error message when the requests fail for module downloads.

Current error

Error: Invalid response from remote module registry

The remote registry at app.terraform.io failed to return a download URL for app.terraform.io/namespace/module/provider 0.1.0.

New error is now uniform between download and list versions requests

Error accessing remote module registry

Failed to retrieve a download URL for app.terraform.io/namespace/module/provider 0.1.0 from app.terraform.io: the request failed after 2 attempts, please try again later: 502 net/http: request canceled (Client.Timeout exceeded while awaiting headers).

The number of retries attempted (if more than 1) is included in the error message for both module and provider discovery errors along with the status code and original error msg.

Logging

2020/02/18 12:30:55 [DEBUG] fetching module versions from "http://127.0.0.1:51466/v1/modules/test-versions/name/provider/versions"
2020/02/18 12:30:55 [DEBUG] GET http://127.0.0.1:51466/v1/modules/test-versions/name/provider/versions
2020/02/18 12:30:55 [TRACE] HTTP client GET request to http://127.0.0.1:51466/v1/modules/test-versions/name/provider/versions
2020/02/18 12:30:55 [DEBUG] GET http://127.0.0.1:51466/v1/modules/test-versions/name/provider/versions (status: 502): retrying in 1s (1 left)
2020/02/18 12:30:56 [INFO] Previous request to the remote registry failed, attempting retry.
2020/02/18 12:30:56 [TRACE] HTTP client GET request to http://127.0.0.1:51466/v1/modules/test-versions/name/provider/versions

@ghost ghost added the sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK label Mar 3, 2020
@mildwonkey mildwonkey requested a review from a team March 3, 2020 17:57
@codecov
Copy link

codecov bot commented Mar 3, 2020

Codecov Report

❗ No coverage uploaded for pull request base (master@add1342). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master   #24260   +/-   ##
=========================================
  Coverage          ?   57.34%           
=========================================
  Files             ?      656           
  Lines             ?    59866           
  Branches          ?        0           
=========================================
  Hits              ?    34329           
  Misses            ?    22492           
  Partials          ?     3045           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update add1342...bb96863. Read the comment docs.

@findkim findkim changed the title Registry retry registry: retryable discovery requests Mar 3, 2020
@findkim findkim merged commit 8f5159a into hashicorp:master Mar 5, 2020
@ghost
Copy link

ghost commented Apr 5, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 5, 2020
@findkim findkim deleted the registry-retry branch August 16, 2021 18:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
sdkv1 [PRs only] Marks changes that may potentially need to be ported to the plugi nSDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants