HDDS-2413. Set configuration variables from annotated java objects

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/XceiverClientManager.java

@@ -29,7 +29,6 @@
 import org.apache.hadoop.hdds.conf.ConfigGroup;
 import org.apache.hadoop.hdds.conf.ConfigType;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.scm.pipeline.Pipeline;
 import org.apache.hadoop.hdds.protocol.proto.HddsProtos;
 import org.apache.hadoop.hdds.security.exception.SCMSecurityException;
@@ -86,8 +85,7 @@ public class XceiverClientManager implements Closeable {
    * @param conf configuration
    */
   public XceiverClientManager(ConfigurationSource conf) throws IOException {
-    this(conf, OzoneConfiguration.of(conf).getObject(ScmClientConfig.class),
-        null);
+    this(conf, conf.getObject(ScmClientConfig.class), null);
   }
 
   public XceiverClientManager(ConfigurationSource conf,

hadoop-hdds/client/src/main/java/org/apache/hadoop/hdds/scm/client/HddsClientUtils.java

@@ -32,7 +32,6 @@
 import org.apache.hadoop.hdds.annotation.InterfaceAudience;
 import org.apache.hadoop.hdds.annotation.InterfaceStability;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.ratis.conf.RatisClientConfig;
 import org.apache.hadoop.hdds.scm.container.common.helpers.StorageContainerException;
 import org.apache.hadoop.io.retry.RetryPolicies;
@@ -250,7 +249,7 @@ public static String getS3VolumeName(ConfigurationSource conf) {
    * Standalone and Ratis client.
    */
   public static int getMaxOutstandingRequests(ConfigurationSource config) {
-    return OzoneConfiguration.of(config)
+    return config
         .getObject(RatisClientConfig.RaftConfig.class)
         .getMaxOutstandingRequests();
   }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/conf/OzoneConfiguration.java

@@ -47,7 +47,7 @@
  */
 @InterfaceAudience.Private
 public class OzoneConfiguration extends Configuration
-    implements ConfigurationSource {
+    implements MutableConfigurationSource {
   static {
     activate();
   }
@@ -72,6 +72,25 @@ public static OzoneConfiguration of(Configuration conf) {
         : new OzoneConfiguration(conf);
   }
 
+  /**
+   * @return a new config object of type {@code T} configured with defaults
+   * and any overrides from XML
+   */
+  public static <T> T newInstanceOf(Class<T> configurationClass) {
+    OzoneConfiguration conf = new OzoneConfiguration();
+    return conf.getObject(configurationClass);
+  }
+
+  /**
+   * @return a new {@code OzoneConfiguration} instance set from the given
+   * {@code configObject}
+   */
+  public static <T> OzoneConfiguration fromObject(T configObject) {
+    OzoneConfiguration conf = new OzoneConfiguration();
+    conf.setFromObject(configObject);
+    return conf;
+  }
+
   public OzoneConfiguration() {
     OzoneConfiguration.activate();
     loadDefaults();

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/DUFactory.java

@@ -25,7 +25,6 @@
 import org.apache.hadoop.hdds.conf.ConfigTag;
 import org.apache.hadoop.hdds.conf.ConfigType;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 
 /**
  * Uses DU for all volumes.  Saves used value in cache file.
@@ -35,14 +34,12 @@ public class DUFactory implements SpaceUsageCheckFactory {
   private static final String DU_CACHE_FILE = "scmUsed";
   private static final String EXCLUDE_PATTERN = "*.tmp.*";
 
-  private static final String CONFIG_PREFIX = "hdds.datanode.du";
-
   private Conf conf;
 
   @Override
   public SpaceUsageCheckFactory setConfiguration(
       ConfigurationSource configuration) {
-    conf = OzoneConfiguration.of(configuration).getObject(Conf.class);
+    conf = configuration.getObject(Conf.class);
     return this;
   }
 
@@ -61,13 +58,11 @@ public SpaceUsageCheckParams paramsFor(File dir) {
   /**
    * Configuration for {@link DUFactory}.
    */
-  @ConfigGroup(prefix = CONFIG_PREFIX)
+  @ConfigGroup(prefix = "hdds.datanode.du")
   public static class Conf {
 
-    private static final String REFRESH_PERIOD = "refresh.period";
-
     @Config(
-        key = REFRESH_PERIOD,
+        key = "refresh.period",
         defaultValue = "1h",
         type = ConfigType.TIME,
         tags = { ConfigTag.DATANODE },
@@ -76,16 +71,12 @@ public static class Conf {
     )
     private long refreshPeriod;
 
-    public void setRefreshPeriod(long millis) {
-      refreshPeriod = millis;
+    public void setRefreshPeriod(Duration duration) {
+      refreshPeriod = duration.toMillis();
     }
 
     public Duration getRefreshPeriod() {
       return Duration.ofMillis(refreshPeriod);
     }
-
-    static String configKeyForRefreshPeriod() {
-      return CONFIG_PREFIX + "." + REFRESH_PERIOD;
-    }
   }
 }

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/DedicatedDiskSpaceUsageFactory.java

@@ -25,7 +25,6 @@
 import org.apache.hadoop.hdds.conf.ConfigTag;
 import org.apache.hadoop.hdds.conf.ConfigType;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 
 /**
  * Uses DedicatedDiskSpaceUsage for all volumes.  Does not save results since
@@ -40,7 +39,7 @@ public class DedicatedDiskSpaceUsageFactory implements SpaceUsageCheckFactory {
   @Override
   public SpaceUsageCheckFactory setConfiguration(
       ConfigurationSource configuration) {
-    conf = OzoneConfiguration.of(configuration).getObject(Conf.class);
+    conf = configuration.getObject(Conf.class);
     return this;
   }
 

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/fs/SpaceUsageCheckFactory.java

@@ -28,7 +28,6 @@
 import org.apache.hadoop.hdds.conf.ConfigGroup;
 import org.apache.hadoop.hdds.conf.ConfigTag;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -70,7 +69,7 @@ default SpaceUsageCheckFactory setConfiguration(ConfigurationSource conf) {
    * instantiated.
    */
   static SpaceUsageCheckFactory create(ConfigurationSource config) {
-    Conf conf = OzoneConfiguration.of(config).getObject(Conf.class);
+    Conf conf = config.getObject(Conf.class);
     Class<? extends SpaceUsageCheckFactory> aClass = null;
     String className = conf.getClassName();
     if (className != null && !className.isEmpty()) {

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/ratis/RatisHelper.java

@@ -30,7 +30,6 @@
 
 import org.apache.hadoop.hdds.StringUtils;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.protocol.DatanodeDetails;
 import org.apache.hadoop.hdds.ratis.conf.RatisClientConfig;
 import org.apache.hadoop.hdds.scm.ScmConfigKeys;
@@ -314,7 +313,7 @@ public static GrpcTlsConfig createTlsClientConfig(SecurityConfig conf,
    * ---------------------------------------------------------------------------
    */
   public static RetryPolicy createRetryPolicy(ConfigurationSource conf) {
-    RatisClientConfig ratisClientConfig = OzoneConfiguration.of(conf)
+    RatisClientConfig ratisClientConfig = conf
         .getObject(RatisClientConfig.class);
     ExponentialBackoffRetry exponentialBackoffRetry =
         createExponentialBackoffPolicy(ratisClientConfig);

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/utils/LegacyHadoopConfigurationSource.java

@@ -22,11 +22,13 @@
 
 import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hdds.conf.ConfigurationSource;
+import org.apache.hadoop.hdds.conf.MutableConfigurationSource;
 
 /**
  * Configuration source to wrap Hadoop Configuration object.
  */
-public class LegacyHadoopConfigurationSource implements ConfigurationSource {
+public class LegacyHadoopConfigurationSource
+    implements MutableConfigurationSource {
 
   private Configuration configuration;
 

hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/SimpleConfiguration.java

@@ -45,7 +45,7 @@ public class SimpleConfiguration extends SimpleConfigurationParent {
   @Config(key = "wait", type = ConfigType.TIME, timeUnit =
       TimeUnit.SECONDS, defaultValue = "30m", description = "Wait time (To "
       + "test TIME config type)", tags = ConfigTag.MANAGEMENT)
-  private long waitTime = 1;
+  private long waitTime;
 
   @PostConstruct
   public void validate() {

hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/SimpleConfigurationParent.java

@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -29,4 +29,8 @@ public class SimpleConfigurationParent {
   public boolean isEnabled() {
     return enabled;
   }
+
+  public void setEnabled(boolean enabled) {
+    this.enabled = enabled;
+  }
 }

hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/conf/TestOzoneConfiguration.java

@@ -1,4 +1,4 @@
-/**
+/*
  * Licensed to the Apache Software Foundation (ASF) under one
  * or more contributor license agreements.  See the NOTICE file
  * distributed with this work for additional information
@@ -74,8 +74,8 @@ public void testGetAllPropertiesByTags() throws Exception {
 
       Path fileResource = new Path(coreDefault.getAbsolutePath());
       conf.addResource(fileResource);
-      Assert.assertEquals(conf.getAllPropertiesByTag("MYCUSTOMTAG")
-          .getProperty("dfs.random.key"), "XYZ");
+      Assert.assertEquals("XYZ", conf.getAllPropertiesByTag("MYCUSTOMTAG")
+          .getProperty("dfs.random.key"));
     }
 
     try (BufferedWriter out = new BufferedWriter(new FileWriter(coreSite))) {
@@ -112,7 +112,7 @@ public void getConfigurationObject() {
 
     Assert.assertEquals("host", configuration.getBindHost());
     Assert.assertEquals("address", configuration.getClientAddress());
-    Assert.assertEquals(true, configuration.isEnabled());
+    Assert.assertTrue(configuration.isEnabled());
     Assert.assertEquals(5555, configuration.getPort());
     Assert.assertEquals(600, configuration.getWaitTime());
   }
@@ -124,17 +124,88 @@ public void getConfigurationObjectWithDefault() {
     SimpleConfiguration configuration =
         ozoneConfiguration.getObject(SimpleConfiguration.class);
 
-    Assert.assertEquals(true, configuration.isEnabled());
+    Assert.assertTrue(configuration.isEnabled());
     Assert.assertEquals(9878, configuration.getPort());
   }
 
+  @Test
+  public void setConfigFromObject() {
+    // GIVEN
+    SimpleConfiguration object = new SimpleConfiguration();
+    object.setBindHost("host");
+    object.setClientAddress("address");
+    object.setEnabled(true);
+    object.setPort(5555);
+    object.setWaitTime(600);
+
+    OzoneConfiguration subject = new OzoneConfiguration();
+
+    // WHEN
+    subject.setFromObject(object);
+
+    // THEN
+    Assert.assertEquals(object.getBindHost(),
+        subject.get("test.scm.client.bind.host"));
+    Assert.assertEquals(object.getClientAddress(),
+        subject.get("test.scm.client.address"));
+    Assert.assertEquals(object.isEnabled(),
+        subject.getBoolean("test.scm.client.enabled", false));
+    Assert.assertEquals(object.getPort(),
+        subject.getInt("test.scm.client.port", 0));
+    Assert.assertEquals(TimeUnit.SECONDS.toMinutes(object.getWaitTime()),
+        subject.getTimeDuration("test.scm.client.wait", 0, TimeUnit.MINUTES));
+  }
+
+  @Test
+  public void setConfigFromObjectWithConfigDefaults() {
+    // GIVEN
+    OzoneConfiguration subject = new OzoneConfiguration();
+    SimpleConfiguration object = subject.getObject(SimpleConfiguration.class);
+
+    // WHEN
+    subject.setFromObject(object);
+
+    // THEN
+    Assert.assertEquals("0.0.0.0",
+        subject.get("test.scm.client.bind.host"));
+    Assert.assertEquals("localhost",
+        subject.get("test.scm.client.address"));
+    Assert.assertTrue(
+        subject.getBoolean("test.scm.client.enabled", false));
+    Assert.assertEquals(9878,
+        subject.getInt("test.scm.client.port", 123));
+    Assert.assertEquals(TimeUnit.MINUTES.toSeconds(30),
+        subject.getTimeDuration("test.scm.client.wait", 555, TimeUnit.SECONDS));
+  }
+
+  @Test
+  public void setConfigFromObjectWithObjectDefaults() {
+    // GIVEN
+    SimpleConfiguration object = new SimpleConfiguration();
+    OzoneConfiguration subject = new OzoneConfiguration();
+
+    // WHEN
+    subject.setFromObject(object);
+
+    // THEN
+    Assert.assertEquals("0.0.0.0",
+        subject.get("test.scm.client.bind.host"));
+    Assert.assertEquals("localhost",
+        subject.get("test.scm.client.address"));
+    Assert.assertFalse(
+        subject.getBoolean("test.scm.client.enabled", false));
+    Assert.assertEquals(0,
+        subject.getInt("test.scm.client.port", 123));
+    Assert.assertEquals(0,
+        subject.getTimeDuration("test.scm.client.wait", 555, TimeUnit.SECONDS));
+  }
+
   @Test(expected = NumberFormatException.class)
   public void postConstructValidation() {
     OzoneConfiguration ozoneConfiguration = new OzoneConfiguration();
     ozoneConfiguration.setInt("test.scm.client.port", -3);
 
-    SimpleConfiguration configuration =
-        ozoneConfiguration.getObject(SimpleConfiguration.class);
+    ozoneConfiguration.getObject(SimpleConfiguration.class);
   }
 
 

hadoop-hdds/common/src/test/java/org/apache/hadoop/hdds/fs/TestDUFactory.java

@@ -21,10 +21,9 @@
 import java.time.Duration;
 
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.junit.Test;
 
-import static org.apache.hadoop.hdds.fs.DUFactory.Conf.configKeyForRefreshPeriod;
 import static org.apache.hadoop.test.GenericTestUtils.getTestDir;
-import org.junit.Test;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertSame;
 
@@ -40,16 +39,21 @@ public void testCreateViaConfig() {
 
   @Test
   public void testParams() {
-    OzoneConfiguration conf = new OzoneConfiguration();
-    conf.set(configKeyForRefreshPeriod(), "1h");
     File dir = getTestDir(getClass().getSimpleName());
+    Duration refresh = Duration.ofHours(1);
+
+    OzoneConfiguration conf = new OzoneConfiguration();
+
+    DUFactory.Conf duConf = conf.getObject(DUFactory.Conf.class);
+    duConf.setRefreshPeriod(refresh);
+    conf.setFromObject(duConf);
 
     SpaceUsageCheckParams params = new DUFactory()
         .setConfiguration(conf)
         .paramsFor(dir);
 
     assertSame(dir, params.getDir());
-    assertEquals(Duration.ofHours(1), params.getRefresh());
+    assertEquals(refresh, params.getRefresh());
     assertSame(DU.class, params.getSource().getClass());
     assertSame(SaveSpaceUsageToFile.class, params.getPersistence().getClass());
   }