From 450ba6790d4758137dcb86a87b643310a7b67c69 Mon Sep 17 00:00:00 2001 From: Brahma Reddy Battula Date: Wed, 29 Aug 2018 00:29:05 +0530 Subject: [PATCH] HDFS-13861. RBF: Illegal Router Admin command leads to printing usage for all commands. Contributed by Ayush Saxena. (cherry picked from commit cb9d371ae2cda1624fc83316ddc09de37d8d0bd3) --- .../hdfs/tools/federation/RouterAdmin.java | 94 +++++++++++++------ .../federation/router/TestRouterAdminCLI.java | 68 ++++++++++++++ 2 files changed, 131 insertions(+), 31 deletions(-) diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java index f88d0a60eb6..46be3732c26 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/main/java/org/apache/hadoop/hdfs/tools/federation/RouterAdmin.java @@ -94,25 +94,58 @@ public class RouterAdmin extends Configured implements Tool { * Print the usage message. */ public void printUsage() { - String usage = "Federation Admin Tools:\n" - + "\t[-add " - + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] " - + "-owner -group -mode ]\n" - + "\t[-update " - + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] " - + "-owner -group -mode ]\n" - + "\t[-rm ]\n" - + "\t[-ls ]\n" - + "\t[-setQuota -nsQuota -ssQuota " - + "]\n" - + "\t[-clrQuota ]\n" - + "\t[-safemode enter | leave | get]\n" - + "\t[-nameservice enable | disable ]\n" - + "\t[-getDisabledNameservices]\n"; - + String usage = getUsage(null); 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 " + + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + + "-owner -group -mode ]"; + } else if (cmd.equals("-update")) { + return "\t[-update " + + " " + + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + + "-owner -group -mode ]"; + } else if (cmd.equals("-rm")) { + return "\t[-rm ]"; + } else if (cmd.equals("-ls")) { + return "\t[-ls ]"; + } else if (cmd.equals("-setQuota")) { + return "\t[-setQuota -nsQuota -ssQuota " + + "]"; + } else if (cmd.equals("-clrQuota")) { + return "\t[-clrQuota ]"; + } else if (cmd.equals("-safemode")) { + return "\t[-safemode enter | leave | get]"; + } else if (cmd.equals("-nameservice")) { + return "\t[-nameservice enable | disable ]"; + } 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 class RouterAdmin extends Configured implements Tool { 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 class RouterAdmin extends Configured implements Tool { } else if ("-getDisabledNameservices".equals(cmd)) { getDisabledNameservices(); } else { - printUsage(); - return exitCode; + throw new IllegalArgumentException("Unknown Command: " + cmd); } } 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. diff --git a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java index 2682e9a2d88..0c7321f7cc9 100644 --- a/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java +++ b/hadoop-hdfs-project/hadoop-hdfs-rbf/src/test/java/org/apache/hadoop/hdfs/server/federation/router/TestRouterAdminCLI.java @@ -436,6 +436,74 @@ public class TestRouterAdminCLI { 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 " + + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + + "-owner -group -mode ]")); + out.reset(); + + argv = new String[] {"-update", src, nsId}; + assertEquals(-1, ToolRunner.run(admin, argv)); + assertTrue(out.toString().contains( + "\t[-update " + + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + + "-owner -group -mode ]")); + out.reset(); + + argv = new String[] {"-rm"}; + assertEquals(-1, ToolRunner.run(admin, argv)); + assertTrue(out.toString().contains("\t[-rm ]")); + out.reset(); + + argv = new String[] {"-setQuota", src}; + assertEquals(-1, ToolRunner.run(admin, argv)); + assertTrue(out.toString() + .contains("\t[-setQuota -nsQuota -ssQuota " + + "]")); + out.reset(); + + argv = new String[] {"-clrQuota"}; + assertEquals(-1, ToolRunner.run(admin, argv)); + assertTrue(out.toString().contains("\t[-clrQuota ]")); + 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 ]")); + out.reset(); + + argv = new String[] {"-Random"}; + assertEquals(-1, ToolRunner.run(admin, argv)); + String expected = "Usage: hdfs routeradmin :\n" + + "\t[-add " + + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + + "-owner -group -mode ]\n" + + "\t[-update " + + " " + "[-readonly] [-order HASH|LOCAL|RANDOM|HASH_ALL] " + + "-owner -group -mode ]\n" + "\t[-rm ]\n" + + "\t[-ls ]\n" + + "\t[-setQuota -nsQuota -ssQuota " + + "]\n" + "\t[-clrQuota ]\n" + + "\t[-safemode enter | leave | get]\n" + + "\t[-nameservice enable | disable ]\n" + + "\t[-getDisabledNameservices]"; + assertTrue(out.toString(), out.toString().contains(expected)); + out.reset(); + } + @Test public void testSetAndClearQuota() throws Exception { String nsId = "ns0";