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]