-
Notifications
You must be signed in to change notification settings - Fork 383
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
fix: retry connections to gce metadata server #284
Conversation
Codecov Report
@@ Coverage Diff @@
## master #284 +/- ##
==========================================
+ Coverage 91.67% 91.71% +0.03%
==========================================
Files 13 13
Lines 829 833 +4
Branches 178 179 +1
==========================================
+ Hits 760 764 +4
Misses 69 69
Continue to review full report at Codecov.
|
@@ -24,7 +24,8 @@ | |||
"gtoken": "^2.1.0", | |||
"jws": "^3.1.4", | |||
"lodash.isstring": "^4.0.1", | |||
"lru-cache": "^4.1.1" | |||
"lru-cache": "^4.1.1", | |||
"retry-axios": "^0.3.0" |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@@ -68,8 +74,11 @@ export class Compute extends OAuth2Client { | |||
try { | |||
// TODO: In 2.0, we should remove the ability to configure the tokenUrl, | |||
// and switch this over to use the gcp-metadata package instead. | |||
res = await this.transporter.request<CredentialRequest>( |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/auth/computeclient.ts
Outdated
res = await ax.request<CredentialRequest>({ | ||
url, | ||
headers: {'Metadata-Flavor': 'Google'}, | ||
raxConfig: {noResponseRetries: 8, retry: 8, instance: ax} |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/auth/computeclient.ts
Outdated
res = await ax.request<CredentialRequest>({ | ||
url, | ||
headers: {'Metadata-Flavor': 'Google'}, | ||
raxConfig: {noResponseRetries: 3, retry: 3, instance: ax} |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
src/auth/computeclient.ts
Outdated
{url, headers: {'Metadata-Flavor': 'Google'}}); | ||
res = await ax.request<CredentialRequest>({ | ||
url, | ||
headers: {'Metadata-Flavor': 'Google'}, |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
test/test.compute.ts
Outdated
nock(HOST_ADDRESS).get(tokenPath).reply(200, { | ||
access_token: 'abc123', | ||
expires_in: 10000 | ||
}) |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
test/test.compute.ts
Outdated
nock(HOST_ADDRESS).get(tokenPath).times(2).reply(500), | ||
nock(HOST_ADDRESS).get(tokenPath).reply(200, { | ||
access_token: 'abc123', | ||
expires_in: 10000 |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
@ofrobots @jonparrott Does this look good? |
{url, headers: {'Metadata-Flavor': 'Google'}}); | ||
res = await ax.request<CredentialRequest>({ | ||
url, | ||
headers: {[gcpMetadata.HEADER_NAME]: 'Google'}, |
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
This comment was marked as spam.
This comment was marked as spam.
Sorry, something went wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM once question addressed adequately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Conceptually looks fine to me.
Resolves #283.
cc @stephenplusplus @jonparrott