diff --git a/hadoop-hdds/cli-common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java b/hadoop-hdds/cli-common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java index da46c2600389..9755a35d2c84 100644 --- a/hadoop-hdds/cli-common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java +++ b/hadoop-hdds/cli-common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java @@ -25,6 +25,7 @@ import java.nio.file.NoSuchFileException; import java.util.Map; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.conf.OzoneConfiguration; import org.apache.hadoop.security.UserGroupInformation; import org.apache.ratis.util.ExitUtils; @@ -93,14 +94,19 @@ public void printError(Throwable error) { final String rawMessage = error.getMessage(); if (verbose || rawMessage == null || rawMessage.isEmpty()) { error.printStackTrace(cmd.getErr()); - } else { - if (error instanceof FileSystemException) { - String errorMessage = handleFileSystemException((FileSystemException) error); - cmd.getErr().println(errorMessage); - } else { - cmd.getErr().println(rawMessage.split("\n")[0]); - } + return; + } + String aclLine = HddsUtils.formatAccessControlExceptionLine(error); + if (aclLine != null) { + cmd.getErr().println(aclLine); + ExitUtils.terminate(EXECUTION_ERROR_EXIT_CODE, aclLine, null); + } + if (error instanceof FileSystemException) { + String errorMessage = handleFileSystemException((FileSystemException) error); + cmd.getErr().println(errorMessage); + return; } + cmd.getErr().println(rawMessage.split("\n")[0]); } @Override 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 fce0f295e33d..bf4213ea8dc4 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 @@ -874,4 +874,26 @@ public static Collection getSCMNodeIds( String scmServiceId = getScmServiceId(configuration); return getSCMNodeIds(configuration, scmServiceId); } + + /** + * If {@code error} exposes an {@link AccessControlException} , returns one line error message. + */ + public static String formatAccessControlExceptionLine(Throwable error) { + for (Throwable t = error; t != null; t = t.getCause()) { + if (t instanceof AccessControlException) { + return t.toString(); + } + } + String msg = error != null ? error.getMessage() : null; + if (msg != null) { + String marker = AccessControlException.class.getName() + ": "; + int i = msg.indexOf(marker); + if (i >= 0) { + int end = msg.indexOf('\n', i); + String line = end < 0 ? msg.substring(i) : msg.substring(i, end); + return line.trim(); + } + } + return null; + } } diff --git a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java index 5740450419c0..eea1932378e4 100644 --- a/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java +++ b/hadoop-hdds/framework/src/main/java/org/apache/hadoop/hdds/utils/HAUtils.java @@ -413,21 +413,26 @@ public RetryAction shouldRetry(Exception e, int retries, int failovers, boolean try { return retriableTask.call(); } catch (Exception ex) { - if (containsAccessControlException(ex)) { - throw new AccessControlException(); + AccessControlException ace = accessControlExceptionInCauseChain(ex); + if (ace != null) { + throw new IOException(ace.getMessage(), ex); } throw new SCMSecurityException("Unable to obtain complete CA list", ex); } } - private static boolean containsAccessControlException(Throwable e) { + private static AccessControlException accessControlExceptionInCauseChain(Throwable e) { while (e != null) { if (e instanceof AccessControlException) { - return true; + return (AccessControlException) e; } e = e.getCause(); } - return false; + return null; + } + + private static boolean containsAccessControlException(Throwable e) { + return accessControlExceptionInCauseChain(e) != null; } private static List waitForCACerts( 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 3a130945ced7..1fb97d6e01a3 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 @@ -122,7 +122,7 @@ private SCMNodeInfo findLeaderNode(ScmClient scmClient) throws IOException { return null; } catch (IOException e) { - throw new IOException("Could not determine leader node", e); + throw new IOException("Could not determine leader node. " + e.getMessage(), e); } } @@ -171,8 +171,7 @@ private void queryNode(ScmClient scmClient, ScmNodeTarget targetScmNode, SCMNode } } } catch (Exception e) { - System.out.printf("%s [%s]: ERROR: Failed to get safe mode status for SCM node: %s%n", - node.getScmClientAddress(), nodeId, e.getMessage()); + rootCommand().printError(e); } } diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java index d6e0840cb7ff..ea296ba98703 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java @@ -26,6 +26,7 @@ import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; +import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.cli.HddsVersionProvider; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -112,7 +113,7 @@ private void printDetails(ScmClient scmClient, long containerID) throws IOExcept container = scmClient.getContainerWithPipeline(containerID); Objects.requireNonNull(container, "Container cannot be null"); } catch (IOException e) { - printError("Unable to retrieve the container details for " + containerID); + rootCommand().printError(e); return; } @@ -120,7 +121,7 @@ private void printDetails(ScmClient scmClient, long containerID) throws IOExcept try { replicas = scmClient.getContainerReplicas(containerID); } catch (IOException e) { - printError("Unable to retrieve the replica details: " + e.getMessage()); + rootCommand().printError(e); } if (json) { @@ -156,6 +157,9 @@ private void printDetails(ScmClient scmClient, long containerID) throws IOExcept if (SCMHAUtils.unwrapException( ioe) instanceof PipelineNotFoundException) { System.out.println("Write Pipeline State: CLOSED"); + } else if (HddsUtils.formatAccessControlExceptionLine(ioe) != null) { + rootCommand().printError(ioe); + return; } else { printError("Failed to retrieve pipeline info"); } diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java index 6d6a6f5afd10..7b1b0267c287 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java @@ -24,6 +24,7 @@ import java.io.IOException; import java.util.ArrayList; import java.util.List; +import org.apache.hadoop.hdds.HddsUtils; import org.apache.hadoop.hdds.cli.HddsVersionProvider; import org.apache.hadoop.hdds.client.ReplicationConfig; import org.apache.hadoop.hdds.protocol.DatanodeDetails; @@ -55,6 +56,10 @@ public class ReconcileSubcommand extends ScmSubcommand { description = "Display the reconciliation status of this container's replicas") private boolean status; + private static boolean isAuthenticationFailure(Throwable t) { + return HddsUtils.formatAccessControlExceptionLine(t) != null; + } + @Override public void execute(ScmClient scmClient) throws IOException { if (status) { @@ -102,7 +107,8 @@ private boolean printReconciliationStatus(ScmClient scmClient, long containerID, .append(". Reconciliation is not supported for open containers") .append(System.lineSeparator()); return false; - } else if (containerInfo.getReplicationType() != HddsProtos.ReplicationType.RATIS) { + } + if (containerInfo.getReplicationType() != HddsProtos.ReplicationType.RATIS) { errorBuilder.append("Cannot get status of container ").append(containerID) .append(". Reconciliation is only supported for Ratis replicated containers") .append(System.lineSeparator()); @@ -112,8 +118,12 @@ private boolean printReconciliationStatus(ScmClient scmClient, long containerID, arrayWriter.write(new ContainerWrapper(containerInfo, replicas)); arrayWriter.flush(); } catch (Exception ex) { + if (isAuthenticationFailure(ex)) { + rootCommand().printError(ex); + } errorBuilder.append("Failed to get reconciliation status of container ") - .append(containerID).append(": ").append(getExceptionMessage(ex)).append(System.lineSeparator()); + .append(containerID).append(": ").append(getExceptionMessage(ex)) + .append(System.lineSeparator()); return false; } return true; @@ -128,8 +138,7 @@ private void executeReconcile(ScmClient scmClient) { System.out.println("Reconciliation has been triggered for container " + containerID); successCount++; } catch (Exception ex) { - System.err.println("Failed to trigger reconciliation for container " + containerID + ": " + - getExceptionMessage(ex)); + rootCommand().printError(ex); failureCount++; } } diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/scm/RotateKeySubCommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/scm/RotateKeySubCommand.java index bf09c8799456..ec93d49fa79a 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/scm/RotateKeySubCommand.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/ozone/admin/scm/RotateKeySubCommand.java @@ -49,7 +49,7 @@ protected void execute(ScmClient scmClient) throws IOException { try { status = client.rotateSecretKeys(force); } catch (IOException e) { - System.err.println("Secret key rotation failed: " + e.getMessage()); + rootCommand().printError(e); return; } if (status) { diff --git a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java index 8e3d2abe3f1b..9433ce46c927 100644 --- a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java +++ b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java @@ -270,10 +270,7 @@ public void testReplicasNotOutputIfError() throws IOException { .collect(Collectors.toList()); assertEquals(0, replica.size()); - Pattern p = Pattern.compile( - "^Unable to retrieve the replica details.*", Pattern.MULTILINE); - Matcher m = p.matcher(errContent.toString(DEFAULT_ENCODING)); - assertTrue(m.find()); + assertThat(errContent.toString(DEFAULT_ENCODING)).contains("Error getting Replicas"); } @Test diff --git a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestReconcileSubcommand.java b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestReconcileSubcommand.java index 8a64b327bbfd..7d38deb2c2ca 100644 --- a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestReconcileSubcommand.java +++ b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestReconcileSubcommand.java @@ -31,6 +31,8 @@ import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -49,6 +51,7 @@ import java.util.Map; import java.util.UUID; import java.util.stream.Collectors; +import org.apache.hadoop.hdds.cli.GenericCli; import org.apache.hadoop.hdds.client.ECReplicationConfig; import org.apache.hadoop.hdds.client.RatisReplicationConfig; import org.apache.hadoop.hdds.client.ReplicationConfig; @@ -58,6 +61,9 @@ import org.apache.hadoop.hdds.scm.container.ContainerInfo; import org.apache.hadoop.hdds.scm.container.ContainerReplicaInfo; import org.apache.hadoop.hdds.server.JsonUtils; +import org.apache.hadoop.security.AccessControlException; +import org.apache.ratis.util.ExitUtils; +import org.apache.ratis.util.ExitUtils.ExitException; import org.assertj.core.api.AbstractStringAssert; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; @@ -83,6 +89,9 @@ public class TestReconcileSubcommand { private static final String DEFAULT_ENCODING = StandardCharsets.UTF_8.name(); + private static final class TestGenericCliRoot extends GenericCli { + } + @BeforeEach public void setup() throws IOException { scmClient = mock(ScmClient.class); @@ -91,6 +100,7 @@ public void setup() throws IOException { System.setOut(new PrintStream(outContent, false, DEFAULT_ENCODING)); System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING)); + ExitUtils.disableSystemExit(); } @AfterEach @@ -98,6 +108,7 @@ public void after() { System.setOut(originalOut); System.setErr(originalErr); System.setIn(originalIn); + ExitUtils.clear(); } @Test @@ -232,7 +243,7 @@ public void testReconcileHandlesInvalidContainer() throws Exception { RuntimeException exception = assertThrows(RuntimeException.class, () -> executeReconcileFromArgs(1)); - assertThatOutput(errContent).contains("Failed to trigger reconciliation for container 1: " + mockMessage); + assertThatOutput(errContent).contains(mockMessage); assertThat(exception.getMessage()).contains("Failed to trigger reconciliation for 1 container"); @@ -302,8 +313,7 @@ public void testReconcileHandlesValidAndInvalidContainers() throws Exception { }); // Should have error messages for EC containers - assertThatOutput(errContent).contains("Failed to trigger reconciliation for container 1: " + EC_CONTAINER_MESSAGE); - assertThatOutput(errContent).contains("Failed to trigger reconciliation for container 3: " + EC_CONTAINER_MESSAGE); + assertThatOutput(errContent).contains(EC_CONTAINER_MESSAGE); assertThatOutput(errContent).doesNotContain("Failed to trigger reconciliation for container 2"); // Exception message should indicate 2 failed containers @@ -315,6 +325,30 @@ public void testReconcileHandlesValidAndInvalidContainers() throws Exception { assertThatOutput(outContent).doesNotContain("container 3"); } + /** + * Tests that the reconciliation loop terminates immediately upon an + * authentication failure. + */ + @Test + public void testReconcileStopsAfterAuthenticationFailure() throws Exception { + mockContainer(1, 3, RatisReplicationConfig.getInstance(THREE), true); + mockContainer(2, 3, RatisReplicationConfig.getInstance(THREE), true); + + IOException authWrapped = new IOException( + "RPC failed", + new AccessControlException("Client cannot authenticate via:[KERBEROS]")); + doThrow(authWrapped).when(scmClient).reconcileContainer(1L); + + assertThrows(ExitException.class, () -> executeReconcileFromArgs(1, 2)); + + verify(scmClient, times(1)).reconcileContainer(1L); + verify(scmClient, never()).reconcileContainer(2L); + + assertThat(errContent.toString(DEFAULT_ENCODING)) + .contains("AccessControlException") + .contains("Client cannot authenticate via:[KERBEROS]"); + } + /** * Invalid container IDs are those that cannot be parsed because they are not positive integers. * When any invalid container ID is passed, the command should fail early instead of proceeding with the valid @@ -367,7 +401,7 @@ public void testUnreachableContainers() throws Exception { assertThrows(RuntimeException.class, () -> parseArgsAndExecute("123", "456")); // Should have error message for unreachable container - assertThatOutput(errContent).contains("Failed to trigger reconciliation for container 456: " + exceptionMessage); + assertThatOutput(errContent).contains(exceptionMessage); assertThatOutput(errContent).doesNotContain("123"); assertThatOutput(outContent).doesNotContain("Reconciliation has been triggered for container 456"); validateReconcileOutput(123); @@ -384,7 +418,13 @@ private void parseArgsAndExecute(String... args) throws Exception { System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING)); ReconcileSubcommand cmd = new ReconcileSubcommand(); - new CommandLine(cmd).parseArgs(args); + TestGenericCliRoot root = new TestGenericCliRoot(); + CommandLine commandLine = new CommandLine(root); + commandLine.addSubcommand("reconcile", cmd); + String[] fullArgs = new String[args.length + 1]; + fullArgs[0] = "reconcile"; + System.arraycopy(args, 0, fullArgs, 1, args.length); + commandLine.parseArgs(fullArgs); cmd.execute(scmClient); } diff --git a/hadoop-ozone/dist/src/main/smoketest/scmha/container-create.robot b/hadoop-ozone/dist/src/main/smoketest/scmha/container-create.robot index 812a66a9cf61..c47448e2f834 100644 --- a/hadoop-ozone/dist/src/main/smoketest/scmha/container-create.robot +++ b/hadoop-ozone/dist/src/main/smoketest/scmha/container-create.robot @@ -21,4 +21,4 @@ Resource ../lib/os.robot *** Test Cases *** Create container without kinit ${output} = Execute And Ignore Error ozone admin container create - Should contain ${output} Permission denied + Should contain ${output} Client cannot authenticate via:[KERBEROS]