lokeshj1703 commented on a change in pull request #1887:
URL: https://github.com/apache/ozone/pull/1887#discussion_r570960691



##########
File path: 
hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/container/common/transport/server/ratis/XceiverServerRatis.java
##########
@@ -413,71 +444,68 @@ public static XceiverServerRatis newXceiverServerRatis(
       DatanodeDetails datanodeDetails, ConfigurationSource ozoneConf,
       ContainerDispatcher dispatcher, ContainerController containerController,
       CertificateClient caClient, StateContext context) throws IOException {
-    int localPort = ozoneConf.getInt(
-        OzoneConfigKeys.DFS_CONTAINER_RATIS_IPC_PORT,
-        OzoneConfigKeys.DFS_CONTAINER_RATIS_IPC_PORT_DEFAULT);
+    Parameters parameters = createTlsParameters(
+        new SecurityConfig(ozoneConf), caClient);
 
-    // Get an available port on current node and
-    // use that as the container port
-    if (ozoneConf.getBoolean(OzoneConfigKeys
-            .DFS_CONTAINER_RATIS_IPC_RANDOM_PORT,
-        OzoneConfigKeys.DFS_CONTAINER_RATIS_IPC_RANDOM_PORT_DEFAULT)) {
-      localPort = 0;
-    }
-    GrpcTlsConfig tlsConfig = createTlsServerConfigForDN(
-          new SecurityConfig(ozoneConf), caClient);
-
-    return new XceiverServerRatis(datanodeDetails, localPort, dispatcher,
-        containerController, context, tlsConfig, ozoneConf);
+    return new XceiverServerRatis(datanodeDetails, dispatcher,
+        containerController, context, ozoneConf, parameters);
   }
 
   // For gRPC server running DN container service with gPRC TLS
-  // No mTLS as the channel is shared for for external client, which
-  // does not have SCM CA issued certificates.
   // In summary:
   // authenticate from server to client is via TLS.
   // authenticate from client to server is via block token (or container 
token).
   // DN Ratis server act as both SSL client and server and we must pass TLS
   // configuration for both.
-  static GrpcTlsConfig createTlsServerConfigForDN(SecurityConfig conf,
+  private static Parameters createTlsParameters(SecurityConfig conf,
       CertificateClient caClient) {
+    Parameters parameters = new Parameters();
+
     if (conf.isSecurityEnabled() && conf.isGrpcTlsEnabled()) {
-      return new GrpcTlsConfig(
+      GrpcTlsConfig serverConfig = new GrpcTlsConfig(
+          caClient.getPrivateKey(), caClient.getCertificate(),
+          caClient.getCACertificate(), true);
+      GrpcConfigKeys.Server.setTlsConf(parameters, serverConfig);
+      GrpcConfigKeys.Admin.setTlsConf(parameters, serverConfig);
+
+      GrpcTlsConfig clientConfig = new GrpcTlsConfig(
           caClient.getPrivateKey(), caClient.getCertificate(),
           caClient.getCACertificate(), false);
+      GrpcConfigKeys.Client.setTlsConf(parameters, clientConfig);
     }
-    return null;
+
+    return parameters;
   }
 
   @Override
   public void start() throws IOException {
     if (!isStarted) {
-      LOG.info("Starting {} {} at port {}", getClass().getSimpleName(),
-          server.getId(), getIPCPort());
+      LOG.info("Starting {} {}", getClass().getSimpleName(), server.getId());
       for (ThreadPoolExecutor executor : chunkExecutors) {
         executor.prestartAllCoreThreads();
       }
       server.start();
 
-      int realPort = server.getServerRpc()
-          .getInetSocketAddress()
-          .getPort();
-
-      if (port == 0) {
-        LOG.info("{} {} is started using port {}", getClass().getSimpleName(),
-            server.getId(), realPort);
-        port = realPort;
-      }
-
-      //register the real port to the datanode details.
-      datanodeDetails.setPort(DatanodeDetails
-          .newPort(DatanodeDetails.Port.Name.RATIS,
-              realPort));
+      RaftServerRpc serverRpc = server.getServerRpc();
+      clientPort = getRealPort(serverRpc.getClientServerAddress(),
+          Port.Name.RATIS);
+      adminPort = getRealPort(serverRpc.getAdminServerAddress(),
+          Port.Name.RATIS_ADMIN);
+      serverPort = getRealPort(serverRpc.getInetSocketAddress(),
+          Port.Name.RATIS_SERVER);
 
       isStarted = true;
     }
   }
 
+  private int getRealPort(InetSocketAddress address, Port.Name name) {

Review comment:
       The real port should be same as configured port?

##########
File path: 
hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConsts.java
##########
@@ -129,6 +129,8 @@
   public static final String OZONE_DB_CHECKPOINT_REQUEST_FLUSH =
       "flushBeforeCheckpoint";
 
+  public static final String SEPARATE_RATIS_PORTS_AVAILABLE = "RatisPorts";
+

Review comment:
       Can we define features in a separate class? We could also define them as 
enums.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to