From 36e759b633250a60c5fc8c27e2f29bd4167da645 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Wed, 15 Nov 2023 17:14:26 +0000 Subject: [PATCH 1/6] Enable multiple IDs passed with spaces on the command line --- .../scm/cli/container/InfoSubcommand.java | 37 ++++++++++++++++--- .../scm/cli/container/TestInfoSubCommand.java | 23 ++++++++++++ 2 files changed, 54 insertions(+), 6 deletions(-) diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java index 9524ce94716f..c20ea02d7ee1 100644 --- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java +++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java @@ -73,14 +73,39 @@ public class InfoSubcommand extends ScmSubcommand { description = "Format output as JSON") private boolean json; - @Parameters(description = "Decimal id of the container.") - private long containerID; + @Parameters(description = "One or more container IDs separated by spaces.") + private String[] containerList; @Override public void execute(ScmClient scmClient) throws IOException { - final ContainerWithPipeline container = scmClient. - getContainerWithPipeline(containerID); - Preconditions.checkNotNull(container, "Container cannot be null"); + boolean first = true; + for (String id : containerList) { + if (!first) { + LOG.info(""); // Emit a blank line + } + first = false; + long containerID; + try { + containerID = Long.parseLong(id); + } catch (NumberFormatException e) { + LOG.error("Invalid container ID: {}", id); + continue; + } + printDetails(scmClient, containerID); + } + } + + private void printDetails(ScmClient scmClient, long containerID) + throws IOException { + final ContainerWithPipeline container; + try { + container = scmClient.getContainerWithPipeline(containerID); + Preconditions.checkNotNull(container, "Container cannot be null"); + } catch (IOException e) { + LOG.error("Unable to retrieve the container details for {}", containerID); + return; + } + List replicas = null; try { replicas = scmClient.getContainerReplicas(containerID); @@ -132,7 +157,7 @@ public void execute(ScmClient scmClient) throws IOException { // Print pipeline of an existing container. String machinesStr = container.getPipeline().getNodes().stream().map( - InfoSubcommand::buildDatanodeDetails) + InfoSubcommand::buildDatanodeDetails) .collect(Collectors.joining(",\n")); LOG.info("Datanodes: [{}]", machinesStr); diff --git a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java index 6058546c97db..ddb81e01927d 100644 --- a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java +++ b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java @@ -92,6 +92,29 @@ public void testReplicaIndexInOutput() throws Exception { testReplicaIncludedInOutput(true); } + @Test + public void testMultipleContainersCanBePassed() throws Exception { + Mockito.when(scmClient.getContainerReplicas(anyLong())) + .thenReturn(getReplicas(true)); + cmd = new InfoSubcommand(); + CommandLine c = new CommandLine(cmd); + c.parseArgs("1", "123", "456", "invalid", "789"); + cmd.execute(scmClient); + + // Ensure we have a log line for each containerID + List logs = appender.getLog(); + List replica = logs.stream() + .filter(m -> m.getRenderedMessage() + .matches("(?s)^Container id: (1|123|456|789).*")) + .collect(Collectors.toList()); + Assertions.assertEquals(4, replica.size()); + + replica = logs.stream() + .filter(m -> m.getRenderedMessage() + .matches("(?s)^Invalid container ID: invalid.*")) + .collect(Collectors.toList()); + Assertions.assertEquals(1, replica.size()); + } private void testReplicaIncludedInOutput(boolean includeIndex) throws IOException { From 1912c88d0a2a43d62576fba69dd9bb2d65d5616d Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Thu, 16 Nov 2023 17:17:27 +0000 Subject: [PATCH 2/6] Changes to ensure correctly formatted JSON --- .../scm/cli/container/InfoSubcommand.java | 40 +++++++++++++++++-- .../scm/cli/container/TestInfoSubCommand.java | 36 +++++++++++++---- 2 files changed, 65 insertions(+), 11 deletions(-) diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java index c20ea02d7ee1..954cf997f2d9 100644 --- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java +++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java @@ -79,20 +79,50 @@ public class InfoSubcommand extends ScmSubcommand { @Override public void execute(ScmClient scmClient) throws IOException { boolean first = true; + printHeader(); for (String id : containerList) { if (!first) { - LOG.info(""); // Emit a blank line + printBreak(); } first = false; long containerID; try { containerID = Long.parseLong(id); } catch (NumberFormatException e) { - LOG.error("Invalid container ID: {}", id); + printError("Invalid container ID: " + id); continue; } printDetails(scmClient, containerID); } + printFooter(); + } + + private void printHeader() { + if (json && containerList.length > 1) { + LOG.info("["); + } + } + + private void printFooter() { + if (json && containerList.length > 1) { + LOG.info("]"); + } + } + + private void printError(String error) { + if (json) { + LOG.info("{ \"error\": \"" + error + "\" }"); + } else { + LOG.info(error); + } + } + + private void printBreak() { + if (json) { + LOG.info(","); + } else { + LOG.info(""); + } } private void printDetails(ScmClient scmClient, long containerID) @@ -102,7 +132,7 @@ private void printDetails(ScmClient scmClient, long containerID) container = scmClient.getContainerWithPipeline(containerID); Preconditions.checkNotNull(container, "Container cannot be null"); } catch (IOException e) { - LOG.error("Unable to retrieve the container details for {}", containerID); + printError("Unable to retrieve the container details for " + containerID); return; } @@ -110,7 +140,9 @@ private void printDetails(ScmClient scmClient, long containerID) try { replicas = scmClient.getContainerReplicas(containerID); } catch (IOException e) { - LOG.error("Unable to retrieve the replica details", e); + if (!json) { + LOG.error("Unable to retrieve the replica details", e); + } } if (json) { diff --git a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java index ddb81e01927d..1488540cc29a 100644 --- a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java +++ b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java @@ -69,7 +69,7 @@ public void setup() throws IOException { scmClient = mock(ScmClient.class); datanodes = createDatanodeDetails(3); Mockito.when(scmClient.getContainerWithPipeline(anyLong())) - .thenReturn(getContainerWithPipeline()); + .then(i -> getContainerWithPipeline(i.getArgument(0))); appender = new TestAppender(); logger = Logger.getLogger( @@ -116,6 +116,30 @@ public void testMultipleContainersCanBePassed() throws Exception { Assertions.assertEquals(1, replica.size()); } + @Test + public void testMultipleContainersCanBePassedJson() throws Exception { + Mockito.when(scmClient.getContainerReplicas(anyLong())) + .thenReturn(getReplicas(true)); + cmd = new InfoSubcommand(); + CommandLine c = new CommandLine(cmd); + c.parseArgs("1", "123", "456", "invalid", "789", "--json"); + cmd.execute(scmClient); + + // Ensure we have a log line for each containerID + List logs = appender.getLog(); + List replica = logs.stream() + .filter(m -> m.getRenderedMessage() + .matches("(?s)^.*\"containerInfo\".*")) + .collect(Collectors.toList()); + Assertions.assertEquals(4, replica.size()); + + replica = logs.stream() + .filter(m -> m.getRenderedMessage() + .matches("(?s)^.*error: \"Invalid container ID: invalid.*")) + .collect(Collectors.toList()); + Assertions.assertEquals(1, replica.size()); + } + private void testReplicaIncludedInOutput(boolean includeIndex) throws IOException { Mockito.when(scmClient.getContainerReplicas(anyLong())) @@ -195,12 +219,9 @@ public void testReplicasNotOutputIfErrorWithJson() throws IOException { cmd.execute(scmClient); List logs = appender.getLog(); - Assertions.assertEquals(2, logs.size()); - String error = logs.get(0).getRenderedMessage(); - String json = logs.get(1).getRenderedMessage(); + Assertions.assertEquals(1, logs.size()); + String json = logs.get(0).getRenderedMessage(); - Assertions.assertTrue(error - .matches("(?s)^Unable to retrieve the replica details.*")); Assertions.assertFalse(json.matches("(?s).*replicas.*")); } @@ -275,7 +296,7 @@ private List getReplicas(boolean includeIndex) { return replicas; } - private ContainerWithPipeline getContainerWithPipeline() { + private ContainerWithPipeline getContainerWithPipeline(long containerID) { Pipeline pipeline = new Pipeline.Builder() .setState(Pipeline.PipelineState.CLOSED) .setReplicationConfig(RatisReplicationConfig.getInstance(THREE)) @@ -284,6 +305,7 @@ private ContainerWithPipeline getContainerWithPipeline() { .build(); ContainerInfo container = new ContainerInfo.Builder() + .setContainerID(containerID) .setSequenceId(1) .setPipelineID(pipeline.getId()) .setUsedBytes(1234) From 18aa33c17a0af6f36745d72a62b3be73e6cd4126 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Fri, 17 Nov 2023 16:00:01 +0000 Subject: [PATCH 3/6] Read from stdin --- .../scm/cli/container/InfoSubcommand.java | 47 ++++++++++++++----- .../scm/cli/container/TestInfoSubCommand.java | 2 +- 2 files changed, 35 insertions(+), 14 deletions(-) diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java index 954cf997f2d9..537a3514f64c 100644 --- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java +++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java @@ -24,6 +24,7 @@ import java.util.HashMap; import java.util.ArrayList; import java.time.Instant; +import java.util.Scanner; import java.util.stream.Collectors; import org.apache.hadoop.hdds.cli.GenericParentCommand; @@ -76,35 +77,55 @@ public class InfoSubcommand extends ScmSubcommand { @Parameters(description = "One or more container IDs separated by spaces.") private String[] containerList; + private boolean multiContainer = false; + @Override public void execute(ScmClient scmClient) throws IOException { boolean first = true; + if (containerList == null || containerList.length > 1) { + multiContainer = true; + } printHeader(); - for (String id : containerList) { - if (!first) { - printBreak(); + if (containerList == null) { + Scanner scanner = new Scanner(System.in, "UTF-8"); + System.in. + while (scanner.hasNextLine()) { + String id = scanner.nextLine().trim(); + printOutput(scmClient, id, first); + first = false; } - first = false; - long containerID; - try { - containerID = Long.parseLong(id); - } catch (NumberFormatException e) { - printError("Invalid container ID: " + id); - continue; + } else { + for (String id : containerList) { + printOutput(scmClient, id, first); + first = false; } - printDetails(scmClient, containerID); } printFooter(); } + private void printOutput(ScmClient scmClient, String id, boolean first) + throws IOException { + if (!first) { + printBreak(); + } + long containerID; + try { + containerID = Long.parseLong(id); + } catch (NumberFormatException e) { + printError("Invalid container ID: " + id); + return; + } + printDetails(scmClient, containerID); + } + private void printHeader() { - if (json && containerList.length > 1) { + if (json && multiContainer) { LOG.info("["); } } private void printFooter() { - if (json && containerList.length > 1) { + if (json && multiContainer) { LOG.info("]"); } } diff --git a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java index 1488540cc29a..bb998ad0d5d3 100644 --- a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java +++ b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java @@ -135,7 +135,7 @@ public void testMultipleContainersCanBePassedJson() throws Exception { replica = logs.stream() .filter(m -> m.getRenderedMessage() - .matches("(?s)^.*error: \"Invalid container ID: invalid.*")) + .matches("(?s)^.*\"error\": \"Invalid container ID: invalid.*")) .collect(Collectors.toList()); Assertions.assertEquals(1, replica.size()); } From bc616196ba8701501cec138005021f261cdab842 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Tue, 21 Nov 2023 16:11:50 +0000 Subject: [PATCH 4/6] Read from stdin only if dash is passed --- .../scm/cli/container/InfoSubcommand.java | 15 +++-- .../scm/cli/container/TestInfoSubCommand.java | 66 +++++++++++++++++++ 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java index 537a3514f64c..f85bc37a501d 100644 --- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java +++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java @@ -74,7 +74,11 @@ public class InfoSubcommand extends ScmSubcommand { description = "Format output as JSON") private boolean json; - @Parameters(description = "One or more container IDs separated by spaces.") + @Parameters(description = "One or more container IDs separated by spaces. " + + "To read from stdin, specify '-' and supply the container IDs " + + "separated by newlines.", + arity = "1..*", + paramLabel = "") private String[] containerList; private boolean multiContainer = false; @@ -82,13 +86,16 @@ public class InfoSubcommand extends ScmSubcommand { @Override public void execute(ScmClient scmClient) throws IOException { boolean first = true; - if (containerList == null || containerList.length > 1) { + boolean stdin = false; + if (containerList.length > 1) { multiContainer = true; + } else if (containerList[0].equals("-")) { + stdin = true; } + printHeader(); - if (containerList == null) { + if (stdin) { Scanner scanner = new Scanner(System.in, "UTF-8"); - System.in. while (scanner.hasNextLine()) { String id = scanner.nextLine().trim(); printOutput(scmClient, id, first); diff --git a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java index bb998ad0d5d3..b12a99cd9d58 100644 --- a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java +++ b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java @@ -39,7 +39,12 @@ import org.mockito.Mockito; import picocli.CommandLine; +import java.io.ByteArrayInputStream; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; +import java.io.PrintStream; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.UUID; @@ -49,6 +54,7 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState.CLOSED; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.mockito.ArgumentMatchers.anyLong; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -64,6 +70,15 @@ public class TestInfoSubCommand { private Logger logger; private TestAppender appender; + 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); @@ -75,11 +90,17 @@ public void setup() throws IOException { logger = Logger.getLogger( org.apache.hadoop.hdds.scm.cli.container.InfoSubcommand.class); logger.addAppender(appender); + + System.setOut(new PrintStream(outContent, false, DEFAULT_ENCODING)); + System.setErr(new PrintStream(errContent, false, DEFAULT_ENCODING)); } @AfterEach public void after() { logger.removeAppender(appender); + System.setOut(originalOut); + System.setErr(originalErr); + System.setIn(originalIn); } @Test @@ -92,6 +113,16 @@ public void testReplicaIndexInOutput() throws Exception { testReplicaIncludedInOutput(true); } + @Test + public void testErrorWhenNoContainerIDParam() throws Exception { + cmd = new InfoSubcommand(); + assertThrows(CommandLine.MissingParameterException.class, () -> { + CommandLine c = new CommandLine(cmd); + c.parseArgs(); + cmd.execute(scmClient); + }); + } + @Test public void testMultipleContainersCanBePassed() throws Exception { Mockito.when(scmClient.getContainerReplicas(anyLong())) @@ -100,7 +131,23 @@ public void testMultipleContainersCanBePassed() throws Exception { CommandLine c = new CommandLine(cmd); c.parseArgs("1", "123", "456", "invalid", "789"); cmd.execute(scmClient); + validateMultiOutput(); + } + + @Test + public void testContainersCanBeReadFromStdin() throws IOException { + String input = "1\n123\n456\ninvalid\n789\n"; + inContent = new ByteArrayInputStream(input.getBytes()); + System.setIn(inContent); + cmd = new InfoSubcommand(); + CommandLine c = new CommandLine(cmd); + c.parseArgs("-"); + cmd.execute(scmClient); + validateMultiOutput(); + } + + private void validateMultiOutput() { // Ensure we have a log line for each containerID List logs = appender.getLog(); List replica = logs.stream() @@ -116,6 +163,21 @@ public void testMultipleContainersCanBePassed() throws Exception { Assertions.assertEquals(1, replica.size()); } + @Test + public void testContainersCanBeReadFromStdinJson() + throws IOException { + String input = "1\n123\n456\ninvalid\n789\n"; + inContent = new ByteArrayInputStream(input.getBytes()); + System.setIn(inContent); + cmd = new InfoSubcommand(); + CommandLine c = new CommandLine(cmd); + c.parseArgs("-", "--json"); + cmd.execute(scmClient); + + validateJsonMultiOutput(); + } + + @Test public void testMultipleContainersCanBePassedJson() throws Exception { Mockito.when(scmClient.getContainerReplicas(anyLong())) @@ -125,6 +187,10 @@ public void testMultipleContainersCanBePassedJson() throws Exception { c.parseArgs("1", "123", "456", "invalid", "789", "--json"); cmd.execute(scmClient); + validateJsonMultiOutput(); + } + + private void validateJsonMultiOutput() { // Ensure we have a log line for each containerID List logs = appender.getLog(); List replica = logs.stream() From cc1aec8146b04ae8b1b1b4a204bd8b2bdd6d0e97 Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Wed, 22 Nov 2023 10:08:17 +0000 Subject: [PATCH 5/6] Errors to stdout and some formatting changes --- .../scm/cli/container/InfoSubcommand.java | 24 ++++++------- .../scm/cli/container/TestInfoSubCommand.java | 36 +++++++++---------- 2 files changed, 26 insertions(+), 34 deletions(-) diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java index f85bc37a501d..ad12a93bd5d6 100644 --- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java +++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java @@ -91,6 +91,8 @@ public void execute(ScmClient scmClient) throws IOException { multiContainer = true; } else if (containerList[0].equals("-")) { stdin = true; + // Assume multiple containers if reading from stdin + multiContainer = true; } printHeader(); @@ -112,9 +114,6 @@ public void execute(ScmClient scmClient) throws IOException { private void printOutput(ScmClient scmClient, String id, boolean first) throws IOException { - if (!first) { - printBreak(); - } long containerID; try { containerID = Long.parseLong(id); @@ -122,7 +121,7 @@ private void printOutput(ScmClient scmClient, String id, boolean first) printError("Invalid container ID: " + id); return; } - printDetails(scmClient, containerID); + printDetails(scmClient, containerID, first); } private void printHeader() { @@ -138,11 +137,7 @@ private void printFooter() { } private void printError(String error) { - if (json) { - LOG.info("{ \"error\": \"" + error + "\" }"); - } else { - LOG.info(error); - } + System.err.println(error); } private void printBreak() { @@ -153,8 +148,8 @@ private void printBreak() { } } - private void printDetails(ScmClient scmClient, long containerID) - throws IOException { + private void printDetails(ScmClient scmClient, long containerID, + boolean first) throws IOException { final ContainerWithPipeline container; try { container = scmClient.getContainerWithPipeline(containerID); @@ -168,11 +163,12 @@ private void printDetails(ScmClient scmClient, long containerID) try { replicas = scmClient.getContainerReplicas(containerID); } catch (IOException e) { - if (!json) { - LOG.error("Unable to retrieve the replica details", e); - } + printError("Unable to retrieve the replica details: " + e.getMessage()); } + if (!first) { + printBreak(); + } if (json) { if (container.getPipeline().size() != 0) { ContainerWithPipelineAndReplicas wrapper = diff --git a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java index b12a99cd9d58..c88e4fd216b1 100644 --- a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java +++ b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java @@ -29,7 +29,6 @@ import org.apache.hadoop.hdds.scm.pipeline.PipelineID; import org.apache.hadoop.hdds.scm.pipeline.PipelineNotFoundException; import org.apache.log4j.AppenderSkeleton; -import org.apache.log4j.Level; import org.apache.log4j.Logger; import org.apache.log4j.spi.LoggingEvent; import org.junit.jupiter.api.AfterEach; @@ -44,6 +43,7 @@ import java.io.IOException; import java.io.InputStream; import java.io.PrintStream; +import java.io.UnsupportedEncodingException; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; @@ -55,6 +55,7 @@ import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.LifeCycleState.CLOSED; import static org.apache.hadoop.hdds.protocol.proto.HddsProtos.ReplicationFactor.THREE; 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.ArgumentMatchers.any; import static org.mockito.Mockito.mock; @@ -147,7 +148,7 @@ public void testContainersCanBeReadFromStdin() throws IOException { validateMultiOutput(); } - private void validateMultiOutput() { + private void validateMultiOutput() throws UnsupportedEncodingException { // Ensure we have a log line for each containerID List logs = appender.getLog(); List replica = logs.stream() @@ -156,11 +157,10 @@ private void validateMultiOutput() { .collect(Collectors.toList()); Assertions.assertEquals(4, replica.size()); - replica = logs.stream() - .filter(m -> m.getRenderedMessage() - .matches("(?s)^Invalid container ID: invalid.*")) - .collect(Collectors.toList()); - Assertions.assertEquals(1, replica.size()); + Pattern p = Pattern.compile( + "^Invalid\\scontainer\\sID:\\sinvalid.*", Pattern.MULTILINE); + Matcher m = p.matcher(errContent.toString(DEFAULT_ENCODING)); + assertTrue(m.find()); } @Test @@ -190,7 +190,7 @@ public void testMultipleContainersCanBePassedJson() throws Exception { validateJsonMultiOutput(); } - private void validateJsonMultiOutput() { + private void validateJsonMultiOutput() throws UnsupportedEncodingException { // Ensure we have a log line for each containerID List logs = appender.getLog(); List replica = logs.stream() @@ -199,11 +199,10 @@ private void validateJsonMultiOutput() { .collect(Collectors.toList()); Assertions.assertEquals(4, replica.size()); - replica = logs.stream() - .filter(m -> m.getRenderedMessage() - .matches("(?s)^.*\"error\": \"Invalid container ID: invalid.*")) - .collect(Collectors.toList()); - Assertions.assertEquals(1, replica.size()); + Pattern p = Pattern.compile( + "^Invalid\\scontainer\\sID:\\sinvalid.*", Pattern.MULTILINE); + Matcher m = p.matcher(errContent.toString(DEFAULT_ENCODING)); + assertTrue(m.find()); } private void testReplicaIncludedInOutput(boolean includeIndex) @@ -266,13 +265,10 @@ public void testReplicasNotOutputIfError() throws IOException { .collect(Collectors.toList()); Assertions.assertEquals(0, replica.size()); - // Ensure we have an error logged: - List error = logs.stream() - .filter(m -> m.getLevel() == Level.ERROR) - .collect(Collectors.toList()); - Assertions.assertEquals(1, error.size()); - Assertions.assertTrue(error.get(0).getRenderedMessage() - .matches("(?s)^Unable to retrieve the replica details.*")); + Pattern p = Pattern.compile( + "^Unable to retrieve the replica details.*", Pattern.MULTILINE); + Matcher m = p.matcher(errContent.toString(DEFAULT_ENCODING)); + assertTrue(m.find()); } @Test From db531bedf3c1b4540c8b5573a03ee6396d9c6ada Mon Sep 17 00:00:00 2001 From: S O'Donnell Date: Wed, 22 Nov 2023 11:29:09 +0000 Subject: [PATCH 6/6] Fix findbugs and failing tests caused by rebase --- .../hadoop/hdds/scm/cli/container/InfoSubcommand.java | 2 +- .../hdds/scm/cli/container/TestInfoSubCommand.java | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java index ad12a93bd5d6..8ed9f520b29d 100644 --- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java +++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/InfoSubcommand.java @@ -206,7 +206,7 @@ private void printDetails(ScmClient scmClient, long containerID, ioe) instanceof PipelineNotFoundException) { LOG.info("Write Pipeline State: CLOSED"); } else { - LOG.error("Failed to retrieve pipeline info"); + printError("Failed to retrieve pipeline info"); } } LOG.info("Container State: {}", container.getContainerInfo().getState()); diff --git a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java index c88e4fd216b1..9b264312fdf4 100644 --- a/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java +++ b/hadoop-hdds/tools/src/test/java/org/apache/hadoop/hdds/scm/cli/container/TestInfoSubCommand.java @@ -86,6 +86,8 @@ public void setup() throws IOException { datanodes = createDatanodeDetails(3); Mockito.when(scmClient.getContainerWithPipeline(anyLong())) .then(i -> getContainerWithPipeline(i.getArgument(0))); + Mockito.when(scmClient.getPipeline(any())) + .thenThrow(new PipelineNotFoundException("Pipeline not found.")); appender = new TestAppender(); logger = Logger.getLogger( @@ -138,7 +140,7 @@ public void testMultipleContainersCanBePassed() throws Exception { @Test public void testContainersCanBeReadFromStdin() throws IOException { String input = "1\n123\n456\ninvalid\n789\n"; - inContent = new ByteArrayInputStream(input.getBytes()); + inContent = new ByteArrayInputStream(input.getBytes(DEFAULT_ENCODING)); System.setIn(inContent); cmd = new InfoSubcommand(); CommandLine c = new CommandLine(cmd); @@ -167,7 +169,7 @@ private void validateMultiOutput() throws UnsupportedEncodingException { public void testContainersCanBeReadFromStdinJson() throws IOException { String input = "1\n123\n456\ninvalid\n789\n"; - inContent = new ByteArrayInputStream(input.getBytes()); + inContent = new ByteArrayInputStream(input.getBytes(DEFAULT_ENCODING)); System.setIn(inContent); cmd = new InfoSubcommand(); CommandLine c = new CommandLine(cmd); @@ -209,8 +211,6 @@ private void testReplicaIncludedInOutput(boolean includeIndex) throws IOException { Mockito.when(scmClient.getContainerReplicas(anyLong())) .thenReturn(getReplicas(includeIndex)); - Mockito.when(scmClient.getPipeline(any())) - .thenThrow(new PipelineNotFoundException("Pipeline not found.")); cmd = new InfoSubcommand(); CommandLine c = new CommandLine(cmd); c.parseArgs("1"); @@ -251,8 +251,6 @@ private void testReplicaIncludedInOutput(boolean includeIndex) public void testReplicasNotOutputIfError() throws IOException { Mockito.when(scmClient.getContainerReplicas(anyLong())) .thenThrow(new IOException("Error getting Replicas")); - Mockito.when(scmClient.getPipeline(any())) - .thenThrow(new PipelineNotFoundException("Pipeline not found.")); cmd = new InfoSubcommand(); CommandLine c = new CommandLine(cmd); c.parseArgs("1");