HDDS-2413. Set configuration variables from annotated java objects

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

@@ -35,8 +35,6 @@ 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
@@ -61,13 +59,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 +72,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/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());
   }

hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationReflectionUtil.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
@@ -21,6 +21,8 @@
 import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Modifier;
+import java.util.Deque;
+import java.util.LinkedList;
 
 /**
  * Reflection utilities for configuration injection.
@@ -162,4 +164,80 @@ public static <T> void callPostConstruct(Class<T> configurationClass,
       }
     }
   }
+
+  public static <T> void updateConfiguration(ConfigurationSource config,
+      T object, String prefix) {
+
+    Class<?> configClass = object.getClass();
+    Deque<Class<?>> classes = new LinkedList<>();
+    classes.addLast(configClass);
+    Class<?> superclass = configClass.getSuperclass();
+    while (superclass != null) {
+      classes.addFirst(superclass);
+      superclass = superclass.getSuperclass();
+    }
+
+    for (Class<?> cl : classes) {
+      updateConfigurationFromObject(config, cl, object, prefix);
+    }
+  }
+
+  private static <T> void updateConfigurationFromObject(
+      ConfigurationSource config, Class<?> configClass, T configObject,
+      String prefix) {
+
+    for (Field field : configClass.getDeclaredFields()) {
+      if (field.isAnnotationPresent(Config.class)) {
+        Config configAnnotation = field.getAnnotation(Config.class);
+        String fieldLocation = configClass + "." + field.getName();
+        String key = prefix + "." + configAnnotation.key();
+        ConfigType type = configAnnotation.type();
+
+        if (type == ConfigType.AUTO) {
+          type = detectConfigType(field.getType(), fieldLocation);
+        }
+
+        //Note: default value is handled by ozone-default.xml. Here we can
+        //use any default.
+        boolean accessChanged = false;
+        try {
+          if (!field.isAccessible()) {
+            field.setAccessible(true);
+            accessChanged = true;
+          }
+          switch (type) {
+          case STRING:
+            Object value = field.get(configObject);
+            if (value != null) {
+              config.set(key, String.valueOf(value));
+            }
+            break;
+          case INT:
+            config.setInt(key, field.getInt(configObject));
+            break;
+          case BOOLEAN:
+            config.setBoolean(key, field.getBoolean(configObject));
+            break;
+          case LONG:
+            config.setLong(key, field.getLong(configObject));
+            break;
+          case TIME:
+            config.setTimeDuration(key, field.getLong(configObject),
+                configAnnotation.timeUnit());
+            break;
+          default:
+            throw new ConfigurationException(
+                "Unsupported ConfigType " + type + " on " + fieldLocation);
+          }
+        } catch (IllegalAccessException e) {
+          throw new ConfigurationException(
+              "Can't inject configuration to " + fieldLocation, e);
+        } finally {
+          if (accessChanged) {
+            field.setAccessible(false);
+          }
+        }
+      }
+    }
+  }
 }

hadoop-hdds/config/src/main/java/org/apache/hadoop/hdds/conf/ConfigurationSource.java

@@ -17,6 +17,8 @@
  */
 package org.apache.hadoop.hdds.conf;
 
+import org.apache.hadoop.hdds.conf.TimeDurationUtil.ParsedTimeDuration;
+
 import java.io.IOException;
 import java.util.Arrays;
 import java.util.Collection;
@@ -37,9 +39,6 @@ public interface ConfigurationSource {
 
   char[] getPassword(String key) throws IOException;
 
-  @Deprecated
-    //TODO: user read only configs and don't use it to store actual port
-    // numbers.
   void set(String key, String value);
 
   default String get(String key, String defaultValue) {
@@ -52,6 +51,10 @@ default int getInt(String key, int defaultValue) {
     return value != null ? Integer.parseInt(value) : defaultValue;
   }
 
+  default void setInt(String name, int value) {
+    set(name, Integer.toString(value));
+  }
+
   /**
    * Get the value of the <code>name</code> property as a set of comma-delimited
    * <code>int</code> values.
@@ -76,11 +79,19 @@ default long getLong(String key, long defaultValue) {
     return value != null ? Long.parseLong(value) : defaultValue;
   }
 
+  default void setLong(String name, long value) {
+    set(name, Long.toString(value));
+  }
+
   default boolean getBoolean(String key, boolean defaultValue) {
     String value = get(key);
     return value != null ? Boolean.parseBoolean(value) : defaultValue;
   }
 
+  default void setBoolean(String name, boolean value) {
+    set(name, Boolean.toString(value));
+  }
+
   default float getFloat(String key, float defaultValue) {
     String value = get(key);
     return value != null ? Float.parseFloat(value) : defaultValue;
@@ -155,6 +166,13 @@ default <T> T getObject(Class<T> configurationClass) {
 
   }
 
+  default <T> void setFromObject(T object) {
+    ConfigGroup configGroup =
+        object.getClass().getAnnotation(ConfigGroup.class);
+    String prefix = configGroup.prefix();
+    ConfigurationReflectionUtil.updateConfiguration(this, object, prefix);
+  }
+
   /**
    * Get the value of the <code>name</code> property as a <code>Class</code>
    * implementing the interface specified by <code>xface</code>.
@@ -237,6 +255,10 @@ default long getTimeDuration(String name, long defaultValue,
     }
   }
 
+  default void setTimeDuration(String name, long value, TimeUnit unit) {
+    set(name, value + ParsedTimeDuration.unitFor(unit).suffix());
+  }
+
   default long getTimeDuration(String name, String defaultValue,
       TimeUnit unit) {
     String vStr = get(name);