Making GRPC Port Configurable #507 (#508)

* Making GRPC Port Configurable The port on which GRPC communication happens is currently hardcoded. Added the support of reading the value from stanza, to make it easier for users to change the port. Co-authored-by: Aditya Jindal <“[email protected]”>
opendistro-for-elasticsearch · Nov 3, 2020 · d3c6ccc · d3c6ccc
1 parent dd0c097
commit d3c6ccc
Show file tree

Hide file tree

Showing 10 changed files with 87 additions and 55 deletions.
diff --git a/pa_config/performance-analyzer.properties b/pa_config/performance-analyzer.properties
@@ -17,6 +17,9 @@ cleanup-metrics-db-files = true
 # WebService exposed by App's port
 webservice-listener-port = 9600
 
+# Port for RPC Communication
+rpc-port = 9650
+
 # Metric DB File Prefix Path location
 metrics-db-file-prefix-path = /tmp/metricsdb_
 

diff --git a/.../java/com/amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerApp.java b/.../java/com/amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerApp.java
@@ -265,11 +265,11 @@ public static ClientServers createClientServers(final GRPCConnectionManager conn
     boolean useHttps = settings.getHttpsEnabled();
     return createClientServers(
         connectionManager,
-        Util.RPC_PORT,
+        settings.getRpcPort(),
         new MetricsServerHandler(),
         new MetricsRestUtil(),
         useHttps,
-        settings.getSettingValue(PerformanceAnalyzerWebServer.WEBSERVICE_PORT_CONF_NAME),
+        settings.getWebServicePort(),
         settings.getSettingValue(PerformanceAnalyzerWebServer.WEBSERVICE_BIND_HOST_NAME),
         appContext);
   }
@@ -279,7 +279,7 @@ public static ClientServers createClientServers(final GRPCConnectionManager conn
                                                   final MetricsServerHandler metricsServerHandler,
                                                   final MetricsRestUtil metricsRestUtil,
                                                   boolean useHttps,
-                                                  final String webServerPortFromSetting,
+                                                  int webServerPort,
                                                   final String hostFromSetting,
                                                   final AppContext appContext) {
     NetServer netServer = new NetServer(rpcPort, 1, useHttps);
@@ -290,7 +290,7 @@ public static ClientServers createClientServers(final GRPCConnectionManager conn
     }
 
     HttpServer httpServer =
-        PerformanceAnalyzerWebServer.createInternalServer(webServerPortFromSetting, hostFromSetting, useHttps);
+        PerformanceAnalyzerWebServer.createInternalServer(webServerPort, hostFromSetting, useHttps);
 
     if (metricsRestUtil != null) {
       httpServer.createContext(QUERY_URL, new QueryMetricsRequestHandler(netClient, metricsRestUtil, appContext));

diff --git a/...com/amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerWebServer.java b/...com/amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerWebServer.java
@@ -42,28 +42,24 @@
 public class PerformanceAnalyzerWebServer {
 
   private static final Logger LOG = LogManager.getLogger(PerformanceAnalyzerWebServer.class);
-  public static final int WEBSERVICE_DEFAULT_PORT = 9600;
   @VisibleForTesting
   public static final String WEBSERVICE_BIND_HOST_NAME = "webservice-bind-host";
-  @VisibleForTesting
-  public static final String WEBSERVICE_PORT_CONF_NAME = "webservice-listener-port";
   // Use system default for max backlog.
   private static final int INCOMING_QUEUE_LENGTH = 1;
 
-  public static HttpServer createInternalServer(String portFromSetting, String hostFromSetting, boolean httpsEnabled) {
-    int internalPort = getPortNumber(portFromSetting);
+  public static HttpServer createInternalServer(int webServerPort, String hostFromSetting, boolean httpsEnabled) {
     try {
       Security.addProvider(new BouncyCastleProvider());
       HttpServer server;
       if (httpsEnabled) {
-        server = createHttpsServer(internalPort, hostFromSetting);
+        server = createHttpsServer(webServerPort, hostFromSetting);
       } else {
-        server = createHttpServer(internalPort, hostFromSetting);
+        server = createHttpServer(webServerPort, hostFromSetting);
       }
       server.setExecutor(Executors.newCachedThreadPool());
       return server;
     } catch (java.net.BindException ex) {
-      LOG.error("Could not create HttpServer on port {}", internalPort, ex);
+      LOG.error("Could not create HttpServer on port {}", webServerPort, ex);
       Runtime.getRuntime().halt(1);
     } catch (Exception ex) {
       ex.printStackTrace();
@@ -157,25 +153,4 @@ private static HttpServer createHttpServer(int readerPort, String bindHost) thro
 
     return server;
   }
-
-  private static int getPortNumber(String readerPortValue) {
-    try {
-      if (readerPortValue == null) {
-        LOG.info(
-            "{} not configured; using default value: {}",
-            WEBSERVICE_PORT_CONF_NAME,
-            WEBSERVICE_DEFAULT_PORT);
-        return WEBSERVICE_DEFAULT_PORT;
-      }
-
-      return Integer.parseInt(readerPortValue);
-    } catch (Exception ex) {
-      LOG.error(
-          "Invalid Configuration: {} Using default value: {} AND Error: {}",
-          WEBSERVICE_PORT_CONF_NAME,
-          WEBSERVICE_DEFAULT_PORT,
-          ex.toString());
-      return WEBSERVICE_DEFAULT_PORT;
-    }
-  }
 }
diff --git a/...n/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/config/PluginSettings.java b/...n/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/config/PluginSettings.java
@@ -20,7 +20,6 @@
 import com.google.common.annotations.VisibleForTesting;
 
 import java.io.File;
-import java.security.InvalidParameterException;
 import java.util.Properties;
 
 import javax.annotation.Nullable;
@@ -49,6 +48,10 @@ public class PluginSettings {
   private static final long BATCH_METRICS_RETENTION_PERIOD_MINUTES_DEFAULT = 7;
   private static final long BATCH_METRICS_RETENTION_PERIOD_MINUTES_MIN = 1;
   private static final long BATCH_METRICS_RETENTION_PERIOD_MINUTES_MAX = 60;
+  public static final String RPC_PORT_CONF_NAME = "rpc-port";
+  public static final int RPC_DEFAULT_PORT = 9650;
+  public static final String WEBSERVICE_PORT_CONF_NAME = "webservice-listener-port";
+  public static final int WEBSERVICE_DEFAULT_PORT = 9600;
 
   /** Determines whether the metricsdb files should be cleaned up. */
   public static final String DB_FILE_CLEANUP_CONF_NAME = "cleanup-metrics-db-files";
@@ -67,6 +70,9 @@ public class PluginSettings {
   /** Determines how many minutes worth of metricsdb files will be retained if batch metrics is enabled. */
   private long batchMetricsRetentionPeriodMinutes;
 
+  private int rpcPort;
+  private int webServicePort;
+
   static {
     Util.invokePrivilegedAndLogError(PluginSettings::createInstance);
   }
@@ -91,6 +97,14 @@ public long getBatchMetricsRetentionPeriodMinutes() {
     return batchMetricsRetentionPeriodMinutes;
   }
 
+  public int getRpcPort() {
+    return rpcPort;
+  }
+
+  public int getWebServicePort() {
+    return webServicePort;
+  }
+
   @VisibleForTesting
   public void setBatchMetricsRetentionPeriodMinutes(long batchMetricsRetentionPeriodMinutes) {
     this.batchMetricsRetentionPeriodMinutes = batchMetricsRetentionPeriodMinutes;
@@ -151,6 +165,8 @@ private PluginSettings(String cfPath) {
     metricsDeletionInterval = DELETION_INTERVAL_DEFAULT;
     writerQueueSize = WRITER_QUEUE_SIZE_DEFAULT;
     batchMetricsRetentionPeriodMinutes = BATCH_METRICS_RETENTION_PERIOD_MINUTES_DEFAULT;
+    rpcPort = RPC_DEFAULT_PORT;
+    webServicePort = WEBSERVICE_DEFAULT_PORT;
     if (cfPath == null || cfPath.isEmpty()) {
       this.configFilePath = DEFAULT_CONFIG_FILE_PATH;
     } else {
@@ -166,6 +182,7 @@ private PluginSettings(String cfPath) {
       loadHttpsEnabled();
       loadMetricsDBFilesCleanupEnabled();
       loadBatchMetricsRetentionPeriodMinutesFromConfig();
+      loadPortsFromConfig();
     } catch (ConfigFileException e) {
       LOG.error(
           "Loading config file {} failed with error: {}. Disabling plugin.",
@@ -181,12 +198,15 @@ private PluginSettings(String cfPath) {
     }
     LOG.info(
         "Config: metricsLocation: {}, metricsDeletionInterval: {}, httpsEnabled: {},"
-            + " cleanup-metrics-db-files: {}, batch-metrics-retention-period-minutes: {}",
+            + " cleanup-metrics-db-files: {}, batch-metrics-retention-period-minutes: {}, rpc-port: {}, "
+            +    "webservice-port {}",
         metricsLocation,
         metricsDeletionInterval,
         httpsEnabled,
         shouldCleanupMetricsDBFiles,
-        batchMetricsRetentionPeriodMinutes);
+        batchMetricsRetentionPeriodMinutes,
+        rpcPort,
+        webServicePort);
   }
 
   public static PluginSettings instance() {
@@ -321,4 +341,37 @@ private void loadBatchMetricsRetentionPeriodMinutesFromConfig() {
               batchMetricsRetentionPeriodMinutes);
     }
   }
+
+  public void loadPortsFromConfig() {
+    try {
+      String rpcPortValue = settings.getProperty(RPC_PORT_CONF_NAME);
+      String webServicePortValue = settings.getProperty(WEBSERVICE_PORT_CONF_NAME);
+      if (rpcPortValue == null) {
+        LOG.info(
+                "{} not configured; using default value: {}",
+                RPC_PORT_CONF_NAME,
+                RPC_DEFAULT_PORT);
+        this.rpcPort = RPC_DEFAULT_PORT;
+      } else {
+        this.rpcPort = Integer.parseInt(rpcPortValue);
+      }
+      if (webServicePortValue == null) {
+        LOG.info(
+                "{} not configured; using default value: {}",
+                WEBSERVICE_PORT_CONF_NAME,
+                WEBSERVICE_DEFAULT_PORT);
+        this.webServicePort = WEBSERVICE_DEFAULT_PORT;
+      } else {
+        this.webServicePort = Integer.parseInt(webServicePortValue);
+      }
+    } catch (Exception ex) {
+      LOG.error(
+              "Invalid Configuration: {} Using default value: {} AND Error: {}",
+              RPC_PORT_CONF_NAME,
+              RPC_DEFAULT_PORT,
+              ex.toString());
+      this.rpcPort = RPC_DEFAULT_PORT;
+      this.webServicePort = WEBSERVICE_DEFAULT_PORT;
+    }
+  }
 }
diff --git a/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/core/Util.java b/src/main/java/com/amazon/opendistro/elasticsearch/performanceanalyzer/core/Util.java
@@ -29,8 +29,6 @@ public class Util {
   public static final String RCA_QUERY_URL = "/_opendistro/_performanceanalyzer/rca";
   public static final String ACTIONS_QUERY_URL = "/_opendistro/_performanceanalyzer/actions";
   public static final String ES_HOME = System.getProperty("es.path.home");
-  // TODO: Make this configurable.
-  public static final int RPC_PORT = 9650;
   public static final String PLUGIN_LOCATION =
           ES_HOME
           + File.separator

diff --git a/...azon/opendistro/elasticsearch/performanceanalyzer/rca/framework/util/InstanceDetails.java b/...azon/opendistro/elasticsearch/performanceanalyzer/rca/framework/util/InstanceDetails.java
@@ -15,6 +15,7 @@
 
 package com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.framework.util;
 
+import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.PluginSettings;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.core.Util;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics.NodeRole;
@@ -101,7 +102,7 @@ public String toString() {
   private final int grpcPort;
 
   public InstanceDetails(AllMetrics.NodeRole role, Id instanceId, Ip instanceIp, boolean isMaster) {
-    this(role, instanceId, instanceIp, isMaster, Util.RPC_PORT);
+    this(role, instanceId, instanceIp, isMaster, PluginSettings.instance().getRpcPort());
   }
 
   public InstanceDetails(AllMetrics.NodeRole role, Id instanceId, Ip instanceIp, boolean isMaster, int grpcPort) {

diff --git a/...zon/opendistro/elasticsearch/performanceanalyzer/reader/ClusterDetailsEventProcessor.java b/...zon/opendistro/elasticsearch/performanceanalyzer/reader/ClusterDetailsEventProcessor.java
@@ -15,14 +15,13 @@
 
 package com.amazon.opendistro.elasticsearch.performanceanalyzer.reader;
 
+import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.PluginSettings;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.overrides.ConfigOverridesApplier;
-import com.amazon.opendistro.elasticsearch.performanceanalyzer.core.Util;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.PerformanceAnalyzerMetrics;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.RcaControllerHelper;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.reader_writer_shared.Event;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.util.JsonConverter;
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.collect.ImmutableList;
 import java.util.ArrayList;
 import java.util.Collections;
@@ -170,7 +169,7 @@ public static class NodeDetails {
     private String hostAddress;
     private String role;
     private Boolean isMasterNode;
-    private int grpcPort = Util.RPC_PORT;
+    private int grpcPort = PluginSettings.instance().getRpcPort();
 
     NodeDetails(String stringifiedMetrics) {
       Map<String, Object> map = JsonConverter
@@ -183,7 +182,7 @@ public static class NodeDetails {
     }
 
     public NodeDetails(AllMetrics.NodeRole role, String id, String hostAddress, boolean isMaster) {
-      this(role, id, hostAddress, isMaster, Util.RPC_PORT);
+      this(role, id, hostAddress, isMaster, PluginSettings.instance().getRpcPort());
     }
 
     public NodeDetails(final NodeDetails other) {

diff --git a/...amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerWebServerTest.java b/...amazon/opendistro/elasticsearch/performanceanalyzer/PerformanceAnalyzerWebServerTest.java
@@ -1,5 +1,7 @@
 package com.amazon.opendistro.elasticsearch.performanceanalyzer;
 
+import static com.amazon.opendistro.elasticsearch.performanceanalyzer.config.PluginSettings.WEBSERVICE_PORT_CONF_NAME;
+
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.PluginSettings;
 import com.sun.net.httpserver.HttpServer;
 
@@ -47,7 +49,7 @@ public class PerformanceAnalyzerWebServerTest {
     public void setup() {
         // Save old PluginSettings values
         oldBindHost = PluginSettings.instance().getSettingValue(PerformanceAnalyzerWebServer.WEBSERVICE_BIND_HOST_NAME);
-        oldPort = PluginSettings.instance().getSettingValue(PerformanceAnalyzerWebServer.WEBSERVICE_PORT_CONF_NAME);
+        oldPort = PluginSettings.instance().getProperty(WEBSERVICE_PORT_CONF_NAME);
         oldCertificateFilePath = PluginSettings.instance().getSettingValue(CertificateUtils.CERTIFICATE_FILE_PATH);
         oldPrivateKeyFilePath = PluginSettings.instance().getSettingValue(CertificateUtils.PRIVATE_KEY_FILE_PATH);
         oldTrustedCasFilePath = PluginSettings.instance().getSettingValue(CertificateUtils.TRUSTED_CAS_FILE_PATH);
@@ -57,7 +59,7 @@ public void setup() {
         oldHttpsEnabled = PluginSettings.instance().getHttpsEnabled();
         // Update bind host, port, and server certs for the test
         PluginSettings.instance().overrideProperty(PerformanceAnalyzerWebServer.WEBSERVICE_BIND_HOST_NAME, BIND_HOST);
-        PluginSettings.instance().overrideProperty(PerformanceAnalyzerWebServer.WEBSERVICE_PORT_CONF_NAME, PORT);
+        PluginSettings.instance().overrideProperty(WEBSERVICE_PORT_CONF_NAME, PORT);
         ClassLoader classLoader = getClass().getClassLoader();
         PluginSettings.instance().overrideProperty(CertificateUtils.CERTIFICATE_FILE_PATH,
                 Objects.requireNonNull(classLoader.getResource("tls/server/localhost.crt")).getFile());
@@ -74,9 +76,9 @@ public void tearDown() {
             PluginSettings.instance().overrideProperty(PerformanceAnalyzerWebServer.WEBSERVICE_BIND_HOST_NAME, "localhost");
         }
         if (oldPort != null) {
-            PluginSettings.instance().overrideProperty(PerformanceAnalyzerWebServer.WEBSERVICE_PORT_CONF_NAME, oldPort);
+            PluginSettings.instance().overrideProperty(WEBSERVICE_PORT_CONF_NAME, oldPort);
         } else {
-            PluginSettings.instance().overrideProperty(PerformanceAnalyzerWebServer.WEBSERVICE_PORT_CONF_NAME, "9600");
+            PluginSettings.instance().overrideProperty(WEBSERVICE_PORT_CONF_NAME, "9600");
         }
         if (oldCertificateFilePath != null) {
             PluginSettings.instance().overrideProperty(CertificateUtils.CERTIFICATE_FILE_PATH, oldCertificateFilePath);
@@ -110,7 +112,7 @@ public void tearDown() {
 
     public void initializeServer(boolean useHttps) {
         PluginSettings.instance().setHttpsEnabled(useHttps);
-        server = PerformanceAnalyzerWebServer.createInternalServer(PORT, BIND_HOST, useHttps);
+        server = PerformanceAnalyzerWebServer.createInternalServer(Integer.parseInt(PORT), BIND_HOST, useHttps);
         Assert.assertNotNull(server);
         server.setExecutor(Executors.newFixedThreadPool(1));
         // Setup basic /test endpoint. When the server receives any request on /test, it responds with "hello"

diff --git a/...amazon/opendistro/elasticsearch/performanceanalyzer/rca/integTests/framework/Cluster.java b/...amazon/opendistro/elasticsearch/performanceanalyzer/rca/integTests/framework/Cluster.java
@@ -17,6 +17,7 @@
 
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.PerformanceAnalyzerApp;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.PerformanceAnalyzerWebServer;
+import com.amazon.opendistro.elasticsearch.performanceanalyzer.config.PluginSettings;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.core.Util;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.metrics.AllMetrics;
 import com.amazon.opendistro.elasticsearch.performanceanalyzer.rca.integTests.framework.annotations.AMetric;
@@ -119,8 +120,8 @@ public Cluster(final ClusterType type, final File clusterDir, final boolean useH
   }
 
   private void createMultiNodeDedicatedMaster() {
-    int currWebServerPort = PerformanceAnalyzerWebServer.WEBSERVICE_DEFAULT_PORT;
-    int currGrpcServerPort = Util.RPC_PORT;
+    int currWebServerPort = PluginSettings.WEBSERVICE_DEFAULT_PORT;
+    int currGrpcServerPort = PluginSettings.RPC_DEFAULT_PORT;
     int hostIdx = 0;
 
     createHost(hostIdx, AllMetrics.NodeRole.ELECTED_MASTER, currWebServerPort, currGrpcServerPort);
@@ -200,16 +201,16 @@ public void startRcaControllerThread() {
   }
 
   private void createSingleNodeCluster() {
-    int currWebServerPort = PerformanceAnalyzerWebServer.WEBSERVICE_DEFAULT_PORT;
-    int currGrpcServerPort = Util.RPC_PORT;
+    int currWebServerPort = PluginSettings.WEBSERVICE_DEFAULT_PORT;
+    int currGrpcServerPort = PluginSettings.RPC_DEFAULT_PORT;
     int hostIdx = 0;
 
     createHost(hostIdx, AllMetrics.NodeRole.ELECTED_MASTER, currWebServerPort, currGrpcServerPort);
   }
 
   private void createMultiNodeCoLocatedMaster() {
-    int currWebServerPort = PerformanceAnalyzerWebServer.WEBSERVICE_DEFAULT_PORT;
-    int currGrpcServerPort = Util.RPC_PORT;
+    int currWebServerPort = PluginSettings.WEBSERVICE_DEFAULT_PORT;
+    int currGrpcServerPort = PluginSettings.RPC_DEFAULT_PORT;
     int hostIdx = 0;
 
     createHost(hostIdx, AllMetrics.NodeRole.ELECTED_MASTER, currWebServerPort, currGrpcServerPort);

diff --git a/...om/amazon/opendistro/elasticsearch/performanceanalyzer/rca/integTests/framework/Host.java b/...om/amazon/opendistro/elasticsearch/performanceanalyzer/rca/integTests/framework/Host.java
@@ -154,7 +154,7 @@ public void createServersAndThreads(final ThreadProvider threadProvider) {
         null,
         null,
         useHttps,
-        String.valueOf(webServerPort),
+        webServerPort,
         null,  // A null host is fine as this will use the loopback address
         this.appContext);