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-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

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]
    * </pre>
-   * @param args
+   * @param args Command line arguments.
-   * @return
+   * @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
+   * @param args Command line arguments.
-   *          Command line arguments
    * @throws Exception
    */
   public static void main(String[] args) throws Exception {

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");