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

Author: Gargi-jais11

hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java

@@ -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("[");
@@ -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);
@@ -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 =

hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java

@@ -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();
@@ -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();
@@ -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();
@@ -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)

hadoop-ozone/dist/src/main/smoketest/admincli/container.robot

@@ -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