HDDS-16138. Add compatibility coverage for SCM Ratis role output over IPv6

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<Arguments> 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<SecurityConfig> mockedSecurityConfigConstruction = mockConstruction(SecurityConfig.class);
+        MockedStatic<RaftServer> staticMockedRaftServer = mockStatic(RaftServer.class);
+        MockedStatic<RatisUtil> 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<String> roles = scmRatisServer.getRatisRoles();
+
+      assertEquals(Arrays.asList(expectedRole), roles);
+    }
+  }
+
   @Test
-  public void testGetRatisRolesWithIPv6() throws Exception {
+  public void testGetRatisRolesKeepsHostname() throws Exception {
     try (
         MockedConstruction<SecurityConfig> mockedSecurityConfigConstruction = mockConstruction(SecurityConfig.class);
         MockedStatic<RaftServer> 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<String> 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(":");
     }
   }
 

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerRatisRoles.java

@@ -0,0 +1,126 @@
+/*
+ * 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<String> 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<String> 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());
+  }
+
+  /**
+   * Verify that the JMX view reports invalid role strings as an error row.
+   */
+  @ParameterizedTest
+  @ValueSource(strings = {"", "scm1.example.com:9894"})
+  public void testGetScmRatisRolesWithUnparseableRole(String role) {
+    StorageContainerManager scm = scmWith(healthyServer(Collections.singletonList(role)));
+
+    List<List<String>> roles = scm.getScmRatisRoles();
+
+    assertEquals(1, roles.size());
+    assertEquals(1, roles.get(0).size());
+    assertThat(roles.get(0).get(0)).startsWith("Exception Occurred, ");
+  }
+}

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<SCMNodeInfo> nodes)
+      throws IOException {
     try {
       List<String> 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;
     }