HADOOP-19129: [ABFS] Test Fixes and Test Script Bug Fixes

hadoop-tools/hadoop-azure/dev-support/testrun-scripts/testsupport.sh

@@ -21,8 +21,20 @@ combtestfile=$resourceDir
 combtestfile+=abfs-combination-test-configs.xml
 logdir=dev-support/testlogs/
 
-testresultsregex="Results:(\n|.)*?Tests run:"
+# Regex to filter out final test stats
+testresultsregex="Tests run: [0-9]+, Failures: [0-9]+, Errors: [0-9]+, Skipped: [0-9]+$"
+
+# Regex to filter out the test that failed due to unexpected output or error.
+failedTestRegex1="<<< FAILURE!$"
+
+# Regex to filter out the test that failed due to runtime exception.
+failedTestRegex2="<<< ERROR!$"
+
+# Regex to remove the formatting used by mvn output for better regex matching.
+removeFormattingRegex="s/\x1b\[[0-9;]*m//g"
 accountConfigFileSuffix="_settings.xml"
+separatorbar1="============================================================"
+separatorbar2="------------------------------"
 testOutputLogFolder=$logdir
 testlogfilename=combinationTestLogFile
 
@@ -59,17 +71,23 @@ ENDOFFILE
     logOutput "Exiting. Number of properties and values differ for $combination"
     exit 1
   fi
+  echo "$separatorbar1"
+  echo "$combination"
+  echo "$separatorbar1"
+
+  # First include the account specific configurations.
+  xmlstarlet ed -P -L -s /configuration -t elem -n include -v "" $combtestfile
+  xmlstarlet ed -P -L -i /configuration/include -t attr -n href -v "$accountConfigFile" $combtestfile
+  xmlstarlet ed -P -L -i /configuration/include -t attr -n xmlns -v "http://www.w3.org/2001/XInclude" $combtestfile
+
+  # Override the combination specific configurations.
   for ((i = 0; i < propertiessize; i++)); do
     key=${PROPERTIES[$i]}
     val=${VALUES[$i]}
     echo "Combination specific property setting: [ key=$key , value=$val ]"
     changeconf "$key" "$val"
   done
   formatxml "$combtestfile"
-  xmlstarlet ed -P -L -s /configuration -t elem -n include -v "" $combtestfile
-  xmlstarlet ed -P -L -i /configuration/include -t attr -n href -v "$accountConfigFile" $combtestfile
-  xmlstarlet ed -P -L -i /configuration/include -t attr -n xmlns -v "http://www.w3.org/2001/XInclude" $combtestfile
-  formatxml $combtestfile
   echo ' '
   echo "Activated [$combtestfile] - for account: $accountName for combination $combination"
   testlogfilename="$testOutputLogFolder/Test-Logs-$combination.txt"
@@ -81,6 +99,8 @@ ENDOFFILE
     echo "Running test for combination $combination on account $accountName [ProcessCount=$processcount]"
     logOutput "Test run report can be seen in $testlogfilename"
     mvn -T 1C -Dparallel-tests=abfs -Dscale -DtestsThreadCount="$processcount" verify >> "$testlogfilename" || true
+    # Remove the formatting used by mvn output for better regex matching.
+    sed -i "$removeFormattingRegex" "$testlogfilename"
     ENDTIME=$(date +%s)
     summary
   fi
@@ -102,19 +122,37 @@ ENDOFFILE
 summary() {
   {
     echo ""
+    echo "$separatorbar1"
     echo "$combination"
-    echo "========================"
-    pcregrep -M "$testresultsregex" "$testlogfilename"
+    echo "$separatorbar1"
+    summarycontent
   } >> "$aggregatedTestResult"
   printf "\n----- Test results -----\n"
-  pcregrep -M "$testresultsregex" "$testlogfilename"
+  summarycontent
   secondstaken=$((ENDTIME - STARTTIME))
   mins=$((secondstaken / 60))
   secs=$((secondstaken % 60))
   printf "\nTime taken: %s mins %s secs.\n" "$mins" "$secs"
-  echo "Find test result for the combination ($combination) in: $testlogfilename"
+  logOutput "For Error details refer to Test run report in: $testlogfilename"
   logOutput "Consolidated test result is saved in: $aggregatedTestResult"
-  echo "------------------------"
+  echo "$separatorbar2"
+}
+
+summarycontent() {
+  output=$(pcregrep -M "$failedTestRegex1" "$testlogfilename" || true)
+  if [ -n "$output" ]; then
+    echo "$output"
+  fi
+  output=$(pcregrep -M "$failedTestRegex2" "$testlogfilename" || true)
+  if [ -n "$output" ]; then
+    echo ""
+    echo "$output"
+  fi
+  output=$(pcregrep -M "$testresultsregex" "$testlogfilename" || true)
+  if [ -n "$output" ]; then
+    echo ""
+    echo "$output"
+  fi
 }
 
 checkdependencies() {

hadoop-tools/hadoop-azure/src/main/java/org/apache/hadoop/fs/azurebfs/services/AbfsOutputStream.java

@@ -593,7 +593,7 @@ private void writeAppendBlobCurrentBufferToService() throws IOException {
         "writeCurrentBufferToService", "append")) {
       AppendRequestParameters reqParams = new AppendRequestParameters(offset, 0,
           bytesLength, APPEND_MODE, true, leaseId, isExpectHeaderEnabled);
-      AbfsRestOperation op = client.append(path, uploadData.toByteArray(),
+      AbfsRestOperation op = getClient().append(path, uploadData.toByteArray(),
           reqParams, cachedSasToken.get(), contextEncryptionAdapter,
           new TracingContext(tracingContext));
       cachedSasToken.update(op.getSasToken());
@@ -606,7 +606,7 @@ private void writeAppendBlobCurrentBufferToService() throws IOException {
       outputStreamStatistics.uploadFailed(bytesLength);
       failureWhileSubmit(ex);
     } finally {
-      IOUtils.close(uploadData);
+      IOUtils.close(uploadData, activeBlock);
     }
   }
 

hadoop-tools/hadoop-azure/src/site/markdown/testing_azure.md

@@ -581,7 +581,7 @@ This will delete the containers; the output log of the test run will
 provide the details and summary of the operation.
 
 
-## Testing the Azure ABFS Client
+# Testing the Azure ABFS Client
 
 Azure Data Lake Storage Gen 2 (ADLS Gen 2) is a set of capabilities dedicated to
 big data analytics, built on top of Azure Blob Storage. The ABFS and ABFSS
@@ -602,29 +602,26 @@ various test combinations, it will:
 2. Run tests for all combinations
 3. Summarize results across all the test combination runs.
 
-Below are the pre-requiste steps to follow:
-1. Copy
+Below are the pre-requisite steps to follow:
+1. Copy `./src/test/resources/azure-auth-keys.xml.template` to
+`./src/test/resources/azure-auth-keys.xml`
+1. Update account names that should be used in the test run for HNS and non-HNS
+combinations in the 2 properties present in the xml (account name should be
+without domain part), namely
+   1. `fs.azure.hnsTestAccountName`: Specify the HNS Enabled Account
+   2. `fs.azure.nonHnsTestAccountName`: Specify the HNS Disabled Account
 
-        ./src/test/resources/azure-auth-keys.xml.template
-        TO
-        ./src/test/resources/azure-auth-keys.xml
-  Update account names that should be used in the test run for HNS and non-HNS
-  combinations in the 2 properties present in the xml (account name should be
-  without domain part), namely
+   Note: `azure-auth-keys.xml` is listed in .gitignore, so any accidental account name leak is prevented.
 
-    fs.azure.hnsTestAccountName
-    fs.azure.nonHnsTestAccountName
-  azure-auth-keys.xml is listed in .gitignore, so any accidental account name leak is prevented.
+    ```
+    XInclude is supported, so for extra security secrets may be
+    kept out of the source tree then referenced through an XInclude element:
 
-```
-XInclude is supported, so for extra security secrets may be
-kept out of the source tree then referenced through an XInclude element:
-
-      <include xmlns="http://www.w3.org/2001/XInclude"
-        href="/users/self/.secrets/auth-keys.xml" />
-```
+          <include xmlns="http://www.w3.org/2001/XInclude"
+            href="/users/self/.secrets/auth-keys.xml" />
+    ```
 
-2. Create account config files (one config file per account) in folder:
+1. Create account config files (one config file per account) in folder:
 
         ./src/test/resources/accountSettings/
    Follow the instruction in the start of the template file
@@ -634,6 +631,8 @@ kept out of the source tree then referenced through an XInclude element:
    New files created in folder accountSettings is listed in .gitignore to
    prevent accidental cred leaks.
 
+You are all set to run the test script.
+
 **To run PR validation:** Running command
 * `dev-support/testrun-scripts/runtests.sh` will prompt as below:
 ```bash
@@ -664,6 +663,9 @@ Set the active test combination to run the action:
 2) HNS-SharedKey           4) AppendBlob-HNS-OAuth    6) Quit
 #? 1
 
+============================================================
+HNS-OAuth
+============================================================
 Combination specific property setting: [ key=fs.azure.account.auth.type , value=OAuth ]
 
 Activated [src/test/resources/abfs-combination-test-configs.xml] - for account: snvijayacontracttest for combination HNS-OAuth
@@ -682,6 +684,46 @@ consolidated results of all the combination runs will be saved into a file as
 Test-Results.log in the same folder. When run for PR validation, the
 consolidated test results needs to be pasted into the PR comment section.
 
+**Aggregated Test Results**: `Test-Results.txt` file will show the aggregated results
+across all th combinations ran as part of script in following format
+```bash
+    ============================================================
+    HNS-OAuth
+    ============================================================
+    [ERROR] testAbfsHttpSendStatistics(org.apache.hadoop.fs.azurebfs.ITestAbfsNetworkStatistics)  Time elapsed: 3.137 s  <<< FAILURE!
+    [ERROR] testBlobDataContributor(org.apache.hadoop.fs.azurebfs.ITestAzureBlobFileSystemOauth)  Time elapsed: 4.154 s  <<< ERROR!
+
+    [WARNING] Tests run: 137, Failures: 0, Errors: 0, Skipped: 2
+    [ERROR] Tests run: 623, Failures: 1, Errors: 0, Skipped: 73
+    [ERROR] Tests run: 340, Failures: 0, Errors: 1, Skipped: 55
+
+    ============================================================
+    HNS-SharedKey
+    ============================================================
+    [ERROR] testAbfsHttpSendStatistics(org.apache.hadoop.fs.azurebfs.ITestAbfsNetworkStatistics)  Time elapsed: 2.175 s  <<< FAILURE!
+
+    [WARNING] Tests run: 137, Failures: 0, Errors: 0, Skipped: 3
+    [ERROR] Tests run: 623, Failures: 1, Errors: 0, Skipped: 42
+    [WARNING] Tests run: 340, Failures: 0, Errors: 0, Skipped: 41
+
+    ============================================================
+    NonHNS-SharedKey
+    ============================================================
+    [ERROR] testNonRecursiveDeleteWithPagination(org.apache.hadoop.fs.azurebfs.services.ITestAbfsPaginatedDelete)  Time elapsed: 0.85 s  <<< ERROR!
+
+    [WARNING] Tests run: 137, Failures: 0, Errors: 0, Skipped: 9
+    [ERROR] Tests run: 607, Failures: 1, Errors: 1, Skipped: 269
+    [WARNING] Tests run: 340, Failures: 0, Errors: 0, Skipped: 44
+
+    ============================================================
+    AppendBlob-HNS-OAuth
+    ============================================================
+
+    [WARNING] Tests run: 137, Failures: 0, Errors: 0, Skipped: 2
+    [ERROR] Tests run: 623, Failures: 0, Errors: 0, Skipped: 73
+    [ERROR] Tests run: 340, Failures: 0, Errors: 0, Skipped: 79
+```
+
 **To add a new test combination:** Templates for mandatory test combinations
 for PR validation are present in `dev-support/testrun-scripts/runtests.sh`.
 If a new one needs to be added, add a combination to

hadoop-tools/hadoop-azure/src/test/java/org/apache/hadoop/fs/azurebfs/AbstractAbfsIntegrationTest.java

@@ -107,18 +107,11 @@ protected AbstractAbfsIntegrationTest() throws Exception {
     abfsConfig = new AbfsConfiguration(rawConfig, accountName);
 
     authType = abfsConfig.getEnum(FS_AZURE_ACCOUNT_AUTH_TYPE_PROPERTY_NAME, AuthType.SharedKey);
+    assumeValidAuthConfigsPresent();
+
     abfsScheme = authType == AuthType.SharedKey ? FileSystemUriSchemes.ABFS_SCHEME
             : FileSystemUriSchemes.ABFS_SECURE_SCHEME;
 
-    if (authType == AuthType.SharedKey) {
-      assumeTrue("Not set: " + FS_AZURE_ACCOUNT_KEY,
-          abfsConfig.get(FS_AZURE_ACCOUNT_KEY) != null);
-      // Update credentials
-    } else {
-      assumeTrue("Not set: " + FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME,
-          abfsConfig.get(FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME) != null);
-    }
-
     final String abfsUrl = this.getFileSystemName() + "@" + this.getAccountName();
     URI defaultUri = null;
 
@@ -131,7 +124,7 @@ protected AbstractAbfsIntegrationTest() throws Exception {
     this.testUrl = defaultUri.toString();
     abfsConfig.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY, defaultUri.toString());
     abfsConfig.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION, true);
-    if (abfsConfig.get(FS_AZURE_TEST_APPENDBLOB_ENABLED) == "true") {
+    if (isAppendBlobEnabled()) {
       String appendblobDirs = this.testUrl + "," + abfsConfig.get(FS_AZURE_CONTRACT_TEST_URI);
       rawConfig.set(FS_AZURE_APPEND_BLOB_KEY, appendblobDirs);
     }
@@ -265,26 +258,28 @@ public AccessTokenProvider getAccessTokenProvider(final AzureBlobFileSystem fs)
   }
 
   public void loadConfiguredFileSystem() throws Exception {
-      // disable auto-creation of filesystem
-      abfsConfig.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION,
+    // disable auto-creation of filesystem
+    abfsConfig.setBoolean(AZURE_CREATE_REMOTE_FILESYSTEM_DURING_INITIALIZATION,
           false);
 
-      // AbstractAbfsIntegrationTest always uses a new instance of FileSystem,
-      // need to disable that and force filesystem provided in test configs.
-      String[] authorityParts =
-          (new URI(rawConfig.get(FS_AZURE_CONTRACT_TEST_URI))).getRawAuthority().split(
-        AbfsHttpConstants.AZURE_DISTRIBUTED_FILE_SYSTEM_AUTHORITY_DELIMITER, 2);
-      this.fileSystemName = authorityParts[0];
+    // AbstractAbfsIntegrationTest always uses a new instance of FileSystem,
+    // need to disable that and force filesystem provided in test configs.
+    assumeValidTestConfigPresent(this.getRawConfiguration(), FS_AZURE_CONTRACT_TEST_URI);
 
-      // Reset URL with configured filesystem
-      final String abfsUrl = this.getFileSystemName() + "@" + this.getAccountName();
-      URI defaultUri = null;
+    String[] authorityParts =
+        (new URI(rawConfig.get(FS_AZURE_CONTRACT_TEST_URI))).getRawAuthority().split(
+      AbfsHttpConstants.AZURE_DISTRIBUTED_FILE_SYSTEM_AUTHORITY_DELIMITER, 2);
+    this.fileSystemName = authorityParts[0];
 
-      defaultUri = new URI(abfsScheme, abfsUrl, null, null, null);
+    // Reset URL with configured filesystem
+    final String abfsUrl = this.getFileSystemName() + "@" + this.getAccountName();
+    URI defaultUri = null;
+
+    defaultUri = new URI(abfsScheme, abfsUrl, null, null, null);
 
-      this.testUrl = defaultUri.toString();
-      abfsConfig.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY,
-          defaultUri.toString());
+    this.testUrl = defaultUri.toString();
+    abfsConfig.set(CommonConfigurationKeysPublic.FS_DEFAULT_NAME_KEY,
+        defaultUri.toString());
 
     useConfiguredFileSystem = true;
   }
@@ -536,8 +531,35 @@ protected long assertAbfsStatistics(AbfsStatistic statistic,
   }
 
   protected void assumeValidTestConfigPresent(final Configuration conf, final String key) {
-    String configuredValue = conf.get(key);
+    String configuredValue = conf.get(accountProperty(key, accountName),
+        conf.get(key, ""));
     Assume.assumeTrue(String.format("Missing Required Test Config: %s.", key),
-        configuredValue != null && !configuredValue.isEmpty());
+        !configuredValue.isEmpty());
+  }
+
+  protected void assumeValidAuthConfigsPresent() {
+    final AuthType currentAuthType = getAuthType();
+    Assume.assumeFalse(
+        "SAS Based Authentication Not Allowed For Integration Tests",
+        currentAuthType == AuthType.SAS);
+    if (currentAuthType == AuthType.SharedKey) {
+      assumeValidTestConfigPresent(getRawConfiguration(), FS_AZURE_ACCOUNT_KEY);
+    } else if (currentAuthType == AuthType.OAuth) {
+      assumeValidTestConfigPresent(getRawConfiguration(),
+          FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME);
+      assumeValidTestConfigPresent(getRawConfiguration(),
+          FS_AZURE_ACCOUNT_OAUTH_CLIENT_ID);
+      assumeValidTestConfigPresent(getRawConfiguration(),
+          FS_AZURE_ACCOUNT_OAUTH_CLIENT_SECRET);
+      assumeValidTestConfigPresent(getRawConfiguration(),
+          FS_AZURE_ACCOUNT_OAUTH_CLIENT_ENDPOINT);
+    } else if (currentAuthType == AuthType.Custom) {
+      assumeValidTestConfigPresent(getRawConfiguration(),
+          FS_AZURE_ACCOUNT_TOKEN_PROVIDER_TYPE_PROPERTY_NAME);
+    }
+  }
+
+  protected boolean isAppendBlobEnabled() {
+    return getRawConfiguration().getBoolean(FS_AZURE_TEST_APPENDBLOB_ENABLED, false);
   }
 }