This is an automated email from the ASF dual-hosted git repository.

sarvekshayr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/ozone.git


The following commit(s) were added to refs/heads/master by this push:
     new 95867e884fd HDDS-14578. Ozone admin command gives inconsistent error 
messages on expired keytab. (#10160)
95867e884fd is described below

commit 95867e884fdb3ce957c2694fba83914a332be089
Author: sravani <[email protected]>
AuthorDate: Fri May 8 19:35:31 2026 +0530

    HDDS-14578. Ozone admin command gives inconsistent error messages on 
expired keytab. (#10160)
---
 .../org/apache/hadoop/hdds/cli/GenericCli.java     | 20 ++++++---
 .../java/org/apache/hadoop/hdds/HddsUtils.java     | 22 ++++++++++
 .../java/org/apache/hadoop/hdds/utils/HAUtils.java | 15 ++++---
 .../hdds/scm/cli/SafeModeCheckSubcommand.java      |  5 +--
 .../hdds/scm/cli/container/InfoSubcommand.java     |  8 +++-
 .../scm/cli/container/ReconcileSubcommand.java     | 17 ++++++--
 .../ozone/admin/scm/RotateKeySubCommand.java       |  2 +-
 .../hdds/scm/cli/container/TestInfoSubCommand.java |  5 +--
 .../scm/cli/container/TestReconcileSubcommand.java | 50 +++++++++++++++++++---
 .../main/smoketest/scmha/container-create.robot    |  2 +-
 10 files changed, 114 insertions(+), 32 deletions(-)

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 6d6463b0940..b12330ab2d0 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 fce0f295e33..bf4213ea8dc 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<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;
+  }
 }
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 5740450419c..eea1932378e 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<String> 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 3a130945ced..1fb97d6e01a 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 d6e0840cb7f..ea296ba9870 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 6d6a6f5afd1..7b1b0267c28 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 bf09c879945..ec93d49fa79 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 8e3d2abe3f1..9433ce46c92 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 8a64b327bbf..7d38deb2c2c 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 812a66a9cf6..c47448e2f83 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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to