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

store: fail faster if not running in EC2 #415

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

Conversation

kevinburkesegment
Copy link
Contributor

If you are running chamber outside of EC2 without a region configured, the binary will take 5 seconds to fail because the default ec2 metadata API timeout is 5 seconds.

Instead, take a few milliseconds to determine whether we are even running on EC2. We attempt to do this by checking the contents of different files on the disk, e.g. /sys/devices/virtual/dmi/id/board_asset_tag, and then if all of those fail, attempting to see if anything is listening on the metadata host.

This can help prevent the 5 second failure timeout.

If you are running `chamber` outside of EC2 without a region
configured, the binary will take 5 seconds to fail because the default
ec2 metadata API timeout is 5 seconds.

Instead, take a few milliseconds to determine whether
we are even running on EC2. We attempt to do this by
checking the contents of different files on the disk, e.g.
/sys/devices/virtual/dmi/id/board_asset_tag, and then if all of those
fail, attempting to see if anything is listening on the metadata host.

This can help prevent the 5 second failure timeout.
@kevinburkesegment kevinburkesegment requested a review from a team as a code owner August 24, 2023 22:11
@mckern
Copy link
Contributor

mckern commented Aug 25, 2023

What's the advantage to using this approach instead of using ec2metadata's Available()?

It looks like Available() is intended for exactly this use case, and implementing Available() is a very small addition:

 		ec2metadataSvc := ec2metadata.New(session)
+		if !ec2metadataSvc.Available() {
+			return nil, nil, err
+		}
+

@kevinburkesegment
Copy link
Contributor Author

Reading through the source I believe that that endpoint would also have a 5 second timeout. I can test it experimentally though.

@mckern
Copy link
Contributor

mckern commented Aug 25, 2023

Reading through the source I believe that that endpoint would also have a 5 second timeout. I can test it experimentally though.

Yeah, I think you're right but can't we just feed it a temporary timeout context (note, this is decidedly incomplete, and only serves as an example):

 		ec2metadataSvc := ec2metadata.New(session)
+		ctx, cancelFn := context.WithTimeout(aws.BackgroundContext(), 20*time.Millisecond)
+		defer cancelFn()
+		if !ec2metadataSvc.AvailableWithContext(ctx) {
+			return nil, nil, fmt.Errorf("EC2 Metadata service not available")
+		}
+
chamber $ aws-okta exec ci-read -- time ./test.sh
+ unset AWS_DEFAULT_REGION AWS_REGION
+ chamber --backend s3 --backend-s3-bucket segment-secrets-ci export whatever
Error: EC2 Metadata service not available
        0.08 real         0.03 user         0.01 sys

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

Successfully merging this pull request may close these issues.

2 participants