HADOOP-18508. Support multiple s3a integration test runs on same bucket in parallel

* adds job id to fork id
* adds option to disable root tests
* documents this
* gets more tests working under the job/fork subdir rather
  than absolute paths. tracking them down is tricky.

Change-Id: I95530f883307b66971c0300631993798967b6739

Author: steveloughran

hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java

@@ -81,6 +81,12 @@ public abstract class FileContextMainOperationsBaseTest  {
   protected final FileContextTestHelper fileContextTestHelper =
     createFileContextHelper();
 
+  /**
+   * Create the test helper.
+   * Important: this is invoked during the construction of the base class,
+   * so is very brittle.
+   * @return a test helper.
+   */
   protected FileContextTestHelper createFileContextHelper() {
     return new FileContextTestHelper();
   }
@@ -107,7 +113,7 @@ public boolean accept(Path file) {
 
   private static final byte[] data = getFileData(numBlocks,
       getDefaultBlockSize());
-  
+
   @Before
   public void setUp() throws Exception {
     File testBuildData = GenericTestUtils.getRandomizedTestDir();

hadoop-tools/hadoop-aws/pom.xml

@@ -56,6 +56,13 @@
 
     <!-- Is prefetch enabled? -->
     <fs.s3a.prefetch.enabled>unset</fs.s3a.prefetch.enabled>
+
+    <!-- Job ID; allows for parallel jobs on same bucket -->
+    <job.id>0</job.id>
+    <!-- are root tests enabled. Set to false when running parallel jobs on same bucket -->
+    <root.tests.enabled>unset</root.tests.enabled>
+
+
   </properties>
 
   <profiles>
@@ -162,7 +169,7 @@
                     <!-- surefire.forkNumber won't do the parameter -->
                     <!-- substitution.  Putting a prefix in front of it like -->
                     <!-- "fork-" makes it work. -->
-                    <test.unique.fork.id>fork-000${surefire.forkNumber}</test.unique.fork.id>
+                    <test.unique.fork.id>job-${job.id}-fork-000${surefire.forkNumber}</test.unique.fork.id>
                     <!-- Propagate scale parameters -->
                     <fs.s3a.scale.test.enabled>${fs.s3a.scale.test.enabled}</fs.s3a.scale.test.enabled>
                     <fs.s3a.scale.test.huge.filesize>${fs.s3a.scale.test.huge.filesize}</fs.s3a.scale.test.huge.filesize>
@@ -173,14 +180,14 @@
                     <test.default.timeout>${test.integration.timeout}</test.default.timeout>
                     <!-- Prefetch -->
                     <fs.s3a.prefetch.enabled>${fs.s3a.prefetch.enabled}</fs.s3a.prefetch.enabled>
+                    <!-- are root tests enabled. Set to false when running parallel jobs on same bucket -->
+                    <fs.s3a.root.tests.enabled>${root.tests.enabled}</fs.s3a.root.tests.enabled>
+
                   </systemPropertyVariables>
                   <!-- Some tests cannot run in parallel.  Tests that cover -->
                   <!-- access to the root directory must run in isolation -->
                   <!-- from anything else that could modify the bucket. -->
-                  <!-- S3A tests that cover multi-part upload must run in -->
-                  <!-- isolation, because the file system is configured to -->
-                  <!-- purge existing multi-part upload data on -->
-                  <!-- initialization.  MiniYARNCluster has not yet been -->
+                  <!-- MiniYARNCluster has not yet been -->
                   <!-- changed to handle parallel test execution gracefully. -->
                   <!-- Exclude all of these tests from parallel execution, -->
                   <!-- and instead run them sequentially in a separate -->
@@ -224,6 +231,9 @@
                     <fs.s3a.directory.marker.audit>${fs.s3a.directory.marker.audit}</fs.s3a.directory.marker.audit>
                     <!-- Prefetch -->
                     <fs.s3a.prefetch.enabled>${fs.s3a.prefetch.enabled}</fs.s3a.prefetch.enabled>
+                    <!-- are root tests enabled. Set to false when running parallel jobs on same bucket -->
+                    <fs.s3a.root.tests.enabled>${root.tests.enabled}</fs.s3a.root.tests.enabled>
+                    <test.unique.fork.id>job-${job.id}</test.unique.fork.id>
                   </systemPropertyVariables>
                   <!-- Do a sequential run for tests that cannot handle -->
                   <!-- parallel execution. -->

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

@@ -518,6 +518,50 @@ Otherwise, set a large timeout in `fs.s3a.scale.test.timeout`
 The tests are executed in an order to only clean up created files after
 the end of all the tests. If the tests are interrupted, the test data will remain.
 
+## Testing through continuous integration
+
+Supporting 
+parallel test jobs against the same bucket
+
+For CI testing of the module, including the integration tests,
+it is generally necessary to support testing multiple PRs simultaneously.
+
+To do this
+1. A job ID must be supplied in the `job.id` property, so each job works on an isolated directory
+   tree. This should be a number or unique string, which will be used within a path element, so
+   must only contain characters valid in an S3/hadoop path element.
+2. Root directory tests need to be disabled by setting `fs.s3a.root.tests.enabled` to
+   `false`, either in the command line to maven or in the XML configurations.
+
+```
+mvn verify -T 1C -Dparallel-tests -DtestsThreadCount=14 -Dscale -Dfs.s3a.root.tests.enabled=false -Djob.id=001
+```   
+
+This parallel execution feature is only for isolated builds sharing a single S3 bucket; it does
+not support parallel builds and tests from the same local source tree.
+
+Without the root tests being executed, set up a scheduled job to purge the test bucket of all
+data on a regular basis, to keep costs down. Running the test `ITestS3AContractRootDir` is
+sufficient for this, as is a `hadoop fs` command. 
+
+```
+hadoop fs -rm -r s3a://s3a-tests-us-west-1/*
+```
+
+### Securing CI builds
+
+It's clearly unsafe to have CI infrastructure testing PRs submitted to apache github account
+with AWS credentials -which is why it isn't done by the Yetus-initiated builds.
+
+Anyone doing this privately should
+* Review patches before triggering the tests.
+* Have a dedicated IAM role with restricted access to the test bucket, any KMS keys used, and the
+  external bucket containing the CSV test file.
+* Have a build process which generates short-lived session credentials for this role.
+* And 
+
+
+
 ## <a name="alternate_s3"></a> Load tests.
 
 Some are designed to overload AWS services with more

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRootDir.java

@@ -27,6 +27,8 @@
 import org.apache.hadoop.fs.contract.AbstractFSContract;
 import org.apache.hadoop.fs.s3a.S3AFileSystem;
 
+import static org.apache.hadoop.fs.s3a.S3ATestUtils.maybeSkipRootTests;
+
 /**
  * root dir operations against an S3 bucket.
  */
@@ -36,6 +38,12 @@ public class ITestS3AContractRootDir extends
   private static final Logger LOG =
       LoggerFactory.getLogger(ITestS3AContractRootDir.class);
 
+  @Override
+  public void setup() throws Exception {
+    super.setup();
+    maybeSkipRootTests(getFileSystem().getConf());
+  }
+
   @Override
   protected AbstractFSContract createContract(Configuration conf) {
     return new S3AContract(conf);

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

@@ -354,8 +354,10 @@ public void shouldBeAbleToSwitchOnS3PathStyleAccessViaConfigProperty()
           clientOptions.isPathStyleAccess());
       byte[] file = ContractTestUtils.toAsciiByteArray("test file");
       ContractTestUtils.writeAndRead(fs,
-          new Path("/path/style/access/testFile"), file, file.length,
-              (int) conf.getLongBytes(Constants.FS_S3A_BLOCK_SIZE, file.length), false, true);
+          createTestPath(new Path("/path/style/access/testFile")),
+          file, file.length,
+          (int) conf.getLongBytes(Constants.FS_S3A_BLOCK_SIZE, file.length),
+          false, true);
     } catch (final AWSS3IOException e) {
       LOG.error("Caught exception: ", e);
       // Catch/pass standard path style access behaviour when live bucket

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

@@ -48,6 +48,7 @@
 import static org.apache.hadoop.fs.s3a.S3ATestUtils.createTestPath;
 import static org.apache.hadoop.fs.s3a.S3ATestUtils.disableFilesystemCaching;
 import static org.apache.hadoop.fs.s3a.S3ATestUtils.getTestBucketName;
+import static org.apache.hadoop.fs.s3a.S3ATestUtils.maybeSkipRootTests;
 import static org.apache.hadoop.fs.s3a.S3ATestUtils.removeBaseAndBucketOverrides;
 import static org.apache.hadoop.test.LambdaTestUtils.intercept;
 
@@ -129,6 +130,9 @@ protected Configuration createConfiguration() {
   public void setup() throws Exception {
     super.setup();
     assumeEnabled();
+    // although not a root dir test, this confuses paths enough it shouldn't be run in
+    // parallel with other jobs
+    maybeSkipRootTests(getConfiguration());
   }
 
   @Override

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

@@ -18,7 +18,11 @@
 
 package org.apache.hadoop.fs.s3a;
 
+import java.io.IOException;
+
+import org.junit.Assert;
 import org.junit.Ignore;
+import org.junit.Test;
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.fs.FSMainOperationsBaseTest;
@@ -54,6 +58,26 @@ public void tearDown() throws Exception {
     }
   }
 
+
+  @Test
+  @Override
+  public void testWDAbsolute() throws IOException {
+    Path absoluteDir = getTestRootPath(fSys, "test/existingDir");
+    fSys.mkdirs(absoluteDir);
+    fSys.setWorkingDirectory(absoluteDir);
+    Assert.assertEquals(absoluteDir, fSys.getWorkingDirectory());
+  }
+
+  /**
+   * Get an absolute directory for the test {@link #testWDAbsolute()}.
+   *
+   * @return a fully qualified path.
+   */
+  protected Path getAbsoluteDirectoryForWorkingDir() {
+    return new Path(new Path(fSys.getUri()),
+        createTestPath(new Path("/test/existingDir")));
+  }
+
   @Override
   @Ignore("Permissions not supported")
   public void testListStatusThrowsExceptionForUnreadableDir() {

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

@@ -251,4 +251,15 @@ public interface S3ATestConstants {
    * Value: {@value}.
    */
   String PROJECT_BUILD_DIRECTORY_PROPERTY = "project.build.directory";
+
+  /**
+   * System property for root tests being enabled: {@value}.
+   */
+  String ROOT_TESTS_ENABLED = "fs.s3a.root.tests.enabled";
+
+
+  /**
+   * Default policy on root tests: {@value}.
+   */
+  boolean DEFAULT_ROOT_TESTS_ENABLED = true;
 }

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

@@ -485,7 +485,7 @@ public static void skipIfStorageClassTestsDisabled(
    */
   public static Path createTestPath(Path defVal) {
     String testUniqueForkId =
-        System.getProperty(S3ATestConstants.TEST_UNIQUE_FORK_ID);
+        System.getProperty(TEST_UNIQUE_FORK_ID);
     return testUniqueForkId == null ? defVal :
         new Path("/" + testUniqueForkId, "test");
   }
@@ -1510,4 +1510,13 @@ public static S3AInputStream getS3AInputStream(
   public static void disablePrefetching(Configuration conf) {
     removeBaseAndBucketOverrides(conf, PREFETCH_ENABLED_KEY);
   }
+
+  /**
+   * Skip root tests if the system properties/config says so.
+   * @param conf configuration to check
+   */
+  public static void maybeSkipRootTests(Configuration conf) {
+    assume("Root tests disabled",
+        getTestPropertyBool(conf, ROOT_TESTS_ENABLED, DEFAULT_ROOT_TESTS_ENABLED));
+  }
 }

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/fileContext/ITestS3AFileContextMainOperations.java

@@ -3,29 +3,36 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- *      http://www.apache.org/licenses/LICENSE-2.0
+ * http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
  * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 package org.apache.hadoop.fs.s3a.fileContext;
 
 import java.io.IOException;
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.FileContextMainOperationsBaseTest;
-import org.apache.hadoop.fs.s3a.S3ATestUtils;
+import java.util.UUID;
+
 import org.junit.Before;
 import org.junit.Ignore;
 import org.junit.Test;
 
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.fs.FileContextMainOperationsBaseTest;
+import org.apache.hadoop.fs.FileContextTestHelper;
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.s3a.S3ATestUtils;
+
 /**
  * S3A implementation of FileContextMainOperationsBaseTest.
  */
 public class ITestS3AFileContextMainOperations
-        extends FileContextMainOperationsBaseTest {
+    extends FileContextMainOperationsBaseTest {
+
 
   @Before
   public void setUp() throws IOException, Exception {
@@ -34,6 +41,19 @@ public void setUp() throws IOException, Exception {
     super.setUp();
   }
 
+
+  /**
+   * Called before even our own constructor and fields are
+   * inited.
+   * @return a test helper using the s3a test path.
+   */
+  @Override
+  protected FileContextTestHelper createFileContextHelper() {
+    Path testPath =
+        S3ATestUtils.createTestPath(new Path("/" + UUID.randomUUID()));
+    return new FileContextTestHelper(testPath.toUri().toString());
+  }
+
   @Override
   protected boolean listCorruptedBlocksSupported() {
     return false;