chihsuan commented on code in PR #10831:
URL: https://github.com/apache/ozone/pull/10831#discussion_r3682197542
##########
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()` is still not IPv6-safe here. For example, it
treats the final `:1` in the bare IPv6 address `2001:db8::1` as a port and
returns `2001:db8:`.
Could we extract the host with Guava's `HostAndPort.fromString()`?
https://github.com/apache/ozone/blob/06d972484dc9d9b643d363711d6378e09f80a140/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java#L202-L212
##########
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:
This could throw an unchecked `IllegalArgumentException` for malformed
entries, such as "" or "host:9894". The old `parts.length < 3` check skipped
such entries, should we preserve the previous behavior?
##########
hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java:
##########
@@ -243,6 +243,65 @@ public static String getHostPortString(String host, int
port) {
return HostAndPort.fromParts(host, port).toString();
}
+ /**
+ * Parse a Ratis role string produced by
+ * {@code SCMRatisServerImpl.getRatisRoles()} into its constituent fields.
+ * The format is {@code [host]:port:ROLE:id:hostIP} where host and hostIP
+ * may be bracketed IPv6 literals.
+ *
+ * @param roleString the encoded role string
+ * @return a 5-element array: {host, port, role, id, hostIP}
+ */
+ public static String[] parseRatisRoleString(String roleString) {
+ Preconditions.checkArgument(roleString != null && !roleString.isEmpty(),
+ "Role string must not be null or empty");
+
+ // Parse from the right: the last field is hostIP (possibly bracketed),
+ // then id (uuid, no colons), then role (LEADER/FOLLOWER, no colons),
+ // and the remainder is host:port (which may be bracketed IPv6).
+ int idx = roleString.length();
+
+ // Field 5: hostIP — may be bracketed IPv6 like [2001:db8::1]
+ String hostIp;
+ if (roleString.charAt(idx - 1) == ']') {
+ int bracket = roleString.lastIndexOf('[');
+ Preconditions.checkArgument(bracket > 0,
+ "Malformed role string (unmatched bracket): %s", roleString);
+ hostIp = roleString.substring(bracket + 1, idx - 1);
+ idx = bracket - 1;
Review Comment:
This assumes the character before `[` is the `:` separator without checking.
Would it be worth adding a `Preconditions.checkArgument` here?
--
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]
