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

fingerprint: handle incomplete AWS imitation APIs #7509

Merged
merged 4 commits into from
Mar 26, 2020

Conversation

notnoop
Copy link
Contributor

@notnoop notnoop commented Mar 26, 2020

Fix a regression where we accidentally started treating non-AWS
environments as AWS environments, resulting in bad networking settings.

Two factors some at play:

First, in [1], we accidentally switched the ultimate AWS test from
checking ami-id to instance-id. This means that nomad started
treating more environments as AWS; e.g. Hetzner implements instance-id
but not ami-id.

Second, some of these environments return empty values instead of
errors! Hetzner returns empty 200 response for local-ipv4, resulting
into bad networking configuration.

This change fix the situation by restoring the check to ami-id and
ensuring that we only set network configuration when the ip address is
not-empty. Also, be more defensive around response whitespace input.

Build https://circleci.com/gh/hashicorp/nomad/51933 shows the behavior (or failures) in these environments.

[1] #6779

Fixes #7232

Mahmood Ali added 2 commits March 26, 2020 11:13
Test that nomad doesn't set empty/bad network configuration when in an
environment that does incomplete immitation of EC2 Metadata API.
Fix a regression where we accidentally started treating non-AWS
environments as AWS environments, resulting in bad networking settings.

Two factors some at play:

First, in [1], we accidentally switched the ultimate AWS test from
checking `ami-id` to `instance-id`.  This means that nomad started
treating more environments as AWS; e.g. Hetzner implements `instance-id`
but not `ami-id`.

Second, some of these environments return empty values instead of
errors!  Hetzner returns empty 200 response for `local-ipv4`, resulting
into bad networking configuration.

This change fix the situation by restoring the check to `ami-id` and
ensuring that we only set network configuration when the ip address is
not-empty.  Also, be more defensive around response whitespace input.

[1] #6779
@notnoop notnoop requested a review from cgbaker March 26, 2020 15:30
@notnoop notnoop self-assigned this Mar 26, 2020
Copy link
Contributor

@cgbaker cgbaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow, what a nasty issue. good job, and thanks for adding the tests in the first commit.

}
}
newNetwork = &structs.NetworkResource{
Device: "eth0",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized we probably shouldn't assume network being eth0 :( - will fix that in a follow up PR to ensure that we don't override cases where a user specifically set network interface.

@notnoop notnoop merged commit dc39c11 into master Mar 26, 2020
@notnoop notnoop deleted the b-ec2metadata-outside-aws branch March 26, 2020 22:27
@krim
Copy link

krim commented Mar 27, 2020

@notnoop thanks a lot!
When it will be in a release?

@notnoop
Copy link
Contributor Author

notnoop commented Apr 2, 2020

This will be in 0.11.0 - it's in beta now and should be out in final soon.

notnoop pushed a commit that referenced this pull request Apr 9, 2020
fingerprint: handle incomplete AWS imitation APIs
@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v0.10.4 Unable to deploy job due to "network: no networks available"
3 participants