From 4b63e3e504e9e226acda67f9d880ab923da2ea97 Mon Sep 17 00:00:00 2001
From: Weiwei Yang <wwei@apache.org>
Date: Sat, 1 Jul 2017 15:22:22 +0800
Subject: [PATCH] HDFS-12002. Ozone : SCM cli misc fixes/improvements.
 Contributed by Chen Liang.

---
 .../container/ContainerCommandHandler.java    |  5 ++-
 .../cli/container/DeleteContainerHandler.java | 24 ++++++++---
 .../cli/container/InfoContainerHandler.java   | 27 ++++++++++--
 .../hadoop/ozone/web/ozShell/Shell.java       |  1 +
 .../apache/hadoop/ozone/scm/TestSCMCli.java   | 43 ++++++++++++++-----
 5 files changed, 79 insertions(+), 21 deletions(-)

diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/ContainerCommandHandler.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/ContainerCommandHandler.java
index f4e428ea83..822c8b51d9 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/ContainerCommandHandler.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/ContainerCommandHandler.java
@@ -92,9 +92,9 @@ private static void addCommandsOption(Options options) {
     Option createContainer =
         new Option(CONTAINER_CREATE, false, "Create container");
     Option infoContainer =
-        new Option(CONTAINER_INFO, true, "Info container");
+        new Option(CONTAINER_INFO, false, "Info container");
     Option deleteContainer =
-        new Option(CONTAINER_DELETE, true, "Delete container");
+        new Option(CONTAINER_DELETE, false, "Delete container");
 
     options.addOption(createContainer);
     options.addOption(deleteContainer);
@@ -107,6 +107,7 @@ public static void addOptions(Options options) {
     // for create container options.
     CreateContainerHandler.addOptions(options);
     DeleteContainerHandler.addOptions(options);
+    InfoContainerHandler.addOptions(options);
     // TODO : add other options such as delete, close etc.
   }
 }
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/DeleteContainerHandler.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/DeleteContainerHandler.java
index fc83e05001..0782af3b04 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/DeleteContainerHandler.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/DeleteContainerHandler.java
@@ -30,14 +30,16 @@
 import java.io.IOException;
 
 import static org.apache.hadoop.ozone.scm.cli.SCMCLI.CMD_WIDTH;
+import static org.apache.hadoop.ozone.scm.cli.SCMCLI.HELP_OP;
 
 /**
  * This is the handler that process delete container command.
  */
 public class DeleteContainerHandler extends OzoneCommandHandler {
 
-  protected static final String CONTAINER_DELETE = "del";
+  protected static final String CONTAINER_DELETE = "delete";
   protected static final String OPT_FORCE = "f";
+  protected static final String OPT_CONTAINER_NAME = "c";
 
   public DeleteContainerHandler(ScmClient scmClient) {
     super(scmClient);
@@ -46,9 +48,17 @@ public DeleteContainerHandler(ScmClient scmClient) {
   @Override
   public void execute(CommandLine cmd) throws IOException {
     Preconditions.checkArgument(cmd.hasOption(CONTAINER_DELETE),
-        "Expecting command del");
+        "Expecting command delete");
+    if (!cmd.hasOption(OPT_CONTAINER_NAME)) {
+      displayHelp();
+      if (!cmd.hasOption(HELP_OP)) {
+        throw new IOException("Expecting container name");
+      } else {
+        return;
+      }
+    }
 
-    String containerName = cmd.getOptionValue(CONTAINER_DELETE);
+    String containerName = cmd.getOptionValue(OPT_CONTAINER_NAME);
 
     Pipeline pipeline = getScmClient().getContainer(containerName);
     if (pipeline == null) {
@@ -61,11 +71,12 @@ public void execute(CommandLine cmd) throws IOException {
     logOut("Container %s deleted.", containerName);
   }
 
-  @Override public void displayHelp() {
+  @Override
+  public void displayHelp() {
     Options options = new Options();
     addOptions(options);
     HelpFormatter helpFormatter = new HelpFormatter();
-    helpFormatter.printHelp(CMD_WIDTH, "hdfs scm -container -del <option>",
+    helpFormatter.printHelp(CMD_WIDTH, "hdfs scm -container -delete <option>",
         "where <option> is", options, "");
   }
 
@@ -74,5 +85,8 @@ public static void addOptions(Options options) {
         false,
         "forcibly delete a container");
     options.addOption(forceOpt);
+    Option containerNameOpt = new Option(OPT_CONTAINER_NAME,
+        true, "Specify container name");
+    options.addOption(containerNameOpt);
   }
 }
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/InfoContainerHandler.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/InfoContainerHandler.java
index c32036e9dc..fc9ef0c372 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/InfoContainerHandler.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/scm/cli/container/InfoContainerHandler.java
@@ -20,6 +20,7 @@
 import com.google.common.base.Preconditions;
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.HelpFormatter;
+import org.apache.commons.cli.Option;
 import org.apache.commons.cli.Options;
 
 import org.apache.hadoop.hdfs.ozone.protocol.proto.ContainerProtos.ContainerData;
@@ -31,12 +32,16 @@
 import java.io.IOException;
 import java.util.stream.Collectors;
 
+import static org.apache.hadoop.ozone.scm.cli.SCMCLI.CMD_WIDTH;
+import static org.apache.hadoop.ozone.scm.cli.SCMCLI.HELP_OP;
+
 /**
  * This is the handler that process container info command.
  */
 public class InfoContainerHandler extends OzoneCommandHandler {
 
   public static final String CONTAINER_INFO = "info";
+  protected static final String OPT_CONTAINER_NAME = "c";
 
   /**
    * Constructs a handler object.
@@ -52,7 +57,15 @@ public void execute(CommandLine cmd) throws IOException {
     if (!cmd.hasOption(CONTAINER_INFO)) {
       throw new IOException("Expecting container info");
     }
-    String containerName = cmd.getOptionValue(CONTAINER_INFO);
+    if (!cmd.hasOption(OPT_CONTAINER_NAME)) {
+      displayHelp();
+      if (!cmd.hasOption(HELP_OP)) {
+        throw new IOException("Expecting container name");
+      } else {
+        return;
+      }
+    }
+    String containerName = cmd.getOptionValue(OPT_CONTAINER_NAME);
     Pipeline pipeline = getScmClient().getContainer(containerName);
     Preconditions.checkNotNull(pipeline, "Pipeline cannot be null");
 
@@ -84,8 +97,16 @@ public void execute(CommandLine cmd) throws IOException {
 
   @Override
   public void displayHelp() {
+    Options options = new Options();
+    addOptions(options);
     HelpFormatter helpFormatter = new HelpFormatter();
-    helpFormatter.printHelp("hdfs scm -container -info <container name>",
-        new Options());
+    helpFormatter.printHelp(CMD_WIDTH, "hdfs scm -container -info <option>",
+        "where <option> is", options, "");
+  }
+
+  public static void addOptions(Options options) {
+    Option containerNameOpt = new Option(OPT_CONTAINER_NAME,
+        true, "Specify container name");
+    options.addOption(containerNameOpt);
   }
 }
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/web/ozShell/Shell.java b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/web/ozShell/Shell.java
index 294d8f57d2..f846b6d85c 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/web/ozShell/Shell.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/main/java/org/apache/hadoop/ozone/web/ozShell/Shell.java
@@ -110,6 +110,7 @@ public static void main(String[] argv) throws Exception {
     try {
       res = ToolRunner.run(shell, argv);
     } catch (Exception ex) {
+      System.err.println("ERROR: " + ex.getMessage());
       System.exit(1);
     }
     System.exit(res);
diff --git a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/scm/TestSCMCli.java b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/scm/TestSCMCli.java
index 77865e94c7..4e8214c631 100644
--- a/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/scm/TestSCMCli.java
+++ b/hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/ozone/scm/TestSCMCli.java
@@ -164,7 +164,7 @@ public void testDeleteContainer() throws Exception {
     Assert.assertTrue(containerExist(containerName));
 
     // Gracefully delete a container should fail because it is open.
-    delCmd = new String[] {"-container", "-del", containerName};
+    delCmd = new String[] {"-container", "-delete", "-c", containerName};
     testErr = new ByteArrayOutputStream();
     exitCode = runCommandAndGetOutput(delCmd, null, testErr);
     assertEquals(ResultCode.EXECUTION_ERROR, exitCode);
@@ -184,7 +184,7 @@ public void testDeleteContainer() throws Exception {
     Assert.assertTrue(containerExist(containerName));
 
     // Try force delete again.
-    delCmd = new String[] {"-container", "-del", containerName, "-f"};
+    delCmd = new String[] {"-container", "-delete", "-c", containerName, "-f"};
     exitCode = runCommandAndGetOutput(delCmd, null, null);
     assertEquals(ResultCode.SUCCESS, exitCode);
     Assert.assertFalse(containerExist(containerName));
@@ -201,7 +201,7 @@ public void testDeleteContainer() throws Exception {
     Assert.assertTrue(containerExist(containerName));
 
     // Successfully delete an empty container.
-    delCmd = new String[] {"-container", "-del", containerName};
+    delCmd = new String[] {"-container", "-delete", "-c", containerName};
     exitCode = runCommandAndGetOutput(delCmd, null, null);
     assertEquals(ResultCode.SUCCESS, exitCode);
     Assert.assertFalse(containerExist(containerName));
@@ -216,7 +216,7 @@ public void testDeleteContainer() throws Exception {
     // 3. Test to delete a non-exist container.
     // ****************************************
     containerName = "non-exist-container";
-    delCmd = new String[] {"-container", "-del", containerName};
+    delCmd = new String[] {"-container", "-delete", "-c", containerName};
     testErr = new ByteArrayOutputStream();
     exitCode = runCommandAndGetOutput(delCmd, null, testErr);
     assertEquals(ResultCode.EXECUTION_ERROR, exitCode);
@@ -259,7 +259,7 @@ public void testInfoContainer() throws Exception {
     ContainerData data = new ContainerData(cname);
     containerManager.createContainer(pipeline, data);
 
-    info = new String[]{"-container", "-info", cname};
+    info = new String[]{"-container", "-info", "-c", cname};
     ByteArrayOutputStream out = new ByteArrayOutputStream();
     exitCode = runCommandAndGetOutput(info, out, null);
     assertEquals(ResultCode.SUCCESS, exitCode);
@@ -280,7 +280,7 @@ public void testInfoContainer() throws Exception {
     KeyUtils.getDB(data, conf).put(cname.getBytes(),
         "someKey".getBytes());
 
-    info = new String[]{"-container", "-info", cname};
+    info = new String[]{"-container", "-info", "-c", cname};
     exitCode = runCommandAndGetOutput(info, out, null);
     assertEquals(ResultCode.SUCCESS, exitCode);
 
@@ -307,7 +307,7 @@ public void testInfoContainer() throws Exception {
         .collect(Collectors.toList());
     String metadataStr = StringUtils.join(", ", metaList);
 
-    info = new String[]{"-container", "-info", cname};
+    info = new String[]{"-container", "-info", "-c", cname};
     exitCode = runCommandAndGetOutput(info, out, null);
     assertEquals(ResultCode.SUCCESS, exitCode);
 
@@ -322,7 +322,7 @@ public void testInfoContainer() throws Exception {
     // Close last container and test info again.
     containerManager.closeContainer(cname);
 
-    info = new String[]{"-container", "-info", cname};
+    info = new String[]{"-container", "-info", "-c", cname};
     exitCode = runCommandAndGetOutput(info, out, null);
     assertEquals(ResultCode.SUCCESS, exitCode);
     data = containerManager.readContainer(cname);
@@ -375,9 +375,9 @@ public void testHelp() throws Exception {
     String expected1 =
         "usage: hdfs scm -container <commands> <options>\n" +
         "where <commands> can be one of the following\n" +
-        " -create       Create container\n" +
-        " -del <arg>    Delete container\n" +
-        " -info <arg>   Info container\n";
+        " -create   Create container\n" +
+        " -delete   Delete container\n" +
+        " -info     Info container\n";
 
     assertEquals(expected1, testContent.toString());
     testContent.reset();
@@ -389,6 +389,27 @@ public void testHelp() throws Exception {
         "where <option> is\n" +
         " -c <arg>   Specify container name\n";
     assertEquals(expected2, testContent.toString());
+    testContent.reset();
+
+    String[] args3 = {"-container", "-delete", "-help"};
+    assertEquals(ResultCode.SUCCESS, cli.run(args3));
+    String expected3 =
+        "usage: hdfs scm -container -delete <option>\n" +
+        "where <option> is\n" +
+        " -c <arg>   Specify container name\n" +
+        " -f         forcibly delete a container\n";
+    assertEquals(expected3, testContent.toString());
+    testContent.reset();
+
+    String[] args4 = {"-container", "-info", "-help"};
+    assertEquals(ResultCode.SUCCESS, cli.run(args4));
+    String expected4 =
+        "usage: hdfs scm -container -info <option>\n" +
+        "where <option> is\n" +
+        " -c <arg>   Specify container name\n";
+    assertEquals(expected4, testContent.toString());
+    testContent.reset();
+
     System.setOut(init);
   }
 }