From 675b97bd9464486e09afcf60c75c0e244b1552bd Mon Sep 17 00:00:00 2001 From: Armin Date: Mon, 17 Jun 2019 21:36:31 +0200 Subject: [PATCH 1/2] Fix NetworkUtilsTests * Follow up to #42109: * Now that we know that the test failures come from `veth` type network interfaces being created/destroyed concurrently to the test run, we can fix this test by simply ignoring these interfaces to prevent making a syscall to them when they don't exist anymore --- .../org/elasticsearch/common/network/NetworkUtilsTests.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/server/src/test/java/org/elasticsearch/common/network/NetworkUtilsTests.java b/server/src/test/java/org/elasticsearch/common/network/NetworkUtilsTests.java index ddc40961b073c..9b659cd0e54dd 100644 --- a/server/src/test/java/org/elasticsearch/common/network/NetworkUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/common/network/NetworkUtilsTests.java @@ -90,7 +90,10 @@ public void testFilter() throws Exception { public void testAddressInterfaceLookup() throws Exception { for (NetworkInterface netIf : NetworkUtils.getInterfaces()) { try { - if (!netIf.isUp() || Collections.list(netIf.getInetAddresses()).isEmpty()) { + // Ignoring virtual ethernet devices since e.g. Docker could be creating or removing them in the background breaking tests. + final String interfaceName = netIf.getName(); + if ((interfaceName != null && interfaceName.startsWith("veth")) || netIf.isUp() == false + || Collections.list(netIf.getInetAddresses()).isEmpty()) { continue; } } catch (SocketException e) { From 0f2010575b03c202d0776d74f7cfcffe85d45b30 Mon Sep 17 00:00:00 2001 From: Armin Date: Wed, 19 Jun 2019 08:12:14 +0200 Subject: [PATCH 2/2] CR: change test to only test name lookup --- .../common/network/NetworkUtils.java | 8 +++- .../common/network/NetworkUtilsTests.java | 38 ++++++------------- 2 files changed, 18 insertions(+), 28 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/common/network/NetworkUtils.java b/server/src/main/java/org/elasticsearch/common/network/NetworkUtils.java index 4e8f871ed3044..6a49f0d7658d4 100644 --- a/server/src/main/java/org/elasticsearch/common/network/NetworkUtils.java +++ b/server/src/main/java/org/elasticsearch/common/network/NetworkUtils.java @@ -200,10 +200,14 @@ static InetAddress[] getGlobalAddresses() throws IOException { static InetAddress[] getAllAddresses() throws IOException { return filterAllAddresses(address -> true, "no up-and-running addresses found"); } - + + static Optional maybeGetInterfaceByName(List networkInterfaces, String name) { + return networkInterfaces.stream().filter(netIf -> name.equals(netIf.getName())).findFirst(); + } + /** Returns addresses for the given interface (it must be marked up) */ static InetAddress[] getAddressesForInterface(String name) throws SocketException { - Optional networkInterface = getInterfaces().stream().filter((netIf) -> name.equals(netIf.getName())).findFirst(); + Optional networkInterface = maybeGetInterfaceByName(getInterfaces(), name); if (networkInterface.isPresent() == false) { throw new IllegalArgumentException("No interface named '" + name + "' found, got " + getInterfaces()); diff --git a/server/src/test/java/org/elasticsearch/common/network/NetworkUtilsTests.java b/server/src/test/java/org/elasticsearch/common/network/NetworkUtilsTests.java index 9b659cd0e54dd..48fc767cf9980 100644 --- a/server/src/test/java/org/elasticsearch/common/network/NetworkUtilsTests.java +++ b/server/src/test/java/org/elasticsearch/common/network/NetworkUtilsTests.java @@ -20,13 +20,15 @@ package org.elasticsearch.common.network; import org.elasticsearch.test.ESTestCase; +import org.elasticsearch.test.hamcrest.OptionalMatchers; import java.net.InetAddress; import java.net.NetworkInterface; -import java.net.SocketException; -import java.util.Collections; +import java.util.List; +import java.util.Optional; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; /** * Tests for network utils. Please avoid using any methods that cause DNS lookups! @@ -80,30 +82,14 @@ public void testFilter() throws Exception { assertArrayEquals(new InetAddress[] { InetAddress.getByName("::1") }, NetworkUtils.filterIPV6(addresses)); } - /** - * Test that selecting by name is possible and properly matches the addresses on all interfaces and virtual - * interfaces. - * - * Note that to avoid that this test fails when interfaces are down or they do not have addresses assigned to them, - * they are ignored. - */ - public void testAddressInterfaceLookup() throws Exception { - for (NetworkInterface netIf : NetworkUtils.getInterfaces()) { - try { - // Ignoring virtual ethernet devices since e.g. Docker could be creating or removing them in the background breaking tests. - final String interfaceName = netIf.getName(); - if ((interfaceName != null && interfaceName.startsWith("veth")) || netIf.isUp() == false - || Collections.list(netIf.getInetAddresses()).isEmpty()) { - continue; - } - } catch (SocketException e) { - throw new AssertionError("Failed to check if interface [" + netIf + "] is up", e); - } - - String name = netIf.getName(); - InetAddress[] expectedAddresses = Collections.list(netIf.getInetAddresses()).toArray(new InetAddress[0]); - InetAddress[] foundAddresses = NetworkUtils.getAddressesForInterface(name); - assertArrayEquals(expectedAddresses, foundAddresses); + // test that selecting by name is possible + public void testMaybeGetInterfaceByName() throws Exception { + final List networkInterfaces = NetworkUtils.getInterfaces(); + for (NetworkInterface netIf : networkInterfaces) { + final Optional maybeNetworkInterface = + NetworkUtils.maybeGetInterfaceByName(networkInterfaces, netIf.getName()); + assertThat(maybeNetworkInterface, OptionalMatchers.isPresent()); + assertThat(maybeNetworkInterface.get().getName(), equalTo(netIf.getName())); } }