HDDS-6561. Modify default bucket layout to LEGACY.

hadoop-hdds/common/src/main/java/org/apache/hadoop/ozone/OzoneConfigKeys.java

@@ -443,12 +443,6 @@ public final class OzoneConfigKeys {
   public static final boolean OZONE_CLIENT_KEY_LATEST_VERSION_LOCATION_DEFAULT =
       true;
 
-  public static final String OZONE_CLIENT_TEST_OFS_DEFAULT_BUCKET_LAYOUT =
-      "ozone.client.test.ofs.default.bucket.layout";
-
-  public static final String OZONE_CLIENT_TEST_OFS_BUCKET_LAYOUT_DEFAULT =
-      "FILE_SYSTEM_OPTIMIZED";
-
   public static final String OZONE_CLIENT_REQUIRED_OM_VERSION_MIN_KEY =
       "ozone.client.required.om.version.min";
 

hadoop-hdds/common/src/main/resources/ozone-default.xml

@@ -3043,7 +3043,7 @@
 
   <property>
     <name>ozone.default.bucket.layout</name>
-    <value>OBJECT_STORE</value>
+    <value/>
     <tag>OZONE, MANAGEMENT</tag>
     <description>
       Default bucket layout used by Ozone Manager during bucket creation when a client does not specify the
@@ -3053,21 +3053,6 @@
       FILE_SYSTEM_OPTIMIZED: This layout allows the bucket to support atomic rename/delete operations and
       also allows interoperability between S3 and FS APIs. Keys written via S3 API with a "/" delimiter
       will create intermediate directories.
-      For testing purposes LEGACY layout is also allowed. LEGACY layout is not recommended and will be removed in the
-      future.
-    </description>
-  </property>
-
-  <property>
-    <name>ozone.client.test.ofs.default.bucket.layout</name>
-    <value>FILE_SYSTEM_OPTIMIZED</value>
-    <tag>OZONE, CLIENT</tag>
-    <description>
-      This configuration is used for testing purposes. Default bucket layout value when buckets are created using OFS.
-      Supported values are LEGACY and FILE_SYSTEM_OPTIMIZED.
-      FILE_SYSTEM_OPTIMIZED: This layout allows the bucket to support atomic rename/delete operations and
-      also allows interoperability between S3 and FS APIs. Keys written via S3 API with a "/" delimiter
-      will create intermediate directories.
     </description>
   </property>
 </configuration>

hadoop-hdds/docs/content/feature/PrefixFSO.md

@@ -68,12 +68,12 @@ The following configuration can be configured in `ozone-site.xml` to define the
 if the client has not specified the bucket layout argument.
 Supported values are `OBJECT_STORE` and `FILE_SYSTEM_OPTIMIZED`.
 
-By default, the buckets will default to `OBJECT_STORE` behaviour.
+By default, this config value is empty. Ozone will default to `LEGACY` bucket layout if it finds an empty config value.
 
 ```XML
 
 <property>
     <name>ozone.default.bucket.layout</name>
-    <value>OBJECT_STORE</value>
+    <value/>
 </property>
 ```

hadoop-ozone/common/src/main/java/org/apache/hadoop/ozone/om/OMConfigKeys.java

@@ -280,9 +280,11 @@ private OMConfigKeys() {
   public static final String OZONE_DEFAULT_BUCKET_LAYOUT =
       "ozone.default.bucket.layout";
   public static final String OZONE_DEFAULT_BUCKET_LAYOUT_DEFAULT =
-      BucketLayout.OBJECT_STORE.name();
+      BucketLayout.LEGACY.name();
   public static final String OZONE_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED =
       BucketLayout.FILE_SYSTEM_OPTIMIZED.name();
+  public static final String OZONE_BUCKET_LAYOUT_OBJECT_STORE =
+      BucketLayout.OBJECT_STORE.name();
 
   /**
    * Configuration properties for Directory Deleting Service.

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystem.java

@@ -199,9 +199,6 @@ public static void initClusterAndEnv() throws IOException,
           bucketLayout.name());
     } else {
       bucketLayout = BucketLayout.LEGACY;
-      // We need the OFS buckets to be in LEGACY layout for this test.
-      conf.set(OzoneConfigKeys.OZONE_CLIENT_TEST_OFS_DEFAULT_BUCKET_LAYOUT,
-          BucketLayout.LEGACY.name());
       conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT,
           bucketLayout.name());
       conf.setBoolean(OMConfigKeys.OZONE_OM_ENABLE_FILESYSTEM_PATHS,
@@ -520,22 +517,29 @@ public void testListStatusInBucket() throws Exception {
     Path dir1 = new Path(root, "dir1");
     Path dir12 = new Path(dir1, "dir12");
     Path dir2 = new Path(root, "dir2");
-    fs.mkdirs(dir12);
-    fs.mkdirs(dir2);
-
-    // ListStatus on root should return dir1 (even though /dir1 key does not
-    // exist) and dir2 only. dir12 is not an immediate child of root and
-    // hence should not be listed.
-    FileStatus[] fileStatuses = ofs.listStatus(root);
-    Assert.assertEquals(
-        "FileStatus should return only the immediate children",
-        2, fileStatuses.length);
-
-    // Verify that dir12 is not included in the result of the listStatus on root
-    String fileStatus1 = fileStatuses[0].getPath().toUri().getPath();
-    String fileStatus2 = fileStatuses[1].getPath().toUri().getPath();
-    Assert.assertNotEquals(fileStatus1, dir12.toString());
-    Assert.assertNotEquals(fileStatus2, dir12.toString());
+    try {
+      fs.mkdirs(dir12);
+      fs.mkdirs(dir2);
+
+      // ListStatus on root should return dir1 (even though /dir1 key does not
+      // exist) and dir2 only. dir12 is not an immediate child of root and
+      // hence should not be listed.
+      FileStatus[] fileStatuses = ofs.listStatus(root);
+      Assert.assertEquals(
+          "FileStatus should return only the immediate children",
+          2, fileStatuses.length);
+
+      // Verify that dir12 is not included in the result of the listStatus on
+      // root
+      String fileStatus1 = fileStatuses[0].getPath().toUri().getPath();
+      String fileStatus2 = fileStatuses[1].getPath().toUri().getPath();
+      Assert.assertNotEquals(fileStatus1, dir12.toString());
+      Assert.assertNotEquals(fileStatus2, dir12.toString());
+    } finally {
+      // cleanup
+      fs.delete(dir1, true);
+      fs.delete(dir2, true);
+    }
   }
 
   /**
@@ -546,25 +550,27 @@ public void testListStatusOnLargeDirectory() throws Exception {
     Path root = new Path("/" + volumeName + "/" + bucketName);
     Set<String> paths = new TreeSet<>();
     int numDirs = LISTING_PAGE_SIZE + LISTING_PAGE_SIZE / 2;
-    for (int i = 0; i < numDirs; i++) {
-      Path p = new Path(root, String.valueOf(i));
-      fs.mkdirs(p);
-      paths.add(p.getName());
-    }
-
-    FileStatus[] fileStatuses = ofs.listStatus(root);
-    Assert.assertEquals(
-        "Total directories listed do not match the existing directories",
-        numDirs, fileStatuses.length);
+    try {
+      for (int i = 0; i < numDirs; i++) {
+        Path p = new Path(root, String.valueOf(i));
+        fs.mkdirs(p);
+        paths.add(p.getName());
+      }
 
-    for (int i = 0; i < numDirs; i++) {
-      Assert.assertTrue(paths.contains(fileStatuses[i].getPath().getName()));
-    }
+      FileStatus[] fileStatuses = ofs.listStatus(root);
+      Assert.assertEquals(
+          "Total directories listed do not match the existing directories",
+          numDirs, fileStatuses.length);
 
-    // Cleanup
-    for (int i = 0; i < numDirs; i++) {
-      Path p = new Path(root, String.valueOf(i));
-      fs.delete(p, true);
+      for (int i = 0; i < numDirs; i++) {
+        Assert.assertTrue(paths.contains(fileStatuses[i].getPath().getName()));
+      }
+    } finally {
+      // Cleanup
+      for (int i = 0; i < numDirs; i++) {
+        Path p = new Path(root, String.valueOf(i));
+        fs.delete(p, true);
+      }
     }
   }
 
@@ -1445,16 +1451,21 @@ private void checkInvalidPath(Path path) throws Exception {
   public void testRenameFile() throws Exception {
     final String dir = "/dir" + new Random().nextInt(1000);
     Path dirPath = new Path(getBucketPath() + dir);
-    getFs().mkdirs(dirPath);
-
     Path file1Source = new Path(getBucketPath() + dir
         + "/file1_Copy");
-    ContractTestUtils.touch(getFs(), file1Source);
     Path file1Destin = new Path(getBucketPath() + dir + "/file1");
-    assertTrue("Renamed failed", getFs().rename(file1Source, file1Destin));
-    assertTrue("Renamed failed: /dir/file1", getFs().exists(file1Destin));
-    FileStatus[] fStatus = getFs().listStatus(dirPath);
-    assertEquals("Renamed failed", 1, fStatus.length);
+    try {
+      getFs().mkdirs(dirPath);
+
+      ContractTestUtils.touch(getFs(), file1Source);
+      assertTrue("Renamed failed", getFs().rename(file1Source, file1Destin));
+      assertTrue("Renamed failed: /dir/file1", getFs().exists(file1Destin));
+      FileStatus[] fStatus = getFs().listStatus(dirPath);
+      assertEquals("Renamed failed", 1, fStatus.length);
+    } finally {
+      // clean up
+      fs.delete(dirPath, true);
+    }
   }
 
 
@@ -1492,25 +1503,32 @@ public void testRenameToParentDir() throws Exception {
     final String dir1 = root + "/dir1";
     final String dir2 = dir1 + "/dir2";
     final Path dir2SourcePath = new Path(getBucketPath() + dir2);
-    getFs().mkdirs(dir2SourcePath);
     final Path destRootPath = new Path(getBucketPath() + root);
-
     Path file1Source = new Path(getBucketPath() + dir1 + "/file2");
-    ContractTestUtils.touch(getFs(), file1Source);
-
-    // rename source directory to its parent directory(destination).
-    assertTrue("Rename failed", getFs().rename(dir2SourcePath, destRootPath));
-    final Path expectedPathAfterRename =
-        new Path(getBucketPath() + root + "/dir2");
-    assertTrue("Rename failed",
-        getFs().exists(expectedPathAfterRename));
-
-    // rename source file to its parent directory(destination).
-    assertTrue("Rename failed", getFs().rename(file1Source, destRootPath));
-    final Path expectedFilePathAfterRename =
-        new Path(getBucketPath() + root + "/file2");
-    assertTrue("Rename failed",
-        getFs().exists(expectedFilePathAfterRename));
+    try {
+      getFs().mkdirs(dir2SourcePath);
+
+      ContractTestUtils.touch(getFs(), file1Source);
+
+      // rename source directory to its parent directory(destination).
+      assertTrue("Rename failed", getFs().rename(dir2SourcePath, destRootPath));
+      final Path expectedPathAfterRename =
+          new Path(getBucketPath() + root + "/dir2");
+      assertTrue("Rename failed",
+          getFs().exists(expectedPathAfterRename));
+
+      // rename source file to its parent directory(destination).
+      assertTrue("Rename failed", getFs().rename(file1Source, destRootPath));
+      final Path expectedFilePathAfterRename =
+          new Path(getBucketPath() + root + "/file2");
+      assertTrue("Rename failed",
+          getFs().exists(expectedFilePathAfterRename));
+    } finally {
+      // clean up
+      fs.delete(file1Source, true);
+      fs.delete(dir2SourcePath, true);
+      fs.delete(destRootPath, true);
+    }
   }
 
   /**
@@ -1535,6 +1553,9 @@ public void testRenameDirToItsOwnSubDir() throws Exception {
           " its own subdirectory");
     } catch (IllegalArgumentException e) {
       //expected
+    } finally {
+      // clean up
+      fs.delete(sourceRoot, true);
     }
   }
 

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/fs/ozone/TestRootedOzoneFileSystemWithFSO.java

@@ -166,13 +166,16 @@ public void testRenameDirToItsOwnSubDir() throws Exception {
     final Path dir1Path = new Path(getBucketPath() + dir1);
     // Add a sub-dir1 to the directory to be moved.
     final Path subDir1 = new Path(dir1Path, "sub_dir1");
-    getFs().mkdirs(subDir1);
-    LOG.info("Created dir1 {}", subDir1);
-
     final Path sourceRoot = new Path(getBucketPath() + root);
-    LOG.info("Rename op-> source:{} to destin:{}", sourceRoot, subDir1);
-    //  rename should fail and return false
-    Assert.assertFalse(getFs().rename(sourceRoot, subDir1));
+    try {
+      getFs().mkdirs(subDir1);
+      LOG.info("Created dir1 {}", subDir1);
+      LOG.info("Rename op-> source:{} to destin:{}", sourceRoot, subDir1);
+      //  rename should fail and return false
+      Assert.assertFalse(getFs().rename(sourceRoot, subDir1));
+    } finally {
+      getFs().delete(sourceRoot, true);
+    }
   }
 
   @Override

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/om/TestObjectStore.java

@@ -84,17 +84,17 @@ public void testCreateBucketWithBucketLayout() throws Exception {
     store.createVolume(sampleVolumeName);
     OzoneVolume volume = store.getVolume(sampleVolumeName);
 
-    // Case 1: Bucket layout: Empty and OM default bucket layout: OBJECT_STORE
+    // Case 1: Bucket layout: Empty and OM default bucket layout: LEGACY
     BucketArgs.Builder builder = BucketArgs.newBuilder();
     volume.createBucket(sampleBucketName, builder.build());
     OzoneBucket bucket = volume.getBucket(sampleBucketName);
     Assert.assertEquals(sampleBucketName, bucket.getName());
-    Assert.assertEquals(BucketLayout.OBJECT_STORE,
+    Assert.assertEquals(BucketLayout.LEGACY,
         bucket.getBucketLayout());
 
-    // Case 2: Bucket layout: DEFAULT
+    // Case 2: Bucket layout: OBJECT_STORE
     sampleBucketName = UUID.randomUUID().toString();
-    builder.setBucketLayout(BucketLayout.DEFAULT);
+    builder.setBucketLayout(BucketLayout.OBJECT_STORE);
     volume.createBucket(sampleBucketName, builder.build());
     bucket = volume.getBucket(sampleBucketName);
     Assert.assertEquals(sampleBucketName, bucket.getName());
@@ -107,7 +107,16 @@ public void testCreateBucketWithBucketLayout() throws Exception {
     volume.createBucket(sampleBucketName, builder.build());
     bucket = volume.getBucket(sampleBucketName);
     Assert.assertEquals(sampleBucketName, bucket.getName());
-    Assert.assertNotEquals(BucketLayout.LEGACY, bucket.getBucketLayout());
+    Assert.assertEquals(BucketLayout.LEGACY, bucket.getBucketLayout());
+
+    // Case 3: Bucket layout: FILE_SYSTEM_OPTIMIZED
+    sampleBucketName = UUID.randomUUID().toString();
+    builder.setBucketLayout(BucketLayout.FILE_SYSTEM_OPTIMIZED);
+    volume.createBucket(sampleBucketName, builder.build());
+    bucket = volume.getBucket(sampleBucketName);
+    Assert.assertEquals(sampleBucketName, bucket.getName());
+    Assert.assertEquals(BucketLayout.FILE_SYSTEM_OPTIMIZED,
+        bucket.getBucketLayout());
   }
 
   /**

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/recon/TestReconWithOzoneManagerFSO.java

@@ -32,7 +32,6 @@
 import org.apache.hadoop.ozone.container.ContainerTestHelper;
 import org.apache.hadoop.ozone.container.TestHelper;
 import org.apache.hadoop.ozone.om.OMConfigKeys;
-import org.apache.hadoop.ozone.om.helpers.BucketLayout;
 import org.apache.hadoop.ozone.recon.api.NSSummaryEndpoint;
 import org.apache.hadoop.ozone.recon.api.types.NamespaceSummaryResponse;
 import org.apache.hadoop.ozone.recon.api.types.EntityType;
@@ -67,7 +66,7 @@ public class TestReconWithOzoneManagerFSO {
   public static void init() throws Exception {
     conf = new OzoneConfiguration();
     conf.set(OMConfigKeys.OZONE_DEFAULT_BUCKET_LAYOUT,
-        BucketLayout.FILE_SYSTEM_OPTIMIZED.name());
+        OMConfigKeys.OZONE_BUCKET_LAYOUT_FILE_SYSTEM_OPTIMIZED);
     cluster =
             MiniOzoneCluster.newBuilder(conf)
                     .setNumDatanodes(1)

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java

@@ -963,20 +963,4 @@ public void testListVolumeBucketKeyShouldPrintValidJsonArray()
     testVolumes.forEach(vol -> execute(ozoneShell, new String[] {
         "volume", "delete", vol}));
   }
-
-  @Test
-  public void testClientBucketLayoutValidation() throws Exception {
-    String[] args = new String[]{
-        "bucket", "create", "o3://" + omServiceId + "/volume7" + "/bucketTest1",
-        "--layout", "LEGACY"
-    };
-    try {
-      execute(ozoneShell, args);
-      Assert.fail("Should throw exception on unsupported bucket layouts!");
-    } catch (Exception e) {
-      GenericTestUtils.assertExceptionContains(
-          "expected one of [FILE_SYSTEM_OPTIMIZED, OBJECT_STORE] ",
-          e);
-    }
-  }
 }