errose28 commented on code in PR #7944:
URL: https://github.com/apache/ozone/pull/7944#discussion_r2274223704


##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ContainerIDParameters.java:
##########
@@ -31,4 +32,34 @@ public class ContainerIDParameters extends ItemsFromStdin {
   public void setContainerIDs(List<String> arguments) {
     setItems(arguments);
   }
+
+  public List<Long> getValidatedIDs() {
+    List<Long> containerIDs = new ArrayList<>(size());
+    boolean allValid = true;
+
+    for (String input: this) {
+      boolean idValid = true;
+      try {
+        long id = Long.parseLong(input);
+        if (id <= 0) {
+          idValid = false;
+        } else {
+          containerIDs.add(id);
+        }
+      } catch (NumberFormatException e) {
+        idValid = false;
+      }
+
+      if (!idValid) {
+        allValid = false;
+        System.err.println("Container ID must be a positive integer, got: " + 
input);
+      }
+    }
+
+    // After errors have been printed for all invalid containers, exit PicoCLI 
with a non-zero result.
+    if (!allValid) {
+      throw new IllegalArgumentException();

Review Comment:
   I've added an error message for checking status of open containers and tests 
for this as well.



##########
hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java:
##########
@@ -34,15 +45,183 @@
     versionProvider = HddsVersionProvider.class)
 public class ReconcileSubcommand extends ScmSubcommand {
 
-  @CommandLine.Parameters(description = "ID of the container to reconcile")
-  private long containerId;
+  @CommandLine.Mixin
+  private ContainerIDParameters containerList;
+
+  @CommandLine.Option(names = { "--status" },
+      defaultValue = "false",
+      fallbackValue = "true",
+      description = "Display the reconciliation status of this container's 
replicas")
+  private boolean status;
 
   @Override
   public void execute(ScmClient scmClient) throws IOException {
-    scmClient.reconcileContainer(containerId);
-    System.out.println("Reconciliation has been triggered for container " + 
containerId);
-    // TODO HDDS-12078 allow status to be checked from the reconcile 
subcommand directly.
-    System.out.println("Use \"ozone admin container info --json " + 
containerId + "\" to see the checksums of each " +
-        "container replica");
+    if (status) {
+      executeStatus(scmClient);
+    } else {
+      executeReconcile(scmClient);
+    }
+  }
+
+  private void executeStatus(ScmClient scmClient) throws IOException {
+    // Do validation outside the json array writer, otherwise failed 
validation will print an empty json array.
+    List<Long> containerIDs = containerList.getValidatedIDs();
+    try (SequenceWriter arrayWriter = JsonUtils.getSequenceWriter(System.out)) 
{
+      // Since status is retrieved using container info, do client side 
validation that it is only used for Ratis
+      // containers. If EC containers are given, print a  message to stderr 
and eventually exit non-zero, but continue
+      // processing the remaining containers.
+      int failureCount = 0;
+      for (Long containerID : containerIDs) {
+        if (!printReconciliationStatus(scmClient, containerID, arrayWriter)) {
+          failureCount++;
+        }
+      }
+      if (failureCount > 0) {
+        throw new RuntimeException("Failed to process reconciliation status 
for " + failureCount + " containers");
+      }
+    }
+    // Array writer will not add a newline to the end.
+    System.out.println();
+  }
+
+  private boolean printReconciliationStatus(ScmClient scmClient, long 
containerID, SequenceWriter arrayWriter) {
+    try {
+      ContainerInfo containerInfo = scmClient.getContainer(containerID);
+      if (containerInfo.getReplicationType() != 
HddsProtos.ReplicationType.RATIS) {

Review Comment:
   I've added an error message for checking status of open containers and tests 
for this as well.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


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

Reply via email to