HDDS-7329. Extend ozone admin datanode usageinfo and list info to accept hostname parameter

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/client/ScmClient.java

@@ -400,15 +400,15 @@ List<DeletedBlocksTransactionInfo> getFailedDeletedBlockTxn(int count,
   int resetDeletedBlockRetryCount(List<Long> txIDs) throws IOException;
 
   /**
-   * Get usage information of datanode by ipaddress or uuid.
+   * Get usage information of datanode by address or uuid.
    *
-   * @param ipaddress datanode ipaddress String
+   * @param address datanode address String
    * @param uuid datanode uuid String
    * @return List of DatanodeUsageInfoProto. Each element contains info such as
    * capacity, SCMused, and remaining space.
    * @throws IOException
    */
-  List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(String ipaddress,
+  List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(String address,
                                                                String uuid)
       throws IOException;
 

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/protocol/StorageContainerLocationProtocol.java

@@ -410,9 +410,9 @@ StartContainerBalancerResponseProto startContainerBalancer(
   boolean getContainerBalancerStatus() throws IOException;
 
   /**
-   * Get Datanode usage information by ip or uuid.
+   * Get Datanode usage information by ip or hostname or uuid.
    *
-   * @param ipaddress datanode IP address String
+   * @param address datanode IP address or Hostname String
    * @param uuid datanode UUID String
    * @param clientVersion Client's version number
    * @return List of DatanodeUsageInfoProto. Each element contains info such as
@@ -421,7 +421,7 @@ StartContainerBalancerResponseProto startContainerBalancer(
    * @see org.apache.hadoop.ozone.ClientVersion
    */
   List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      String ipaddress, String uuid, int clientVersion) throws IOException;
+      String address, String uuid, int clientVersion) throws IOException;
 
   /**
    * Get usage information of most or least used datanodes.

hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/scm/protocolPB/StorageContainerLocationProtocolClientSideTranslatorPB.java

@@ -932,19 +932,19 @@ public boolean getContainerBalancerStatus() throws IOException {
   /**
    * Builds request for datanode usage information and receives response.
    *
-   * @param ipaddress Address String
+   * @param address Address String
    * @param uuid UUID String
    * @return List of DatanodeUsageInfoProto. Each element contains info such as
    * capacity, SCMUsed, and remaining space.
    * @throws IOException
    */
   @Override
   public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      String ipaddress, String uuid, int clientVersion) throws IOException {
+      String address, String uuid, int clientVersion) throws IOException {
 
     DatanodeUsageInfoRequestProto request =
         DatanodeUsageInfoRequestProto.newBuilder()
-            .setIpaddress(ipaddress)
+            .setIpaddress(address)
             .setUuid(uuid)
             .build();
 

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/SCMNodeManager.java

@@ -82,6 +82,7 @@
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
@@ -379,34 +380,30 @@ public RegisteredCommand register(
       datanodeDetails.setIpAddress(dnAddress.getHostAddress());
     }
 
-    String dnsName;
-    String networkLocation;
+    final String ipAddress = datanodeDetails.getIpAddress();
+    final String hostName = datanodeDetails.getHostName();
     datanodeDetails.setNetworkName(datanodeDetails.getUuidString());
-    if (useHostname) {
-      dnsName = datanodeDetails.getHostName();
-    } else {
-      dnsName = datanodeDetails.getIpAddress();
-    }
-    networkLocation = nodeResolve(dnsName);
+    String networkLocation = nodeResolve(useHostname ? hostName : ipAddress);
     if (networkLocation != null) {
       datanodeDetails.setNetworkLocation(networkLocation);
     }
 
+    final UUID uuid = datanodeDetails.getUuid();
     if (!isNodeRegistered(datanodeDetails)) {
       try {
         clusterMap.add(datanodeDetails);
         nodeStateManager.addNode(datanodeDetails, layoutInfo);
         // Check that datanode in nodeStateManager has topology parent set
         DatanodeDetails dn = nodeStateManager.getNode(datanodeDetails);
         Preconditions.checkState(dn.getParent() != null);
-        addToDnsToUuidMap(dnsName, datanodeDetails.getUuid());
+        addToDnsToUuidMap(uuid, ipAddress, hostName);
         // Updating Node Report, as registration is successful
         processNodeReport(datanodeDetails, nodeReport);
-        LOG.info("Registered Data node : {}", datanodeDetails.toDebugString());
+        LOG.info("Registered datanode: {}", datanodeDetails.toDebugString());
         scmNodeEventPublisher.fireEvent(SCMEvents.NEW_NODE, datanodeDetails);
       } catch (NodeAlreadyExistsException e) {
         if (LOG.isTraceEnabled()) {
-          LOG.trace("Datanode is already registered. Datanode: {}",
+          LOG.trace("Datanode is already registered: {}",
               datanodeDetails);
         }
       } catch (NodeNotFoundException e) {
@@ -416,32 +413,20 @@ public RegisteredCommand register(
     } else {
       // Update datanode if it is registered but the ip or hostname changes
       try {
-        final DatanodeInfo datanodeInfo =
-                nodeStateManager.getNode(datanodeDetails);
-        if (!datanodeInfo.getIpAddress().equals(datanodeDetails.getIpAddress())
-                || !datanodeInfo.getHostName()
-                .equals(datanodeDetails.getHostName())) {
-          LOG.info("Updating data node {} from {} to {}",
+        final DatanodeInfo oldNode = nodeStateManager.getNode(datanodeDetails);
+        if (updateDnsToUuidMap(oldNode.getHostName(), oldNode.getIpAddress(),
+            hostName, ipAddress, uuid)) {
+          LOG.info("Updating datanode {} from {} to {}",
                   datanodeDetails.getUuidString(),
-                  datanodeInfo,
+                  oldNode,
                   datanodeDetails);
-          clusterMap.update(datanodeInfo, datanodeDetails);
-
-          String oldDnsName;
-          if (useHostname) {
-            oldDnsName = datanodeInfo.getHostName();
-          } else {
-            oldDnsName = datanodeInfo.getIpAddress();
-          }
-          updateDnsToUuidMap(oldDnsName, dnsName, datanodeDetails.getUuid());
-
+          clusterMap.update(oldNode, datanodeDetails);
           nodeStateManager.updateNode(datanodeDetails, layoutInfo);
           DatanodeDetails dn = nodeStateManager.getNode(datanodeDetails);
           Preconditions.checkState(dn.getParent() != null);
           processNodeReport(datanodeDetails, nodeReport);
-          LOG.info("Updated Datanode to: {}", dn);
-          scmNodeEventPublisher
-                  .fireEvent(SCMEvents.NODE_ADDRESS_UPDATE, dn);
+          LOG.info("Updated datanode to: {}", dn);
+          scmNodeEventPublisher.fireEvent(SCMEvents.NODE_ADDRESS_UPDATE, dn);
         }
       } catch (NodeNotFoundException e) {
         LOG.error("Cannot find datanode {} from nodeStateManager",
@@ -458,31 +443,49 @@ public RegisteredCommand register(
   /**
    * Add an entry to the dnsToUuidMap, which maps hostname / IP to the DNs
    * running on that host. As each address can have many DNs running on it,
-   * this is a one to many mapping.
+   * and each host can have multiple addresses,
+   * this is a many to many mapping.
    *
-   * @param addr the hostname or IP of the node
    * @param uuid the UUID of the registered node.
+   * @param addresses hostname and/or IP of the node
    */
-  private synchronized void addToDnsToUuidMap(String addr, UUID uuid) {
-    dnsToUuidMap.computeIfAbsent(addr, k -> ConcurrentHashMap.newKeySet())
-        .add(uuid);
+  private synchronized void addToDnsToUuidMap(UUID uuid, String... addresses) {
+    for (String addr : addresses) {
+      if (!Strings.isNullOrEmpty(addr)) {
+        dnsToUuidMap.computeIfAbsent(addr, k -> ConcurrentHashMap.newKeySet())
+            .add(uuid);
+      }
+    }
   }
 
-  private synchronized void removeFromDnsToUuidMap(String addr, UUID uuid) {
-    Set<UUID> dnSet = dnsToUuidMap.get(addr);
-    if (dnSet != null && dnSet.remove(uuid) && dnSet.isEmpty()) {
-      dnsToUuidMap.remove(addr);
+  private synchronized void removeFromDnsToUuidMap(UUID uuid, String address) {
+    if (address != null) {
+      Set<UUID> dnSet = dnsToUuidMap.get(address);
+      if (dnSet != null && dnSet.remove(uuid) && dnSet.isEmpty()) {
+        dnsToUuidMap.remove(address);
+      }
     }
   }
 
-  private synchronized void updateDnsToUuidMap(
-      String oldDnsName, String newDnsName, UUID uuid) {
-    Preconditions.checkNotNull(oldDnsName, "old address == null");
-    Preconditions.checkNotNull(newDnsName, "new address == null");
-    if (!oldDnsName.equals(newDnsName)) {
-      removeFromDnsToUuidMap(oldDnsName, uuid);
-      addToDnsToUuidMap(newDnsName, uuid);
+  private boolean updateDnsToUuidMap(
+      String oldHostName, String oldIpAddress,
+      String newHostName, String newIpAddress,
+      UUID uuid) {
+    final boolean ipChanged = !Objects.equals(oldIpAddress, newIpAddress);
+    final boolean hostNameChanged = !Objects.equals(oldHostName, newHostName);
+    if (ipChanged || hostNameChanged) {
+      synchronized (this) {
+        if (ipChanged) {
+          removeFromDnsToUuidMap(uuid, oldIpAddress);
+          addToDnsToUuidMap(uuid, newIpAddress);
+        }
+        if (hostNameChanged) {
+          removeFromDnsToUuidMap(uuid, oldHostName);
+          addToDnsToUuidMap(uuid, newHostName);
+        }
+      }
     }
+    return ipChanged || hostNameChanged;
   }
 
   /**

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMClientProtocolServer.java

@@ -1106,17 +1106,17 @@ public boolean getContainerBalancerStatus() {
 
   /**
    * Get Datanode usage info such as capacity, SCMUsed, and remaining by ip
-   * or uuid.
+   * or hostname or uuid.
    *
-   * @param ipaddress Datanode Address String
+   * @param address Datanode Address String
    * @param uuid Datanode UUID String
    * @return List of DatanodeUsageInfoProto. Each element contains usage info
    * such as capacity, SCMUsed, and remaining space.
    * @throws IOException if admin authentication fails
    */
   @Override
   public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      String ipaddress, String uuid, int clientVersion) throws IOException {
+      String address, String uuid, int clientVersion) throws IOException {
 
     // check admin authorisation
     try {
@@ -1130,8 +1130,8 @@ public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
     List<DatanodeDetails> nodes = new ArrayList<>();
     if (!Strings.isNullOrEmpty(uuid)) {
       nodes.add(scm.getScmNodeManager().getNodeByUuid(uuid));
-    } else if (!Strings.isNullOrEmpty(ipaddress)) {
-      nodes = scm.getScmNodeManager().getNodesByAddress(ipaddress);
+    } else if (!Strings.isNullOrEmpty(address)) {
+      nodes = scm.getScmNodeManager().getNodesByAddress(address);
     } else {
       throw new IOException(
           "Could not get datanode with the specified parameters."

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/node/TestSCMNodeManager.java

@@ -1794,26 +1794,6 @@ public void testHandlingSCMCommandEvent()
     }
   }
 
-  /**
-   * Test add node into network topology during node register. Datanode
-   * uses Ip address to resolve network location.
-   */
-  @Test
-  public void testScmRegisterNodeWithIpAddress()
-      throws IOException, InterruptedException, AuthenticationException {
-    testScmRegisterNodeWithNetworkTopology(false);
-  }
-
-  /**
-   * Test add node into network topology during node register. Datanode
-   * uses hostname to resolve network location.
-   */
-  @Test
-  public void testScmRegisterNodeWithHostname()
-      throws IOException, InterruptedException, AuthenticationException {
-    testScmRegisterNodeWithNetworkTopology(true);
-  }
-
   /**
    * Test add node into a 4-layer network topology during node register.
    */
@@ -1859,11 +1839,15 @@ public void testScmRegisterNodeWith4LayerNetworkTopology()
     }
   }
 
-  private void testScmRegisterNodeWithNetworkTopology(boolean useHostname)
+  @ParameterizedTest
+  @ValueSource(booleans = {true, false})
+  void testScmRegisterNodeWithNetworkTopology(boolean useHostname)
       throws IOException, InterruptedException, AuthenticationException {
     OzoneConfiguration conf = getConf();
     conf.setTimeDuration(OZONE_SCM_HEARTBEAT_PROCESS_INTERVAL, 1000,
         MILLISECONDS);
+    conf.setBoolean(DFSConfigKeysLegacy.DFS_DATANODE_USE_DN_HOSTNAME,
+        useHostname);
 
     // create table mapping file
     String[] hostNames = {"host1", "host2", "host3", "host4"};
@@ -1875,9 +1859,7 @@ private void testScmRegisterNodeWithNetworkTopology(boolean useHostname)
     conf.set(NET_TOPOLOGY_NODE_SWITCH_MAPPING_IMPL_KEY,
         "org.apache.hadoop.net.TableMapping");
     conf.set(NET_TOPOLOGY_TABLE_MAPPING_FILE_KEY, mapFile);
-    if (useHostname) {
-      conf.set(DFSConfigKeysLegacy.DFS_DATANODE_USE_DN_HOSTNAME, "true");
-    }
+
     final int nodeCount = hostNames.length;
     // use default IP address to resolve node
     try (SCMNodeManager nodeManager = createNodeManager(conf)) {
@@ -1899,13 +1881,11 @@ private void testScmRegisterNodeWithNetworkTopology(boolean useHostname)
           assertEquals("/rack1", node.getNetworkLocation()));
 
       // test get node
-      if (useHostname) {
-        Arrays.stream(hostNames).forEach(hostname -> assertNotEquals(0,
-            nodeManager.getNodesByAddress(hostname).size()));
-      } else {
-        Arrays.stream(ipAddress).forEach(ip -> assertNotEquals(0,
-            nodeManager.getNodesByAddress(ip).size()));
-      }
+      Arrays.stream(hostNames).forEach(hostname -> assertNotEquals(0,
+          nodeManager.getNodesByAddress(hostname).size()));
+
+      Arrays.stream(ipAddress).forEach(ip -> assertNotEquals(0,
+          nodeManager.getNodesByAddress(ip).size()));
     }
   }
 
@@ -1998,15 +1978,14 @@ void testGetNodesByAddress(boolean useHostname)
       }
       // test get node
       assertEquals(0, nodeManager.getNodesByAddress(null).size());
-      if (useHostname) {
-        assertEquals(2, nodeManager.getNodesByAddress("host1").size());
-        assertEquals(1, nodeManager.getNodesByAddress("host2").size());
-        assertEquals(0, nodeManager.getNodesByAddress("unknown").size());
-      } else {
-        assertEquals(2, nodeManager.getNodesByAddress("1.2.3.4").size());
-        assertEquals(1, nodeManager.getNodesByAddress("2.3.4.5").size());
-        assertEquals(0, nodeManager.getNodesByAddress("1.9.8.7").size());
-      }
+
+      assertEquals(2, nodeManager.getNodesByAddress("host1").size());
+      assertEquals(1, nodeManager.getNodesByAddress("host2").size());
+      assertEquals(0, nodeManager.getNodesByAddress("unknown").size());
+
+      assertEquals(2, nodeManager.getNodesByAddress("1.2.3.4").size());
+      assertEquals(1, nodeManager.getNodesByAddress("2.3.4.5").size());
+      assertEquals(0, nodeManager.getNodesByAddress("1.9.8.7").size());
     }
   }
 

hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/ContainerOperationClient.java

@@ -519,8 +519,8 @@ public int resetDeletedBlockRetryCount(List<Long> txIDs) throws IOException {
 
   @Override
   public List<HddsProtos.DatanodeUsageInfoProto> getDatanodeUsageInfo(
-      String ipaddress, String uuid) throws IOException {
-    return storageContainerLocationClient.getDatanodeUsageInfo(ipaddress,
+      String address, String uuid) throws IOException {
+    return storageContainerLocationClient.getDatanodeUsageInfo(address,
         uuid, ClientVersion.CURRENT_VERSION);
   }
 

hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java

@@ -52,6 +52,11 @@ public class ListInfoSubcommand extends ScmSubcommand {
       defaultValue = "")
   private String uuid;
 
+  @CommandLine.Option(names = {"--hostname"},
+      description = "Show info by datanode hostname.",
+      defaultValue = "")
+  private String hostname;
+
   @CommandLine.Option(names = {"--operational-state"},
       description = "Show info by datanode NodeOperationalState(" +
           "IN_SERVICE, " +
@@ -82,6 +87,10 @@ public void execute(ScmClient scmClient) throws IOException {
       allNodes = allNodes.filter(p -> p.getDatanodeDetails().getIpAddress()
           .compareToIgnoreCase(ipaddress) == 0);
     }
+    if (!Strings.isNullOrEmpty(hostname)) {
+      allNodes = allNodes.filter(p -> p.getDatanodeDetails().getHostName()
+          .compareToIgnoreCase(hostname) == 0);
+    }
     if (!Strings.isNullOrEmpty(uuid)) {
       allNodes = allNodes.filter(p ->
           p.getDatanodeDetails().getUuidString().equals(uuid));