diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DecommissionStatusSubCommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DecommissionStatusSubCommand.java index 75a58ceb4b19..fd7aaec1bfc7 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DecommissionStatusSubCommand.java +++ b/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 allNodes = scmClient.queryNode(DECOMMISSIONING, null, HddsProtos.QueryScope.CLUSTER, "").stream(); - List decommissioningNodes = - DecommissionUtils.getDecommissioningNodesList(allNodes, uuid, ipAddress); - if (!Strings.isNullOrEmpty(uuid)) { + List 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; } diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java index ad0f0a7ef9cb..2b7813f46164 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/ListInfoSubcommand.java +++ b/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 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 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() diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/NodeSelectionMixin.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/NodeSelectionMixin.java new file mode 100644 index 000000000000..255ff83fb026 --- /dev/null +++ b/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; + } +} diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/UsageInfoSubcommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/UsageInfoSubcommand.java index 2d486dab495a..cc14f5d2b23e 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/UsageInfoSubcommand.java +++ b/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 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") diff --git a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDecommissionStatusSubCommand.java b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDecommissionStatusSubCommand.java index f7660ab10011..08371d16fad3 100644 --- a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/datanode/TestDecommissionStatusSubCommand.java +++ b/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,8 +135,9 @@ 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 @@ -137,7 +145,7 @@ public void testIdOptionDecommissionStatusSuccess() throws IOException { 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 getNodeDetails(int n) { List nodesList = new ArrayList<>(); diff --git a/hadoop-ozone/dist/src/main/smoketest/admincli/datanode.robot b/hadoop-ozone/dist/src/main/smoketest/admincli/datanode.robot index b3e14c9bbb6e..d4e7e7a2b48b 100644 --- a/hadoop-ozone/dist/src/main/smoketest/admincli/datanode.robot +++ b/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