diff --git a/AGENTS.md b/AGENTS.md index 95d60ed5b73b..5bd695b1f438 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -10,7 +10,7 @@ - Reuse existing Ozone and Ratis utilities when the surrounding code already uses them. Prefer extending an existing helper over duplicating logic or adding a new one-off abstraction. - If there are multiple reasonable interpretations, state the tradeoff and ask instead of guessing. -- Do not wrap lines early just to make them look uniform. The project limit is 120 characters; use the space. +- Do not wrap lines early just to make them look uniform. The checkstyle maximum (see `hadoop-hdds/dev-support/checkstyle/checkstyle.xml`) is 120 characters for Java. Use the full 120 characters before wrapping; never break a line that fits on one line. - Use established Ozone vocabulary in code, docs, and PR text: SCM, OM, datanode, container, pipeline, volume, bucket, key, snapshot, Recon, FSO, OBS, and S3 Gateway. diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java index bf4213ea8dc4..335b8934c21d 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java @@ -228,6 +228,21 @@ public static OptionalInt getHostPort(String value) { } } + /** + * Combine a host and port into a "host:port" string, wrapping the host in + * square brackets when it is an IPv6 literal (for example + * {@code [2001:db8::1]:9858}). A bare IPv6 literal joined to a port with a + * plain colon is ambiguous and cannot be parsed by Ratis/gRPC targets or + * URI-based address parsers. + * + * @param host a hostname, IPv4 literal, or (bracketed or bare) IPv6 literal + * @param port the port number + * @return the combined address, bracketed for IPv6 literals + */ + public static String getHostPortString(String host, int port) { + return HostAndPort.fromParts(host, port).toString(); + } + /** * Retrieve a number, trying the supplied config keys in order. * Each config value may be absent diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/NodeDetails.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/NodeDetails.java index 0984048bd513..ae871a8a86fd 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/NodeDetails.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/NodeDetails.java @@ -103,11 +103,7 @@ public String getHostAddress() { } public String getRatisHostPortStr() { - StringBuilder hostPort = new StringBuilder(); - hostPort.append(getHostName()) - .append(':') - .append(ratisPort); - return hostPort.toString(); + return HddsUtils.getHostPortString(getHostName(), ratisPort); } public int getRatisPort() { diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java index b2813bad1e2e..4a30248f4a76 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java @@ -36,6 +36,7 @@ import java.util.stream.Collectors; import javax.net.ssl.TrustManager; import org.apache.hadoop.hdds.HddsConfigKeys; +import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -125,18 +126,13 @@ public static UUID toDatanodeId(RaftProtos.RaftPeerProto peerId) { } private static String toRaftPeerAddress(DatanodeDetails id, Port.Name port) { - if (datanodeUseHostName()) { - final String address = - id.getHostName() + ":" + id.getPort(port).getValue(); - LOG.debug("Datanode is using hostname for raft peer address: {}", - address); - return address; - } else { - final String address = - id.getIpAddress() + ":" + id.getPort(port).getValue(); - LOG.debug("Datanode is using IP for raft peer address: {}", address); - return address; - } + final boolean useHostName = datanodeUseHostName(); + final String address = HddsUtils.getHostPortString( + useHostName ? id.getHostName() : id.getIpAddress(), + id.getPort(port).getValue()); + LOG.debug("Datanode is using {} for raft peer address: {}", + useHostName ? "hostname" : "IP", address); + return address; } public static RaftPeerId toRaftPeerId(DatanodeDetails id) { diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java index 650db8c5439f..95e2cb2d5645 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/TestHddsUtils.java @@ -59,6 +59,21 @@ void testGetHostName() { HddsUtils.getHostName(":1234")); } + @Test + void testGetHostPortString() { + // Hostnames and IPv4 literals are joined with a plain colon. + assertEquals("host1:9858", HddsUtils.getHostPortString("host1", 9858)); + assertEquals("1.2.3.4:9858", HddsUtils.getHostPortString("1.2.3.4", 9858)); + + // Bare IPv6 literals must be bracketed so the result is an unambiguous + // Ratis/gRPC target. + assertEquals("[2001:db8::1]:9858", HddsUtils.getHostPortString("2001:db8::1", 9858)); + assertEquals("[::1]:9858", HddsUtils.getHostPortString("::1", 9858)); + + // Already-bracketed IPv6 literals keep a single pair of brackets. + assertEquals("[2001:db8::1]:9858", HddsUtils.getHostPortString("[2001:db8::1]", 9858)); + } + static List validPaths() { return Arrays.asList( Arguments.of("/", "/"), diff --git a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestRatisHelper.java b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestRatisHelper.java index 2f10f550bb5a..aab920954f72 100644 --- a/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestRatisHelper.java +++ b/hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/ratis/TestRatisHelper.java @@ -21,7 +21,11 @@ import static org.junit.jupiter.api.Assertions.assertNull; import org.apache.hadoop.hdds.conf.OzoneConfiguration; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.DatanodeID; +import org.apache.hadoop.hdds.protocol.MockDatanodeDetails; import org.apache.ratis.conf.RaftProperties; +import org.apache.ratis.protocol.RaftPeer; import org.junit.jupiter.api.Test; /** @@ -122,4 +126,15 @@ public void testCreateRaftServerProperties() { assertNull(raftProperties.get("raft.client.rpc.request.timeout")); } + + @Test + public void testRaftPeerAddressBracketsIpv6() { + // hdds.datanode.use.datanode.hostname defaults to false, so the datanode + // IP is used for the raft peer address. An IPv6 literal must be bracketed + // for the Ratis/gRPC peer target to be parsed correctly. + DatanodeDetails dn = MockDatanodeDetails.createDatanodeDetails( + DatanodeID.randomID(), "dn-ipv6", "2001:db8::1", "/default-rack"); + RaftPeer peer = RatisHelper.toRaftPeer(dn); + assertEquals("[2001:db8::1]:0", peer.getAddress()); + } }