fix context timeout associated with imds apis #1732
Conversation
|
This really sounds like a bug in the SDK to me, we're not the first ones to run into it: aws/aws-sdk-go-v2#1247 IIUC the SDK is overriding the context we pass and as a result, making our retry options useless? |
| imds-mock --config-file $CONFIG_PATH & | ||
| [ "${IMDS_MOCK_ONLY_CONFIGURE:-}" = "true" ] || imds-mock --config-file $CONFIG_PATH & | ||
| export AWS_EC2_METADATA_SERVICE_ENDPOINT=http://localhost:1338 | ||
| $HOME/.local/bin/moto_server -p5000 & | ||
| [ "${AWS_MOCK_ONLY_CONFIGURE:-}" = "true" ] || $HOME/.local/bin/moto_server -p5000 & |
There was a problem hiding this comment.
maybe just split this into 2 helpers, mock::imds and mock::aws? I combined them so that our standard set of mocks didn't get unruly, but it's not that big of a deal
There was a problem hiding this comment.
i was considering it, and I would probably still use an ONLY_CONFIGURE env so that i can enable the service part way through the run if that's fine.
I could just export the variable myself but i figured its cleaner using the mock as the mechanism
There was a problem hiding this comment.
there's not much pressure since this is a pretty unique test so lets not worry about it for now :)
right, the underlying middleware for this client pretty much throws a context timeout onto your request, xref. |
ndbaker1
force-pushed
the
imds-fix
branch
2 times, most recently
from
87eb3d9
to
d91b41f
Compare
|
updated the sdk to the versions with the fix, then also just moved the userdata helper into the imds wrapper |
| DisableDefaultTimeout: true, | ||
| Retryer: retry.NewStandard(func(so *retry.StandardOptions) { | ||
| so.MaxAttempts = 15 | ||
| so.MaxBackoff = 1 * time.Second |
There was a problem hiding this comment.
this means 1 second between each attempt, right?
There was a problem hiding this comment.
yea, the underlying exponential retryer just behaves linearly for 1 second backoff
|
/ci |
|
@cartermckinnon roger that! I've dispatched a workflow. 👍 |
|
@cartermckinnon the workflow that you requested has completed. 🎉
|
Issue #, if available:
Description of changes:
the ec2imds apis fail fast in the
GetUserDatacall, because of the context driven error:notice that we only pass
context.TODO()into this call, but the underlying middleware for the aws sdk call to imds adds its own deadline if one is missing or trivially set as 0.https://github.com/aws/aws-sdk-go-v2/blob/main/feature/ec2/imds/request_middleware.go#L268-L280
in this PR we are setting a context timeout greater than the retry period.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.
Testing Done
add e2e tests to help with this validation.
See this guide for recommended testing for PRs. Some tests may not apply. Completing tests and providing additional validation steps are not required, but it is recommended and may reduce review time and time to merge.