diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaInfo.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaInfo.java index 24bc4d6d32cb..b9b9d679d63b 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaInfo.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/ContainerReplicaInfo.java @@ -17,16 +17,11 @@ package org.apache.hadoop.hdds.scm.container; -import static org.apache.hadoop.hdds.HddsUtils.checksumToString; - -import com.fasterxml.jackson.core.JsonGenerator; -import com.fasterxml.jackson.databind.JsonSerializer; -import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.annotation.JsonSerialize; -import java.io.IOException; import java.util.UUID; import org.apache.hadoop.hdds.protocol.DatanodeDetails; import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.server.JsonUtils; /** * Class which stores ContainerReplica details on the client. @@ -41,7 +36,7 @@ public final class ContainerReplicaInfo { private long keyCount; private long bytesUsed; private int replicaIndex = -1; - @JsonSerialize(using = LongToHexJsonSerializer.class) + @JsonSerialize(using = JsonUtils.ChecksumSerializer.class) private long dataChecksum; public static ContainerReplicaInfo fromProto( @@ -100,13 +95,6 @@ public long getDataChecksum() { return dataChecksum; } - private static class LongToHexJsonSerializer extends JsonSerializer { - @Override - public void serialize(Long value, JsonGenerator gen, SerializerProvider provider) throws IOException { - gen.writeString(checksumToString(value)); - } - } - /** * Builder for ContainerReplicaInfo class. */ diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/server/JsonUtils.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/server/JsonUtils.java index 633864b2c123..54637458a30c 100644 --- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/server/JsonUtils.java +++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/server/JsonUtils.java @@ -18,21 +18,26 @@ package org.apache.hadoop.hdds.server; import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.core.JsonGenerator; import com.fasterxml.jackson.core.JsonProcessingException; import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.JsonSerializer; import com.fasterxml.jackson.databind.MappingIterator; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; import com.fasterxml.jackson.databind.ObjectWriter; import com.fasterxml.jackson.databind.SequenceWriter; import com.fasterxml.jackson.databind.SerializationFeature; +import com.fasterxml.jackson.databind.SerializerProvider; import com.fasterxml.jackson.databind.node.ArrayNode; import com.fasterxml.jackson.databind.node.ObjectNode; import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import java.io.File; import java.io.IOException; +import java.io.OutputStream; import java.io.Reader; import java.util.List; +import org.apache.hadoop.hdds.HddsUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -76,6 +81,23 @@ public static String toJsonString(Object obj) throws IOException { return MAPPER.writeValueAsString(obj); } + /** + * Returns a {@link SequenceWriter} that will write to and close the provided output stream when it is closed. + * If the sequence is being written to stdout and more stdout output is needed later, use + * {@link #getStdoutSequenceWriter} instead. + */ + public static SequenceWriter getSequenceWriter(OutputStream stream) throws IOException { + return WRITER.writeValuesAsArray(stream); + } + + /** + * Returns a {@link SequenceWriter} that will write to stdout but not close stdout for more output once the sequence + * writer is closed. + */ + public static SequenceWriter getStdoutSequenceWriter() throws IOException { + return getSequenceWriter(new NonClosingOutputStream(System.out)); + } + public static String toJsonStringWIthIndent(Object obj) { try { return INDENT_OUTPUT_MAPPER.writeValueAsString(obj); @@ -107,6 +129,10 @@ public static T readFromReader(Reader reader, Class valueType) throws IOE return MAPPER.readValue(reader, valueType); } + public static ObjectMapper getDefaultMapper() { + return MAPPER; + } + /** * Utility to sequentially write a large collection of items to a file. */ @@ -132,4 +158,47 @@ public static List readFromFile(File file, Class itemType) } } + /** + * Serializes a checksum stored as a long into its json string representation. + */ + public static class ChecksumSerializer extends JsonSerializer { + @Override + public void serialize(Long value, JsonGenerator gen, SerializerProvider provider) throws IOException { + gen.writeString(HddsUtils.checksumToString(value)); + } + } + + private static class NonClosingOutputStream extends OutputStream { + + private final OutputStream delegate; + + NonClosingOutputStream(OutputStream delegate) { + this.delegate = delegate; + } + + @Override + public void write(int b) throws IOException { + delegate.write(b); + } + + @Override + public void write(byte[] b) throws IOException { + delegate.write(b); + } + + @Override + public void write(byte[] b, int off, int len) throws IOException { + delegate.write(b, off, len); + } + + @Override + public void flush() throws IOException { + delegate.flush(); + } + + @Override + public void close() { + // Ignore close to keep the underlying stream open + } + } } diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ContainerIDParameters.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ContainerIDParameters.java index 4b14b40c13f6..36e615a5829e 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ContainerIDParameters.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ContainerIDParameters.java @@ -17,6 +17,7 @@ package org.apache.hadoop.hdds.scm.cli.container; +import java.util.ArrayList; import java.util.List; import org.apache.hadoop.hdds.cli.ItemsFromStdin; import picocli.CommandLine; @@ -25,10 +26,42 @@ @CommandLine.Command public class ContainerIDParameters extends ItemsFromStdin { + @CommandLine.Spec + private CommandLine.Model.CommandSpec spec; + @CommandLine.Parameters(description = "Container IDs" + FORMAT_DESCRIPTION, arity = "1..*", paramLabel = "") public void setContainerIDs(List arguments) { setItems(arguments); } + + public List getValidatedIDs() { + List containerIDs = new ArrayList<>(size()); + List invalidIDs = new ArrayList<>(); + + 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) { + invalidIDs.add(input); + } + } + + if (!invalidIDs.isEmpty()) { + throw new CommandLine.ParameterException(spec.commandLine(), + "Container IDs must be positive integers. Invalid container IDs: " + String.join(" ", invalidIDs)); + } + return containerIDs; + } } diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java index 2c3ad44c9798..34a5c48656f2 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java @@ -69,6 +69,7 @@ public void execute(ScmClient scmClient) throws IOException { multiContainer = containerList.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); first = false; diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java index ba82c8c14842..0b88cec37f9a 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListSubcommand.java @@ -182,6 +182,7 @@ private void listAllContainers(ScmClient scmClient, SequenceWriter writer, } while (fetchedCount > 0); } + // TODO HDDS-13593 Remove this in favor of JsonUtils#getStdoutSequenceWriter. private static class NonClosingOutputStream extends OutputStream { private final OutputStream delegate; diff --git a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java index 79df162cf090..a714ad759df2 100644 --- a/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java +++ b/hadoop-ozone/cli-admin/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ReconcileSubcommand.java @@ -17,15 +17,27 @@ package org.apache.hadoop.hdds.scm.cli.container; +import com.fasterxml.jackson.annotation.JsonInclude; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.databind.SequenceWriter; +import com.fasterxml.jackson.databind.annotation.JsonSerialize; import java.io.IOException; +import java.util.ArrayList; +import java.util.List; import org.apache.hadoop.hdds.cli.HddsVersionProvider; +import org.apache.hadoop.hdds.client.ReplicationConfig; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; import org.apache.hadoop.hdds.scm.cli.ScmSubcommand; import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.container.ContainerInfo; +import org.apache.hadoop.hdds.scm.container.ContainerReplicaInfo; +import org.apache.hadoop.hdds.server.JsonUtils; import picocli.CommandLine; import picocli.CommandLine.Command; /** - * This is the handler that process container list command. + * Handle the container reconcile CLI command. */ @Command( name = "reconcile", @@ -34,15 +46,218 @@ 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 containerIDs = containerList.getValidatedIDs(); + int failureCount = 0; + StringBuilder errorBuilder = new StringBuilder(); + try (SequenceWriter arrayWriter = JsonUtils.getStdoutSequenceWriter()) { + // 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. + for (Long containerID : containerIDs) { + if (!printReconciliationStatus(scmClient, containerID, arrayWriter, errorBuilder)) { + failureCount++; + } + } + arrayWriter.flush(); + } + // Sequence writer will not add a newline to the end. + System.out.println(); + System.out.flush(); + // Flush all json output before printing errors. + if (errorBuilder.length() > 0) { + System.err.print(errorBuilder); + } + if (failureCount > 0) { + throw new RuntimeException("Failed to process reconciliation status for " + failureCount + " container" + + (failureCount > 1 ? "s" : "")); + } + } + + private boolean printReconciliationStatus(ScmClient scmClient, long containerID, SequenceWriter arrayWriter, + StringBuilder errorBuilder) { + try { + ContainerInfo containerInfo = scmClient.getContainer(containerID); + if (containerInfo.isOpen()) { + errorBuilder.append("Cannot get status of container ").append(containerID) + .append(". Reconciliation is not supported for open containers\n"); + return false; + } else if (containerInfo.getReplicationType() != HddsProtos.ReplicationType.RATIS) { + errorBuilder.append("Cannot get status of container ").append(containerID) + .append(". Reconciliation is only supported for Ratis replicated containers\n"); + return false; + } + List replicas = scmClient.getContainerReplicas(containerID); + arrayWriter.write(new ContainerWrapper(containerInfo, replicas)); + arrayWriter.flush(); + } catch (Exception ex) { + errorBuilder.append("Failed to get reconciliation status of container ") + .append(containerID).append(": ").append(getExceptionMessage(ex)).append('\n'); + return false; + } + return true; + } + + private void executeReconcile(ScmClient scmClient) { + int failureCount = 0; + int successCount = 0; + for (Long containerID : containerList.getValidatedIDs()) { + try { + scmClient.reconcileContainer(containerID); + System.out.println("Reconciliation has been triggered for container " + containerID); + successCount++; + } catch (Exception ex) { + System.err.println("Failed to trigger reconciliation for container " + containerID + ": " + + getExceptionMessage(ex)); + failureCount++; + } + } + + if (successCount > 0) { + System.out.println("\nUse \"ozone admin container reconcile --status\" to see the checksums of each container " + + "replica"); + } + if (failureCount > 0) { + throw new RuntimeException("Failed to trigger reconciliation for " + failureCount + " container" + + (failureCount > 1 ? "s" : "")); + } + } + + /** + * Hadoop RPC puts the server side stack trace within the exception message. This method is a workaround to not + * display that to the user. + */ + private String getExceptionMessage(Exception ex) { + return ex.getMessage().split("\n", 2)[0]; + } + + /** + * Used to json serialize the container and replica information for output. + */ + private static class ContainerWrapper { + private final long containerID; + private final HddsProtos.LifeCycleState state; + private final ReplicationConfig replicationConfig; + private boolean replicasMatch; + private final List replicas; + + ContainerWrapper(ContainerInfo info, List replicas) { + this.containerID = info.getContainerID(); + this.state = info.getState(); + this.replicationConfig = info.getReplicationConfig(); + + this.replicas = new ArrayList<>(); + this.replicasMatch = true; + long firstChecksum = 0; + if (!replicas.isEmpty()) { + firstChecksum = replicas.get(0).getDataChecksum(); + } + for (ContainerReplicaInfo replica: replicas) { + replicasMatch = replicasMatch && (firstChecksum == replica.getDataChecksum()); + this.replicas.add(new ReplicaWrapper(replica)); + } + } + + public long getContainerID() { + return containerID; + } + + public HddsProtos.LifeCycleState getState() { + return state; + } + + public ReplicationConfig getReplicationConfig() { + return replicationConfig; + } + + public boolean getReplicasMatch() { + return replicasMatch; + } + + public List getReplicas() { + return replicas; + } + } + + private static class ReplicaWrapper { + private final DatanodeWrapper datanode; + private final String state; + private int replicaIndex; + @JsonSerialize(using = JsonUtils.ChecksumSerializer.class) + private final long dataChecksum; + + ReplicaWrapper(ContainerReplicaInfo replica) { + this.datanode = new DatanodeWrapper(replica.getDatanodeDetails()); + this.state = replica.getState(); + // Only display replica index when it has a positive value for EC. + if (replica.getReplicaIndex() > 0) { + this.replicaIndex = replica.getReplicaIndex(); + } + this.dataChecksum = replica.getDataChecksum(); + } + + public DatanodeWrapper getDatanode() { + return datanode; + } + + public String getState() { + return state; + } + + /** + * Replica index is only included in the output if it is non-zero, which will be the case for EC. + * For Ratis, avoid printing all zero replica indices to avoid confusion. + */ + @JsonInclude(JsonInclude.Include.NON_DEFAULT) + public int getReplicaIndex() { + return replicaIndex; + } + + public long getDataChecksum() { + return dataChecksum; + } + } + + private static class DatanodeWrapper { + private final DatanodeDetails dnDetails; + + DatanodeWrapper(DatanodeDetails dnDetails) { + this.dnDetails = dnDetails; + } + + @JsonProperty(index = 5) + public String getID() { + return dnDetails.getUuidString(); + } + + @JsonProperty(index = 10) + public String getHostname() { + return dnDetails.getHostName(); + } + + // Without specifying a value, Jackson will try to serialize this as "ipaddress". + @JsonProperty(index = 15, value = "ipAddress") + public String getIPAddress() { + return dnDetails.getIpAddress(); + } } } diff --git a/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestReconcileSubcommand.java b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestReconcileSubcommand.java new file mode 100644 index 000000000000..8a64b327bbfd --- /dev/null +++ b/hadoop-ozone/cli-admin/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestReconcileSubcommand.java @@ -0,0 +1,553 @@ +/* + * 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.container; + +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState.CLOSED; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState.OPEN; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.ONE; +import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.api.Assertions.assertEquals; +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.ArgumentMatchers.anyLong; +import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +import com.fasterxml.jackson.core.type.TypeReference; +import com.fasterxml.jackson.databind.JsonNode; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; +import java.io.IOException; +import java.io.InputStream; +import java.io.PrintStream; +import java.io.StringReader; +import java.nio.charset.StandardCharsets; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.UUID; +import java.util.stream.Collectors; +import org.apache.hadoop.hdds.client.ECReplicationConfig; +import org.apache.hadoop.hdds.client.RatisReplicationConfig; +import org.apache.hadoop.hdds.client.ReplicationConfig; +import org.apache.hadoop.hdds.protocol.DatanodeDetails; +import org.apache.hadoop.hdds.protocol.proto.HddsProtos; +import org.apache.hadoop.hdds.scm.client.ScmClient; +import org.apache.hadoop.hdds.scm.container.ContainerInfo; +import org.apache.hadoop.hdds.scm.container.ContainerReplicaInfo; +import org.apache.hadoop.hdds.server.JsonUtils; +import org.assertj.core.api.AbstractStringAssert; +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import picocli.CommandLine; + +/** + * Tests the `ozone admin container reconcile` CLI. + */ +public class TestReconcileSubcommand { + + private static final String EC_CONTAINER_MESSAGE = "Reconciliation is only supported for Ratis replicated containers"; + private static final String OPEN_CONTAINER_MESSAGE = "Reconciliation is not supported for open containers"; + + private ScmClient scmClient; + + private final ByteArrayOutputStream outContent = new ByteArrayOutputStream(); + private final ByteArrayOutputStream errContent = new ByteArrayOutputStream(); + private ByteArrayInputStream inContent; + private final PrintStream originalOut = System.out; + private final PrintStream originalErr = System.err; + private final InputStream originalIn = System.in; + + private static final String DEFAULT_ENCODING = StandardCharsets.UTF_8.name(); + + @BeforeEach + public void setup() throws IOException { + scmClient = mock(ScmClient.class); + + doNothing().when(scmClient).reconcileContainer(anyLong()); + + System.setOut(new PrintStream(outContent, false, DEFAULT_ENCODING)); + System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING)); + } + + @AfterEach + public void after() { + System.setOut(originalOut); + System.setErr(originalErr); + System.setIn(originalIn); + } + + @Test + public void testWithMatchingReplicas() throws Exception { + mockContainer(1); + mockContainer(2); + mockContainer(3); + validateOutput(true, 1, 2, 3); + } + + /** + * When no replicas are present, the "replicasMatch" field should be set to true. + */ + @Test + public void testReplicasMatchWithNoReplicas() throws Exception { + mockContainer(1, 0, RatisReplicationConfig.getInstance(THREE), true); + validateOutput(true, 1); + } + + /** + * When one replica is present, the "replicasMatch" field should be set to true. + */ + @Test + public void testReplicasMatchWithOneReplica() throws Exception { + mockContainer(1, 1, RatisReplicationConfig.getInstance(ONE), true); + validateOutput(true, 1); + } + + @Test + public void testWithMismatchedReplicas() throws Exception { + mockContainer(1, 3, RatisReplicationConfig.getInstance(THREE), false); + mockContainer(2, 3, RatisReplicationConfig.getInstance(THREE), false); + validateOutput(false, 1, 2); + } + + @Test + public void testNoInput() throws Exception { + // PicoCLI should reject commands with no arguments. + assertThrows(CommandLine.MissingParameterException.class, this::executeStatusFromArgs); + assertThrows(CommandLine.MissingParameterException.class, this::executeReconcileFromArgs); + + // When reading from stdin, the arguments are valid, but an empty list results in no output. + executeReconcileFromStdin(); + assertThatOutput(outContent).isEmpty(); + assertThatOutput(errContent).isEmpty(); + + executeStatusFromStdin(); + // Status command should output empty JSON array + String output = outContent.toString(DEFAULT_ENCODING); + JsonNode jsonOutput = JsonUtils.readTree(output); + assertThat(jsonOutput.isArray()).isTrue(); + assertThat(jsonOutput.isEmpty()).isTrue(); + assertThatOutput(errContent).isEmpty(); + } + + /** + * When multiple arguments are given, they are treated as container IDs. Mixing "-" to read from stdin with + * ID arguments will result in "-" raising an invalid container ID error. + */ + @Test + public void testRejectsStdinAndArgs() throws Exception { + mockContainer(1); + // Test sending reconcile command. + Exception reconcileEx = assertThrows(RuntimeException.class, () -> parseArgsAndExecute("1", "-")); + assertThat(reconcileEx.getMessage()) + .contains("Container IDs must be positive integers. Invalid container IDs: -"); + assertThatOutput(outContent).isEmpty(); + + // Test checking status. + Exception statusEx = assertThrows(RuntimeException.class, () -> parseArgsAndExecute("--status", "1", "-")); + assertThat(statusEx.getMessage()) + .contains("Container IDs must be positive integers. Invalid container IDs: -"); + assertThatOutput(outContent).isEmpty(); + } + + /** + * When the `--status` flag is passed, the client will check the replication type and raise an error if the container + * returned is EC. The server lets us get information about containers of any type. + */ + @Test + public void testStatusRejectsECContainer() throws Exception { + mockContainer(1, 3, new ECReplicationConfig(3, 2), true); + + RuntimeException exception = assertThrows(RuntimeException.class, () -> executeStatusFromArgs(1)); + + assertThatOutput(errContent).contains("Cannot get status of container 1"); + assertThatOutput(errContent).contains(EC_CONTAINER_MESSAGE); + + assertThat(exception.getMessage()).contains("Failed to process reconciliation status for 1 container"); + + // Should have empty JSON array output since no containers were processed + String output = outContent.toString(DEFAULT_ENCODING); + JsonNode jsonOutput = JsonUtils.readTree(output); + assertTrue(jsonOutput.isArray()); + assertTrue(jsonOutput.isEmpty()); + } + + /** + * When the `--status` flag is passed, the client will check the container state and raise an error if the container + * returned is open. The server lets us get information about containers in any state. + */ + @Test + public void testStatusRejectsOpenContainer() throws Exception { + mockOpenContainer(1, 3, RatisReplicationConfig.getInstance(THREE)); + + RuntimeException exception = assertThrows(RuntimeException.class, () -> executeStatusFromArgs(1)); + + assertThatOutput(errContent).contains("Cannot get status of container 1"); + assertThatOutput(errContent).contains(OPEN_CONTAINER_MESSAGE); + + assertThat(exception.getMessage()).contains("Failed to process reconciliation status for 1 container"); + + // Should have empty JSON array output since no containers were processed + String output = outContent.toString(DEFAULT_ENCODING); + JsonNode jsonOutput = JsonUtils.readTree(output); + assertTrue(jsonOutput.isArray()); + assertTrue(jsonOutput.isEmpty()); + } + + /** + * Reconciliation is not supported for open or EC containers. This is checked on the server side by SCM when it gets + * a request to reconcile a container. Since the server side is mocked in these tests, this test checks that when any + * exception is thrown back from the server, its message is printed by the client. + */ + @Test + public void testReconcileHandlesInvalidContainer() throws Exception { + mockContainer(1); + + // Mock reconcile to fail for EC container + final String mockMessage = "Mock SCM rejection of container"; + doThrow(new IOException(mockMessage)).when(scmClient).reconcileContainer(1L); + + RuntimeException exception = assertThrows(RuntimeException.class, () -> executeReconcileFromArgs(1)); + + assertThatOutput(errContent).contains("Failed to trigger reconciliation for container 1: " + mockMessage); + + assertThat(exception.getMessage()).contains("Failed to trigger reconciliation for 1 container"); + + // Should have no successful reconcile output + assertThatOutput(outContent).doesNotContain("Reconciliation has been triggered for container 1"); + } + + /** + * When`--status` is given and a mix of Open, Ratis, and EC containers are returned from the server, + * the client should only print results for the closed Ratis containers. Errors for the other containers should be + * printed. + */ + @Test + public void testStatusHandlesValidAndInvalidContainers() throws Exception { + mockContainer(1, 3, new ECReplicationConfig(3, 2), true); + // Container ID 2 is the only valid one. + mockContainer(2, 3, RatisReplicationConfig.getInstance(THREE), true); + mockContainer(3, 3, new ECReplicationConfig(6, 3), true); + mockOpenContainer(4, 3, RatisReplicationConfig.getInstance(THREE)); + + // Test status output - should process Ratis container but fail due to EC containers + RuntimeException exception = assertThrows(RuntimeException.class, () -> { + executeStatusFromArgs(1, 2, 3, 4); + }); + + // Should have error messages for EC and open containers + assertThatOutput(errContent).contains("Cannot get status of container 1"); + assertThatOutput(errContent).contains("Cannot get status of container 3"); + assertThatOutput(errContent).contains("Cannot get status of container 4"); + assertThatOutput(errContent).contains(EC_CONTAINER_MESSAGE); + assertThatOutput(errContent).contains(OPEN_CONTAINER_MESSAGE); + assertThatOutput(errContent).doesNotContain("2"); + + // Exception message should indicate 3 failed containers + assertThat(exception.getMessage()).contains("Failed to process reconciliation status for 3 containers"); + + // Should have output for only container 2: the closed ratis container. + validateStatusOutput(true, 2); + + // Verify that EC containers 1 and 3 and open container 4 are not present in JSON output + String output = outContent.toString(DEFAULT_ENCODING); + JsonNode jsonOutput = JsonUtils.readTree(output); + assertThat(jsonOutput.isArray()).isTrue(); + for (JsonNode containerNode : jsonOutput) { + int containerID = containerNode.get("containerID").asInt(); + assertThat(containerID).isNotIn(1, 3, 4); + } + } + + /** + * Give a mix of valid and invalid containers to reconcile, and mock the server to return errors for the invalid ones. + * The valid containers should still be processed. + */ + @Test + public void testReconcileHandlesValidAndInvalidContainers() throws Exception { + mockContainer(1, 3, new ECReplicationConfig(3, 2), true); + mockContainer(2, 3, RatisReplicationConfig.getInstance(THREE), true); + mockContainer(3, 3, new ECReplicationConfig(6, 3), true); + + // Mock reconcile to fail for EC containers + doThrow(new IOException(EC_CONTAINER_MESSAGE)).when(scmClient).reconcileContainer(1L); + doThrow(new IOException(EC_CONTAINER_MESSAGE)).when(scmClient).reconcileContainer(3L); + + // Test reconcile command - should process Ratis container but fail for EC containers + RuntimeException exception = assertThrows(RuntimeException.class, () -> { + executeReconcileFromArgs(1, 2, 3); + }); + + // Should have error messages for EC containers + assertThatOutput(errContent).contains("Failed to trigger reconciliation for container 1: " + EC_CONTAINER_MESSAGE); + assertThatOutput(errContent).contains("Failed to trigger reconciliation for container 3: " + EC_CONTAINER_MESSAGE); + assertThatOutput(errContent).doesNotContain("Failed to trigger reconciliation for container 2"); + + // Exception message should indicate 2 failed containers + assertThat(exception.getMessage()).contains("Failed to trigger reconciliation for 2 containers"); + + // Should have reconcile success output for container 2 (Ratis) only + validateReconcileOutput(2); + assertThatOutput(outContent).doesNotContain("container 1"); + assertThatOutput(outContent).doesNotContain("container 3"); + } + + /** + * Invalid container IDs are those that cannot be parsed because they are not positive integers. + * When any invalid container ID is passed, the command should fail early instead of proceeding with the valid + * entries. All invalid container IDs should be displayed in the error message, not just the first one. + */ + @Test + public void testSomeInvalidContainerIDs() throws Exception { + // Test status command + Exception statusEx = + assertThrows(RuntimeException.class, () -> parseArgsAndExecute("--status", "123", "invalid", "-1", "456")); + + // Should have error messages for invalid container IDs only. + assertThat(statusEx.getMessage()) + .contains("Container IDs must be positive integers. Invalid container IDs: invalid -1") + .doesNotContain("123", "456"); + assertThatOutput(errContent).doesNotContain("123"); + assertThatOutput(errContent).doesNotContain("456"); + assertThatOutput(outContent).isEmpty(); + + // Test reconcile command + Exception reconcileEx = + assertThrows(RuntimeException.class, () -> parseArgsAndExecute("123", "invalid", "-1", "456")); + + // Should have error messages for invalid IDs + assertThat(reconcileEx.getMessage()) + .contains("Container IDs must be positive integers. Invalid container IDs: invalid -1") + .doesNotContain("123", "456"); + assertThatOutput(errContent).doesNotContain("123"); + assertThatOutput(errContent).doesNotContain("456"); + assertThatOutput(outContent).isEmpty(); + } + + @Test + public void testUnreachableContainers() throws Exception { + final String exceptionMessage = "Container not found"; + + mockContainer(123); + doThrow(new IOException(exceptionMessage)).when(scmClient).getContainer(456L); + + // Test status command - should throw exception due to unreachable containers + assertThrows(RuntimeException.class, () -> parseArgsAndExecute("--status", "123", "456")); + + // Should have error messages for unreachable containers + assertThatOutput(errContent).contains("Failed to get reconciliation status of container 456: " + exceptionMessage); + assertThatOutput(errContent).doesNotContain("123"); + validateStatusOutput(true, 123); + + // Test reconcile command - should also throw exception + doThrow(new IOException(exceptionMessage)).when(scmClient).reconcileContainer(456L); + + assertThrows(RuntimeException.class, () -> parseArgsAndExecute("123", "456")); + // Should have error message for unreachable container + assertThatOutput(errContent).contains("Failed to trigger reconciliation for container 456: " + exceptionMessage); + assertThatOutput(errContent).doesNotContain("123"); + assertThatOutput(outContent).doesNotContain("Reconciliation has been triggered for container 456"); + validateReconcileOutput(123); + } + + private void parseArgsAndExecute(String... args) throws Exception { + // Create fresh streams and command objects for each execution, otherwise stale results may interfere with tests. + if (inContent != null) { + inContent.reset(); + } + outContent.reset(); + errContent.reset(); + System.setOut(new PrintStream(outContent, false, DEFAULT_ENCODING)); + System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING)); + + ReconcileSubcommand cmd = new ReconcileSubcommand(); + new CommandLine(cmd).parseArgs(args); + cmd.execute(scmClient); + } + + private void validateOutput(boolean replicasMatch, long... containerIDs) throws Exception { + // Test reconcile and status with arguments. + executeStatusFromArgs(containerIDs); + validateStatusOutput(replicasMatch, containerIDs); + executeReconcileFromArgs(containerIDs); + validateReconcileOutput(containerIDs); + + // Test reconcile and status with stdin. + executeStatusFromStdin(containerIDs); + validateStatusOutput(replicasMatch, containerIDs); + executeReconcileFromStdin(containerIDs); + validateReconcileOutput(containerIDs); + } + + private void executeStatusFromArgs(long... containerIDs) throws Exception { + List args = Arrays.stream(containerIDs) + .mapToObj(Long::toString) + .collect(Collectors.toList()); + args.add(0, "--status"); + parseArgsAndExecute(args.toArray(new String[]{})); + } + + private void executeReconcileFromArgs(long... containerIDs) throws Exception { + List args = Arrays.stream(containerIDs) + .mapToObj(Long::toString) + .collect(Collectors.toList()); + parseArgsAndExecute(args.toArray(new String[]{})); + } + + private void executeStatusFromStdin(long... containerIDs) throws Exception { + String inputIDs = Arrays.stream(containerIDs) + .mapToObj(Long::toString) + .collect(Collectors.joining("\n")); + inContent = new ByteArrayInputStream(inputIDs.getBytes(DEFAULT_ENCODING)); + System.setIn(inContent); + parseArgsAndExecute("-", "--status"); + } + + private void executeReconcileFromStdin(long... containerIDs) throws Exception { + String inputIDs = Arrays.stream(containerIDs) + .mapToObj(Long::toString) + .collect(Collectors.joining("\n")); + inContent = new ByteArrayInputStream(inputIDs.getBytes(DEFAULT_ENCODING)); + System.setIn(inContent); + parseArgsAndExecute("-"); + } + + private void validateStatusOutput(boolean replicasMatch, long... containerIDs) throws Exception { + String output = outContent.toString(DEFAULT_ENCODING); + // Output should be pretty-printed and end in a newline. + assertThat(output).endsWith("\n"); + + List containerOutputList = JsonUtils.getDefaultMapper() + .readValue(new StringReader(output), new TypeReference>() { }); + assertEquals(containerIDs.length, containerOutputList.size()); + for (Object containerJson: containerOutputList) { + Map containerOutput = (Map)containerJson; + long containerID = (Integer)containerOutput.get("containerID"); + ContainerInfo expectedContainerInfo = scmClient.getContainer(containerID); + List expectedReplicas = scmClient.getContainerReplicas(containerID); + + Map repConfig = (Map)containerOutput.get("replicationConfig"); + + // Check container level fields. + assertEquals(expectedContainerInfo.getContainerID(), ((Integer)containerOutput.get("containerID")).longValue()); + assertEquals(expectedContainerInfo.getState().toString(), containerOutput.get("state")); + assertEquals(expectedContainerInfo.getReplicationConfig().getReplicationType().toString(), + repConfig.get("replicationType")); + assertEquals(replicasMatch, containerOutput.get("replicasMatch")); + + // Check replica fields. + List replicaOutputList = (List)containerOutput.get("replicas"); + assertEquals(expectedReplicas.size(), replicaOutputList.size()); + for (int i = 0; i < expectedReplicas.size(); i++) { + Map replicaOutput = (Map)replicaOutputList.get(i); + ContainerReplicaInfo expectedReplica = expectedReplicas.get(i); + + // Check container replica info. + assertEquals(expectedReplica.getState(), replicaOutput.get("state")); + assertEquals(Long.toHexString(expectedReplica.getDataChecksum()), replicaOutput.get("dataChecksum")); + // Replica index should only be output for EC containers. It has no meaning for Ratis containers. + if (expectedContainerInfo.getReplicationType().equals(HddsProtos.ReplicationType.RATIS)) { + assertFalse(replicaOutput.containsKey("replicaIndex")); + } else { + assertEquals(expectedReplica.getReplicaIndex(), replicaOutput.get("replicaIndex")); + } + + // Check datanode info. + Map dnOutput = (Map)replicaOutput.get("datanode"); + DatanodeDetails expectedDnDetails = expectedReplica.getDatanodeDetails(); + + assertEquals(expectedDnDetails.getHostName(), dnOutput.get("hostname")); + assertEquals(expectedDnDetails.getUuidString(), dnOutput.get("id")); + assertEquals(expectedDnDetails.getIpAddress(), dnOutput.get("ipAddress")); + // Datanode output should be brief and only contain the above three identifiers. + assertEquals(3, dnOutput.size()); + } + } + } + + private void validateReconcileOutput(long... containerIDs) throws Exception { + for (long id: containerIDs) { + verify(scmClient, atLeastOnce()).reconcileContainer(id); + assertThatOutput(outContent).contains("Reconciliation has been triggered for container " + id); + } + } + + private AbstractStringAssert assertThatOutput(ByteArrayOutputStream stream) throws Exception { + return assertThat(stream.toString(DEFAULT_ENCODING)); + } + + private void mockContainer(long containerID) throws Exception { + mockContainer(containerID, 3, RatisReplicationConfig.getInstance(THREE), true); + } + + private void mockOpenContainer(long containerID, int numReplicas, ReplicationConfig repConfig) throws Exception { + mockContainer(containerID, numReplicas, repConfig, OPEN, true); + } + + private void mockContainer(long containerID, int numReplicas, ReplicationConfig repConfig, boolean replicasMatch) + throws Exception { + mockContainer(containerID, numReplicas, repConfig, CLOSED, replicasMatch); + } + + private void mockContainer(long containerID, int numReplicas, ReplicationConfig repConfig, + HddsProtos.LifeCycleState state, boolean replicasMatch) throws Exception { + ContainerInfo container = new ContainerInfo.Builder() + .setContainerID(containerID) + .setState(state) + .setReplicationConfig(repConfig) + .build(); + when(scmClient.getContainer(containerID)).thenReturn(container); + + List replicas = new ArrayList<>(); + int replicaIndex = 1; + for (int i = 0; i < numReplicas; i++) { + DatanodeDetails dn = DatanodeDetails.newBuilder() + .setHostName("dn") + .setUuid(UUID.randomUUID()) + .setIpAddress("127.0.0.1") + .build(); + + ContainerReplicaInfo.Builder replicaBuilder = new ContainerReplicaInfo.Builder() + .setContainerID(containerID) + .setState(state.name()) + .setDatanodeDetails(dn); + if (repConfig.getReplicationType() != HddsProtos.ReplicationType.RATIS) { + replicaBuilder.setReplicaIndex(replicaIndex++); + } + if (replicasMatch) { + if (state == OPEN) { + replicaBuilder.setDataChecksum(0); + } else { + replicaBuilder.setDataChecksum(123); + } + } else { + replicaBuilder.setDataChecksum(i); + } + replicas.add(replicaBuilder.build()); + } + when(scmClient.getContainerReplicas(containerID)).thenReturn(replicas); + } +} diff --git a/hadoop-ozone/dist/src/main/smoketest/admincli/container.robot b/hadoop-ozone/dist/src/main/smoketest/admincli/container.robot index d419cbf7aecd..e9450c9de595 100644 --- a/hadoop-ozone/dist/src/main/smoketest/admincli/container.robot +++ b/hadoop-ozone/dist/src/main/smoketest/admincli/container.robot @@ -36,12 +36,19 @@ Container is closed Container checksums should match [arguments] ${container} ${expected_checksum} - ${data_checksum1} = Execute ozone admin container info "${container}" --json | jq -r '.replicas[0].dataChecksum' | head -n1 - ${data_checksum2} = Execute ozone admin container info "${container}" --json | jq -r '.replicas[1].dataChecksum' | head -n1 - ${data_checksum3} = Execute ozone admin container info "${container}" --json | jq -r '.replicas[2].dataChecksum' | head -n1 + ${data_checksum1} = Execute ozone admin container reconcile --status "${container}" | jq -r '.[].replicas[0].dataChecksum' + ${data_checksum2} = Execute ozone admin container reconcile --status "${container}" | jq -r '.[].replicas[1].dataChecksum' + ${data_checksum3} = Execute ozone admin container reconcile --status "${container}" | jq -r '.[].replicas[2].dataChecksum' Should be equal as strings ${data_checksum1} ${expected_checksum} Should be equal as strings ${data_checksum2} ${expected_checksum} Should be equal as strings ${data_checksum3} ${expected_checksum} + # Verify that container info shows the same checksums as reconcile status + ${info_checksum1} = Execute ozone admin container info "${container}" --json | jq -r '.replicas[0].dataChecksum' + ${info_checksum2} = Execute ozone admin container info "${container}" --json | jq -r '.replicas[1].dataChecksum' + ${info_checksum3} = Execute ozone admin container info "${container}" --json | jq -r '.replicas[2].dataChecksum' + Should be equal as strings ${data_checksum1} ${info_checksum1} + Should be equal as strings ${data_checksum2} ${info_checksum2} + Should be equal as strings ${data_checksum3} ${info_checksum3} *** Test Cases *** Create container @@ -181,10 +188,9 @@ Reset user Cannot reconcile open container # At this point we should have an open Ratis Three container. ${container} = Execute ozone admin container list --state OPEN | jq -r '.[] | select(.replicationConfig.replicationFactor == "THREE") | .containerID' | head -n1 + # Reconciling and querying status of open containers is not supported Execute and check rc ozone admin container reconcile "${container}" 255 - # The container should not yet have any replica checksums since it is still open. - # 0 is the hex value of an empty checksum. - Container checksums should match ${container} 0 + Execute and check rc ozone admin container reconcile --status "${container}" 255 Close container ${container} = Execute ozone admin container list --state OPEN | jq -r '.[] | select(.replicationConfig.replicationFactor == "THREE") | .containerID' | head -1 @@ -196,9 +202,8 @@ Close container Wait until keyword succeeds 1min 10sec Container is closed ${container} Reconcile closed container - # Check that info does not show replica checksums, since manual reconciliation has not yet been triggered. ${container} = Execute ozone admin container list --state CLOSED | jq -r '.[] | select(.replicationConfig.replicationFactor == "THREE") | .containerID' | head -1 - ${data_checksum} = Execute ozone admin container info "${container}" --json | jq -r '.replicas[].dataChecksum' | head -n1 + ${data_checksum} = Execute ozone admin container reconcile --status "${container}" | jq -r '.[].replicas[0].dataChecksum' # Once the container is closed, the data checksum should be populated Should Not Be Equal As Strings 0 ${data_checksum} Container checksums should match ${container} ${data_checksum}