Skip to content

HDDS-13592. Make InfoSubcommand validate all container IDs before proceeding #9001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Tejaskriya merged 3 commits into from
Sep 16, 2025
Merged

HDDS-13592. Make InfoSubcommand validate all container IDs before proceeding #9001

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9e730e4
use getValidatedIDs to validate and convert container IDs by CLI
Sep 3, 2025
49dcd9e
fixed robot test
Sep 4, 2025
49a7d88
removed first param
Sep 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 10 additions & 21 deletions ...zone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,30 +65,23 @@ public class InfoSubcommand extends ScmSubcommand {

@Override
public void execute(ScmClient scmClient) throws IOException {
// validate all container IDs and fail fast
List<Long> containerIDs = containerList.getValidatedIDs();

boolean first = true;
multiContainer = containerList.size() > 1;
multiContainer = containerIDs.size() > 1;

printHeader();
// TODO HDDS-13592: Use ContainerIDParameters#getValidatedIDs to automatically handle type conversion and fail fast.
for (String id : containerList) {
printOutput(scmClient, id, first);
for (Long containerID : containerIDs) {
if (!first) {
printBreak();
}
printDetails(scmClient, containerID);
first = false;
}
printFooter();
}

private void printOutput(ScmClient scmClient, String id, boolean first)
throws IOException {
long containerID;
try {
containerID = Long.parseLong(id);
} catch (NumberFormatException e) {
printError("Invalid container ID: " + id);
return;
}
printDetails(scmClient, containerID, first);
}

private void printHeader() {
if (json && multiContainer) {
System.out.println("[");
Expand All @@ -113,8 +106,7 @@ private void printBreak() {
}
}

private void printDetails(ScmClient scmClient, long containerID,
boolean first) throws IOException {
private void printDetails(ScmClient scmClient, long containerID) throws IOException {
final ContainerWithPipeline container;
try {
container = scmClient.getContainerWithPipeline(containerID);
Expand All @@ -131,9 +123,6 @@ private void printDetails(ScmClient scmClient, long containerID,
printError("Unable to retrieve the replica details: " + e.getMessage());
}

if (!first) {
printBreak();
}
if (json) {
if (!container.getPipeline().isEmpty()) {
ContainerWithPipelineAndReplicas wrapper =
Expand Down
55 changes: 42 additions & 13 deletions .../cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,14 +119,22 @@ public void testMultipleContainersCanBePassed() throws Exception {
when(scmClient.getContainerReplicas(anyLong())).thenReturn(getReplicas(true));
cmd = new InfoSubcommand();
CommandLine c = new CommandLine(cmd);
c.parseArgs("1", "123", "456", "invalid", "789");
c.parseArgs("1", "123", "456", "789");
cmd.execute(scmClient);
validateMultiOutput();
}

@Test
public void testMultipleInvalidContainerIdFails() throws Exception {
cmd = new InfoSubcommand();
CommandLine c = new CommandLine(cmd);
c.parseArgs("1", "invalid", "-2", "0.5");
validateInvalidContainerIDOutput();
}

@Test
public void testContainersCanBeReadFromStdin() throws IOException {
String input = "1\n123\n456\ninvalid\n789\n";
String input = "1\n123\n456\n789\n";
ByteArrayInputStream inContent = new ByteArrayInputStream(input.getBytes(DEFAULT_ENCODING));
System.setIn(inContent);
cmd = new InfoSubcommand();
Expand All @@ -137,23 +145,38 @@ public void testContainersCanBeReadFromStdin() throws IOException {
validateMultiOutput();
}

@Test
public void testInvalidContainerIdFromStdinFails() throws Exception {
String input = "1\ninvalid\n-2\n0.5\n";
ByteArrayInputStream inContent = new ByteArrayInputStream(input.getBytes(DEFAULT_ENCODING));
System.setIn(inContent);
cmd = new InfoSubcommand();
CommandLine c = new CommandLine(cmd);
c.parseArgs("-");
validateInvalidContainerIDOutput();
}

private void validateMultiOutput() throws UnsupportedEncodingException {
// Ensure we have a log line for each containerID
List<String> replica = Arrays.stream(outContent.toString(DEFAULT_ENCODING).split("\n"))
.filter(m -> m.matches("(?s)^Container id: (1|123|456|789).*"))
.collect(Collectors.toList());
assertEquals(4, replica.size());
}

Pattern p = Pattern.compile(
"^Invalid\\scontainer\\sID:\\sinvalid.*", Pattern.MULTILINE);
Matcher m = p.matcher(errContent.toString(DEFAULT_ENCODING));
assertTrue(m.find());
private void validateInvalidContainerIDOutput() throws Exception {
CommandLine.ParameterException ex = assertThrows(
CommandLine.ParameterException.class, () -> cmd.execute(scmClient));

assertThat(ex.getMessage())
.isEqualTo("Container IDs must be positive integers. Invalid container IDs: invalid -2 0.5");
assertThat(outContent.toString(DEFAULT_ENCODING)).isEmpty();
}

@Test
public void testContainersCanBeReadFromStdinJson()
throws IOException {
String input = "1\n123\n456\ninvalid\n789\n";
String input = "1\n123\n456\n789\n";
ByteArrayInputStream inContent = new ByteArrayInputStream(input.getBytes(DEFAULT_ENCODING));
System.setIn(inContent);
cmd = new InfoSubcommand();
Expand All @@ -164,12 +187,23 @@ public void testContainersCanBeReadFromStdinJson()
validateJsonMultiOutput();
}

@Test
public void testInvalidContainerIdFromStdinJsonFails() throws Exception {
String input = "1\ninvalid\n-2\n0.5\n";
ByteArrayInputStream inContent = new ByteArrayInputStream(input.getBytes(DEFAULT_ENCODING));
System.setIn(inContent);
cmd = new InfoSubcommand();
CommandLine c = new CommandLine(cmd);
c.parseArgs("-", "--json");
validateInvalidContainerIDOutput();
}

@Test
public void testMultipleContainersCanBePassedJson() throws Exception {
when(scmClient.getContainerReplicas(anyLong())).thenReturn(getReplicas(true));
cmd = new InfoSubcommand();
CommandLine c = new CommandLine(cmd);
c.parseArgs("1", "123", "456", "invalid", "789", "--json");
c.parseArgs("1", "123", "456", "789", "--json");
cmd.execute(scmClient);

validateJsonMultiOutput();
Expand All @@ -181,11 +215,6 @@ private void validateJsonMultiOutput() throws UnsupportedEncodingException {
.filter(m -> m.matches("(?s)^.*\"containerInfo\".*"))
.collect(Collectors.toList());
assertEquals(4, replica.size());

Pattern p = Pattern.compile(
"^Invalid\\scontainer\\sID:\\sinvalid.*", Pattern.MULTILINE);
Matcher m = p.matcher(errContent.toString(DEFAULT_ENCODING));
assertTrue(m.find());
}

private void testReplicaIncludedInOutput(boolean includeIndex)
Expand Down
5 changes: 5 additions & 0 deletions hadoop-ozone/dist/src/main/smoketest/admincli/container.robot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ Container info
Should contain ${output} Pipeline id
Should contain ${output} Datanodes

Container info should fail with invalid container ID
${output} = Execute And Ignore Error ozone admin container info "${CONTAINER}" -2 0.5 abc
Should contain ${output} Container IDs must be positive integers.
Should contain ${output} Invalid container IDs: -2 0.5 abc

Verbose container info
${output} = Execute ozone admin --verbose container info "${CONTAINER}"
Should contain ${output} Pipeline Info
Expand Down