From 409f306c50ae95a734610c9377b53f048d72ef2b Mon Sep 17 00:00:00 2001 From: Steve Loughran Date: Wed, 26 Oct 2022 18:11:04 +0100 Subject: [PATCH] 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 --- .../fs/FileContextMainOperationsBaseTest.java | 8 +++- hadoop-tools/hadoop-aws/pom.xml | 20 ++++++--- .../site/markdown/tools/hadoop-aws/testing.md | 44 +++++++++++++++++++ .../contract/s3a/ITestS3AContractRootDir.java | 8 ++++ .../hadoop/fs/s3a/ITestS3AConfiguration.java | 6 ++- .../hadoop/fs/s3a/ITestS3AEncryptionSSEC.java | 4 ++ .../fs/s3a/ITestS3AFSMainOperations.java | 24 ++++++++++ .../hadoop/fs/s3a/S3ATestConstants.java | 11 +++++ .../apache/hadoop/fs/s3a/S3ATestUtils.java | 11 ++++- .../ITestS3AFileContextMainOperations.java | 30 ++++++++++--- .../s3a/scale/AbstractSTestS3AHugeFiles.java | 10 +++++ .../fs/s3a/scale/ITestS3AConcurrentOps.java | 4 +- .../hadoop/fs/s3a/scale/S3AScaleTestBase.java | 2 +- .../tools/ITestMarkerToolRootOperations.java | 2 + .../apache/hadoop/fs/s3a/yarn/ITestS3A.java | 41 ++++++----------- 15 files changed, 180 insertions(+), 45 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java index 4c90490b090..6897a0d1943 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/FileContextMainOperationsBaseTest.java +++ b/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 abstract class FileContextMainOperationsBaseTest { private static final byte[] data = getFileData(numBlocks, getDefaultBlockSize()); - + @Before public void setUp() throws Exception { File testBuildData = GenericTestUtils.getRandomizedTestDir(); diff --git a/hadoop-tools/hadoop-aws/pom.xml b/hadoop-tools/hadoop-aws/pom.xml index 6ebf1c71f0d..9c23a035162 100644 --- a/hadoop-tools/hadoop-aws/pom.xml +++ b/hadoop-tools/hadoop-aws/pom.xml @@ -56,6 +56,13 @@ unset + + + 0 + + unset + + @@ -162,7 +169,7 @@ - fork-000${surefire.forkNumber} + job-${job.id}-fork-000${surefire.forkNumber} ${fs.s3a.scale.test.enabled} ${fs.s3a.scale.test.huge.filesize} @@ -173,14 +180,14 @@ ${test.integration.timeout} ${fs.s3a.prefetch.enabled} + + ${root.tests.enabled} + - - - - + @@ -224,6 +231,9 @@ ${fs.s3a.directory.marker.audit} ${fs.s3a.prefetch.enabled} + + ${root.tests.enabled} + job-${job.id} diff --git a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/testing.md b/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/testing.md index d2ed9ede017..9bbe39df334 100644 --- a/hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/testing.md +++ b/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 + + + ## Load tests. Some are designed to overload AWS services with more diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRootDir.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRootDir.java index 5335de1b324..cd5c078a9ed 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/contract/s3a/ITestS3AContractRootDir.java +++ b/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.AbstractContractRootDirectoryTest; 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); diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java index 26d00bc7d35..a5806af8d2c 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AConfiguration.java @@ -354,8 +354,10 @@ public class ITestS3AConfiguration { 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 diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionSSEC.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionSSEC.java index 64e37bf832b..c7bfdb30ba0 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AEncryptionSSEC.java +++ b/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.Constants.SERVER_SIDE_ENCRYPTION_KEY; 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 @@ public class ITestS3AEncryptionSSEC extends AbstractTestS3AEncryption { 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 diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFSMainOperations.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFSMainOperations.java index 6669e8426af..ea6aeb35b56 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/ITestS3AFSMainOperations.java +++ b/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 class ITestS3AFSMainOperations extends FSMainOperationsBaseTest { } } + + @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() { diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestConstants.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestConstants.java index a6269c43766..986115d3412 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestConstants.java +++ b/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; } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java index 469562f9b33..30104854427 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/S3ATestUtils.java @@ -485,7 +485,7 @@ public final class S3ATestUtils { */ 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 final class S3ATestUtils { 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)); + } } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/fileContext/ITestS3AFileContextMainOperations.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/fileContext/ITestS3AFileContextMainOperations.java index 3b4eaf4a806..b28f88e43b4 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/fileContext/ITestS3AFileContextMainOperations.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/fileContext/ITestS3AFileContextMainOperations.java @@ -3,7 +3,7 @@ * 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, @@ -11,21 +11,28 @@ * 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 class ITestS3AFileContextMainOperations 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; diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractSTestS3AHugeFiles.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractSTestS3AHugeFiles.java index f8d47011de3..6aff74be860 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractSTestS3AHugeFiles.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/AbstractSTestS3AHugeFiles.java @@ -98,6 +98,16 @@ public abstract class AbstractSTestS3AHugeFiles extends S3AScaleTestBase { DEFAULT_HUGE_FILESIZE); } + /** + * Test dir deletion is removed from test case teardown so the + * subsequent tests see the output. + * @throws IOException failure + */ + @Override + protected void deleteTestDirInTeardown() throws IOException { + /* no-op */ + } + /** * Get the name of this test suite, which is used in path generation. * Base implementation uses {@link #getBlockOutputBufferName()} for this. diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/ITestS3AConcurrentOps.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/ITestS3AConcurrentOps.java index df5cd46fffa..9dd8583855b 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/ITestS3AConcurrentOps.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/ITestS3AConcurrentOps.java @@ -67,8 +67,8 @@ public class ITestS3AConcurrentOps extends S3AScaleTestBase { super.setup(); auxFs = getNormalFileSystem(); - testRoot = path("/ITestS3AConcurrentOps"); - testRoot = S3ATestUtils.createTestPath(testRoot); + // this is set to the method path, even in test setup. + testRoot = methodPath(); } private S3AFileSystem getNormalFileSystem() throws Exception { diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/S3AScaleTestBase.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/S3AScaleTestBase.java index 514c6cf8869..38839ba0ddc 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/S3AScaleTestBase.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/scale/S3AScaleTestBase.java @@ -83,7 +83,7 @@ public class S3AScaleTestBase extends AbstractS3ATestBase { @Override public void setup() throws Exception { super.setup(); - testPath = path("/tests3ascale"); + testPath = path("tests3ascale"); LOG.debug("Scale test operation count = {}", getOperationCount()); enabled = getTestPropertyBool( getConf(), diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestMarkerToolRootOperations.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestMarkerToolRootOperations.java index 02fec81513f..6d50fa72303 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestMarkerToolRootOperations.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/tools/ITestMarkerToolRootOperations.java @@ -26,6 +26,7 @@ import org.junit.runners.MethodSorters; import org.apache.hadoop.fs.Path; +import static org.apache.hadoop.fs.s3a.S3ATestUtils.maybeSkipRootTests; import static org.apache.hadoop.fs.s3a.tools.MarkerTool.AUDIT; import static org.apache.hadoop.fs.s3a.tools.MarkerTool.CLEAN; import static org.apache.hadoop.fs.s3a.tools.MarkerTool.MARKERS; @@ -42,6 +43,7 @@ public class ITestMarkerToolRootOperations extends AbstractMarkerToolTest { @Override public void setup() throws Exception { super.setup(); + maybeSkipRootTests(getConfiguration()); rootPath = getFileSystem().makeQualified(new Path("/")); } diff --git a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/yarn/ITestS3A.java b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/yarn/ITestS3A.java index 7d2c1dc3023..037eda97427 100644 --- a/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/yarn/ITestS3A.java +++ b/hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/yarn/ITestS3A.java @@ -17,49 +17,34 @@ */ package org.apache.hadoop.fs.s3a.yarn; -import org.apache.hadoop.conf.Configuration; +import java.util.EnumSet; + +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.Timeout; + import org.apache.hadoop.fs.CreateFlag; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileContext; import org.apache.hadoop.fs.FsStatus; import org.apache.hadoop.fs.Path; - -import org.junit.After; -import org.junit.Before; -import org.junit.Rule; -import org.junit.Test; -import org.junit.rules.Timeout; - -import java.util.EnumSet; +import org.apache.hadoop.fs.s3a.AbstractS3ATestBase; import org.apache.hadoop.fs.s3a.S3ATestUtils; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertTrue; - /** * S3A tests through the {@link FileContext} API. */ -public class ITestS3A { +public class ITestS3A extends AbstractS3ATestBase { private FileContext fc; @Rule public final Timeout testTimeout = new Timeout(90000); - @Before - public void setUp() throws Exception { - Configuration conf = new Configuration(); - fc = S3ATestUtils.createTestFileContext(conf); - } - @After - public void tearDown() throws Exception { - if (fc != null) { - fc.delete(getTestPath(), true); - } - } - - protected Path getTestPath() { - return S3ATestUtils.createTestPath(new Path("/tests3afc")); + @Override + public void setup() throws Exception { + super.setup(); + fc = S3ATestUtils.createTestFileContext(getConfiguration()); } @Test @@ -77,7 +62,7 @@ public class ITestS3A { @Test public void testS3ACreateFileInSubDir() throws Exception { - Path dirPath = getTestPath(); + Path dirPath = methodPath(); fc.mkdir(dirPath, FileContext.DIR_DEFAULT_PERM, true); Path filePath = new Path(dirPath, "file"); try (FSDataOutputStream file = fc.create(filePath, EnumSet.of(CreateFlag