AdyChechani commented on code in PR #10831:
URL: https://github.com/apache/ozone/pull/10831#discussion_r3719243860


##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/SafeModeCheckSubcommand.java:
##########
@@ -104,16 +106,16 @@ private SCMNodeInfo findLeaderNode(ScmClient scmClient) 
throws IOException {
     try {
       List<String> roles = scmClient.getScmRoles();
       for (String role : roles) {
-        String[] parts = role.split(":");
-        if (parts.length < 3 || !"LEADER".equalsIgnoreCase(parts[2])) {
+        String[] parts = HddsUtils.parseRatisRoleString(role);

Review Comment:
   The old `parts.length < 3` check doesn't apply here since 
`parseRatisRoleString()` either returns exactly 5 fields or throws; there's no 
partial result to length-check against.
   Wrapped `parseRatisRoleString()` in a try-catch for 
`IllegalArgumentException` and continue to skip malformed entries (same pattern 
used in `GetScmRatisRolesSubcommand`).



##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/SafeModeCheckSubcommand.java:
##########
@@ -176,32 +178,34 @@ private void queryNode(ScmClient scmClient, ScmNodeTarget 
targetScmNode, SCMNode
   }
 
   /**
-   * Check if the given SCMNodeInfo matches the target address.
-   * Tries to match by direct string comparison and by resolved address.
+   * Check if the given addresses match by comparing host portions and ports.
+   * Inputs may be bare hosts or host:port strings. Handles IPv6 equivalence
+   * (e.g. 2001:db8::1 vs 2001:db8:0:0:0:0:0:1) by resolving to InetAddress.
    */
   private boolean matchesAddress(String address1, String address2) {
     if (address1.equalsIgnoreCase(address2)) {
       return true;
     }
 
     try {
-      // Parse both addresses into host:port components
-      String[] parts1 = address1.split(":", 2);
-      String[] parts2 = address2.split(":", 2);
-
-      String host1 = parts1[0];
-      String host2 = parts2[0];
-      
-      // Hostnames must match
-      if (!host1.equalsIgnoreCase(host2)) {
+      String host1 = HddsUtils.getHostName(address1).orElse(address1);

Review Comment:
   `HddsUtils.getHostName()` delegates to Guava's `HostAndPort.fromString()` 
which correctly handles bare IPv6, when it sees multiple colons it treats the 
entire string as an IPv6 host with no port (which means, 
`HostAndPort.fromString("2001:db8::1").getHost()` returns **2001:db8::1**, and 
`hasPort()` returns **false**). So this is already IPv6-safe.



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