Revert "HADOOP-19485. S3A: Test failures after SDK upgrade"

This reverts commit 89a988b0a88871370b9ea8551194c64fb0d8a1ae.

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/AwsSdkWorkarounds.java

@@ -18,10 +18,9 @@
 
 package org.apache.hadoop.fs.s3a.impl;
 
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
 import org.apache.hadoop.classification.VisibleForTesting;
+import org.apache.hadoop.fs.s3a.impl.logging.LogControl;
+import org.apache.hadoop.fs.s3a.impl.logging.LogControllerFactory;
 
 /**
  * This class exists to support workarounds for parts of the AWS SDK
@@ -36,20 +35,16 @@ public final class AwsSdkWorkarounds {
   public static final String TRANSFER_MANAGER =
       "software.amazon.awssdk.transfer.s3.S3TransferManager";
 
-  private static final Logger LOG = LoggerFactory.getLogger(AwsSdkWorkarounds.class);
-
   private AwsSdkWorkarounds() {
   }
 
   /**
    * Prepare logging before creating AWS clients.
-   * There is currently no logging to require tuning,
-   * so this only logs at trace that it was invoked.
    * @return true if the log tuning operation took place.
    */
   public static boolean prepareLogging() {
-    LOG.trace("prepareLogging()");
-    return true;
+    return LogControllerFactory.createController().
+        setLogLevel(TRANSFER_MANAGER, LogControl.LogLevel.ERROR);
   }
 
   /**
@@ -58,6 +53,7 @@ public final class AwsSdkWorkarounds {
    */
   @VisibleForTesting
   static boolean restoreNoisyLogging() {
-    return true;
+    return LogControllerFactory.createController().
+        setLogLevel(TRANSFER_MANAGER, LogControl.LogLevel.INFO);
   }
 }

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

@@ -24,9 +24,9 @@ import java.net.UnknownHostException;
 import java.nio.file.AccessDeniedException;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.concurrent.atomic.AtomicReference;
 
 import org.assertj.core.api.Assertions;
+import org.junit.Ignore;
 import org.junit.Test;
 import software.amazon.awssdk.awscore.AwsExecutionAttribute;
 import software.amazon.awssdk.awscore.exception.AwsServiceException;
@@ -106,10 +106,6 @@ public class ITestS3AEndpointRegion extends AbstractS3ATestBase {
 
   public static final String EXCEPTION_THROWN_BY_INTERCEPTOR = "Exception thrown by interceptor";
 
-  /**
-   * Text to include in assertions.
-   */
-  private static final AtomicReference<String> EXPECTED_MESSAGE = new AtomicReference<>();
   /**
    * New FS instance which will be closed in teardown.
    */
@@ -580,7 +576,7 @@ public class ITestS3AEndpointRegion extends AbstractS3ATestBase {
         .isFalse();
   }
 
-  private static final class RegionInterceptor implements ExecutionInterceptor {
+  private final class RegionInterceptor implements ExecutionInterceptor {
     private final String endpoint;
     private final String region;
     private final boolean isFips;
@@ -595,49 +591,28 @@ public class ITestS3AEndpointRegion extends AbstractS3ATestBase {
     public void beforeExecution(Context.BeforeExecution context,
         ExecutionAttributes executionAttributes)  {
 
-
-      // extract state from the execution attributes.
-      final Boolean endpointOveridden =
-          executionAttributes.getAttribute(AwsExecutionAttribute.ENDPOINT_OVERRIDDEN);
-      final String clientEndpoint =
-          executionAttributes.getAttribute(AwsExecutionAttribute.CLIENT_ENDPOINT).toString();
-      final Boolean fipsEnabled = executionAttributes.getAttribute(
-          AwsExecutionAttribute.FIPS_ENDPOINT_ENABLED);
-      final String reg = executionAttributes.getAttribute(AwsExecutionAttribute.AWS_REGION).
-          toString();
-
-      String state = "SDK beforeExecution callback; "
-          + "endpointOveridden=" + endpointOveridden
-          + "; clientEndpoint=" + clientEndpoint
-          + "; fipsEnabled=" + fipsEnabled
-          + "; region=" + reg;
-
       if (endpoint != null && !endpoint.endsWith(CENTRAL_ENDPOINT)) {
-        Assertions.assertThat(endpointOveridden)
-            .describedAs("Endpoint not overridden in %s. Client Config=%s",
-                state, EXPECTED_MESSAGE.get())
-            .isTrue();
-
-        Assertions.assertThat(clientEndpoint)
-            .describedAs("There is an endpoint mismatch in %s. Client Config=%s",
-                state, EXPECTED_MESSAGE.get())
-            .isEqualTo("https://" + endpoint);
+        Assertions.assertThat(
+                executionAttributes.getAttribute(AwsExecutionAttribute.ENDPOINT_OVERRIDDEN))
+            .describedAs("Endpoint not overridden").isTrue();
+
+        Assertions.assertThat(
+                executionAttributes.getAttribute(AwsExecutionAttribute.CLIENT_ENDPOINT).toString())
+            .describedAs("There is an endpoint mismatch").isEqualTo("https://" + endpoint);
       } else {
-        Assertions.assertThat(endpointOveridden)
-            .describedAs("Attribute endpointOveridden is null in %s. Client Config=%s",
-                state, EXPECTED_MESSAGE.get())
-            .isEqualTo(false);
+        Assertions.assertThat(
+                executionAttributes.getAttribute(AwsExecutionAttribute.ENDPOINT_OVERRIDDEN))
+            .describedAs("Endpoint is overridden").isEqualTo(null);
       }
 
-      Assertions.assertThat(reg)
-          .describedAs("Incorrect region set in %s. Client Config=%s",
-              state, EXPECTED_MESSAGE.get())
-          .isEqualTo(region);
+      Assertions.assertThat(
+              executionAttributes.getAttribute(AwsExecutionAttribute.AWS_REGION).toString())
+          .describedAs("Incorrect region set").isEqualTo(region);
 
       // verify the fips state matches expectation.
-      Assertions.assertThat(fipsEnabled)
-          .describedAs("Incorrect FIPS flag set in %s; Client Config=%s",
-              state, EXPECTED_MESSAGE.get())
+      Assertions.assertThat(executionAttributes.getAttribute(
+          AwsExecutionAttribute.FIPS_ENDPOINT_ENABLED))
+          .describedAs("Incorrect FIPS flag set in execution attributes")
           .isNotNull()
           .isEqualTo(isFips);
 
@@ -662,11 +637,6 @@ public class ITestS3AEndpointRegion extends AbstractS3ATestBase {
       String endpoint, String configuredRegion, String expectedRegion, boolean isFips)
       throws IOException {
 
-    String expected =
-        "endpoint=" + endpoint + "; region=" + configuredRegion
-        + "; expectedRegion=" + expectedRegion + "; isFips=" + isFips;
-    LOG.info("Creating S3 client with {}", expected);
-    EXPECTED_MESSAGE.set(expected);
     List<ExecutionInterceptor> interceptors = new ArrayList<>();
     interceptors.add(new RegionInterceptor(endpoint, expectedRegion, isFips));
 

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

@@ -32,9 +32,12 @@ import org.apache.hadoop.test.GenericTestUtils;
 import static org.apache.hadoop.test.GenericTestUtils.LogCapturer.captureLogs;
 
 /**
- * Tests for any AWS SDK workaround code.
+ * Verify that noisy transfer manager logs are turned off.
  * <p>
- * These tests are inevitably brittle against SDK updates.
+ * This is done by creating new FS instances and then
+ * requesting an on-demand transfer manager from the store.
+ * As this is only done once per FS instance, a new FS is
+ * required per test case.
  */
 public class ITestAwsSdkWorkarounds extends AbstractS3ATestBase {
 
@@ -50,6 +53,13 @@ public class ITestAwsSdkWorkarounds extends AbstractS3ATestBase {
   private static final Logger XFER_LOG =
       LoggerFactory.getLogger(AwsSdkWorkarounds.TRANSFER_MANAGER);
 
+  /**
+   * This is the string which keeps being printed.
+   * {@value}.
+   */
+  private static final String FORBIDDEN =
+      "The provided S3AsyncClient is an instance of MultipartS3AsyncClient";
+
   /**
    * Marginal test run speedup by skipping needless test dir cleanup.
    * @throws IOException failure
@@ -60,7 +70,23 @@ public class ITestAwsSdkWorkarounds extends AbstractS3ATestBase {
   }
 
   /**
-   * Test instantiation with logging enabled.
+   * Test instantiation with logging disabled.
+   */
+  @Test
+  public void testQuietLogging() throws Throwable {
+    // simulate the base state of logging
+    noisyLogging();
+    // creating a new FS switches to quiet logging
+    try (S3AFileSystem newFs = newFileSystem()) {
+      String output = createAndLogTransferManager(newFs);
+      Assertions.assertThat(output)
+          .describedAs("LOG output")
+          .doesNotContain(FORBIDDEN);
+    }
+  }
+
+  /**
+   * Test instantiation with logging disabled.
    */
   @Test
   public void testNoisyLogging() throws Throwable {
@@ -69,8 +95,9 @@ public class ITestAwsSdkWorkarounds extends AbstractS3ATestBase {
       noisyLogging();
       String output = createAndLogTransferManager(newFs);
       Assertions.assertThat(output)
-          .describedAs("LOG output")
-          .isEmpty();
+          .describedAs("LOG output does not contain the forbidden text."
+              + " Has the SDK been fixed?")
+          .contains(FORBIDDEN);
     }
   }
 

hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/sdk/TestAWSV2SDK.java

@@ -57,7 +57,7 @@ public class TestAWSV2SDK extends AbstractHadoopTestBase {
     assertThat(v2ClassPath)
             .as("AWS V2 SDK should be present on the classpath").isNotNull();
     List<String> listOfV2SdkClasses = getClassNamesFromJarFile(v2ClassPath);
-    String awsSdkPrefix = "software/amazon/";
+    String awsSdkPrefix = "software/amazon/awssdk";
     List<String> unshadedClasses = new ArrayList<>();
     for (String awsSdkClass : listOfV2SdkClasses) {
       if (!awsSdkClass.startsWith(awsSdkPrefix)) {