From 48d0b548492a3fc0b072543be81b5e1b0ea1f278 Mon Sep 17 00:00:00 2001
From: Anu Engineer <aengineer@apache.org>
Date: Thu, 10 May 2018 12:43:13 -0700
Subject: [PATCH] HDDS-31. Fix TestSCMCli. Contributed by Lokesh Jain.

---
 .../common/helpers/ContainerInfo.java         | 26 ++++++
 .../cli/container/CreateContainerHandler.java |  8 +-
 .../cli/container/ListContainerHandler.java   |  8 +-
 .../apache/hadoop/ozone/scm/TestSCMCli.java   | 80 +++++++++----------
 4 files changed, 74 insertions(+), 48 deletions(-)

diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/common/helpers/ContainerInfo.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/common/helpers/ContainerInfo.java
index 0bd4c26d42a..10fd96c175a 100644
--- a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/common/helpers/ContainerInfo.java
+++ b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/scm/container/common/helpers/ContainerInfo.java
@@ -18,6 +18,10 @@
 
 package org.apache.hadoop.hdds.scm.container.common.helpers;
 
+import com.fasterxml.jackson.annotation.JsonAutoDetect;
+import com.fasterxml.jackson.annotation.PropertyAccessor;
+import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.ObjectWriter;
 import com.google.common.base.Preconditions;
 import org.apache.commons.lang3.builder.EqualsBuilder;
 import org.apache.commons.lang3.builder.HashCodeBuilder;
@@ -25,6 +29,7 @@ import org.apache.hadoop.hdds.scm.container.ContainerID;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
 import org.apache.hadoop.util.Time;
 
+import java.io.IOException;
 import java.util.Comparator;
 
 /**
@@ -32,6 +37,17 @@ import java.util.Comparator;
  */
 public class ContainerInfo
     implements Comparator<ContainerInfo>, Comparable<ContainerInfo> {
+
+  private static final ObjectWriter WRITER;
+
+  static {
+    ObjectMapper mapper = new ObjectMapper();
+    mapper.setVisibility(PropertyAccessor.FIELD, JsonAutoDetect.Visibility.ANY);
+    mapper
+        .setVisibility(PropertyAccessor.GETTER, JsonAutoDetect.Visibility.NONE);
+    WRITER = mapper.writer();
+  }
+
   private HddsProtos.LifeCycleState state;
   private Pipeline pipeline;
   // Bytes allocated by SCM for clients.
@@ -254,6 +270,16 @@ public class ContainerInfo
     return this.compare(this, o);
   }
 
+  /**
+   * Returns a JSON string of this object.
+   *
+   * @return String - json string
+   * @throws IOException
+   */
+  public String toJsonString() throws IOException {
+    return WRITER.writeValueAsString(this);
+  }
+
   /**
    * Builder class for ContainerInfo.
    */
diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/CreateContainerHandler.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/CreateContainerHandler.java
index c0ff1f7a6d6..278ee30842f 100644
--- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/CreateContainerHandler.java
+++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/CreateContainerHandler.java
@@ -44,6 +44,10 @@ public class CreateContainerHandler extends OzoneCommandHandler {
 
   @Override
   public void execute(CommandLine cmd) throws IOException {
+    if (cmd.hasOption(HELP_OP)) {
+      displayHelp();
+    }
+
     if (!cmd.hasOption(CONTAINER_CREATE)) {
       throw new IOException("Expecting container create");
     }
@@ -57,7 +61,7 @@ public class CreateContainerHandler extends OzoneCommandHandler {
   public void displayHelp() {
     Options options = new Options();
     HelpFormatter helpFormatter = new HelpFormatter();
-    helpFormatter.printHelp(CMD_WIDTH, "hdfs scm -container -create <option>",
-        "where <option> is", options, "");
+    helpFormatter.printHelp(CMD_WIDTH, "hdfs scm -container -create",
+        null, options, null);
   }
 }
diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListContainerHandler.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListContainerHandler.java
index 42dae65ade3..3483b3e69aa 100644
--- a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListContainerHandler.java
+++ b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/container/ListContainerHandler.java
@@ -24,7 +24,6 @@ import org.apache.commons.cli.Options;
 import org.apache.hadoop.hdds.scm.cli.OzoneCommandHandler;
 import org.apache.hadoop.hdds.scm.client.ScmClient;
 import org.apache.hadoop.hdds.scm.container.common.helpers.ContainerInfo;
-import org.apache.hadoop.hdds.scm.container.common.helpers.Pipeline;
 import org.apache.hadoop.ozone.web.utils.JsonUtils;
 
 import java.io.IOException;
@@ -87,14 +86,15 @@ public class ListContainerHandler extends OzoneCommandHandler {
 
     // Output data list
     for (ContainerInfo container : containerList) {
-      outputContainerPipeline(container.getPipeline());
+      outputContainerInfo(container);
     }
   }
 
-  private void outputContainerPipeline(Pipeline pipeline) throws IOException {
+  private void outputContainerInfo(ContainerInfo containerInfo)
+      throws IOException {
     // Print container report info.
     logOut("%s", JsonUtils.toJsonStringWithDefaultPrettyPrinter(
-        pipeline.toJsonString()));
+        containerInfo.toJsonString()));
   }
 
   @Override
diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestSCMCli.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestSCMCli.java
index 888b72e61f7..2d8577c056b 100644
--- a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestSCMCli.java
+++ b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/scm/TestSCMCli.java
@@ -55,7 +55,6 @@ import static org.apache.hadoop.hdds.scm.cli.ResultCode.EXECUTION_ERROR;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
 import static org.junit.Assert.assertFalse;
 /**
  * This class tests the CLI of SCM.
@@ -126,21 +125,12 @@ public class TestSCMCli {
 
   @Test
   public void testCreateContainer() throws Exception {
-    long containerID = ContainerTestHelper.getTestContainerID();
-    try {
-      scm.getClientProtocolServer().getContainer(containerID);
-      fail("should not be able to get the container");
-    } catch (IOException ioe) {
-      assertTrue(ioe.getMessage().contains(
-          "Specified key does not exist. key : " + containerID));
-    }
-    String[] args = {"-container", "-create", "-c",
-        Long.toString(containerID)};
+    ByteArrayOutputStream testContent = new ByteArrayOutputStream();
+    PrintStream testPrintOut = new PrintStream(testContent);
+    System.setOut(testPrintOut);
+    String[] args = {"-container", "-create"};
     assertEquals(ResultCode.SUCCESS, cli.run(args));
-    ContainerInfo container = scm.getClientProtocolServer()
-        .getContainer(containerID);
-    assertNotNull(container);
-    assertEquals(containerID, container.containerID());
+    assertEquals("", testContent.toString());
   }
 
   private boolean containerExist(long containerID) {
@@ -215,9 +205,9 @@ public class TestSCMCli {
     ContainerInfo emptyContainer = containerOperationClient
         .createContainer(xceiverClientManager.getType(),
             HddsProtos.ReplicationFactor.ONE, containerOwner);
-    containerOperationClient.closeContainer(container.getContainerID(),
+    containerOperationClient.closeContainer(emptyContainer.getContainerID(),
         container.getPipeline());
-    Assert.assertTrue(containerExist(container.getContainerID()));
+    Assert.assertTrue(containerExist(emptyContainer.getContainerID()));
 
     // Successfully delete an empty container.
     delCmd = new String[] {"-container", "-delete", "-c",
@@ -252,7 +242,7 @@ public class TestSCMCli {
     DatanodeDetails datanodeDetails = cluster.getHddsDatanodes().get(0)
         .getDatanodeDetails();
     String formatStr =
-        "Container Name: %s\n" +
+        "Container id: %s\n" +
         "Container State: %s\n" +
         "Container DB Path: %s\n" +
         "Container Path: %s\n" +
@@ -261,7 +251,7 @@ public class TestSCMCli {
         "Datanodes: [%s]\n";
 
     String formatStrWithHash =
-        "Container Name: %s\n" +
+        "Container id: %s\n" +
         "Container State: %s\n" +
         "Container Hash: %s\n" +
         "Container DB Path: %s\n" +
@@ -271,8 +261,9 @@ public class TestSCMCli {
         "Datanodes: [%s]\n";
 
     // Test a non-exist container
-    String cname = "nonExistContainer";
-    String[] info = {"-container", "-info", cname};
+    String containerID =
+        Long.toString(ContainerTestHelper.getTestContainerID());
+    String[] info = { "-container", "-info", containerID };
     int exitCode = runCommandAndGetOutput(info, null, null);
     assertEquals("Expected Execution Error, Did not find that.",
         EXECUTION_ERROR, exitCode);
@@ -286,14 +277,16 @@ public class TestSCMCli {
             readContainer(container.getContainerID(),
                 container.getPipeline()), conf);
 
-    info = new String[]{"-container", "-info", "-c", cname};
+    info = new String[] { "-container", "-info", "-c",
+        Long.toString(container.getContainerID()) };
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     exitCode = runCommandAndGetOutput(info, out, null);
     assertEquals("Expected Success, did not find it.", ResultCode.SUCCESS,
             exitCode);
 
     String openStatus = data.isOpen() ? "OPEN" : "CLOSED";
-    String expected = String.format(formatStr, cname, openStatus,
+    String expected =
+        String.format(formatStr, container.getContainerID(), openStatus,
         data.getDBPath(), data.getContainerPath(), "",
         datanodeDetails.getHostName(), datanodeDetails.getHostName());
     assertEquals(expected, out.toString());
@@ -307,14 +300,16 @@ public class TestSCMCli {
     data = ContainerData
         .getFromProtBuf(containerOperationClient.readContainer(
             container.getContainerID(), container.getPipeline()), conf);
-    KeyUtils.getDB(data, conf).put(cname.getBytes(), "someKey".getBytes());
+    KeyUtils.getDB(data, conf)
+        .put(containerID.getBytes(), "someKey".getBytes());
 
-    info = new String[]{"-container", "-info", "-c", cname};
+    info = new String[] { "-container", "-info", "-c",
+        Long.toString(container.getContainerID()) };
     exitCode = runCommandAndGetOutput(info, out, null);
     assertEquals(ResultCode.SUCCESS, exitCode);
 
     openStatus = data.isOpen() ? "OPEN" : "CLOSED";
-    expected = String.format(formatStr, cname, openStatus,
+    expected = String.format(formatStr, container.getContainerID(), openStatus,
         data.getDBPath(), data.getContainerPath(), "",
         datanodeDetails.getHostName(), datanodeDetails.getHostName());
     assertEquals(expected, out.toString());
@@ -326,7 +321,8 @@ public class TestSCMCli {
     containerOperationClient.closeContainer(
         container.getContainerID(), container.getPipeline());
 
-    info = new String[] {"-container", "-info", "-c", cname};
+    info = new String[] { "-container", "-info", "-c",
+        Long.toString(container.getContainerID()) };
     exitCode = runCommandAndGetOutput(info, out, null);
     assertEquals(ResultCode.SUCCESS, exitCode);
     data = ContainerData
@@ -334,9 +330,10 @@ public class TestSCMCli {
             container.getContainerID(), container.getPipeline()), conf);
 
     openStatus = data.isOpen() ? "OPEN" : "CLOSED";
-    expected = String.format(formatStrWithHash, cname, openStatus,
-        data.getHash(), data.getDBPath(), data.getContainerPath(),
-        "", datanodeDetails.getHostName(), datanodeDetails.getHostName());
+    expected = String
+        .format(formatStrWithHash, container.getContainerID(), openStatus,
+            data.getHash(), data.getDBPath(), data.getContainerPath(), "",
+            datanodeDetails.getHostName(), datanodeDetails.getHostName());
     assertEquals(expected, out.toString());
   }
 
@@ -413,16 +410,16 @@ public class TestSCMCli {
     args = new String[] {"-container", "-list", "-start",
         startContainerIDStr};
     exitCode = runCommandAndGetOutput(args, out, err);
-    assertEquals(ResultCode.SUCCESS, exitCode);
-    assertTrue(out.toString().isEmpty());
+    assertEquals(ResultCode.EXECUTION_ERROR, exitCode);
+    assertTrue(err.toString().contains(
+        "java.io.IOException: Expecting container count"));
   }
 
   @Test
   public void testCloseContainer() throws Exception {
-    long containerID = ContainerTestHelper.getTestContainerID();
-    String[] args = {"-container", "-create", "-c",
-        Long.toString(containerID)};
-    assertEquals(ResultCode.SUCCESS, cli.run(args));
+    long containerID = containerOperationClient
+        .createContainer(xceiverClientManager.getType(),
+            HddsProtos.ReplicationFactor.ONE, containerOwner).getContainerID();
     ContainerInfo container = scm.getClientProtocolServer()
         .getContainer(containerID);
     assertNotNull(container);
@@ -477,7 +474,7 @@ public class TestSCMCli {
     String[] args2 = {"-container", "-create", "-help"};
     assertEquals(ResultCode.SUCCESS, cli.run(args2));
     String expected2 =
-        "usage: hdfs scm -container -create\n";
+        "usage: hdfs scm -container -create\n\n";
     assertEquals(expected2, testContent.toString());
     testContent.reset();
 
@@ -502,11 +499,10 @@ public class TestSCMCli {
 
     String[] args5 = {"-container", "-list", "-help"};
     assertEquals(ResultCode.SUCCESS, cli.run(args5));
-    String expected5 =
-        "usage: hdfs scm -container -list <option>\n" +
-            "where <option> can be the following\n" +
-            " -start <arg>    Specify start container id, required\n" +
-            " -count <arg>   Specify count number name\n";
+    String expected5 = "usage: hdfs scm -container -list <option>\n"
+        + "where <option> can be the following\n"
+        + " -count <arg>   Specify count number, required\n"
+        + " -start <arg>   Specify start container id\n";
     assertEquals(expected5, testContent.toString());
     testContent.reset();