From 9467bef8ea9a62658b32dd43a76f4f98087d1986 Mon Sep 17 00:00:00 2001 From: Alejandro Abdelnur Date: Thu, 21 Aug 2014 18:59:21 +0000 Subject: [PATCH] HADOOP-10816. KeyShell returns -1 on error to the shell, should be 1. (Mike Yoder via wang) Conflicts: hadoop-common-project/hadoop-common/CHANGES.txt git-svn-id: https://svn.apache.org/repos/asf/hadoop/common/branches/branch-2@1619532 13f79535-47bb-0310-9956-ffa450edef68 --- .../hadoop-common/CHANGES.txt | 3 ++ .../apache/hadoop/crypto/key/KeyShell.java | 41 ++++++++++++------- .../hadoop/crypto/key/TestKeyShell.java | 16 ++++---- 3 files changed, 37 insertions(+), 23 deletions(-) diff --git a/hadoop-common-project/hadoop-common/CHANGES.txt b/hadoop-common-project/hadoop-common/CHANGES.txt index 14b3de13c65..dddb5d5231c 100644 --- a/hadoop-common-project/hadoop-common/CHANGES.txt +++ b/hadoop-common-project/hadoop-common/CHANGES.txt @@ -247,6 +247,9 @@ Release 2.6.0 - UNRELEASED HADOOP-10611. KMS, keyVersion name should not be assumed to be keyName@versionNumber. (tucu) + HADOOP-10816. KeyShell returns -1 on error to the shell, should be 1. + (Mike Yoder via wang) + Release 2.5.0 - 2014-08-11 INCOMPATIBLE CHANGES diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java index 5b83e389fea..dd3190999c1 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/crypto/key/KeyShell.java @@ -57,6 +57,16 @@ public class KeyShell extends Configured implements Tool { private boolean userSuppliedProvider = false; + /** + * Primary entry point for the KeyShell; called via main(). + * + * @param args Command line arguments. + * @return 0 on success and 1 on failure. This value is passed back to + * the unix shell, so we must follow shell return code conventions: + * the return code is an unsigned character, and 0 means success, and + * small positive integers mean failure. + * @throws Exception + */ @Override public int run(String[] args) throws Exception { int exitCode = 0; @@ -68,11 +78,11 @@ public class KeyShell extends Configured implements Tool { if (command.validate()) { command.execute(); } else { - exitCode = -1; + exitCode = 1; } } catch (Exception e) { e.printStackTrace(err); - return -1; + return 1; } return exitCode; } @@ -86,8 +96,8 @@ public class KeyShell extends Configured implements Tool { * % hadoop key list [-provider providerPath] * % hadoop key delete keyName [--provider providerPath] [-i] * - * @param args - * @return + * @param args Command line arguments. + * @return 0 on success, 1 on failure. * @throws IOException */ private int init(String[] args) throws IOException { @@ -105,7 +115,7 @@ public class KeyShell extends Configured implements Tool { command = new CreateCommand(keyName, options); if ("--help".equals(keyName)) { printKeyShellUsage(); - return -1; + return 1; } } else if (args[i].equals("delete")) { String keyName = "--help"; @@ -116,7 +126,7 @@ public class KeyShell extends Configured implements Tool { command = new DeleteCommand(keyName); if ("--help".equals(keyName)) { printKeyShellUsage(); - return -1; + return 1; } } else if (args[i].equals("roll")) { String keyName = "--help"; @@ -127,7 +137,7 @@ public class KeyShell extends Configured implements Tool { command = new RollCommand(keyName); if ("--help".equals(keyName)) { printKeyShellUsage(); - return -1; + return 1; } } else if ("list".equals(args[i])) { command = new ListCommand(); @@ -145,13 +155,13 @@ public class KeyShell extends Configured implements Tool { out.println("\nAttributes must be in attribute=value form, " + "or quoted\nlike \"attribute = value\"\n"); printKeyShellUsage(); - return -1; + return 1; } if (attributes.containsKey(attr)) { out.println("\nEach attribute must correspond to only one value:\n" + "atttribute \"" + attr + "\" was repeated\n" ); printKeyShellUsage(); - return -1; + return 1; } attributes.put(attr, val); } else if ("--provider".equals(args[i]) && moreTokens) { @@ -163,17 +173,17 @@ public class KeyShell extends Configured implements Tool { interactive = true; } else if ("--help".equals(args[i])) { printKeyShellUsage(); - return -1; + return 1; } else { printKeyShellUsage(); ToolRunner.printGenericCommandUsage(System.err); - return -1; + return 1; } } if (command == null) { printKeyShellUsage(); - return -1; + return 1; } if (!attributes.isEmpty()) { @@ -491,10 +501,11 @@ public class KeyShell extends Configured implements Tool { } /** - * Main program. + * main() entry point for the KeyShell. While strictly speaking the + * return is void, it will System.exit() with a return code: 0 is for + * success and 1 for failure. * - * @param args - * Command line arguments + * @param args Command line arguments. * @throws Exception */ public static void main(String[] args) throws Exception { diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java index 6b24a25288f..e22949ee7ad 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/crypto/key/TestKeyShell.java @@ -161,7 +161,7 @@ public class TestKeyShell { KeyShell ks = new KeyShell(); ks.setConf(new Configuration()); rc = ks.run(args1); - assertEquals(-1, rc); + assertEquals(1, rc); assertTrue(outContent.toString().contains("key1 has not been created.")); } @@ -174,7 +174,7 @@ public class TestKeyShell { KeyShell ks = new KeyShell(); ks.setConf(new Configuration()); rc = ks.run(args1); - assertEquals(-1, rc); + assertEquals(1, rc); assertTrue(outContent.toString().contains("key1 has not been created.")); } @@ -187,7 +187,7 @@ public class TestKeyShell { KeyShell ks = new KeyShell(); ks.setConf(new Configuration()); rc = ks.run(args1); - assertEquals(-1, rc); + assertEquals(1, rc); assertTrue(outContent.toString().contains("There are no valid " + "KeyProviders configured.")); } @@ -216,7 +216,7 @@ public class TestKeyShell { config.set(KeyProviderFactory.KEY_PROVIDER_PATH, "user:///"); ks.setConf(config); rc = ks.run(args1); - assertEquals(-1, rc); + assertEquals(1, rc); assertTrue(outContent.toString().contains("There are no valid " + "KeyProviders configured.")); } @@ -262,19 +262,19 @@ public class TestKeyShell { final String[] args2 = {"create", "keyattr2", "--provider", jceksProvider, "--attr", "=bar"}; rc = ks.run(args2); - assertEquals(-1, rc); + assertEquals(1, rc); /* Not in attribute = value form */ outContent.reset(); args2[5] = "foo"; rc = ks.run(args2); - assertEquals(-1, rc); + assertEquals(1, rc); /* No attribute or value */ outContent.reset(); args2[5] = "="; rc = ks.run(args2); - assertEquals(-1, rc); + assertEquals(1, rc); /* Legal: attribute is a, value is b=c */ outContent.reset(); @@ -308,7 +308,7 @@ public class TestKeyShell { "--attr", "foo=bar", "--attr", "foo=glarch"}; rc = ks.run(args4); - assertEquals(-1, rc); + assertEquals(1, rc); /* Clean up to be a good citizen */ deleteKey(ks, "keyattr1");