HADOOP-15843. s3guard bucket-info command to not print a stack trace on bucket-not-found.

Contributed by Adam Antal.

(Revised patch applied after stevel committed the wrong one; that has been reverted)

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/s3guard/S3GuardTool.java

@@ -552,6 +552,10 @@ public abstract class S3GuardTool extends Configured implements Tool {
     @Override
     public int run(String[] args, PrintStream out) throws Exception {
       List<String> paths = parseArgs(args);
+      if (paths.isEmpty()) {
+        errorln(getUsage());
+        throw invalidArgs("no arguments");
+      }
       Map<String, String> options = new HashMap<>();
       checkIfS3BucketIsGuarded(paths);
 
@@ -1639,6 +1643,11 @@ public abstract class S3GuardTool extends Configured implements Tool {
     } catch (ExitUtil.ExitException e) {
       // explicitly raised exit code
       exit(e.getExitCode(), e.toString());
+    } catch (FileNotFoundException e) {
+      // Bucket doesn't exist or similar - return code of 44, "404".
+      errorln(e.toString());
+      LOG.debug("Not found:", e);
+      exit(EXIT_NOT_FOUND, e.toString());
     } catch (Throwable e) {
       e.printStackTrace(System.err);
       exit(ERROR, e.toString());

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3guard.md

@@ -1006,6 +1006,20 @@ There's are limit on how often you can change the capacity of an DynamoDB table;
 if you call set-capacity too often, it fails. Wait until the after the time indicated
 and try again.
 
+### Error `Invalid region specified`
+
+```
+java.io.IOException: Invalid region specified "iceland-2":
+  Region can be configured with fs.s3a.s3guard.ddb.region:
+  us-gov-west-1, us-east-1, us-east-2, us-west-1, us-west-2,
+  eu-west-1, eu-west-2, eu-west-3, eu-central-1, ap-south-1,
+  ap-southeast-1, ap-southeast-2, ap-northeast-1, ap-northeast-2,
+  sa-east-1, cn-north-1, cn-northwest-1, ca-central-1
+  at org.apache.hadoop.fs.s3a.s3guard.DynamoDBClientFactory$DefaultDynamoDBClientFactory.getRegion
+  at org.apache.hadoop.fs.s3a.s3guard.DynamoDBClientFactory$DefaultDynamoDBClientFactory.createDynamoDBClient
+```
+
+The region specified in `fs.s3a.s3guard.ddb.region` is invalid.
 
 ## Other Topics
 

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/s3guard/AbstractS3GuardToolTestBase.java

@@ -25,8 +25,10 @@ import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStreamReader;
 import java.net.URI;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashSet;
+import java.util.List;
 import java.util.Set;
 import java.util.UUID;
 import java.util.concurrent.Callable;
@@ -52,6 +54,7 @@ import org.apache.hadoop.util.ExitUtil;
 import org.apache.hadoop.util.StringUtils;
 
 import static org.apache.hadoop.fs.s3a.Constants.METADATASTORE_AUTHORITATIVE;
+import static org.apache.hadoop.fs.s3a.Constants.S3GUARD_DDB_REGION_KEY;
 import static org.apache.hadoop.fs.s3a.Constants.S3GUARD_DDB_TABLE_CREATE_KEY;
 import static org.apache.hadoop.fs.s3a.Constants.S3GUARD_DDB_TABLE_NAME_KEY;
 import static org.apache.hadoop.fs.s3a.Constants.S3GUARD_METASTORE_NULL;
@@ -346,28 +349,95 @@ public abstract class AbstractS3GuardToolTestBase extends AbstractS3ATestBase {
   @Test
   public void testSetCapacityFailFastIfNotGuarded() throws Exception{
     Configuration conf = getConfiguration();
-    conf.set(S3GUARD_DDB_TABLE_NAME_KEY, UUID.randomUUID().toString());
-    conf.set(S3GUARD_DDB_TABLE_CREATE_KEY, Boolean.FALSE.toString());
+    bindToNonexistentTable(conf);
+    String bucket = rawFs.getBucket();
+    clearBucketOption(conf, bucket, S3_METADATA_STORE_IMPL);
+    clearBucketOption(conf, bucket, S3GUARD_DDB_TABLE_NAME_KEY);
+    clearBucketOption(conf, bucket, S3GUARD_DDB_TABLE_CREATE_KEY);
     conf.set(S3_METADATA_STORE_IMPL, S3GUARD_METASTORE_NULL);
 
     S3GuardTool.SetCapacity cmdR = new S3GuardTool.SetCapacity(conf);
-    String[] argsR = new String[]{cmdR.getName(),
-        "s3a://" + getFileSystem().getBucket()};
+    String[] argsR = new String[]{
+        cmdR.getName(),
+        "s3a://" + getFileSystem().getBucket()
+    };
 
     intercept(IllegalStateException.class, "unguarded",
-        () -> run(argsR));
+        () -> cmdR.run(argsR));
+  }
+
+  /**
+   * Binds the configuration to a nonexistent table.
+   * @param conf
+   */
+  private void bindToNonexistentTable(final Configuration conf) {
+    conf.set(S3GUARD_DDB_TABLE_NAME_KEY, UUID.randomUUID().toString());
+    conf.unset(S3GUARD_DDB_REGION_KEY);
+    conf.setBoolean(S3GUARD_DDB_TABLE_CREATE_KEY, false);
+  }
+
+  /**
+   * Make an S3GuardTool of the specific subtype with binded configuration
+   * to a nonexistent table.
+   * @param tool
+   */
+  private S3GuardTool makeBindedTool(Class<? extends S3GuardTool> tool)
+      throws Exception {
+    Configuration conf = getConfiguration();
+    // set a table as a safety check in case the test goes wrong
+    // and deletes it.
+    bindToNonexistentTable(conf);
+    return tool.getDeclaredConstructor(Configuration.class).newInstance(conf);
+  }
+
+  @Test
+  public void testToolsNoBucket() throws Throwable {
+    List<Class<? extends S3GuardTool>> tools =
+        Arrays.asList(S3GuardTool.Destroy.class, S3GuardTool.BucketInfo.class,
+            S3GuardTool.Diff.class, S3GuardTool.Import.class,
+            S3GuardTool.Prune.class, S3GuardTool.SetCapacity.class,
+            S3GuardTool.Uploads.class);
+
+    for (Class<? extends S3GuardTool> tool : tools) {
+      S3GuardTool cmdR = makeBindedTool(tool);
+      describe("Calling " + cmdR.getName() + " on a bucket that does not exist.");
+      String[] argsR = new String[]{
+          cmdR.getName(),
+          S3A_THIS_BUCKET_DOES_NOT_EXIST
+      };
+      intercept(FileNotFoundException.class,
+          () -> cmdR.run(argsR));
+    }
+  }
+
+  @Test
+  public void testToolsNoArgsForBucketAndDDBTable() throws Throwable {
+    List<Class<? extends S3GuardTool>> tools =
+        Arrays.asList(S3GuardTool.Destroy.class, S3GuardTool.Init.class);
+
+    for (Class<? extends S3GuardTool> tool : tools) {
+      S3GuardTool cmdR = makeBindedTool(tool);
+      describe("Calling " + cmdR.getName() + " without any arguments.");
+      intercept(ExitUtil.ExitException.class,
+          "S3 bucket url or DDB table name have to be provided explicitly",
+          () -> cmdR.run(new String[]{tool.getName()}));
+    }
   }
 
   @Test
-  public void testDestroyNoBucket() throws Throwable {
-    intercept(FileNotFoundException.class,
-        new Callable<Integer>() {
-          @Override
-          public Integer call() throws Exception {
-            return run(S3GuardTool.Destroy.NAME,
-                S3A_THIS_BUCKET_DOES_NOT_EXIST);
-          }
-        });
+  public void testToolsNoArgsForBucket() throws Throwable {
+    List<Class<? extends S3GuardTool>> tools =
+        Arrays.asList(S3GuardTool.BucketInfo.class, S3GuardTool.Diff.class,
+            S3GuardTool.Import.class, S3GuardTool.Prune.class,
+            S3GuardTool.SetCapacity.class, S3GuardTool.Uploads.class);
+
+    for (Class<? extends S3GuardTool> tool : tools) {
+      S3GuardTool cmdR = makeBindedTool(tool);
+      describe("Calling " + cmdR.getName() + " without any arguments.");
+      assertExitCode(S3GuardTool.INVALID_ARGUMENT,
+          intercept(ExitUtil.ExitException.class,
+              () -> cmdR.run(new String[]{tool.getName()})));
+    }
   }
 
   @Test
@@ -382,11 +452,23 @@ public abstract class AbstractS3GuardToolTestBase extends AbstractS3ATestBase {
       exec(cmd, S3GuardTool.BucketInfo.MAGIC_FLAG, name);
     } else {
       // if the FS isn't magic, expect the probe to fail
-      ExitUtil.ExitException e = intercept(ExitUtil.ExitException.class,
-          () -> exec(cmd, S3GuardTool.BucketInfo.MAGIC_FLAG, name));
-      if (e.getExitCode() != E_BAD_STATE) {
-        throw e;
-      }
+      assertExitCode(E_BAD_STATE,
+          intercept(ExitUtil.ExitException.class,
+              () -> exec(cmd, S3GuardTool.BucketInfo.MAGIC_FLAG, name)));
+    }
+  }
+
+  /**
+   * Assert that an exit exception had a specific error code.
+   * @param expectedErrorCode expected code.
+   * @param e exit exception
+   * @throws AssertionError with the exit exception nested inside
+   */
+  protected void assertExitCode(final int expectedErrorCode,
+      final ExitUtil.ExitException e) {
+    if (e.getExitCode() != expectedErrorCode) {
+      throw new AssertionError("Expected error code " +
+          expectedErrorCode + " in " + e, e);
     }
   }