From 981ad093b20d06834483db184b5f39fba3469578 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Tue, 18 Aug 2026 23:23:07 +0800 Subject: [PATCH 1/2] HDDS-16138. Add compatibility coverage for SCM Ratis role output over IPv6 The encoded SCM Ratis role string is a wire format shared by the roles CLI, safe-mode leader detection, the JMX view behind the SCM web UI, and Recon's snapshot download. After HDDS-15774 made the encoding IPv6-safe, only the producer and the roles table were covered, so a regression in the encoding or in any of the other consumers would surface as a runtime failure in an IPv6 deployment rather than as a test failure. Leader detection in `ozone admin safemode status` could not be reached from a test because the subcommand builds its own client, so findLeaderNode now takes the node list as a parameter instead of reading the field. --- .../hdds/scm/ha/TestSCMRatisServerImpl.java | 93 ++++++++++--- ...TestStorageContainerManagerRatisRoles.java | 127 ++++++++++++++++++ .../hdds/scm/cli/SafeModeCheckSubcommand.java | 10 +- .../scm/cli/TestSafeModeCheckSubcommand.java | 124 +++++++++++++++++ .../scm/TestGetScmRatisRolesSubcommand.java | 85 +++++++++++- ...stStorageContainerServiceProviderImpl.java | 53 ++++++++ 6 files changed, 467 insertions(+), 25 deletions(-) create mode 100644 hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerRatisRoles.java create mode 100644 hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/TestSafeModeCheckSubcommand.java diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMRatisServerImpl.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMRatisServerImpl.java index 56b99fb8f657..8ff9a2fb00a7 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMRatisServerImpl.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/ha/TestSCMRatisServerImpl.java @@ -17,9 +17,9 @@ package org.apache.hadoop.hdds.scm.ha; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNull; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -28,9 +28,10 @@ import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; +import java.util.Arrays; import java.util.List; import java.util.UUID; -import org.apache.hadoop.hdds.HddsUtils; +import java.util.stream.Stream; import org.apache.hadoop.hdds.conf.ConfigurationSource; import org.apache.hadoop.hdds.scm.server.StorageContainerManager; import org.apache.hadoop.hdds.security.SecurityConfig; @@ -41,6 +42,9 @@ import org.apache.ratis.protocol.RaftPeerId; import org.apache.ratis.server.RaftServer; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; import org.mockito.MockedConstruction; import org.mockito.MockedStatic; @@ -108,8 +112,68 @@ public void testGetLeaderId() throws Exception { } } + static Stream peerAddressEncodings() { + return Stream.of( + Arguments.of("10.0.0.1:9894", "10.0.0.1:9894:LEADER:peer1:10.0.0.1"), + Arguments.of("[2001:db8::1]:9894", "[2001:db8::1]:9894:LEADER:peer1:[2001:db8::1]")); + } + + /** + * The encoding is a wire format shared with every consumer of + * {@code ozone admin scm roles}, so it is asserted verbatim rather than + * round-tripped through the parser that reads it back. + */ + @ParameterizedTest + @MethodSource("peerAddressEncodings") + public void testGetRatisRolesEncoding(String peerAddress, String expectedRole) throws Exception { + try ( + MockedConstruction mockedSecurityConfigConstruction = mockConstruction(SecurityConfig.class); + MockedStatic staticMockedRaftServer = mockStatic(RaftServer.class); + MockedStatic staticMockedRatisUtil = mockStatic(RatisUtil.class); + ) { + ConfigurationSource conf = mock(ConfigurationSource.class); + StorageContainerManager scm = mock(StorageContainerManager.class); + when(scm.getClusterId()).thenReturn("CID-" + UUID.randomUUID()); + SCMHADBTransactionBuffer dbTransactionBuffer = mock(SCMHADBTransactionBuffer.class); + + RaftServer.Builder raftServerBuilder = mock(RaftServer.Builder.class); + when(raftServerBuilder.setServerId(any())).thenReturn(raftServerBuilder); + when(raftServerBuilder.setProperties(any())).thenReturn(raftServerBuilder); + when(raftServerBuilder.setStateMachineRegistry(any())).thenReturn(raftServerBuilder); + when(raftServerBuilder.setOption(any())).thenReturn(raftServerBuilder); + when(raftServerBuilder.setGroup(any())).thenReturn(raftServerBuilder); + when(raftServerBuilder.setParameters(any())).thenReturn(raftServerBuilder); + + RaftServer raftServer = mock(RaftServer.class); + RaftServer.Division division = mock(RaftServer.Division.class); + when(raftServer.getDivision(any())).thenReturn(division); + when(raftServerBuilder.build()).thenReturn(raftServer); + staticMockedRaftServer.when(RaftServer::newBuilder).thenReturn(raftServerBuilder); + + RaftProperties raftProperties = mock(RaftProperties.class); + staticMockedRatisUtil.when(() -> RatisUtil.newRaftProperties(conf)).thenReturn(raftProperties); + + SecurityConfig sc = new SecurityConfig(conf); + when(sc.isSecurityEnabled()).thenReturn(false); + + SCMRatisServerImpl scmRatisServer = spy(new SCMRatisServerImpl(conf, scm, dbTransactionBuffer)); + + RaftPeer peer = RaftPeer.newBuilder() + .setId(RaftPeerId.valueOf("peer1")) + .setAddress(peerAddress) + .build(); + + when(division.getGroup()).thenReturn(RaftGroup.valueOf(RaftGroupId.randomId(), peer)); + doReturn(peer).when(scmRatisServer).getLeader(); + + List roles = scmRatisServer.getRatisRoles(); + + assertEquals(Arrays.asList(expectedRole), roles); + } + } + @Test - public void testGetRatisRolesWithIPv6() throws Exception { + public void testGetRatisRolesKeepsHostname() throws Exception { try ( MockedConstruction mockedSecurityConfigConstruction = mockConstruction(SecurityConfig.class); MockedStatic staticMockedRaftServer = mockStatic(RaftServer.class); @@ -142,26 +206,21 @@ public void testGetRatisRolesWithIPv6() throws Exception { SCMRatisServerImpl scmRatisServer = spy(new SCMRatisServerImpl(conf, scm, dbTransactionBuffer)); - // IPv6 peer address in the bracketed format that getRatisHostPortStr() produces - RaftPeer ipv6Peer = RaftPeer.newBuilder() + RaftPeer peer = RaftPeer.newBuilder() .setId(RaftPeerId.valueOf("peer1")) - .setAddress("[2001:db8::1]:9894") + .setAddress("localhost:9894") .build(); - RaftGroup raftGroup = RaftGroup.valueOf(RaftGroupId.randomId(), ipv6Peer); - when(division.getGroup()).thenReturn(raftGroup); - doReturn(ipv6Peer).when(scmRatisServer).getLeader(); + when(division.getGroup()).thenReturn(RaftGroup.valueOf(RaftGroupId.randomId(), peer)); + doReturn(peer).when(scmRatisServer).getLeader(); List roles = scmRatisServer.getRatisRoles(); - assertEquals(1, roles.size()); - String roleString = roles.get(0); - String[] parsed = HddsUtils.parseRatisRoleString(roleString); - assertEquals("2001:db8::1", parsed[0]); - assertEquals("9894", parsed[1]); - assertEquals("LEADER", parsed[2]); - assertEquals("peer1", parsed[3]); - assertTrue(!parsed[4].isEmpty(), "hostIP should be resolved"); + // Which address a name resolves to is the resolver's business, so only + // the fields the encoding itself owns are pinned here. + assertEquals(1, roles.size()); + assertThat(roles.get(0)).startsWith("localhost:9894:LEADER:peer1:"); + assertThat(roles.get(0)).doesNotEndWith(":"); } } diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerRatisRoles.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerRatisRoles.java new file mode 100644 index 000000000000..b745a423406b --- /dev/null +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerRatisRoles.java @@ -0,0 +1,127 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.scm.server; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.doCallRealMethod; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; +import org.apache.hadoop.hdds.scm.ha.SCMHAManager; +import org.apache.hadoop.hdds.scm.ha.SCMRatisServer; +import org.apache.ratis.protocol.RaftPeerId; +import org.apache.ratis.server.DivisionInfo; +import org.apache.ratis.server.RaftServer; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Tests the SCM roles exposed over JMX, which the SCM web UI renders. The + * columns are reordered relative to the encoded role string, so the mapping + * is asserted field by field. + */ +public class TestStorageContainerManagerRatisRoles { + + private static final String LEADER_ID = "e428ca07-b2a3-4756-bf9b-a4abb033c7d1"; + private static final String FOLLOWER_ID = "61b1c8e5-da40-4567-8a17-96a0234ba14e"; + + private static StorageContainerManager scmWith(SCMRatisServer server) { + SCMHAManager haManager = mock(SCMHAManager.class); + when(haManager.getRatisServer()).thenReturn(server); + + StorageContainerManager scm = mock(StorageContainerManager.class); + when(scm.getScmHAManager()).thenReturn(haManager); + doCallRealMethod().when(scm).getScmRatisRoles(); + return scm; + } + + private static SCMRatisServer ratisServer(RaftPeerId leaderId, List roles) { + DivisionInfo info = mock(DivisionInfo.class); + when(info.getLeaderId()).thenReturn(leaderId); + + RaftServer.Division division = mock(RaftServer.Division.class); + when(division.getInfo()).thenReturn(info); + + SCMRatisServer server = mock(SCMRatisServer.class); + when(server.getDivision()).thenReturn(division); + when(server.isStopped()).thenReturn(false); + when(server.getRatisRoles()).thenReturn(roles); + return server; + } + + private static SCMRatisServer healthyServer(List roles) { + return ratisServer(RaftPeerId.valueOf("scm1"), roles); + } + + @Test + public void testGetScmRatisRolesIPv4() { + StorageContainerManager scm = scmWith(healthyServer(Arrays.asList( + "scm1.example.com:9894:LEADER:" + LEADER_ID + ":10.0.0.1", + "scm2.example.com:9894:FOLLOWER:" + FOLLOWER_ID + ":10.0.0.2"))); + + assertEquals( + Arrays.asList( + Arrays.asList("scm1.example.com", LEADER_ID, "9894", "LEADER"), + Arrays.asList("scm2.example.com", FOLLOWER_ID, "9894", "FOLLOWER")), + scm.getScmRatisRoles()); + } + + @Test + public void testGetScmRatisRolesIPv6() { + StorageContainerManager scm = scmWith(healthyServer(Arrays.asList( + "[2001:db8::1]:9894:LEADER:" + LEADER_ID + ":[2001:db8:0:0:0:0:0:1]", + "[2001:db8::2]:9894:FOLLOWER:" + FOLLOWER_ID + ":[2001:db8:0:0:0:0:0:2]"))); + + // The host column is a display value, so the brackets that make the + // encoded field unambiguous are stripped back off. + assertEquals( + Arrays.asList( + Arrays.asList("2001:db8::1", LEADER_ID, "9894", "LEADER"), + Arrays.asList("2001:db8::2", FOLLOWER_ID, "9894", "FOLLOWER")), + scm.getScmRatisRoles()); + } + + @Test + public void testGetScmRatisRolesWithoutLeader() { + StorageContainerManager scm = scmWith(ratisServer(null, Collections.emptyList())); + + assertEquals(Collections.singletonList(Collections.singletonList("No leader found")), + scm.getScmRatisRoles()); + } + + /** + * An empty entry is what the producer emits for a peer with no address, so + * it reaches this view like any other role string. + */ + @ParameterizedTest + @ValueSource(strings = {"", "scm1.example.com:9894"}) + public void testGetScmRatisRolesWithUnparseableRole(String role) { + StorageContainerManager scm = scmWith(healthyServer(Collections.singletonList(role))); + + List> roles = scm.getScmRatisRoles(); + + assertEquals(1, roles.size()); + assertEquals(1, roles.get(0).size()); + assertThat(roles.get(0).get(0)).startsWith("Exception Occurred, "); + } +} diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/SafeModeCheckSubcommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/SafeModeCheckSubcommand.java index 7af5ea3d0c63..d6b4c97822e5 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/SafeModeCheckSubcommand.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/SafeModeCheckSubcommand.java @@ -17,6 +17,7 @@ package org.apache.hadoop.hdds.scm.cli; +import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.net.InetAddress; import java.util.List; @@ -85,7 +86,7 @@ private void executeForSingleNode(ScmClient scmClient, ScmNodeTarget targetScmNo SCMNodeInfo targetNode; if (serviceId != null) { // HA mode: find leader - targetNode = findLeaderNode(scmClient); + targetNode = findLeaderNode(scmClient, nodes); if (targetNode == null) { throw new IOException("Could not determine leader node"); } @@ -100,9 +101,12 @@ private void executeForSingleNode(ScmClient scmClient, ScmNodeTarget targetScmNo /** * Find the leader node from SCM roles. * @param scmClient the SCM client + * @param nodes the SCM nodes configured for the service * @return the leader SCMNodeInfo */ - private SCMNodeInfo findLeaderNode(ScmClient scmClient) throws IOException { + @VisibleForTesting + static SCMNodeInfo findLeaderNode(ScmClient scmClient, List nodes) + throws IOException { try { List roles = scmClient.getScmRoles(); for (String role : roles) { @@ -187,7 +191,7 @@ private void queryNode(ScmClient scmClient, ScmNodeTarget targetScmNode, SCMNode * 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) { + private static boolean matchesAddress(String address1, String address2) { if (address1.equalsIgnoreCase(address2)) { return true; } diff --git a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/TestSafeModeCheckSubcommand.java b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/TestSafeModeCheckSubcommand.java new file mode 100644 index 000000000000..173b84f76bc1 --- /dev/null +++ b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/TestSafeModeCheckSubcommand.java @@ -0,0 +1,124 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.hadoop.hdds.scm.cli; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import java.io.IOException; +import java.util.Arrays; +import java.util.List; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.ha.SCMNodeInfo; +import org.apache.hadoop.hdds.scm.net.HostAndPort; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; + +/** + * Tests leader detection in `ozone admin safemode status`, which picks its + * target node by parsing the Ratis role strings returned by SCM. + */ +public class TestSafeModeCheckSubcommand { + + private static final String LEADER_ID = "e428ca07-b2a3-4756-bf9b-a4abb033c7d1"; + private static final String FOLLOWER_ID = "61b1c8e5-da40-4567-8a17-96a0234ba14e"; + + private static SCMNodeInfo node(String nodeId, String host) { + HostAndPort address = new HostAndPort(host, 9860); + return new SCMNodeInfo("scmservice", nodeId, address, address, address, address); + } + + private static SCMNodeInfo findLeader(List roles, List nodes) + throws IOException { + ScmClient scmClient = mock(ScmClient.class); + when(scmClient.getScmRoles()).thenReturn(roles); + + return SafeModeCheckSubcommand.findLeaderNode(scmClient, nodes); + } + + @Test + public void testFindLeaderNodeIPv4() throws Exception { + SCMNodeInfo scm1 = node("scm1", "10.0.0.1"); + SCMNodeInfo scm2 = node("scm2", "10.0.0.2"); + + SCMNodeInfo leader = findLeader( + Arrays.asList( + "scm2.example.com:9894:FOLLOWER:" + FOLLOWER_ID + ":10.0.0.2", + "scm1.example.com:9894:LEADER:" + LEADER_ID + ":10.0.0.1"), + Arrays.asList(scm1, scm2)); + + // The configured nodes carry addresses, not DNS names, so the match comes + // from the trailing resolved-address field rather than the host field. + assertEquals(scm1, leader); + } + + @Test + public void testFindLeaderNodeIPv6() throws Exception { + SCMNodeInfo scm1 = node("scm1", "2001:db8::1"); + SCMNodeInfo scm2 = node("scm2", "2001:db8::2"); + + SCMNodeInfo leader = findLeader( + Arrays.asList( + "[2001:db8::2]:9894:FOLLOWER:" + FOLLOWER_ID + ":[2001:db8:0:0:0:0:0:2]", + "[2001:db8::1]:9894:LEADER:" + LEADER_ID + ":[2001:db8:0:0:0:0:0:1]"), + Arrays.asList(scm1, scm2)); + + assertEquals(scm1, leader); + } + + @Test + public void testFindLeaderNodeMatchesEquivalentIPv6Forms() throws Exception { + // SCM reports the compressed form while the node is configured with the + // expanded one; the two denote the same address and must still match. + SCMNodeInfo scm1 = node("scm1", "2001:db8:0:0:0:0:0:1"); + + SCMNodeInfo leader = findLeader( + Arrays.asList("[2001:db8::1]:9894:LEADER:" + LEADER_ID + ":[2001:db8::1]"), + Arrays.asList(scm1)); + + assertEquals(scm1, leader); + } + + @Test + public void testFindLeaderNodeWithoutLeader() throws Exception { + SCMNodeInfo leader = findLeader( + Arrays.asList("[2001:db8::1]:9894:FOLLOWER:" + FOLLOWER_ID + ":[2001:db8::1]"), + Arrays.asList(node("scm1", "2001:db8::1"))); + + assertNull(leader); + } + + /** + * An empty entry is what the producer emits for a peer with no address, so + * it reaches leader detection like any other role string. + */ + @ParameterizedTest + @ValueSource(strings = {"", "Exception Occurred, No leader found"}) + public void testFindLeaderNodeSkipsUnparseableRole(String unparseable) throws Exception { + SCMNodeInfo scm1 = node("scm1", "2001:db8::1"); + + SCMNodeInfo leader = findLeader( + Arrays.asList(unparseable, "[2001:db8::1]:9894:LEADER:" + LEADER_ID + ":[2001:db8::1]"), + Arrays.asList(scm1)); + + assertEquals(scm1, leader); + } +} diff --git a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/scm/TestGetScmRatisRolesSubcommand.java b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/scm/TestGetScmRatisRolesSubcommand.java index 52c4a4cda2e9..948d7ad0f3c8 100644 --- a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/scm/TestGetScmRatisRolesSubcommand.java +++ b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/scm/TestGetScmRatisRolesSubcommand.java @@ -118,11 +118,86 @@ public void testGetScmHARatisRolesIPv6() throws Exception { try (GenericTestUtils.SystemOutCapturer capture = new GenericTestUtils.SystemOutCapturer()) { cmd.execute(client); - assertThat(capture.getOutput()).contains("2001:db8::1"); - assertThat(capture.getOutput()).contains("9894"); - assertThat(capture.getOutput()).contains("LEADER"); - assertThat(capture.getOutput()).contains("e428ca07-b2a3-4756-bf9b-a4abb033c7d1"); - assertThat(capture.getOutput()).contains("FOLLOWER"); + // Column widths depend on the rendered content, but the column order and + // the value in each one must not shift. + assertThat(capture.getOutput()).containsPattern( + "\\|\\s+2001:db8::1\\s+\\|\\s+9894\\s+\\|\\s+LEADER\\s+\\|" + + "\\s+e428ca07-b2a3-4756-bf9b-a4abb033c7d1\\s+\\|\\s+2001:db8:0:0:0:0:0:1\\s+\\|"); + assertThat(capture.getOutput()).containsPattern( + "\\|\\s+2001:db8::2\\s+\\|\\s+9894\\s+\\|\\s+FOLLOWER\\s+\\|" + + "\\s+61b1c8e5-da40-4567-8a17-96a0234ba14e\\s+\\|\\s+2001:db8:0:0:0:0:0:2\\s+\\|"); + } + } + + @Test + public void testGetScmRatisRolesJson() throws Exception { + GetScmRatisRolesSubcommand cmd = new GetScmRatisRolesSubcommand(); + ScmClient client = mock(ScmClient.class); + CommandLine c = new CommandLine(cmd); + c.parseArgs("--json"); + + List result = new ArrayList<>(); + result.add("scm1.example.com:9894:LEADER:e428ca07-b2a3-4756-bf9b-a4abb033c7d1:10.0.0.1"); + + when(client.getScmRoles()).thenAnswer(invocation -> result); + + try (GenericTestUtils.SystemOutCapturer capture = + new GenericTestUtils.SystemOutCapturer()) { + cmd.execute(client); + assertThat(JsonUtils.readTree(capture.getOutput())).isEqualTo( + JsonUtils.readTree("{\"scm1.example.com\":{" + + "\"address\":\"scm1.example.com:9894\"," + + "\"raftPeerRole\":\"LEADER\"," + + "\"ID\":\"e428ca07-b2a3-4756-bf9b-a4abb033c7d1\"," + + "\"InetAddress\":\"10.0.0.1\"}}")); + } + } + + @Test + public void testGetScmHARatisRolesIPv6Json() throws Exception { + GetScmRatisRolesSubcommand cmd = new GetScmRatisRolesSubcommand(); + ScmClient client = mock(ScmClient.class); + CommandLine c = new CommandLine(cmd); + c.parseArgs("--json"); + + List result = new ArrayList<>(); + result.add("[2001:db8::1]:9894:LEADER:e428ca07-b2a3-4756-bf9b-a4abb033c7d1:[2001:db8:0:0:0:0:0:1]"); + + when(client.getScmRoles()).thenAnswer(invocation -> result); + + try (GenericTestUtils.SystemOutCapturer capture = + new GenericTestUtils.SystemOutCapturer()) { + cmd.execute(client); + // The address is a connectable target so it keeps its brackets; the + // resolved literal in InetAddress is a bare address and stays unbracketed. + assertThat(JsonUtils.readTree(capture.getOutput())).isEqualTo( + JsonUtils.readTree("{\"2001:db8::1\":{" + + "\"address\":\"[2001:db8::1]:9894\"," + + "\"raftPeerRole\":\"LEADER\"," + + "\"ID\":\"e428ca07-b2a3-4756-bf9b-a4abb033c7d1\"," + + "\"InetAddress\":\"2001:db8:0:0:0:0:0:1\"}}")); + } + } + + @Test + public void testGetScmRatisRolesDefaultOutputIsVerbatim() throws Exception { + GetScmRatisRolesSubcommand cmd = new GetScmRatisRolesSubcommand(); + ScmClient client = mock(ScmClient.class); + CommandLine c = new CommandLine(cmd); + c.parseArgs(); + + List result = new ArrayList<>(); + result.add("scm1.example.com:9894:LEADER:e428ca07-b2a3-4756-bf9b-a4abb033c7d1:10.0.0.1"); + result.add("[2001:db8::1]:9894:FOLLOWER:61b1c8e5-da40-4567-8a17-96a0234ba14e:[2001:db8:0:0:0:0:0:1]"); + + when(client.getScmRoles()).thenAnswer(invocation -> result); + + try (GenericTestUtils.SystemOutCapturer capture = + new GenericTestUtils.SystemOutCapturer()) { + cmd.execute(client); + for (String role : result) { + assertThat(capture.getOutput()).contains(role); + } } } diff --git a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestStorageContainerServiceProviderImpl.java b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestStorageContainerServiceProviderImpl.java index 6bf88bc216cc..83de7a0917e5 100644 --- a/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestStorageContainerServiceProviderImpl.java +++ b/hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestStorageContainerServiceProviderImpl.java @@ -17,9 +17,13 @@ package org.apache.hadoop.ozone.recon.spi.impl; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.fail; +import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockConstruction; +import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -29,17 +33,26 @@ import com.google.inject.Injector; import java.io.File; import java.io.IOException; +import java.util.Arrays; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.atomic.AtomicReference; import org.apache.hadoop.hdds.HddsConfigKeys; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.scm.ScmInfo; +import org.apache.hadoop.hdds.scm.ha.InterSCMGrpcClient; import org.apache.hadoop.hdds.scm.pipeline.Pipeline; import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.protocol.StorageContainerLocationProtocol; +import org.apache.hadoop.hdds.utils.HddsServerUtil; import org.apache.hadoop.ozone.recon.ReconUtils; +import org.apache.hadoop.ozone.recon.security.ReconCertificateClient; import org.apache.hadoop.ozone.recon.spi.StorageContainerServiceProvider; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.io.TempDir; +import org.mockito.MockedConstruction; +import org.mockito.MockedStatic; /** * Class to test StorageContainerServiceProviderImpl APIs. @@ -98,4 +111,44 @@ public void testGetPipeline() throws IOException { verify(scmClient, times(1)) .getPipeline(pipelineID); } + + /** + * Recon downloads the SCM snapshot from whichever peer reports itself as + * LEADER, using the resolved address carried in the encoded role string. + */ + @Test + public void testGetSCMDBSnapshotConnectsToLeader() throws Exception { + StorageContainerServiceProvider scmProvider = + injector.getInstance(StorageContainerServiceProvider.class); + StorageContainerLocationProtocol scmClient = + injector.getInstance(StorageContainerLocationProtocol.class); + + when(scmClient.getScmInfo()).thenReturn(new ScmInfo.Builder() + .setClusterId("CID-6a1b5b1e-3f5a-4b5c-8f6d-2a1c3e4f5a6b") + .setScmId("scm1") + .setPeerRoles(Arrays.asList( + "[2001:db8::2]:9894:FOLLOWER:61b1c8e5-da40-4567-8a17-96a0234ba14e:[2001:db8:0:0:0:0:0:2]", + "[2001:db8::1]:9894:LEADER:e428ca07-b2a3-4756-bf9b-a4abb033c7d1:[2001:db8:0:0:0:0:0:1]")) + .build()); + + AtomicReference downloadHost = new AtomicReference<>(); + try (MockedStatic stubbedServerUtil = mockStatic(HddsServerUtil.class); + MockedConstruction stubbedCertClient = + mockConstruction(ReconCertificateClient.class); + MockedConstruction downloaders = + mockConstruction(InterSCMGrpcClient.class, (downloader, context) -> { + downloadHost.set((String) context.arguments().get(0)); + when(downloader.download(any())) + .thenReturn(CompletableFuture.completedFuture(null)); + })) { + scmProvider.getSCMDBSnapshot(); + + assertEquals(1, downloaders.constructed().size()); + } + + // The follower is listed first, so this also pins that the LEADER entry is + // the one selected. The resolved literal is handed over verbatim, unbracketed. + assertEquals("2001:db8:0:0:0:0:0:1", downloadHost.get()); + } + } From 2ceb775a28e48080d5ab968fcdd3b7927d4df1d1 Mon Sep 17 00:00:00 2001 From: rjgoyln Date: Sat, 22 Aug 2026 11:04:42 +0800 Subject: [PATCH 2/2] HDDS-16138. Address review feedback on empty role-string test comments SCM sets an address on every RaftPeer it builds, so describing the empty entry as producer output overstated a path the code does not reach. The cases stay as malformed-input coverage. --- .../hdds/scm/server/TestStorageContainerManagerRatisRoles.java | 3 +-- .../hadoop/hdds/scm/cli/TestSafeModeCheckSubcommand.java | 3 +-- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerRatisRoles.java b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerRatisRoles.java index b745a423406b..e3c9e35e121e 100644 --- a/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerRatisRoles.java +++ b/hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerRatisRoles.java @@ -110,8 +110,7 @@ public void testGetScmRatisRolesWithoutLeader() { } /** - * An empty entry is what the producer emits for a peer with no address, so - * it reaches this view like any other role string. + * Verify that the JMX view reports invalid role strings as an error row. */ @ParameterizedTest @ValueSource(strings = {"", "scm1.example.com:9894"}) diff --git a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/TestSafeModeCheckSubcommand.java b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/TestSafeModeCheckSubcommand.java index 173b84f76bc1..a2af95748f94 100644 --- a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/TestSafeModeCheckSubcommand.java +++ b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/TestSafeModeCheckSubcommand.java @@ -107,8 +107,7 @@ public void testFindLeaderNodeWithoutLeader() throws Exception { } /** - * An empty entry is what the producer emits for a peer with no address, so - * it reaches leader detection like any other role string. + * Verify that leader detection skips invalid role strings and checks later entries. */ @ParameterizedTest @ValueSource(strings = {"", "Exception Occurred, No leader found"})