HDDS-7526. Avoid overwriting replication config on existing bucket when quota is set

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

@@ -343,9 +343,10 @@ public static OmBucketArgs getFromProtobuf(BucketArgs bucketArgs) {
                 bucketArgs.getOwnerName() : null);
     // OmBucketArgs ctor already has more arguments, so setting the default
     // replication config separately.
-    omBucketArgs.setDefaultReplicationConfig(
-        new DefaultReplicationConfig(bucketArgs.getDefaultReplicationConfig()));
-
+    if (bucketArgs.hasDefaultReplicationConfig()) {
+      omBucketArgs.setDefaultReplicationConfig(new DefaultReplicationConfig(
+              bucketArgs.getDefaultReplicationConfig()));
+    }
     if (bucketArgs.hasQuotaInBytes()) {
       omBucketArgs.setQuotaInBytes(bucketArgs.getQuotaInBytes());
     }

hadoop-ozone/common/src/test/java/org/apache/hadoop/ozone/om/helpers/TestOmBucketArgs.java

@@ -18,9 +18,13 @@
 
 package org.apache.hadoop.ozone.om.helpers;
 
+import org.apache.hadoop.hdds.client.DefaultReplicationConfig;
+import org.apache.hadoop.hdds.client.ECReplicationConfig;
 import org.junit.Assert;
 import org.junit.Test;
 
+import static org.apache.hadoop.hdds.client.ReplicationType.EC;
+
 /**
  * Tests for the OmBucketArgs class.
  */
@@ -58,4 +62,30 @@ public void testQuotaIsSetFlagsAreCorrectlySet() {
     Assert.assertEquals(true, argsFromProto.hasQuotaInBytes());
     Assert.assertEquals(true, argsFromProto.hasQuotaInNamespace());
   }
+
+  @Test
+  public void testDefaultReplicationConfigIsSetCorrectly() {
+    OmBucketArgs bucketArgs = OmBucketArgs.newBuilder()
+            .setBucketName("bucket")
+            .setVolumeName("volume")
+            .build();
+
+    OmBucketArgs argsFromProto = OmBucketArgs.getFromProtobuf(
+            bucketArgs.getProtobuf());
+
+    Assert.assertEquals(null, argsFromProto.getDefaultReplicationConfig());
+
+    bucketArgs = OmBucketArgs.newBuilder()
+            .setBucketName("bucket")
+            .setVolumeName("volume")
+            .setDefaultReplicationConfig(new DefaultReplicationConfig(
+                    EC, new ECReplicationConfig(3, 2)))
+            .build();
+
+    argsFromProto = OmBucketArgs.getFromProtobuf(
+            bucketArgs.getProtobuf());
+
+    Assert.assertEquals(EC,
+            argsFromProto.getDefaultReplicationConfig().getType());
+  }
 }

hadoop-ozone/ozone-manager/src/main/java/org/apache/hadoop/ozone/om/request/bucket/OMBucketSetPropertyRequest.java

@@ -202,6 +202,10 @@ public OMClientResponse validateAndUpdateCache(OzoneManager ozoneManager,
       if (defaultReplicationConfig != null) {
         // Resetting the default replication config.
         bucketInfoBuilder.setDefaultReplicationConfig(defaultReplicationConfig);
+      } else if (dbBucketInfo.getDefaultReplicationConfig() != null) {
+        // Retaining existing default replication config
+        bucketInfoBuilder.setDefaultReplicationConfig(
+                  dbBucketInfo.getDefaultReplicationConfig());
       }
 
       bucketInfoBuilder.setCreationTime(dbBucketInfo.getCreationTime());

hadoop-ozone/ozone-manager/src/test/java/org/apache/hadoop/ozone/om/request/bucket/TestOMBucketSetPropertyRequest.java

@@ -444,4 +444,92 @@ public void testValidateAndUpdateCacheWithQuotaNamespaceUsed()
         contains("Cannot update bucket quota. NamespaceQuota requested " +
             "is less than used namespaceQuota"));
   }
+
+  @Test
+  public void testSettingQuotaRetainsReplication() throws Exception {
+    String volumeName1 = UUID.randomUUID().toString();
+    String bucketName1 = UUID.randomUUID().toString();
+    String volumeName2 = UUID.randomUUID().toString();
+    String bucketName2 = UUID.randomUUID().toString();
+
+    /* Bucket with default replication */
+    OMRequestTestUtils.addVolumeAndBucketToDB(
+            volumeName1, bucketName1, omMetadataManager);
+
+    String bucketKey = omMetadataManager
+            .getBucketKey(volumeName1, bucketName1);
+
+    OmBucketInfo dbBucketInfoBefore =
+            omMetadataManager.getBucketTable().get(bucketKey);
+
+    /* Setting quota on a bucket with default replication */
+    OMRequest omRequest = createSetBucketPropertyRequest(volumeName1,
+            bucketName1, true, 20 * GB);
+
+    OMBucketSetPropertyRequest omBucketSetPropertyRequest =
+            new OMBucketSetPropertyRequest(omRequest);
+
+    OMClientResponse omClientResponse = omBucketSetPropertyRequest
+            .validateAndUpdateCache(ozoneManager, 1,
+                    ozoneManagerDoubleBufferHelper);
+
+    Assert.assertEquals(true, omClientResponse.getOMResponse().getSuccess());
+
+    OmBucketInfo dbBucketInfoAfter =
+            omMetadataManager.getBucketTable().get(bucketKey);
+
+    Assert.assertEquals(null,
+            dbBucketInfoAfter.getDefaultReplicationConfig());
+    Assert.assertEquals(
+            dbBucketInfoBefore.getDefaultReplicationConfig(),
+            dbBucketInfoAfter.getDefaultReplicationConfig());
+    Assert.assertEquals(20 * GB,
+            dbBucketInfoAfter.getQuotaInBytes());
+    Assert.assertEquals(1000L,
+            dbBucketInfoAfter.getQuotaInNamespace());
+
+    /* Bucket with EC replication */
+    OmBucketInfo.Builder bucketInfo = new OmBucketInfo.Builder()
+            .setVolumeName(volumeName2)
+            .setBucketName(bucketName2)
+            .setDefaultReplicationConfig(new DefaultReplicationConfig(
+                    EC, new ECReplicationConfig(3, 2)));
+
+    OMRequestTestUtils.addVolumeToDB(volumeName2, omMetadataManager);
+    OMRequestTestUtils.addBucketToDB(omMetadataManager, bucketInfo);
+
+    bucketKey = omMetadataManager
+            .getBucketKey(volumeName2, bucketName2);
+    dbBucketInfoBefore =
+            omMetadataManager.getBucketTable().get(bucketKey);
+
+    /* Setting quota on a bucket with non-default EC replication */
+    omRequest = createSetBucketPropertyRequest(volumeName2,
+            bucketName2, true, 20 * GB);
+
+    omBucketSetPropertyRequest =
+            new OMBucketSetPropertyRequest(omRequest);
+
+    omClientResponse = omBucketSetPropertyRequest
+            .validateAndUpdateCache(ozoneManager, 1,
+                    ozoneManagerDoubleBufferHelper);
+
+    Assert.assertEquals(true, omClientResponse.getOMResponse().getSuccess());
+
+    dbBucketInfoAfter =
+            omMetadataManager.getBucketTable().get(bucketKey);
+
+    Assert.assertEquals(EC,
+            dbBucketInfoAfter.getDefaultReplicationConfig().getType());
+    Assert.assertEquals(
+            dbBucketInfoBefore.getDefaultReplicationConfig().getType(),
+            dbBucketInfoAfter.getDefaultReplicationConfig().getType());
+    Assert.assertEquals(
+            dbBucketInfoBefore.getDefaultReplicationConfig().getFactor(),
+            dbBucketInfoAfter.getDefaultReplicationConfig().getFactor());
+    Assert.assertEquals(20 * GB,
+            dbBucketInfoAfter.getQuotaInBytes());
+    Assert.assertEquals(1000L,
+            dbBucketInfoAfter.getQuotaInNamespace());
+  }
 }