Network: Allow to listen on virtual interfaces. #19568
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Previously when trying to listen on virtual interfaces during bootstrap the application would stop working - the interface couldn't be found by the NetworkUtils class. The NetworkUtils utilize the underlying JDK NetworkInterface class which, when asked to lookup by name only takes physical interfaces into account, failing at virtual (or subinterfaces) ones (returning null). Note that when interating over all interfaces, both physical and virtual ones are taken into account. This changeset asks for all known interfaces, iterates over them and matches on the given name as part of the loop, allowing it to catch both physical and virtual interfaces. As a result, elasticsearch can now also serve on virtual interfaces. A test case has been added which makes sure that all iterable interfaces can be found by their respective name. Note that this PR is a second iteration over the previously merged but later reverted elastic#19537 because it causes tests to fail when interfaces are down. The test has been modified to take this into account now. Closes elastic#17473 Relates elastic#19537
|
Can one of the admins verify this patch? |
|
@rmuir @jasontedor here's the updated PR with the test case being more restrictive to avoid failures on downed interfaces (and to that regard interfaces with no addresses assigned to them, since this is another case where the elastic code throws an exception) |
clintongormley
added
>enhancement
review
:Distributed/Network
| @@ -227,7 +227,14 @@ public static boolean defaultReuseAddress() { | |||
|
|
|||
| /** Returns addresses for the given interface (it must be marked up) */ | |||
| static InetAddress[] getAddressesForInterface(String name) throws SocketException { | |||
| NetworkInterface intf = NetworkInterface.getByName(name); | |||
| NetworkInterface intf = null; | |||
There was a problem hiding this comment.
wouldnt a stream using getInterfaces().stream().filter().findFirst() be nicer here?
There was a problem hiding this comment.
oh I didn't realize the codebase was on java 8 - good idea!
|
Just a quick note: I'm out the next two weeks so I can either fix it up when I'm back or feel free to amend and push if you want to get it in early :) |
|
@spinscale are there any other changes (other than the stream() conversion) that I should make in the next batch? |
spinscale
pushed a commit
to spinscale/elasticsearch
that referenced
this pull request
Sep 13, 2016
Previously when trying to listen on virtual interfaces during bootstrap the application would stop working - the interface couldn't be found by the NetworkUtils class. The NetworkUtils utilize the underlying JDK NetworkInterface class which, when asked to lookup by name only takes physical interfaces into account, failing at virtual (or subinterfaces) ones (returning null). Note that when interating over all interfaces, both physical and virtual ones are taken into account. This changeset asks for all known interfaces, iterates over them and matches on the given name as part of the loop, allowing it to catch both physical and virtual interfaces. As a result, elasticsearch can now also serve on virtual interfaces. A test case has been added which makes sure that all iterable interfaces can be found by their respective name. Note that this PR is a second iteration over the previously merged but later reverted elastic#19537 because it causes tests to fail when interfaces are down. The test has been modified to take this into account now. Closes elastic#17473 Closes elastic#19568 Relates elastic#19537
spinscale
pushed a commit
that referenced
this pull request
Sep 13, 2016
Previously when trying to listen on virtual interfaces during bootstrap the application would stop working - the interface couldn't be found by the NetworkUtils class. The NetworkUtils utilize the underlying JDK NetworkInterface class which, when asked to lookup by name only takes physical interfaces into account, failing at virtual (or subinterfaces) ones (returning null). Note that when interating over all interfaces, both physical and virtual ones are taken into account. This changeset asks for all known interfaces, iterates over them and matches on the given name as part of the loop, allowing it to catch both physical and virtual interfaces. As a result, elasticsearch can now also serve on virtual interfaces. A test case has been added which makes sure that all iterable interfaces can be found by their respective name. Note that this PR is a second iteration over the previously merged but later reverted #19537 because it causes tests to fail when interfaces are down. The test has been modified to take this into account now. Closes #17473 Closes #19568 Relates #19537
spinscale
pushed a commit
that referenced
this pull request
Sep 13, 2016
Previously when trying to listen on virtual interfaces during bootstrap the application would stop working - the interface couldn't be found by the NetworkUtils class. The NetworkUtils utilize the underlying JDK NetworkInterface class which, when asked to lookup by name only takes physical interfaces into account, failing at virtual (or subinterfaces) ones (returning null). Note that when interating over all interfaces, both physical and virtual ones are taken into account. This changeset asks for all known interfaces, iterates over them and matches on the given name as part of the loop, allowing it to catch both physical and virtual interfaces. As a result, elasticsearch can now also serve on virtual interfaces. A test case has been added which makes sure that all iterable interfaces can be found by their respective name. Note that this PR is a second iteration over the previously merged but later reverted #19537 because it causes tests to fail when interfaces are down. The test has been modified to take this into account now. Closes #17473 Closes #19568 Relates #19537
|
👍 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Previously when trying to listen on virtual interfaces during
bootstrap the application would stop working - the interface
couldn't be found by the NetworkUtils class.
The NetworkUtils utilize the underlying JDK NetworkInterface
class which, when asked to lookup by name only takes physical
interfaces into account, failing at virtual (or subinterfaces)
ones (returning null).
Note that when interating over all interfaces, both physical and
virtual ones are taken into account.
This changeset asks for all known interfaces, iterates over them
and matches on the given name as part of the loop, allowing it
to catch both physical and virtual interfaces.
As a result, elasticsearch can now also serve on virtual
interfaces.
A test case has been added which makes sure that all
iterable interfaces can be found by their respective name.
Note that this PR is a second iteration over the previously
merged but later reverted #19537 because it causes tests
to fail when interfaces are down. The test has been modified
to take this into account now.
Closes #17473
Relates #19537