HDFS-13861. RBF: Illegal Router Admin command leads to printing usage for all commands. Contributed by Ayush Saxena.

hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java

@@ -94,25 +94,58 @@ public RouterAdmin(Configuration conf) {
    * Print the usage message.
    */
   public void printUsage() {
-    String usage = "Federation Admin Tools:\n"
+    String usage = getUsage(null);
-        + "\t[-add <source> <nameservice1, nameservice2, ...> <destination> "
-        + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
-        + "-owner <owner> -group <group> -mode <mode>]\n"
-        + "\t[-update <source> <nameservice1, nameservice2, ...> <destination> "
-        + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
-        + "-owner <owner> -group <group> -mode <mode>]\n"
-        + "\t[-rm <source>]\n"
-        + "\t[-ls <path>]\n"
-        + "\t[-setQuota <path> -nsQuota <nsQuota> -ssQuota "
-        + "<quota in bytes or quota size string>]\n"
-        + "\t[-clrQuota <path>]\n"
-        + "\t[-safemode enter | leave | get]\n"
-        + "\t[-nameservice enable | disable <nameservice>]\n"
-        + "\t[-getDisabledNameservices]\n";
-
     System.out.println(usage);
   }
 
+  private void printUsage(String cmd) {
+    String usage = getUsage(cmd);
+    System.out.println(usage);
+  }
+
+  private String getUsage(String cmd) {
+    if (cmd == null) {
+      String[] commands =
+          {"-add", "-update", "-rm", "-ls", "-setQuota", "-clrQuota",
+              "-safemode", "-nameservice", "-getDisabledNameservices"};
+      StringBuilder usage = new StringBuilder();
+      usage.append("Usage: hdfs routeradmin :\n");
+      for (int i = 0; i < commands.length; i++) {
+        usage.append(getUsage(commands[i]));
+        if (i + 1 < commands.length) {
+          usage.append("\n");
+        }
+      }
+      return usage.toString();
+    }
+    if (cmd.equals("-add")) {
+      return "\t[-add <source> <nameservice1, nameservice2, ...> <destination> "
+          + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+          + "-owner <owner> -group <group> -mode <mode>]";
+    } else if (cmd.equals("-update")) {
+      return "\t[-update <source> <nameservice1, nameservice2, ...> "
+          + "<destination> "
+          + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+          + "-owner <owner> -group <group> -mode <mode>]";
+    } else if (cmd.equals("-rm")) {
+      return "\t[-rm <source>]";
+    } else if (cmd.equals("-ls")) {
+      return "\t[-ls <path>]";
+    } else if (cmd.equals("-setQuota")) {
+      return "\t[-setQuota <path> -nsQuota <nsQuota> -ssQuota "
+          + "<quota in bytes or quota size string>]";
+    } else if (cmd.equals("-clrQuota")) {
+      return "\t[-clrQuota <path>]";
+    } else if (cmd.equals("-safemode")) {
+      return "\t[-safemode enter | leave | get]";
+    } else if (cmd.equals("-nameservice")) {
+      return "\t[-nameservice enable | disable <nameservice>]";
+    } else if (cmd.equals("-getDisabledNameservices")) {
+      return "\t[-getDisabledNameservices]";
+    }
+    return getUsage(null);
+  }
+
   @Override
   public int run(String[] argv) throws Exception {
     if (argv.length < 1) {
@@ -129,43 +162,43 @@ public int run(String[] argv) throws Exception {
     if ("-add".equals(cmd)) {
       if (argv.length < 4) {
         System.err.println("Not enough parameters specified for cmd " + cmd);
-        printUsage();
+        printUsage(cmd);
         return exitCode;
       }
     } else if ("-update".equals(cmd)) {
       if (argv.length < 4) {
         System.err.println("Not enough parameters specified for cmd " + cmd);
-        printUsage();
+        printUsage(cmd);
         return exitCode;
       }
-    } else if ("-rm".equalsIgnoreCase(cmd)) {
+    } else if ("-rm".equals(cmd)) {
       if (argv.length < 2) {
         System.err.println("Not enough parameters specified for cmd " + cmd);
-        printUsage();
+        printUsage(cmd);
         return exitCode;
       }
-    } else if ("-setQuota".equalsIgnoreCase(cmd)) {
+    } else if ("-setQuota".equals(cmd)) {
       if (argv.length < 4) {
         System.err.println("Not enough parameters specified for cmd " + cmd);
-        printUsage();
+        printUsage(cmd);
         return exitCode;
       }
-    } else if ("-clrQuota".equalsIgnoreCase(cmd)) {
+    } else if ("-clrQuota".equals(cmd)) {
       if (argv.length < 2) {
         System.err.println("Not enough parameters specified for cmd " + cmd);
-        printUsage();
+        printUsage(cmd);
         return exitCode;
       }
-    } else if ("-safemode".equalsIgnoreCase(cmd)) {
+    } else if ("-safemode".equals(cmd)) {
       if (argv.length < 2) {
         System.err.println("Not enough parameters specified for cmd " + cmd);
-        printUsage();
+        printUsage(cmd);
         return exitCode;
       }
-    } else if ("-nameservice".equalsIgnoreCase(cmd)) {
+    } else if ("-nameservice".equals(cmd)) {
       if (argv.length < 3) {
         System.err.println("Not enough parameters specificed for cmd " + cmd);
-        printUsage();
+        printUsage(cmd);
         return exitCode;
       }
     }
@@ -230,14 +263,13 @@ public int run(String[] argv) throws Exception {
       } else if ("-getDisabledNameservices".equals(cmd)) {
         getDisabledNameservices();
       } else {
-        printUsage();
+        throw new IllegalArgumentException("Unknown Command: " + cmd);
-        return exitCode;
       }
     } catch (IllegalArgumentException arge) {
       debugException = arge;
       exitCode = -1;
       System.err.println(cmd.substring(1) + ": " + arge.getLocalizedMessage());
-      printUsage();
+      printUsage(cmd);
     } catch (RemoteException e) {
       // This is a error returned by the server.
       // Print out the first line of the error message, ignore the stack trace.

hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java

@@ -436,6 +436,74 @@ private void verifyExecutionResult(String mount, boolean canRead,
     assertEquals(rmCommandCode, ToolRunner.run(admin, argv));
   }
 
+  @Test
+  public void testInvalidArgumentMessage() throws Exception {
+    String nsId = "ns0";
+    String src = "/testSource";
+    System.setOut(new PrintStream(out));
+    String[] argv = new String[] {"-add", src, nsId};
+    assertEquals(-1, ToolRunner.run(admin, argv));
+    assertTrue(out.toString().contains(
+        "\t[-add <source> <nameservice1, nameservice2, ...> <destination> "
+            + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+            + "-owner <owner> -group <group> -mode <mode>]"));
+    out.reset();
+
+    argv = new String[] {"-update", src, nsId};
+    assertEquals(-1, ToolRunner.run(admin, argv));
+    assertTrue(out.toString().contains(
+        "\t[-update <source> <nameservice1, nameservice2, ...> <destination> "
+            + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+            + "-owner <owner> -group <group> -mode <mode>]"));
+    out.reset();
+
+    argv = new String[] {"-rm"};
+    assertEquals(-1, ToolRunner.run(admin, argv));
+    assertTrue(out.toString().contains("\t[-rm <source>]"));
+    out.reset();
+
+    argv = new String[] {"-setQuota", src};
+    assertEquals(-1, ToolRunner.run(admin, argv));
+    assertTrue(out.toString()
+        .contains("\t[-setQuota <path> -nsQuota <nsQuota> -ssQuota "
+            + "<quota in bytes or quota size string>]"));
+    out.reset();
+
+    argv = new String[] {"-clrQuota"};
+    assertEquals(-1, ToolRunner.run(admin, argv));
+    assertTrue(out.toString().contains("\t[-clrQuota <path>]"));
+    out.reset();
+
+    argv = new String[] {"-safemode"};
+    assertEquals(-1, ToolRunner.run(admin, argv));
+    assertTrue(out.toString().contains("\t[-safemode enter | leave | get]"));
+    out.reset();
+
+    argv = new String[] {"-nameservice", nsId};
+    assertEquals(-1, ToolRunner.run(admin, argv));
+    assertTrue(out.toString()
+        .contains("\t[-nameservice enable | disable <nameservice>]"));
+    out.reset();
+
+    argv = new String[] {"-Random"};
+    assertEquals(-1, ToolRunner.run(admin, argv));
+    String expected = "Usage: hdfs routeradmin :\n"
+        + "\t[-add <source> <nameservice1, nameservice2, ...> <destination> "
+        + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+        + "-owner <owner> -group <group> -mode <mode>]\n"
+        + "\t[-update <source> <nameservice1, nameservice2, ...> "
+        + "<destination> " + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] "
+        + "-owner <owner> -group <group> -mode <mode>]\n" + "\t[-rm <source>]\n"
+        + "\t[-ls <path>]\n"
+        + "\t[-setQuota <path> -nsQuota <nsQuota> -ssQuota "
+        + "<quota in bytes or quota size string>]\n" + "\t[-clrQuota <path>]\n"
+        + "\t[-safemode enter | leave | get]\n"
+        + "\t[-nameservice enable | disable <nameservice>]\n"
+        + "\t[-getDisabledNameservices]";
+    assertTrue(out.toString(), out.toString().contains(expected));
+    out.reset();
+  }
+
   @Test
   public void testSetAndClearQuota() throws Exception {
     String nsId = "ns0";