HDDS-13098. Create PicoCLI mixin to standardize datanode selection options

hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DecommissionStatusSubCommand.java

@@ -51,35 +51,36 @@ public class DecommissionStatusSubCommand extends ScmSubcommand {
 
   private String errorMessage = "Error getting pipeline and container metrics for ";
 
-  @CommandLine.Option(names = { "--id" },
-      description = "Show info by datanode UUID",
-      defaultValue = "")
-  private String uuid;
-
-  @CommandLine.Option(names = { "--ip" },
-      description = "Show info by datanode ipAddress",
-      defaultValue = "")
-  private String ipAddress;
-
   @CommandLine.Option(names = { "--json" },
       description = "Show output in json format",
       defaultValue = "false")
   private boolean json;
 
+  @CommandLine.Mixin
+  private NodeSelectionMixin nodeSelectionMixin;
+
+  @CommandLine.Spec 
+  private CommandLine.Model.CommandSpec spec;
+
   @Override
   public void execute(ScmClient scmClient) throws IOException {
+    if (!nodeSelectionMixin.getHostname().isEmpty()) {
+      throw new CommandLine.ParameterException(spec.commandLine(),
+          "--hostname option not supported for this command");
+    }
+
     Stream<HddsProtos.Node> allNodes = scmClient.queryNode(DECOMMISSIONING,
         null, HddsProtos.QueryScope.CLUSTER, "").stream();
-    List<HddsProtos.Node> decommissioningNodes =
-        DecommissionUtils.getDecommissioningNodesList(allNodes, uuid, ipAddress);
-    if (!Strings.isNullOrEmpty(uuid)) {
+    List<HddsProtos.Node> decommissioningNodes = DecommissionUtils.getDecommissioningNodesList(allNodes,
+            nodeSelectionMixin.getNodeId(), nodeSelectionMixin.getIp());
+    if (!Strings.isNullOrEmpty(nodeSelectionMixin.getNodeId())) {
       if (decommissioningNodes.isEmpty()) {
-        System.err.println("Datanode: " + uuid + " is not in DECOMMISSIONING");
+        System.err.println("Datanode: " + nodeSelectionMixin.getNodeId() + " is not in DECOMMISSIONING");
         return;
       }
-    } else if (!Strings.isNullOrEmpty(ipAddress)) {
+    } else if (!Strings.isNullOrEmpty(nodeSelectionMixin.getIp())) {
       if (decommissioningNodes.isEmpty()) {
-        System.err.println("Datanode: " + ipAddress + " is not in " +
+        System.err.println("Datanode: " + nodeSelectionMixin.getIp() + " is not in " +
             "DECOMMISSIONING");
         return;
       }

hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java

@@ -44,21 +44,6 @@
     versionProvider = HddsVersionProvider.class)
 public class ListInfoSubcommand extends ScmSubcommand {
 
-  @CommandLine.Option(names = {"--ip"},
-      description = "Show info by ip address.",
-      defaultValue = "")
-  private String ipaddress;
-
-  @CommandLine.Option(names = {"--id"},
-      description = "Show info by datanode UUID.",
-      defaultValue = "")
-  private String uuid;
-
-  @CommandLine.Option(names = {"--hostname"},
-      description = "Show info by datanode hostname.",
-      defaultValue = "")
-  private String hostname;
-
   @CommandLine.Option(names = {"--operational-state"},
       description = "Show info by datanode NodeOperationalState(" +
           "IN_SERVICE, " +
@@ -81,13 +66,16 @@ public class ListInfoSubcommand extends ScmSubcommand {
   @CommandLine.Mixin
   private ListLimitOptions listLimitOptions;
 
+  @CommandLine.Mixin
+  private NodeSelectionMixin nodeSelectionMixin;
+
   private List<Pipeline> pipelines;
 
   @Override
   public void execute(ScmClient scmClient) throws IOException {
     pipelines = scmClient.listPipelines();
-    if (!Strings.isNullOrEmpty(uuid)) {
-      HddsProtos.Node node = scmClient.queryNode(UUID.fromString(uuid));
+    if (!Strings.isNullOrEmpty(nodeSelectionMixin.getNodeId())) {
+      HddsProtos.Node node = scmClient.queryNode(UUID.fromString(nodeSelectionMixin.getNodeId()));
       DatanodeWithAttributes dwa = new DatanodeWithAttributes(DatanodeDetails
           .getFromProtoBuf(node.getNodeID()),
           node.getNodeOperationalStates(0),
@@ -102,13 +90,13 @@ public void execute(ScmClient scmClient) throws IOException {
       return;
     }
     Stream<DatanodeWithAttributes> allNodes = getAllNodes(scmClient).stream();
-    if (!Strings.isNullOrEmpty(ipaddress)) {
+    if (!Strings.isNullOrEmpty(nodeSelectionMixin.getIp())) {
       allNodes = allNodes.filter(p -> p.getDatanodeDetails().getIpAddress()
-          .compareToIgnoreCase(ipaddress) == 0);
+          .compareToIgnoreCase(nodeSelectionMixin.getIp()) == 0);
     }
-    if (!Strings.isNullOrEmpty(hostname)) {
+    if (!Strings.isNullOrEmpty(nodeSelectionMixin.getHostname())) {
       allNodes = allNodes.filter(p -> p.getDatanodeDetails().getHostName()
-          .compareToIgnoreCase(hostname) == 0);
+          .compareToIgnoreCase(nodeSelectionMixin.getHostname()) == 0);
     }
     if (!Strings.isNullOrEmpty(nodeOperationalState)) {
       allNodes = allNodes.filter(p -> p.getOpState().toString()

hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/NodeSelectionMixin.java

@@ -0,0 +1,66 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hdds.scm.cli.datanode;
+
+import com.google.common.base.Strings;
+import picocli.CommandLine;
+
+/**
+ * Picocli mixin providing standardized datanode selection options for consistent CLI usage across commands.
+ */
+public class NodeSelectionMixin {
+
+  @CommandLine.ArgGroup(exclusive = true, multiplicity = "0..1")
+  private Selection selection = new Selection();
+
+  //Precedence order: --node-id > --id (deprecated) > --uuid (deprecated).
+  public String getNodeId() {
+    return !Strings.isNullOrEmpty(selection.nodeId) ? selection.nodeId :
+        !Strings.isNullOrEmpty(selection.id) ? selection.id :
+            !Strings.isNullOrEmpty(selection.uuid) ? selection.uuid : "";
+  }
+
+  public String getHostname() {
+    return selection.hostname;
+  }
+
+  public String getIp() {
+    return selection.ip;
+  }
+
+  static class Selection {
+
+    @CommandLine.Option(names = "--node-id", description = "UUID of the datanode.", defaultValue = "")
+    private String nodeId;
+
+    @Deprecated
+    @CommandLine.Option(names = "--id", description = "UUID of the datanode.", defaultValue = "", hidden = true)
+    private String id;
+
+    @Deprecated
+    @CommandLine.Option(names = "--uuid", description = "UUID of the datanode.", defaultValue = "", hidden = true)
+    private String uuid;
+
+    @CommandLine.Option(names = "--hostname", description = "Hostname of the datanode. " +
+        "Note: not supported for decommission status command", defaultValue = "")
+    private String hostname;
+
+    @CommandLine.Option(names = "--ip", description = "IP address of the datanode", defaultValue = "")
+    private String ip;
+  }
+}

hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/UsageInfoSubcommand.java

@@ -58,7 +58,7 @@ public class UsageInfoSubcommand extends ScmSubcommand {
   }
 
   @CommandLine.ArgGroup(multiplicity = "1")
-  private ExclusiveArguments exclusiveArguments;
+  private NodeSelectionArguments exclusiveArguments;
 
   @CommandLine.Option(names = {"-c", "--count"}, description = "Number of " +
       "datanodes to display (Default: ${DEFAULT-VALUE}).",
@@ -72,16 +72,21 @@ public class UsageInfoSubcommand extends ScmSubcommand {
 
   @Override
   public void execute(ScmClient scmClient) throws IOException {
+    String hostnameOrIp =
+        !Strings.isNullOrEmpty(exclusiveArguments.getIp()) ? exclusiveArguments.getIp()
+            : !Strings.isNullOrEmpty(exclusiveArguments.getHostname()) ? exclusiveArguments.getHostname()
+            : exclusiveArguments.address; //Fallback to deprecated --address for backward compatibility with older CLI.
+
     List<HddsProtos.DatanodeUsageInfoProto> infoList;
     if (count < 1) {
       throw new IOException("Count must be an integer greater than 0.");
     }
 
     // fetch info by ip or hostname or uuid
-    if (!Strings.isNullOrEmpty(exclusiveArguments.address) ||
-        !Strings.isNullOrEmpty(exclusiveArguments.uuid)) {
-      infoList = scmClient.getDatanodeUsageInfo(exclusiveArguments.address,
-          exclusiveArguments.uuid);
+    if (!Strings.isNullOrEmpty(hostnameOrIp) ||
+        !Strings.isNullOrEmpty(exclusiveArguments.getNodeId())) {
+      infoList = scmClient.getDatanodeUsageInfo(hostnameOrIp,
+          exclusiveArguments.getNodeId());
     } else { // get info of most used or least used nodes
       infoList = scmClient.getDatanodeUsageInfo(exclusiveArguments.mostUsed,
           count);
@@ -268,16 +273,14 @@ public long getPipelineCount() {
     }
   }
 
-  private static class ExclusiveArguments {
+  private static class NodeSelectionArguments extends NodeSelectionMixin {
+    @Deprecated
     @CommandLine.Option(names = {"--address"}, paramLabel = "ADDRESS",
         description = "Show info by datanode ip or hostname address.",
-        defaultValue = "")
+        defaultValue = "",
+        hidden = true)
     private String address;
 
-    @CommandLine.Option(names = {"--uuid"}, paramLabel = "UUID", description =
-        "Show info by datanode UUID.", defaultValue = "")
-    private String uuid;
-
     @CommandLine.Option(names = {"-m", "--most-used"},
         description = "Show the most used datanodes.",
         defaultValue = "false")

hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDecommissionStatusSubCommand.java

@@ -18,6 +18,7 @@
 package org.apache.hadoop.hdds.scm.cli.datanode;
 
 import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.any;
 import static org.mockito.Mockito.mock;
@@ -42,6 +43,8 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
 import picocli.CommandLine;
 
 /**
@@ -81,6 +84,8 @@ public void testSuccessWhenDecommissionStatus() throws IOException {
     when(scmClient.getContainersOnDecomNode(any())).thenReturn(containerOnDecom);
     when(scmClient.getMetrics(any())).thenReturn(metrics.get(1));
 
+    CommandLine cmdLine = new CommandLine(cmd);
+    cmdLine.parseArgs();
     cmd.execute(scmClient);
     Pattern p = Pattern.compile("Decommission\\sStatus:\\s" +
             "DECOMMISSIONING\\s-\\s2\\snode\\(s\\)\n");
@@ -112,6 +117,8 @@ public void testNoNodesWhenDecommissionStatus() throws IOException {
         .thenReturn(new ArrayList<>());
     when(scmClient.getContainersOnDecomNode(any())).thenReturn(new HashMap<>());
     when(scmClient.getMetrics(any())).thenReturn(metrics.get(0));
+    CommandLine cmdLine = new CommandLine(cmd);
+    cmdLine.parseArgs();
     cmd.execute(scmClient);
 
     Pattern p = Pattern.compile("Decommission\\sStatus:\\s" +
@@ -128,16 +135,17 @@ public void testNoNodesWhenDecommissionStatus() throws IOException {
     assertFalse(m.find());
   }
 
-  @Test
-  public void testIdOptionDecommissionStatusSuccess() throws IOException {
+  @ParameterizedTest
+  @ValueSource(strings = {"--id", "--node-id"})
+  public void testIdOptionDecommissionStatusSuccess(String flag) throws IOException {
     ScmClient scmClient = mock(ScmClient.class);
     when(scmClient.queryNode(any(), any(), any(), any()))
         .thenAnswer(invocation -> nodes); // 2 nodes decommissioning
     when(scmClient.getContainersOnDecomNode(any())).thenReturn(containerOnDecom);
     when(scmClient.getMetrics(any())).thenReturn(metrics.get(1));
 
     CommandLine c = new CommandLine(cmd);
-    c.parseArgs("--id", nodes.get(0).getNodeID().getUuid());
+    c.parseArgs(flag, nodes.get(0).getNodeID().getUuid());
     cmd.execute(scmClient); // check status of host0
 
     Pattern p = Pattern.compile("Datanode:\\s.*host0\\)");
@@ -153,8 +161,9 @@ public void testIdOptionDecommissionStatusSuccess() throws IOException {
     assertFalse(m.find());
   }
 
-  @Test
-  public void testIdOptionDecommissionStatusFail() throws IOException {
+  @ParameterizedTest
+  @ValueSource(strings = {"--id", "--node-id"})
+  public void testIdOptionDecommissionStatusFail(String flag) throws IOException {
     ScmClient scmClient = mock(ScmClient.class);
     when(scmClient.queryNode(any(), any(), any(), any()))
         .thenAnswer(invocation -> nodes.subList(0, 1)); // host0 decommissioning
@@ -165,7 +174,7 @@ public void testIdOptionDecommissionStatusFail() throws IOException {
     when(scmClient.getMetrics(any())).thenReturn(metrics.get(2));
 
     CommandLine c = new CommandLine(cmd);
-    c.parseArgs("--id", nodes.get(1).getNodeID().getUuid());
+    c.parseArgs(flag, nodes.get(1).getNodeID().getUuid());
     cmd.execute(scmClient); // check status of host1
 
     Pattern p = Pattern.compile("Datanode:\\s(.*)\\sis\\snot\\sin" +
@@ -236,6 +245,20 @@ public void testIpOptionDecommissionStatusFail() throws IOException {
     assertFalse(m.find());
   }
 
+  @Test
+  public void testHostnameOptionThrowsParameterException() throws IOException {
+    ScmClient scmClient = mock(ScmClient.class);
+    CommandLine cmdLine = new CommandLine(cmd);
+    cmdLine.parseArgs("--hostname", "some-host");
+
+    CommandLine.ParameterException ex = assertThrows(
+        CommandLine.ParameterException.class,
+        () -> cmd.execute(scmClient)
+    );
+
+    assertTrue(ex.getMessage().contains("--hostname option not supported for this command"));
+  }
+
   private List<HddsProtos.Node> getNodeDetails(int n) {
     List<HddsProtos.Node> nodesList = new ArrayList<>();
 

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

@@ -41,7 +41,7 @@ List datanodes
 
 Filter list by UUID
     ${uuid} =           Execute      grep '^Datanode:' ${LIST_FILE} | head -1 | awk '{ print \$2 }'
-    ${output} =         Execute      ozone admin datanode list --id "${uuid}"
+    ${output} =         Execute      ozone admin datanode list --node-id "${uuid}"
     Assert Output       ${output}    1    ${uuid}
 
 Filter list by Ip address
@@ -70,22 +70,22 @@ Filter list by NodeState
 
 Get usage info by UUID
     ${uuid} =           Execute      grep '^Datanode:' ${LIST_FILE} | head -1 | awk '{ print \$2 }'
-    ${output} =         Execute      ozone admin datanode usageinfo --uuid "${uuid}"
+    ${output} =         Execute      ozone admin datanode usageinfo --node-id "${uuid}"
     Should contain      ${output}    Usage Information (1 Datanodes)
 
 Get usage info by Ip address
     ${ip} =             Execute      grep '^Datanode:' ${LIST_FILE} | head -1 | awk '{ print \$3 }' | awk -F '[/]' '{ print \$3 }'
-    ${output} =         Execute      ozone admin datanode usageinfo --address "${ip}"
+    ${output} =         Execute      ozone admin datanode usageinfo --ip "${ip}"
     Should contain      ${output}    Usage Information (1 Datanodes)
 
 Get usage info by Hostname
     ${hostname} =       Execute      grep '^Datanode:' ${LIST_FILE} | head -1 | awk '{ print \$3 }' | awk -F '[/]' '{ print \$4 }'
-    ${output} =         Execute      ozone admin datanode usageinfo --address "${hostname}"
+    ${output} =         Execute      ozone admin datanode usageinfo --hostname "${hostname}"
     Should contain      ${output}    Usage Information (1 Datanodes)
 
 Get usage info with invalid address
     ${uuid} =           Execute      grep '^Datanode:' ${LIST_FILE} | head -1 | awk '{ print \$2 }'
-    ${output} =         Execute      ozone admin datanode usageinfo --address "${uuid}"
+    ${output} =         Execute      ozone admin datanode usageinfo --ip "${uuid}"
     Should contain      ${output}    Usage Information (0 Datanodes)
 
 Incomplete command