adoroszlai commented on code in PR #7266: URL: https://github.com/apache/ozone/pull/7266#discussion_r2073271416
########## hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/scm/TestVolumeCommand.java: ########## @@ -0,0 +1,164 @@ +/* + * 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.ozone.scm; + +import static org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.ALL_PORTS; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +import java.util.UUID; +import java.util.concurrent.ThreadLocalRandom; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.datanode.VolumeSubCommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.util.Time; +import org.apache.ozone.test.GenericTestUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import picocli.CommandLine; + +/** + * This unit test is used to verify whether the output of + * `TestVolumeFailureSubCommand` meets the expected results. + */ +public class TestVolumeCommand { + private VolumeSubCommand cmd; + private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); + private final ByteArrayOutputStream errContent = new ByteArrayOutputStream(); + private final PrintStream originalOut = System.out; + private final PrintStream originalErr = System.err; + private static final String DEFAULT_ENCODING = StandardCharsets.UTF_8.name(); + + @BeforeEach + public void setup() throws UnsupportedEncodingException { + cmd = new VolumeSubCommand(); + System.setOut(new PrintStream(outContent, false, DEFAULT_ENCODING)); + System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING)); Review Comment: Do we need to manually set out/err when using `SystemOutCapturer`? ########## hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/ozone/scm/TestVolumeCommand.java: ########## @@ -0,0 +1,164 @@ +/* + * 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.ozone.scm; + +import static org.apache.hadoop.hdds.protocol.DatanodeDetails.Port.Name.ALL_PORTS; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; +import java.io.ByteArrayOutputStream; +import java.io.PrintStream; +import java.io.UnsupportedEncodingException; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.List; +import java.util.Random; +import java.util.UUID; +import java.util.concurrent.ThreadLocalRandom; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.datanode.VolumeSubCommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.util.Time; +import org.apache.ozone.test.GenericTestUtils; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import picocli.CommandLine; + +/** + * This unit test is used to verify whether the output of + * `TestVolumeFailureSubCommand` meets the expected results. + */ +public class TestVolumeCommand { + private VolumeSubCommand cmd; + private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); + private final ByteArrayOutputStream errContent = new ByteArrayOutputStream(); + private final PrintStream originalOut = System.out; + private final PrintStream originalErr = System.err; + private static final String DEFAULT_ENCODING = StandardCharsets.UTF_8.name(); + + @BeforeEach + public void setup() throws UnsupportedEncodingException { + cmd = new VolumeSubCommand(); + System.setOut(new PrintStream(outContent, false, DEFAULT_ENCODING)); + System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING)); + } + + @AfterEach + public void tearDown() { + System.setOut(originalOut); + System.setErr(originalErr); + } + + @Test + public void testCheckVolumeFailureJsonAccuracy() throws Exception { + ScmClient scmClient = mock(ScmClient.class); + when(scmClient.getVolumeInfos("all", "", "", 20, 1)). + thenAnswer(invocation -> getUsageProto()); + + CommandLine c = new CommandLine(cmd); + c.parseArgs("--json"); + + try (GenericTestUtils.SystemOutCapturer capture = + new GenericTestUtils.SystemOutCapturer()) { + cmd.execute(scmClient); + String output = capture.getOutput(); + assertNotNull(output); + ObjectMapper mapper = new ObjectMapper(); + JsonNode json = mapper.readTree(outContent.toString("UTF-8")); + assertTrue(json.isArray()); + assertEquals(5, json.size()); + System.out.println(output); + } + } + + @Test + public void testCheckVolumeFailureTableAccuracy() throws Exception { + ScmClient scmClient = mock(ScmClient.class); + + when(scmClient.getVolumeInfos("all", "", "", 20, 1)). + thenAnswer(invocation -> getUsageProto()); + + CommandLine c = new CommandLine(cmd); + c.parseArgs("--table"); + cmd.execute(scmClient); + + try (GenericTestUtils.SystemOutCapturer capture = + new GenericTestUtils.SystemOutCapturer()) { + cmd.execute(scmClient); + String output = capture.getOutput(); + assertThat(output).contains("/data0/ozonedata/hdds"); + assertThat(output).contains("6.76 TB"); + } + } + + private GetVolumeInfosResponseProto getUsageProto() { + GetVolumeInfosResponseProto.Builder builder = GetVolumeInfosResponseProto.newBuilder(); + List<HddsProtos.VolumeInfoProto> result = new ArrayList<>(); + for (int i = 0; i < 5; i++) { + HddsProtos.DatanodeDetailsProto datanodeDetails = createDatanodeDetails(); + String hostName = datanodeDetails.getHostName(); + String uuId = datanodeDetails.getUuid(); + VolumeInfo volumeInfo = + new VolumeInfo.Builder(). + setHostName(hostName). + setUuid(uuId). + setFailed(true). + setFailureTime(Time.now()). + setVolumeName("/data" + i + "/ozonedata/hdds"). + setCapacity(7430477791683L). + build(); + result.add(volumeInfo.getProtobuf()); + } + builder.setPages(10); + builder.setTotal(10); + builder.setCurrentPage(1); + builder.addAllVolumeInfos(result); + return builder.build(); + } + + private HddsProtos.DatanodeDetailsProto createDatanodeDetails() { + Random random = ThreadLocalRandom.current(); + String ipAddress = random.nextInt(256) + + "." + random.nextInt(256) + + "." + random.nextInt(256) + + "." + random.nextInt(256); + + DatanodeDetails.Builder dn = DatanodeDetails.newBuilder() + .setUuid(UUID.randomUUID()) + .setHostName("localhost" + "-" + ipAddress) + .setIpAddress(ipAddress) + .setPersistedOpState(HddsProtos.NodeOperationalState.IN_SERVICE) + .setPersistedOpStateExpiry(0); + + for (DatanodeDetails.Port.Name name : ALL_PORTS) { + dn.addPort(DatanodeDetails.newPort(name, 0)); + } Review Comment: Use `MockDatanodeDetails`. ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/VolumeSubCommand.java: ########## @@ -0,0 +1,222 @@ +/* + * 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 java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.apache.commons.collections.CollectionUtils; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.VolumeInfoProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.hdds.server.JsonUtils; +import org.apache.hadoop.ozone.utils.FormattingCLIUtils; +import org.apache.hadoop.util.StringUtils; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * We provide a set of volume commands to display the disk information of the DataNode. + */ +@Command( + name = "volumes", + description = "Display the list of volumes on the DataNode.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class VolumeSubCommand extends ScmSubcommand { + + // Display it in JSON format. + @Option(names = { "--json" }, + defaultValue = "false", + description = "Format output as JSON.") + private boolean json; + + // Display it in TABLE format. + @Option(names = { "--table" }, + defaultValue = "false", + description = "Format output as Table.") + private boolean table; + + // PageSize refers to the number of items displayed per page + // in a paginated view. + @Option(names = { "--pageSize" }, + defaultValue = "20", + description = "The number of volume information items displayed per page.") + private int pageSize; + + // The current page. + @Option(names = { "--currentPage" }, + defaultValue = "1", + description = "The current page.") + private int currentPage; + + /** + * We have designed a new option called 'show', + * which includes two selectable configurations: + * 'failed' is used to display failed disks, + * and 'normal' is used to display normal disks. + */ + @Option(names = { "--displayMode" }, + defaultValue = "all", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String displayMode; + + // The UUID identifier of the DataNode. + @Option(names = { "--uuid" }, + defaultValue = "", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String uuid; + + // The HostName identifier of the DataNode. + @Option(names = { "--hostName" }, + defaultValue = "", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String hostName; + + private SimpleDateFormat sdf = new SimpleDateFormat( + "EEE MMM dd HH:mm:ss Z yyyy", Locale.ENGLISH); + + private static final String DATANODE_VOLUME_FAILURES_TITLE = "Datanode Volume"; + + private static final List<String> DATANODE_VOLUME_FAILURES_HEADER = Arrays.asList( + "UUID", "Host Name", "Volume Name", "Volume Status", "Capacity / Capacity Lost", + "Failure Time"); + + private final String[] validModes = {"all", "normal", "failed"}; + + @Override + public void execute(ScmClient client) throws IOException { + + validateDisplayMode(displayMode); + + // Retrieve the volume data based on the conditions. + GetVolumeInfosResponseProto response = + client.getVolumeInfos(displayMode, uuid, hostName, pageSize, currentPage); + + // Print the relevant information if the return value is empty. + if (response == null || CollectionUtils.isEmpty(response.getVolumeInfosList())) { + System.out.println("No volume data was retrieved."); Review Comment: I'm not sure this informational message is needed, but if you think so, at least move it after handling JSON format, which should output valid JSON even for empty results. ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/VolumeSubCommand.java: ########## @@ -0,0 +1,222 @@ +/* + * 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 java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.apache.commons.collections.CollectionUtils; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.VolumeInfoProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.hdds.server.JsonUtils; +import org.apache.hadoop.ozone.utils.FormattingCLIUtils; +import org.apache.hadoop.util.StringUtils; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * We provide a set of volume commands to display the disk information of the DataNode. + */ +@Command( + name = "volumes", + description = "Display the list of volumes on the DataNode.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class VolumeSubCommand extends ScmSubcommand { + + // Display it in JSON format. + @Option(names = { "--json" }, + defaultValue = "false", + description = "Format output as JSON.") + private boolean json; + + // Display it in TABLE format. + @Option(names = { "--table" }, + defaultValue = "false", + description = "Format output as Table.") + private boolean table; + + // PageSize refers to the number of items displayed per page + // in a paginated view. + @Option(names = { "--pageSize" }, + defaultValue = "20", + description = "The number of volume information items displayed per page.") + private int pageSize; + + // The current page. + @Option(names = { "--currentPage" }, + defaultValue = "1", + description = "The current page.") + private int currentPage; Review Comment: Other Ozone CLI commands allow pagination (via `ListOptions` reusable mixin) with `--start` to specify the start item, `--length` for "page size", and `--all` (only one of these last two are allowed). Please try to use options consistent with that. Using `ListOption` directly may not work, since `--prefix` does not seem to be applicable here. ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/VolumeSubCommand.java: ########## @@ -0,0 +1,222 @@ +/* + * 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 java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.apache.commons.collections.CollectionUtils; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.VolumeInfoProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.hdds.server.JsonUtils; +import org.apache.hadoop.ozone.utils.FormattingCLIUtils; +import org.apache.hadoop.util.StringUtils; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * We provide a set of volume commands to display the disk information of the DataNode. + */ +@Command( + name = "volumes", + description = "Display the list of volumes on the DataNode.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class VolumeSubCommand extends ScmSubcommand { + + // Display it in JSON format. + @Option(names = { "--json" }, + defaultValue = "false", + description = "Format output as JSON.") + private boolean json; + + // Display it in TABLE format. + @Option(names = { "--table" }, + defaultValue = "false", + description = "Format output as Table.") + private boolean table; + + // PageSize refers to the number of items displayed per page + // in a paginated view. + @Option(names = { "--pageSize" }, + defaultValue = "20", + description = "The number of volume information items displayed per page.") + private int pageSize; + + // The current page. + @Option(names = { "--currentPage" }, + defaultValue = "1", + description = "The current page.") + private int currentPage; + + /** + * We have designed a new option called 'show', + * which includes two selectable configurations: + * 'failed' is used to display failed disks, + * and 'normal' is used to display normal disks. + */ + @Option(names = { "--displayMode" }, + defaultValue = "all", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String displayMode; Review Comment: Please use an `enum` to limit values allowed. https://picocli.info/#_enum_types ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/VolumeSubCommand.java: ########## @@ -0,0 +1,222 @@ +/* + * 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 java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.apache.commons.collections.CollectionUtils; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.VolumeInfoProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.hdds.server.JsonUtils; +import org.apache.hadoop.ozone.utils.FormattingCLIUtils; +import org.apache.hadoop.util.StringUtils; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * We provide a set of volume commands to display the disk information of the DataNode. + */ +@Command( + name = "volumes", + description = "Display the list of volumes on the DataNode.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class VolumeSubCommand extends ScmSubcommand { + + // Display it in JSON format. + @Option(names = { "--json" }, + defaultValue = "false", + description = "Format output as JSON.") + private boolean json; + + // Display it in TABLE format. + @Option(names = { "--table" }, + defaultValue = "false", + description = "Format output as Table.") + private boolean table; + + // PageSize refers to the number of items displayed per page + // in a paginated view. + @Option(names = { "--pageSize" }, + defaultValue = "20", + description = "The number of volume information items displayed per page.") + private int pageSize; + + // The current page. + @Option(names = { "--currentPage" }, + defaultValue = "1", + description = "The current page.") + private int currentPage; + + /** + * We have designed a new option called 'show', + * which includes two selectable configurations: + * 'failed' is used to display failed disks, + * and 'normal' is used to display normal disks. + */ + @Option(names = { "--displayMode" }, + defaultValue = "all", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String displayMode; + + // The UUID identifier of the DataNode. + @Option(names = { "--uuid" }, + defaultValue = "", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String uuid; + + // The HostName identifier of the DataNode. + @Option(names = { "--hostName" }, + defaultValue = "", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String hostName; + + private SimpleDateFormat sdf = new SimpleDateFormat( + "EEE MMM dd HH:mm:ss Z yyyy", Locale.ENGLISH); + + private static final String DATANODE_VOLUME_FAILURES_TITLE = "Datanode Volume"; + + private static final List<String> DATANODE_VOLUME_FAILURES_HEADER = Arrays.asList( + "UUID", "Host Name", "Volume Name", "Volume Status", "Capacity / Capacity Lost", + "Failure Time"); + + private final String[] validModes = {"all", "normal", "failed"}; + + @Override + public void execute(ScmClient client) throws IOException { + + validateDisplayMode(displayMode); + + // Retrieve the volume data based on the conditions. + GetVolumeInfosResponseProto response = + client.getVolumeInfos(displayMode, uuid, hostName, pageSize, currentPage); + + // Print the relevant information if the return value is empty. + if (response == null || CollectionUtils.isEmpty(response.getVolumeInfosList())) { + System.out.println("No volume data was retrieved."); + return; + } + + List<VolumeInfoProto> volumeInfosList = response.getVolumeInfosList(); + List<VolumeInfo> volumeInfos = convertToVolumeInfos(volumeInfosList); + + // If displayed in JSON format. + if (json) { + System.out.print(JsonUtils.toJsonStringWithDefaultPrettyPrinter(volumeInfos)); Review Comment: Please use `out()` and `err()` inherited from `AbstractSubcommand` for output. ########## hadoop-hdds/interface-client/src/main/proto/hdds.proto: ########## @@ -304,6 +304,15 @@ message RemoveScmResponseProto { optional string scmId = 2; } +message VolumeInfoProto { + required string uuid = 1; + required string hostName = 2; + required string volumeName = 3; + required bool failed = 4; + required int64 failureTime = 5; + required int64 capacity = 6; Review Comment: Add fields as `optional`. https://protobuf.dev/programming-guides/proto2/#field-labels ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/VolumeSubCommand.java: ########## @@ -0,0 +1,222 @@ +/* + * 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 java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.apache.commons.collections.CollectionUtils; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.VolumeInfoProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.hdds.server.JsonUtils; +import org.apache.hadoop.ozone.utils.FormattingCLIUtils; +import org.apache.hadoop.util.StringUtils; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * We provide a set of volume commands to display the disk information of the DataNode. + */ +@Command( + name = "volumes", + description = "Display the list of volumes on the DataNode.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class VolumeSubCommand extends ScmSubcommand { + + // Display it in JSON format. + @Option(names = { "--json" }, + defaultValue = "false", + description = "Format output as JSON.") + private boolean json; + + // Display it in TABLE format. + @Option(names = { "--table" }, + defaultValue = "false", + description = "Format output as Table.") + private boolean table; + + // PageSize refers to the number of items displayed per page + // in a paginated view. + @Option(names = { "--pageSize" }, + defaultValue = "20", + description = "The number of volume information items displayed per page.") + private int pageSize; + + // The current page. + @Option(names = { "--currentPage" }, + defaultValue = "1", + description = "The current page.") + private int currentPage; + + /** + * We have designed a new option called 'show', + * which includes two selectable configurations: + * 'failed' is used to display failed disks, + * and 'normal' is used to display normal disks. + */ + @Option(names = { "--displayMode" }, + defaultValue = "all", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String displayMode; + + // The UUID identifier of the DataNode. + @Option(names = { "--uuid" }, + defaultValue = "", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String uuid; + + // The HostName identifier of the DataNode. + @Option(names = { "--hostName" }, Review Comment: Please avoid camelCase for options. (Also for `--displayMode`.) ```suggestion @Option(names = { "--hostname" }, ``` ########## hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/node/DatanodeInfo.java: ########## @@ -49,7 +51,7 @@ public class DatanodeInfo extends DatanodeDetails { private volatile long lastHeartbeatTime; private long lastStatsUpdatedTime; private int failedVolumeCount; - + private List<VolumeInfoProto> volumeInfos; Review Comment: `volumeInfos` is never assigned. I think the intention was to update this variable in `updateStorageReports` in the block holding `writeLock`. ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/VolumeSubCommand.java: ########## @@ -0,0 +1,222 @@ +/* + * 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 java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.apache.commons.collections.CollectionUtils; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.VolumeInfoProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.hdds.server.JsonUtils; +import org.apache.hadoop.ozone.utils.FormattingCLIUtils; +import org.apache.hadoop.util.StringUtils; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * We provide a set of volume commands to display the disk information of the DataNode. + */ +@Command( + name = "volumes", + description = "Display the list of volumes on the DataNode.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class VolumeSubCommand extends ScmSubcommand { + + // Display it in JSON format. + @Option(names = { "--json" }, + defaultValue = "false", + description = "Format output as JSON.") + private boolean json; + + // Display it in TABLE format. + @Option(names = { "--table" }, + defaultValue = "false", + description = "Format output as Table.") + private boolean table; Review Comment: Please make these [exclusive](https://picocli.info/#_mutually_exclusive_options), like: https://github.com/apache/ozone/blob/af1f98cb3737671df6dce0bdd4af07f8b510dfe7/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/ListOptions.java#L27-L28 https://github.com/apache/ozone/blob/af1f98cb3737671df6dce0bdd4af07f8b510dfe7/hadoop-ozone/cli-shell/src/main/java/org/apache/hadoop/ozone/shell/ListOptions.java#L63-L74 ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/VolumeSubCommand.java: ########## @@ -0,0 +1,222 @@ +/* + * 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 java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.apache.commons.collections.CollectionUtils; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.VolumeInfoProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.hdds.server.JsonUtils; +import org.apache.hadoop.ozone.utils.FormattingCLIUtils; +import org.apache.hadoop.util.StringUtils; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * We provide a set of volume commands to display the disk information of the DataNode. + */ +@Command( + name = "volumes", + description = "Display the list of volumes on the DataNode.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class VolumeSubCommand extends ScmSubcommand { Review Comment: Currently this subcommand is dangling, not added to any parent. If you add it somewhere, it will be available as `ozone ... volumes`. [Suggestion](https://github.com/apache/ozone/pull/7266#discussion_r1819736586) from @errose28 was to add this as `ozone admin datanode volume list`, which allows adding other subcommands related to datanode volumes later. Subcommands are added like this: https://github.com/apache/ozone/blob/af1f98cb3737671df6dce0bdd4af07f8b510dfe7/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/DatanodeCommands.java#L33-L40 ########## hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/datanode/VolumeSubCommand.java: ########## @@ -0,0 +1,222 @@ +/* + * 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 java.io.IOException; +import java.text.SimpleDateFormat; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Locale; +import org.apache.commons.collections.CollectionUtils; +import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos.VolumeInfoProto; +import org.apache.hadoop.hdds.protocol.proto.StorageContainerLocationProtocolProtos.GetVolumeInfosResponseProto; +import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.datanode.VolumeInfo; +import org.apache.hadoop.hdds.server.JsonUtils; +import org.apache.hadoop.ozone.utils.FormattingCLIUtils; +import org.apache.hadoop.util.StringUtils; +import picocli.CommandLine.Command; +import picocli.CommandLine.Option; + +/** + * We provide a set of volume commands to display the disk information of the DataNode. + */ +@Command( + name = "volumes", + description = "Display the list of volumes on the DataNode.", + mixinStandardHelpOptions = true, + versionProvider = HddsVersionProvider.class) +public class VolumeSubCommand extends ScmSubcommand { + + // Display it in JSON format. + @Option(names = { "--json" }, + defaultValue = "false", + description = "Format output as JSON.") + private boolean json; + + // Display it in TABLE format. + @Option(names = { "--table" }, + defaultValue = "false", + description = "Format output as Table.") + private boolean table; + + // PageSize refers to the number of items displayed per page + // in a paginated view. + @Option(names = { "--pageSize" }, + defaultValue = "20", + description = "The number of volume information items displayed per page.") + private int pageSize; + + // The current page. + @Option(names = { "--currentPage" }, + defaultValue = "1", + description = "The current page.") + private int currentPage; + + /** + * We have designed a new option called 'show', + * which includes two selectable configurations: + * 'failed' is used to display failed disks, + * and 'normal' is used to display normal disks. + */ + @Option(names = { "--displayMode" }, + defaultValue = "all", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") + private String displayMode; + + // The UUID identifier of the DataNode. + @Option(names = { "--uuid" }, + defaultValue = "", + description = "failed is used to display failed disks, " + + "normal is used to display normal disks.") Review Comment: description seems to be dup of `--displayMode`. (Same for `--hostName`.) -- 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]
