Copilot commented on code in PR #10755:
URL: https://github.com/apache/ozone/pull/10755#discussion_r3595376205


##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DiskBalancerSubCommandUtil.java:
##########
@@ -95,31 +159,33 @@ public static Map<String, String> 
getAllOperableNodesClientRpcAddress(
       if (node.getNodeStates(0).equals(HddsProtos.NodeState.DEAD)) {
         continue;
       }
-      Port port = details.getPort(Port.Name.CLIENT_RPC);
-      if (port != null) {
-        String address = details.getIpAddress() + ":" + port.getValue();
-        // Format the display string: "hostname (ip:port)" or "ip:port"
-        String hostname = details.getHostName();
-        String display = (hostname != null && !hostname.isEmpty()
-            && !hostname.equals(details.getIpAddress())) ? hostname + " (" + 
address + ")"
-            : address;
-        addressToDisplay.put(address, display);
-      } else {
-        System.out.printf("host: %s(%s) %s port not found%n",
-            details.getHostName(), details.getIpAddress(),
-            Port.Name.CLIENT_RPC.name());
+      try {
+        String address = getClientRpcAddress(details);
+        addressToDisplay.put(address, getDatanodeHostAndIp(node.getNodeID()));
+      } catch (IOException e) {
+        System.out.println(e.getMessage());
       }

Review Comment:
   getAllOperableNodesClientRpcAddress writes resolution errors to stdout. In 
JSON mode (eg `--json --in-service-datanodes`), any stdout output before the 
JSON payload will corrupt the JSON output. Send these messages to stderr 
instead.



##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/AbstractDiskBalancerSubCommand.java:
##########
@@ -154,11 +184,51 @@ private List<String> getTargetDatanodes() {
     if (options.isInServiceDatanodes()) {
       return getAllInServiceDatanodes();
     } else {
-      datanodeDisplayNames = null;  // Non-batch: use user input as-is, no SCM 
for formatting
-      return options.getDatanodes();
+      return resolveExplicitDatanodeAddresses(options.getDatanodes());
     }
   }
 
+  /**
+   * Resolves datanode UUIDs to CLIENT_RPC addresses via SCM.
+   * Hostname and host:port arguments are passed through unchanged.
+   */
+  private List<String> resolveExplicitDatanodeAddresses(List<String> nodeArgs) 
{
+    if 
(nodeArgs.stream().noneMatch(DiskBalancerSubCommandUtil::isDatanodeUuid)) {
+      datanodeDisplayNames = null;
+      return nodeArgs;
+    }
+
+    Map<String, String> displayNames = new LinkedHashMap<>();
+    List<String> resolvedAddresses = new ArrayList<>();
+    try (ScmClient scmClient = new ContainerOperationClient(new 
OzoneConfiguration())) {
+      for (String nodeArg : nodeArgs) {
+        try {
+          DiskBalancerSubCommandUtil.DatanodeTarget target =
+              DiskBalancerSubCommandUtil.resolveDatanodeTarget(scmClient, 
nodeArg);
+          resolvedAddresses.add(target.getClientRpcAddress());
+          displayNames.put(target.getClientRpcAddress(), 
target.getDisplayName());
+        } catch (IOException e) {
+          addResolutionFailure(nodeArg, e.getMessage());
+        }
+      }
+    } catch (IOException e) {
+      System.err.printf("Error resolving datanode address(es). %n%s%n", 
e.getMessage());
+      return null;
+    }

Review Comment:
   If SCM client creation fails (eg SCM down) while resolving UUIDs, this 
method returns null and the whole command aborts, even though non-UUID 
host/host:port arguments could still be executed. This also prevents per-UUID 
failures from being reported in JSON mode. Prefer falling back to executing 
non-UUID arguments and recording UUID resolution failures when SCM cannot be 
contacted.



##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DatanodeParameters.java:
##########
@@ -43,10 +43,11 @@ public class DatanodeParameters extends ItemsFromStdin {
           "  # From file having list of dns to balance",
           "  ozone admin datanode diskbalancer report - < datanode-lists.txt",
           "Port is optional and defaults to 19864 (CLIENT_RPC port).",
-          "Address examples: 'DN-1', 'DN-1:19864', '192.168.1.10'."
+          "Address examples: 'DN-1', 'DN-1:19864', '192.168.1.10',",
+          "  'a3b63511-bdf8-4fa1-8ab6-d19c0e806f84' (datanode UUID)."
       },
       arity = "0..*",
-      paramLabel = "<datanode address>")
+      paramLabel = "<datanode address or id>")

Review Comment:
   The CLI help text currently prints a trailing comma at the end of the 
"Address examples" line (",") which looks like a formatting artifact in the 
rendered usage output. Consider removing the comma and ending the sentence 
cleanly.



-- 
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.

To unsubscribe, e-mail: [email protected]

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