From 77d0b3d43398d770c2175648f49d7d8a09defb63 Mon Sep 17 00:00:00 2001 From: Sid Narayan Date: Thu, 30 Jul 2020 09:50:17 -0700 Subject: [PATCH 1/2] Fix failures in ResourceHeatMapGraphTest When ResourceHeatMapGraphTest was run after PerformanceAnalyzerWebServerTest, we noticed connection refused exceptions that didn't happen when the tests were run in isolation. This commit makes sure that PerformanceAnalyzerWebServerTest cleans up after itself so that subsequent tests will run as expected. --- .../PerformanceAnalyzerWebServerTest.java | 8 ++++++++ .../rca/store/ResourceHeatMapGraphTest.java | 7 ++++++- 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerWebServerTest.java b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerWebServerTest.java index 2d0f818b7..b2be4c218 100644 --- a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerWebServerTest.java +++ b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerWebServerTest.java @@ -70,9 +70,13 @@ public void tearDown() { // Unset all SSL settings if (oldBindHost != null) { PluginSettings.instance().overrideProperty(PerformanceAnalyzerWebServer.WEBSERVICE_BIND_HOST_NAME, oldBindHost); + } else { + PluginSettings.instance().overrideProperty(PerformanceAnalyzerWebServer.WEBSERVICE_BIND_HOST_NAME, "localhost"); } if (oldPort != null) { PluginSettings.instance().overrideProperty(PerformanceAnalyzerWebServer.WEBSERVICE_PORT_CONF_NAME, oldPort); + } else { + PluginSettings.instance().overrideProperty(PerformanceAnalyzerWebServer.WEBSERVICE_PORT_CONF_NAME, "9600"); } if (oldCertificateFilePath != null) { PluginSettings.instance().overrideProperty(CertificateUtils.CERTIFICATE_FILE_PATH, oldCertificateFilePath); @@ -82,6 +86,8 @@ public void tearDown() { } if (oldTrustedCasFilePath != null) { PluginSettings.instance().overrideProperty(CertificateUtils.TRUSTED_CAS_FILE_PATH, oldTrustedCasFilePath); + } else { + PluginSettings.instance().overrideProperty(CertificateUtils.TRUSTED_CAS_FILE_PATH, ""); } if (oldClientCertificateFilePath != null) { PluginSettings.instance().overrideProperty(CertificateUtils.CLIENT_CERTIFICATE_FILE_PATH, oldClientCertificateFilePath); @@ -91,6 +97,8 @@ public void tearDown() { } if (oldClientTrustedCasFilePath != null) { PluginSettings.instance().overrideProperty(CertificateUtils.CLIENT_TRUSTED_CAS_FILE_PATH, oldClientTrustedCasFilePath); + } else { + PluginSettings.instance().overrideProperty(CertificateUtils.CLIENT_TRUSTED_CAS_FILE_PATH, ""); } PluginSettings.instance().setHttpsEnabled(oldHttpsEnabled); diff --git a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ResourceHeatMapGraphTest.java b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ResourceHeatMapGraphTest.java index 90d2c1b85..d39a4afb2 100644 --- a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ResourceHeatMapGraphTest.java +++ b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ResourceHeatMapGraphTest.java @@ -88,7 +88,6 @@ import org.junit.AfterClass; import org.junit.Assert; import org.junit.BeforeClass; -import org.junit.Ignore; import org.junit.Test; public class ResourceHeatMapGraphTest { @@ -151,6 +150,12 @@ public static void shutdown() { clientServers.getHttpServer().stop(0); clientServers.getNetServer().stop(); clientServers.getNetClient().stop(); + + try { + Thread.sleep(1000); + } catch (InterruptedException ie) { + ie.printStackTrace(); + } } private static class AnalysisGraphX extends ElasticSearchAnalysisGraph { From d5182ca55c2ab8a76d895f46ae23204d3befdc95 Mon Sep 17 00:00:00 2001 From: Sid Narayan Date: Thu, 30 Jul 2020 10:24:33 -0700 Subject: [PATCH 2/2] Remove Assert.fail() calls from ResourceHeatMapGraphTest Calls to Assert.fail() obscure the actual Exception in our testing logs. This commit removes the fail() calls and simply throws instead. --- .../rca/store/ResourceHeatMapGraphTest.java | 158 ++++++------------ 1 file changed, 48 insertions(+), 110 deletions(-) diff --git a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ResourceHeatMapGraphTest.java b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ResourceHeatMapGraphTest.java index d39a4afb2..009c9d78e 100644 --- a/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ResourceHeatMapGraphTest.java +++ b/src/test/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/rca/store/ResourceHeatMapGraphTest.java @@ -29,7 +29,6 @@ import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics; import com.amazon.opendistro.elasticsearch.performanceanalyzer.net.GRPCConnectionManager; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.RcaTestHelper; -import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.exceptions.MalformedConfig; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.AnalysisGraph; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.Metric; import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.api.metrics.CPU_Utilization; @@ -67,15 +66,11 @@ import com.google.gson.JsonParser; import com.sun.net.httpserver.HttpServer; import java.io.BufferedReader; -import java.io.IOException; import java.io.InputStreamReader; import java.net.HttpURLConnection; -import java.net.MalformedURLException; -import java.net.ProtocolException; import java.net.URL; import java.nio.file.Path; import java.nio.file.Paths; -import java.sql.SQLException; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; @@ -108,25 +103,9 @@ public class ResourceHeatMapGraphTest { private static AtomicReference networkThreadPoolReference; @BeforeClass - public static void init() { - try { - persistable = PersistenceFactory.create(rcaConf); - } catch (MalformedConfig malformedConfig) { - malformedConfig.printStackTrace(); - Assert.fail(); - } catch (SQLException e) { - e.printStackTrace(); - Assert.fail(); - } catch (IOException e) { - e.printStackTrace(); - Assert.fail(); - } - try { - reader = new SQLiteReader(sqliteFile.toString()); - } catch (SQLException e) { - e.printStackTrace(); - Assert.fail(); - } + public static void init() throws Exception { + persistable = PersistenceFactory.create(rcaConf); + reader = new SQLiteReader(sqliteFile.toString()); AllMetrics.NodeRole nodeRole2 = AllMetrics.NodeRole.ELECTED_MASTER; AppContext appContext = RcaTestHelper.setMyIp("192.168.0.2", nodeRole2); @@ -200,7 +179,7 @@ private List createAndExecuteRcaGraph(AppContext appContext) return connectedComponents; } - private String makeRestRequest(final String[] params) { + private String makeRestRequest(final String[] params) throws Exception { // The params are key/value pairs and therefore there should be even numbers of them. Assert.assertEquals(0, params.length % 2); StringBuilder queryString = new StringBuilder(); @@ -214,63 +193,31 @@ private String makeRestRequest(final String[] params) { uri.append("?").append(queryString); - URL url = null; - try { - url = new URL(uri.toString()); - } catch (MalformedURLException e) { - e.printStackTrace(); - Assert.fail(); - } + URL url = new URL(uri.toString()); String response = ""; - HttpURLConnection connection = null; - - try { - connection = (HttpURLConnection) url.openConnection(); - } catch (IOException e) { - e.printStackTrace(); - Assert.fail(); + HttpURLConnection connection = (HttpURLConnection) url.openConnection(); + connection.setRequestMethod("GET"); + int status = connection.getResponseCode(); + if (status != 200) { + List ret = + new BufferedReader(new InputStreamReader(connection.getErrorStream())).lines().collect(Collectors.toList()); + throw new IllegalStateException(ret.toString()); } - try { - connection.setRequestMethod("GET"); - } catch (ProtocolException e) { - e.printStackTrace(); - connection.disconnect(); - Assert.fail(); - } - - try { - int status = connection.getResponseCode(); - if (status != 200) { - List ret = - new BufferedReader(new InputStreamReader(connection.getErrorStream())).lines().collect(Collectors.toList()); - throw new IllegalStateException(ret.toString()); - } - } catch (IOException e) { - e.printStackTrace(); - connection.disconnect(); - Assert.fail(); - } - - try (BufferedReader in = new BufferedReader( - new InputStreamReader(connection.getInputStream()))) { - String inputLine; - StringBuffer content = new StringBuffer(); - while ((inputLine = in.readLine()) != null) { - content.append(inputLine); - } - response = content.toString(); - } catch (IOException e) { - e.printStackTrace(); - connection.disconnect(); - Assert.fail(); + BufferedReader in = new BufferedReader(new InputStreamReader(connection.getInputStream())); + String inputLine; + StringBuffer content = new StringBuffer(); + while ((inputLine = in.readLine()) != null) { + content.append(inputLine); } + response = content.toString(); + in.close(); return response; } @Test - public void clusterTemperatureProfile() { + public void clusterTemperatureProfile() throws Exception { AppContext appContext = RcaTestHelper.setMyIp("192.168.0.2", AllMetrics.NodeRole.ELECTED_MASTER); List connectedComponents = createAndExecuteRcaGraph(appContext); @@ -308,7 +255,7 @@ public void clusterTemperatureProfile() { } @Test - public void fullNodeTemperatureProfile() { + public void fullNodeTemperatureProfile() throws Exception { AppContext appContext = RcaTestHelper.setMyIp("192.168.0.3", AllMetrics.NodeRole.DATA); createAndExecuteRcaGraph(appContext); verifyFullNodeTemperatureProfile(makeRestRequest( @@ -1061,7 +1008,7 @@ public void construct() { } @Test - public void testHotShardClusterApiResponse() { + public void testHotShardClusterApiResponse() throws Exception { AnalysisGraph analysisGraph = new AnalysisGraphHotShard(); List connectedComponents = RcaUtil.getAnalysisGraphComponents(analysisGraph); @@ -1125,45 +1072,36 @@ public void testHotShardClusterApiResponse() { rcaSchedulerTaskMaster.run(); URL url = null; - try { - url = new URL("http://localhost:9600" + Util.RCA_QUERY_URL + "?name=" + HotShardClusterRca.RCA_TABLE_NAME); - } catch (MalformedURLException e) { - e.printStackTrace(); - Assert.fail(); - } + url = new URL("http://localhost:9600" + Util.RCA_QUERY_URL + "?name=" + + HotShardClusterRca.RCA_TABLE_NAME); - try { - HttpURLConnection con = (HttpURLConnection) url.openConnection(); - con.setRequestMethod("GET"); - - int status = con.getResponseCode(); - System.out.println("Response status: " + status); - try (BufferedReader in = new BufferedReader( - new InputStreamReader(con.getInputStream()))) { - String inputLine; - StringBuffer content = new StringBuffer(); - while ((inputLine = in.readLine()) != null) { - content.append(inputLine); - } - final String hotShardClusterRcaName = HotShardClusterRca.RCA_TABLE_NAME; - final String hotClusterSummaryName = HotClusterSummary.HOT_CLUSTER_SUMMARY_TABLE; + HttpURLConnection con = (HttpURLConnection) url.openConnection(); + con.setRequestMethod("GET"); - JsonParser parser = new JsonParser(); - JsonElement jsonElement = parser.parse(content.toString()); - JsonObject hotShardClusterRca = - jsonElement.getAsJsonObject().get(hotShardClusterRcaName).getAsJsonArray().get(0).getAsJsonObject(); + int status = con.getResponseCode(); + System.out.println("Response status: " + status); + try (BufferedReader in = new BufferedReader( + new InputStreamReader(con.getInputStream()))) { + String inputLine; + StringBuffer content = new StringBuffer(); + while ((inputLine = in.readLine()) != null) { + content.append(inputLine); + } + final String hotShardClusterRcaName = HotShardClusterRca.RCA_TABLE_NAME; + final String hotClusterSummaryName = HotClusterSummary.HOT_CLUSTER_SUMMARY_TABLE; + + JsonParser parser = new JsonParser(); + JsonElement jsonElement = parser.parse(content.toString()); + JsonObject hotShardClusterRca = + jsonElement.getAsJsonObject().get(hotShardClusterRcaName).getAsJsonArray().get(0).getAsJsonObject(); - Assert.assertEquals(hotShardClusterRcaName, hotShardClusterRca.get("rca_name").getAsString()); - Assert.assertEquals("unhealthy", hotShardClusterRca.get("state").getAsString()); + Assert.assertEquals(hotShardClusterRcaName, hotShardClusterRca.get("rca_name").getAsString()); + Assert.assertEquals("unhealthy", hotShardClusterRca.get("state").getAsString()); - JsonObject hotClusterSummary = - hotShardClusterRca.get(hotClusterSummaryName).getAsJsonArray().get(0).getAsJsonObject(); - Assert.assertEquals(1, hotClusterSummary.get("number_of_unhealthy_nodes").getAsInt()); - } - con.disconnect(); - } catch (IOException e) { - e.printStackTrace(); - Assert.fail(); + JsonObject hotClusterSummary = + hotShardClusterRca.get(hotClusterSummaryName).getAsJsonArray().get(0).getAsJsonObject(); + Assert.assertEquals(1, hotClusterSummary.get("number_of_unhealthy_nodes").getAsInt()); } + con.disconnect(); } } \ No newline at end of file