HDDS-14578. Ozone admin command gives inconsistent error messages on expired keytab.

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

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/HddsUtils.java

@@ -874,4 +874,26 @@ public static Collection<String> 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;
+  }
 }

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<String> waitForCACerts(

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);
     }
   }
 

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,15 +113,15 @@ 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;
     }
 
     List<ContainerReplicaInfo> replicas = null;
     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");
         }

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++;
       }
     }

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) {

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

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,13 +100,15 @@ public void setup() throws IOException {
 
     System.setOut(new PrintStream(outContent, false, DEFAULT_ENCODING));
     System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING));
+    ExitUtils.disableSystemExit();
   }
 
   @AfterEach
   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);
   }
 

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]