HADOOP-19595. ABFS: AbfsConfiguration should store account type information (HNS or FNS)

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

@@ -95,6 +95,7 @@ public class AbfsConfiguration{
   private final AbfsServiceType fsConfiguredServiceType;
   private final boolean isSecure;
   private static final Logger LOG = LoggerFactory.getLogger(AbfsConfiguration.class);
+  private Trilean isNamespaceEnabled = null;
 
   @StringConfigurationValidatorAnnotation(ConfigurationKey = FS_AZURE_ACCOUNT_IS_HNS_ENABLED,
       DefaultValue = DEFAULT_FS_AZURE_ACCOUNT_IS_HNS_ENABLED)
@@ -525,8 +526,11 @@ public AbfsConfiguration(final Configuration rawConfig, String accountName)
    * @return TRUE/FALSE value if configured, UNKNOWN if not configured.
    */
   public Trilean getIsNamespaceEnabledAccount() {
-    return Trilean.getTrilean(
-        getString(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, isNamespaceEnabledAccount));
+    if (isNamespaceEnabled == null) {
+      isNamespaceEnabled = Trilean.getTrilean(
+          getString(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, isNamespaceEnabledAccount));
+    }
+    return isNamespaceEnabled;
   }
 
   /**
@@ -1525,8 +1529,8 @@ void setMaxBackoffIntervalMilliseconds(int maxBackoffInterval) {
   }
 
   @VisibleForTesting
-  void setIsNamespaceEnabledAccount(String isNamespaceEnabledAccount) {
-    this.isNamespaceEnabledAccount = isNamespaceEnabledAccount;
+  public void setIsNamespaceEnabledAccount(Trilean isNamespaceEnabledAccount) {
+    this.isNamespaceEnabled = isNamespaceEnabledAccount;
   }
 
   /**

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

@@ -229,6 +229,8 @@ public void initialize(URI uri, Configuration configuration)
      * HNS Account Cannot have Blob Endpoint URI.
      */
     try {
+      // This will update namespaceEnable based on getAcl in case config is not set.
+      // This Information will be stored in abfsConfiguration class.
       abfsConfiguration.validateConfiguredServiceType(
           tryGetIsNamespaceEnabled(initFSTracingContext));
     } catch (InvalidConfigurationValueException ex) {
@@ -296,7 +298,6 @@ public void initialize(URI uri, Configuration configuration)
         }
       }
     }
-    getAbfsStore().updateClientWithNamespaceInfo(new TracingContext(initFSTracingContext));
 
     LOG.trace("Initiate check for delegation token manager");
     if (UserGroupInformation.isSecurityEnabled()) {

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

@@ -186,7 +186,6 @@ public class AzureBlobFileSystemStore implements Closeable, ListingSupport {
 
   private final AbfsConfiguration abfsConfiguration;
   private Set<String> azureInfiniteLeaseDirSet;
-  private volatile Trilean isNamespaceEnabled;
   private final AuthType authType;
   private final UserGroupInformation userGroupInformation;
   private final IdentityTransformerInterface identityTransformer;
@@ -234,8 +233,6 @@ public AzureBlobFileSystemStore(
 
     LOG.trace("AbfsConfiguration init complete");
 
-    this.isNamespaceEnabled = abfsConfiguration.getIsNamespaceEnabledAccount();
-
     this.userGroupInformation = UserGroupInformation.getCurrentUser();
     this.userName = userGroupInformation.getShortUserName();
     LOG.trace("UGI init complete");
@@ -287,18 +284,6 @@ public AzureBlobFileSystemStore(
         "abfs-bounded");
   }
 
-  /**
-   * Updates the client with the namespace information.
-   *
-   * @param tracingContext the tracing context to be used for the operation
-   * @throws AzureBlobFileSystemException if an error occurs while updating the client
-   */
-  public void updateClientWithNamespaceInfo(TracingContext tracingContext)
-      throws AzureBlobFileSystemException {
-    boolean isNamespaceEnabled = getIsNamespaceEnabled(tracingContext);
-    AbfsClient.setIsNamespaceEnabled(isNamespaceEnabled);
-  }
-
   /**
    * Checks if the given key in Azure Storage should be stored as a page
    * blob instead of block blob.
@@ -409,32 +394,32 @@ public boolean getIsNamespaceEnabled(TracingContext tracingContext)
 
   private synchronized boolean getNamespaceEnabledInformationFromServer(
       final TracingContext tracingContext) throws AzureBlobFileSystemException {
-    if (isNamespaceEnabled != Trilean.UNKNOWN) {
-      return isNamespaceEnabled.toBoolean();
+    if (abfsConfiguration.getIsNamespaceEnabledAccount() != Trilean.UNKNOWN) {
+      return isNamespaceEnabled();
     }
     try {
       LOG.debug("Get root ACL status");
       getClient(AbfsServiceType.DFS).getAclStatus(AbfsHttpConstants.ROOT_PATH, tracingContext);
       // If getAcl succeeds, namespace is enabled.
-      isNamespaceEnabled = Trilean.getTrilean(true);
+      setNamespaceEnabled(Trilean.TRUE);
     } catch (AbfsRestOperationException ex) {
       // Get ACL status is a HEAD request, its response doesn't contain errorCode
       // So can only rely on its status code to determine account type.
       if (HttpURLConnection.HTTP_BAD_REQUEST != ex.getStatusCode()) {
         // If getAcl fails with anything other than 400, namespace is enabled.
-        isNamespaceEnabled = Trilean.getTrilean(true);
+        setNamespaceEnabled(Trilean.TRUE);
         // Continue to throw exception as earlier.
         LOG.debug("Failed to get ACL status with non 400. Inferring namespace enabled", ex);
         throw ex;
       }
       // If getAcl fails with 400, namespace is disabled.
       LOG.debug("Failed to get ACL status with 400. "
           + "Inferring namespace disabled and ignoring error", ex);
-      isNamespaceEnabled = Trilean.getTrilean(false);
+      setNamespaceEnabled(Trilean.FALSE);
     } catch (AzureBlobFileSystemException ex) {
       throw ex;
     }
-    return isNamespaceEnabled.toBoolean();
+    return isNamespaceEnabled();
   }
 
   /**
@@ -443,7 +428,7 @@ private synchronized boolean getNamespaceEnabledInformationFromServer(
    */
   @VisibleForTesting
   boolean isNamespaceEnabled() throws TrileanConversionException {
-    return this.isNamespaceEnabled.toBoolean();
+    return abfsConfiguration.getIsNamespaceEnabledAccount().toBoolean();
   }
 
   @VisibleForTesting
@@ -2028,7 +2013,7 @@ DataBlocks.BlockFactory getBlockFactory() {
 
   @VisibleForTesting
   void setNamespaceEnabled(Trilean isNamespaceEnabled){
-    this.isNamespaceEnabled = isNamespaceEnabled;
+    abfsConfiguration.setIsNamespaceEnabledAccount(isNamespaceEnabled);
   }
 
   @VisibleForTesting

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

@@ -69,6 +69,7 @@
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidFileSystemPropertyException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidUriException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.SASTokenProviderException;
+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.TrileanConversionException;
 import org.apache.hadoop.fs.azurebfs.contracts.services.AppendRequestParameters;
 import org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode;
 import org.apache.hadoop.fs.azurebfs.contracts.services.ListResultEntrySchema;
@@ -194,7 +195,6 @@ public abstract class AbfsClient implements Closeable {
   private KeepAliveCache keepAliveCache;
 
   private AbfsApacheHttpClient abfsApacheHttpClient;
-  private static boolean isNamespaceEnabled = false;
 
   /**
    * logging the rename failure if metadata is in an incomplete state.
@@ -1716,18 +1716,15 @@ protected String getUserAgent() {
    * Checks if the namespace is enabled.
    *
    * @return True if the namespace is enabled, false otherwise.
+   * @throws AbfsDriverException if the conversion fails.
    */
-  public static boolean getIsNamespaceEnabled() {
-    return isNamespaceEnabled;
-  }
-
-  /**
-   * Sets the namespace enabled status.
-   *
-   * @param namespaceEnabled True to enable the namespace, false to disable it.
-   */
-  public static void setIsNamespaceEnabled(final boolean namespaceEnabled) {
-    isNamespaceEnabled = namespaceEnabled;
+  public boolean getIsNamespaceEnabled() throws AbfsDriverException {
+    try {
+      return abfsConfiguration.getIsNamespaceEnabledAccount().toBoolean();
+    } catch (TrileanConversionException ex) {
+      LOG.error("Failed to convert namespace enabled account property to boolean", ex);
+      throw new AbfsDriverException("Failed to determine if namespace is enabled", ex);
+    }
   }
 
   protected boolean isRenameResilience() {

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

@@ -1563,9 +1563,11 @@ private Hashtable<String, String> parseCommaSeparatedXmsProperties(String xMsPro
    * @param requestHeaders list of headers to be sent with the request
    *
    * @return client transaction id
+   * @throws AzureBlobFileSystemException if an error occurs while generating the client transaction id
    */
   @VisibleForTesting
-  public String addClientTransactionIdToHeader(List<AbfsHttpHeader> requestHeaders) {
+  public String addClientTransactionIdToHeader(List<AbfsHttpHeader> requestHeaders)
+      throws AzureBlobFileSystemException {
     String clientTransactionId = null;
     // Set client transaction ID if the namespace and client transaction ID config are enabled.
     if (getIsNamespaceEnabled() && getAbfsConfiguration().getIsClientTransactionIdEnabled()) {

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

@@ -33,7 +33,6 @@
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.InvalidConfigurationValueException;
-import org.apache.hadoop.fs.azurebfs.contracts.exceptions.TrileanConversionException;
 import org.apache.hadoop.fs.azurebfs.enums.Trilean;
 import org.apache.hadoop.fs.azurebfs.services.AbfsClient;
 import org.apache.hadoop.fs.azurebfs.services.AbfsRestOperation;
@@ -83,10 +82,6 @@ public void testGetAclCallOnHnsConfigAbsence() throws Exception {
     AzureBlobFileSystemStore store = Mockito.spy(fs.getAbfsStore());
     AbfsClient client = Mockito.spy(fs.getAbfsStore().getClient(AbfsServiceType.DFS));
     Mockito.doReturn(client).when(store).getClient(AbfsServiceType.DFS);
-
-    Mockito.doThrow(TrileanConversionException.class)
-        .when(store)
-        .isNamespaceEnabled();
     store.setNamespaceEnabled(Trilean.UNKNOWN);
 
     TracingContext tracingContext = getSampleTracingContext(fs, true);

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

@@ -282,6 +282,9 @@ private void ensureGetAclDetermineHnsStatusAccuratelyInternal(int statusCode,
     boolean isHnsEnabled = store.getIsNamespaceEnabled(
         getTestTracingContext(getFileSystem(), false));
     Assertions.assertThat(isHnsEnabled).isEqualTo(expectedValue);
+    Assertions.assertThat(store.getClient().getIsNamespaceEnabled())
+        .describedAs("ABFS Client should return same isNameSpace value as store")
+        .isEqualTo(expectedValue);
 
     // GetAcl() should be called only once to determine the HNS status.
     Mockito.verify(mockClient, times(1))
@@ -341,6 +344,76 @@ public void testAccountSpecificConfig() throws Exception {
     }
   }
 
+  /**
+   * Tests the behavior of AbfsConfiguration when the namespace-enabled
+   * configuration set based on config provided.
+   *
+   * Expects the namespace value based on config provided.
+   *
+   * @throws Exception if any error occurs during configuration setup or evaluation
+   */
+  @Test
+  public void testNameSpaceConfig() throws Exception {
+    Configuration configuration = getRawConfiguration();
+    configuration.unset(FS_AZURE_ACCOUNT_IS_HNS_ENABLED);
+    configuration.unset(accountProperty(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, this.getAccountName()));
+    AzureBlobFileSystem abfs = (AzureBlobFileSystem) FileSystem.newInstance(configuration);
+    AbfsConfiguration abfsConfig = new AbfsConfiguration(configuration, "bogusAccountName");
+
+    // Test that the namespace value when config is not set
+    Assertions.assertThat(abfsConfig.getIsNamespaceEnabledAccount())
+        .describedAs("Namespace enabled should be unknown in case config is not set")
+        .isEqualTo(Trilean.UNKNOWN);
+
+    // In case no namespace config is present, file system init calls getAcl() to determine account type.
+    Assertions.assertThat(abfs.getIsNamespaceEnabled(getTestTracingContext(abfs, false)))
+        .describedAs("getIsNamespaceEnabled should return account type based on getAcl() call")
+        .isEqualTo(abfs.getAbfsClient().getIsNamespaceEnabled());
+
+    // In case no namespace config is present, file system init calls getAcl() to determine account type.
+    Assertions.assertThat(abfs.getAbfsStore().getAbfsConfiguration().getIsNamespaceEnabledAccount())
+        .describedAs("getIsNamespaceEnabled() should return updated account type based on getAcl() call")
+        .isNotEqualTo(Trilean.UNKNOWN);
+
+    configuration.set(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, TRUE_STR);
+    abfs = (AzureBlobFileSystem) FileSystem.newInstance(configuration);
+    abfsConfig = new AbfsConfiguration(configuration, "bogusAccountName");
+
+    // Test that the namespace enabled config is set correctly
+    Assertions.assertThat(abfsConfig.getIsNamespaceEnabledAccount())
+        .describedAs("Namespace enabled should be true in case config is set to true")
+        .isEqualTo(Trilean.TRUE);
+
+    // In case namespace config is present, same value will be return.
+    Assertions.assertThat(abfs.getIsNamespaceEnabled(getTestTracingContext(abfs, false)))
+        .describedAs("getIsNamespaceEnabled() should return true when config is set to true")
+        .isEqualTo(true);
+
+    // In case namespace config is present, same value will be return.
+    Assertions.assertThat(abfs.getAbfsClient().getIsNamespaceEnabled())
+        .describedAs("Client's getIsNamespaceEnabled() should return true when config is set to true")
+        .isEqualTo(true);
+
+    configuration.set(FS_AZURE_ACCOUNT_IS_HNS_ENABLED, FALSE_STR);
+    abfs = (AzureBlobFileSystem) FileSystem.newInstance(configuration);
+    abfsConfig = new AbfsConfiguration(configuration, "bogusAccountName");
+
+    // Test that the namespace enabled config is set correctly
+    Assertions.assertThat(abfsConfig.getIsNamespaceEnabledAccount())
+        .describedAs("Namespace enabled should be false in case config is set to false")
+        .isEqualTo(Trilean.FALSE);
+
+    // In case namespace config is present, same value will be return.
+    Assertions.assertThat(abfs.getIsNamespaceEnabled(getTestTracingContext(abfs, false)))
+        .describedAs("getIsNamespaceEnabled() should return false when config is set to false")
+        .isEqualTo(false);
+
+    // In case namespace config is present, same value will be return.
+    Assertions.assertThat(abfs.getAbfsClient().getIsNamespaceEnabled())
+        .describedAs("Client's getIsNamespaceEnabled() should return false when config is set to false")
+        .isEqualTo(false);
+  }
+
   private void assertFileSystemInitWithExpectedHNSSettings(
       Configuration configuration, boolean expectedIsHnsEnabledValue) throws IOException {
     try (AzureBlobFileSystem fs = (AzureBlobFileSystem) FileSystem.newInstance(configuration)) {

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

@@ -39,6 +39,7 @@
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
+import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
 import org.apache.hadoop.fs.azurebfs.utils.TracingContext;
 import org.apache.hadoop.util.functional.FunctionRaisingIOE;
 
@@ -370,7 +371,7 @@ public static void mockGetRenameBlobHandler(AbfsBlobClient blobClient,
    * @param clientTransactionId An array to hold the generated transaction ID.
    */
   public static void mockAddClientTransactionIdToHeader(AbfsDfsClient abfsDfsClient,
-      String[] clientTransactionId) {
+      String[] clientTransactionId) throws AzureBlobFileSystemException {
     Mockito.doAnswer(addClientTransactionId -> {
       clientTransactionId[0] = UUID.randomUUID().toString();
       List<AbfsHttpHeader> headers = addClientTransactionId.getArgument(0);

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

@@ -40,7 +40,6 @@
 import org.apache.hadoop.fs.azurebfs.AzureBlobFileSystemStore;
 import org.apache.hadoop.fs.azurebfs.commit.ResilientCommitByRename;
 import org.apache.hadoop.fs.azurebfs.constants.HttpHeaderConfigurations;
-import org.apache.hadoop.fs.azurebfs.constants.TestConfigurationKeys;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AbfsRestOperationException;
 import org.apache.hadoop.fs.azurebfs.contracts.exceptions.AzureBlobFileSystemException;
 import org.apache.hadoop.fs.azurebfs.contracts.services.AzureServiceErrorCode;
@@ -76,11 +75,8 @@ public class TestAbfsRenameRetryRecovery extends AbstractAbfsIntegrationTest {
   private static final Logger LOG =
       LoggerFactory.getLogger(TestAbfsRenameRetryRecovery.class);
 
-  private boolean isNamespaceEnabled;
-
   public TestAbfsRenameRetryRecovery() throws Exception {
-    isNamespaceEnabled = getConfiguration()
-            .getBoolean(TestConfigurationKeys.FS_AZURE_TEST_NAMESPACE_ENABLED_ACCOUNT, false);
+    // do nothing
   }
 
   /**
@@ -461,10 +457,10 @@ public void testExistingPathCorrectlyRejected() throws Exception {
    */
   @Test
   public void testRenameRecoveryUnsupportedForFlatNamespace() throws Exception {
-    Assume.assumeTrue(!isNamespaceEnabled);
     // In DFS endpoint, renamePath is O(1) API call and idempotency issue can happen.
     // For blob endpoint, client orchestrates the rename operation.
     assumeDfsServiceType();
+    assumeHnsDisabled();
     AzureBlobFileSystem fs = getFileSystem();
     AzureBlobFileSystemStore abfsStore = fs.getAbfsStore();
     TracingContext testTracingContext = getTestTracingContext(fs, false);