HDDS-1192. Support -conf command line argument in GenericCli

Author: kittinanasi

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java

@@ -22,6 +22,7 @@
 import java.util.concurrent.Callable;
 
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.fs.Path;
 
 import com.google.common.annotations.VisibleForTesting;
 import picocli.CommandLine;
@@ -41,6 +42,9 @@ public class GenericCli implements Callable<Void>, GenericParentCommand {
   @Option(names = {"-D", "--set"})
   private Map<String, String> configurationOverrides = new HashMap<>();
 
+  @Option(names = {"-conf"})
+  private String configurationPath;
+
   private final CommandLine cmd;
 
   public GenericCli() {
@@ -70,19 +74,19 @@ protected void printError(Throwable error) {
     } else {
       System.err.println(error.getMessage().split("\n")[0]);
     }
-    if(error instanceof MissingSubcommandException){
-      System.err.println(((MissingSubcommandException) error).getUsage());
-    }
   }
 
   @Override
   public Void call() throws Exception {
-    throw new MissingSubcommandException(cmd.getUsageMessage());
+    throw new MissingSubcommandException(cmd);
   }
 
   @Override
   public OzoneConfiguration createOzoneConfiguration() {
     OzoneConfiguration ozoneConf = new OzoneConfiguration();
+    if (configurationPath != null) {
+      ozoneConf.addResource(new Path(configurationPath));
+    }
     if (configurationOverrides != null) {
       for (Entry<String, String> entry : configurationOverrides.entrySet()) {
         ozoneConf.set(entry.getKey(), entry.getValue());

hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/MissingSubcommandException.java

@@ -17,19 +17,15 @@
  */
 package org.apache.hadoop.hdds.cli;
 
+import picocli.CommandLine;
+
 /**
  * Exception to throw if subcommand is not selected but required.
  */
-public class MissingSubcommandException extends RuntimeException {
-
-  private String usage;
+public class MissingSubcommandException extends CommandLine.ParameterException {
 
-  public MissingSubcommandException(String usage) {
-    super("Incomplete command");
-    this.usage = usage;
+  public MissingSubcommandException(CommandLine cmd) {
+    super(cmd, "Incomplete command");
   }
 
-  public String getUsage() {
-    return usage;
-  }
 }

hadoop-hdds/container-service/src/main/java/org/apache/hadoop/ozone/HddsDatanodeService.java

@@ -53,6 +53,7 @@
 import java.net.InetAddress;
 import java.security.KeyPair;
 import java.security.cert.CertificateException;
+import java.util.Arrays;
 import java.util.List;
 import java.util.UUID;
 
@@ -81,64 +82,46 @@ public class HddsDatanodeService extends GenericCli implements ServicePlugin {
   private CertificateClient dnCertClient;
   private String component;
   private HddsDatanodeHttpServer httpServer;
+  private boolean printBanner;
+  private String[] args;
 
-  /**
-   * Default constructor.
-   */
-  public HddsDatanodeService() {
-    this(null);
+  public HddsDatanodeService(boolean printBanner, String[] args) {
+    this.printBanner = printBanner;
+    this.args = args != null ? Arrays.copyOf(args, args.length) : null;
   }
 
   /**
-   * Constructs {@link HddsDatanodeService} using the provided {@code conf}
-   * value.
+   * Create an Datanode instance based on the supplied command-line arguments.
+   * <p>
+   * This method is intended for unit tests only. It suppresses the
+   * startup/shutdown message and skips registering Unix signal handlers.
    *
-   * @param conf OzoneConfiguration
+   * @param args      command line arguments.
+   * @return Datanode instance
    */
-  public HddsDatanodeService(Configuration conf) {
-    if (conf == null) {
-      this.conf = new OzoneConfiguration();
-    } else {
-      this.conf = new OzoneConfiguration(conf);
-    }
-  }
-
   @VisibleForTesting
   public static HddsDatanodeService createHddsDatanodeService(
-      String[] args, Configuration conf) {
-    return createHddsDatanodeService(args, conf, false);
+      String[] args) {
+    return createHddsDatanodeService(args, false);
   }
 
   /**
    * Create an Datanode instance based on the supplied command-line arguments.
-   * <p>
-   * This method is intended for unit tests only. It suppresses the
-   * startup/shutdown message and skips registering Unix signal handlers.
    *
    * @param args        command line arguments.
-   * @param conf        HDDS configuration
    * @param printBanner if true, then log a verbose startup message.
    * @return Datanode instance
    */
   private static HddsDatanodeService createHddsDatanodeService(
-      String[] args, Configuration conf, boolean printBanner) {
-    if (args.length == 0 && printBanner) {
-      StringUtils
-          .startupShutdownMessage(HddsDatanodeService.class, args, LOG);
-
-    }
-    return new HddsDatanodeService(conf);
+      String[] args, boolean printBanner) {
+    return new HddsDatanodeService(printBanner, args);
   }
 
   public static void main(String[] args) {
     try {
-      Configuration conf = new OzoneConfiguration();
       HddsDatanodeService hddsDatanodeService =
-          createHddsDatanodeService(args, conf, true);
-      if (hddsDatanodeService != null) {
-        hddsDatanodeService.start(null);
-        hddsDatanodeService.join();
-      }
+          createHddsDatanodeService(args, true);
+      hddsDatanodeService.run(args);
     } catch (Throwable e) {
       LOG.error("Exception in HddsDatanodeService.", e);
       terminate(1, e);
@@ -149,19 +132,43 @@ public static Logger getLogger() {
     return LOG;
   }
 
+  @Override
+  public Void call() throws Exception {
+    if (printBanner) {
+      StringUtils
+          .startupShutdownMessage(HddsDatanodeService.class, args, LOG);
+    }
+    start(createOzoneConfiguration());
+    join();
+    return null;
+  }
+
+  public void setConfiguration(OzoneConfiguration configuration) {
+    this.conf = configuration;
+  }
+
   /**
    * Starts HddsDatanode services.
    *
    * @param service The service instance invoking this method
    */
   @Override
   public void start(Object service) {
+    if (service instanceof Configurable) {
+      start(new OzoneConfiguration(((Configurable) service).getConf()));
+    } else {
+      start(new OzoneConfiguration());
+    }
+  }
+
+  public void start(OzoneConfiguration configuration) {
+    setConfiguration(configuration);
+    start();
+  }
 
+  public void start() {
     DefaultMetricsSystem.initialize("HddsDatanode");
     OzoneConfiguration.activate();
-    if (service instanceof Configurable) {
-      conf = new OzoneConfiguration(((Configurable) service).getConf());
-    }
     if (HddsUtils.isHddsEnabled(conf)) {
       try {
         String hostname = HddsUtils.getHostName(conf);
@@ -404,11 +411,13 @@ public DatanodeStateMachine getDatanodeStateMachine() {
   }
 
   public void join() {
-    try {
-      datanodeStateMachine.join();
-    } catch (InterruptedException e) {
-      Thread.currentThread().interrupt();
-      LOG.info("Interrupted during StorageContainerManager join.");
+    if (datanodeStateMachine != null) {
+      try {
+        datanodeStateMachine.join();
+      } catch (InterruptedException e) {
+        Thread.currentThread().interrupt();
+        LOG.info("Interrupted during StorageContainerManager join.");
+      }
     }
   }
 

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsDatanodeService.java

@@ -40,7 +40,7 @@
  */
 public class TestHddsDatanodeService {
   private File testDir;
-  private Configuration conf;
+  private OzoneConfiguration conf;
   private HddsDatanodeService service;
   private String[] args = new String[] {};
 
@@ -64,8 +64,8 @@ public void tearDown() {
 
   @Test
   public void testStartup() throws IOException {
-    service = HddsDatanodeService.createHddsDatanodeService(args, conf);
-    service.start(null);
+    service = HddsDatanodeService.createHddsDatanodeService(args);
+    service.start(conf);
     service.join();
 
     assertNotNull(service.getDatanodeDetails());

hadoop-hdds/container-service/src/test/java/org/apache/hadoop/ozone/TestHddsSecureDatanodeInit.java

@@ -83,10 +83,10 @@ public static void setUp() throws Exception {
         ServicePlugin.class);
     securityConfig = new SecurityConfig(conf);
 
-    service = HddsDatanodeService.createHddsDatanodeService(args, conf);
+    service = HddsDatanodeService.createHddsDatanodeService(args);
     dnLogs = GenericTestUtils.LogCapturer.captureLogs(getLogger());
     callQuietly(() -> {
-      service.start(null);
+      service.start(conf);
       return null;
     });
     callQuietly(() -> {

hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/scm/cli/SafeModeCommands.java

@@ -54,7 +54,6 @@ public SCMCLI getParent() {
   @Override
   public Void call() throws Exception {
     throw new MissingSubcommandException(
-        this.parent.getCmd().getSubcommands().get("safemode").
-        getUsageMessage());
+        this.parent.getCmd().getSubcommands().get("safemode"));
   }
 }

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/MiniOzoneClusterImpl.java

@@ -23,7 +23,6 @@
 import java.util.Optional;
 import org.apache.commons.io.FileUtils;
 import org.apache.hadoop.classification.InterfaceAudience;
-import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.conf.StorageUnit;
 import org.apache.hadoop.hdds.HddsConfigKeys;
 import org.apache.hadoop.hdds.protocol.DatanodeDetails;
@@ -104,9 +103,9 @@ public class MiniOzoneClusterImpl implements MiniOzoneCluster {
    * @throws IOException if there is an I/O error
    */
   MiniOzoneClusterImpl(OzoneConfiguration conf,
-                               OzoneManager ozoneManager,
-                               StorageContainerManager scm,
-                               List<HddsDatanodeService> hddsDatanodes) {
+                       OzoneManager ozoneManager,
+                       StorageContainerManager scm,
+                       List<HddsDatanodeService> hddsDatanodes) {
     this.conf = conf;
     this.ozoneManager = ozoneManager;
     this.scm = scm;
@@ -275,7 +274,7 @@ public void restartHddsDatanode(int i, boolean waitForDatanode)
     datanodeService.stop();
     datanodeService.join();
     // ensure same ports are used across restarts.
-    Configuration config = datanodeService.getConf();
+    OzoneConfiguration config = datanodeService.getConf();
     int currentPort = datanodeService.getDatanodeDetails()
         .getPort(DatanodeDetails.Port.Name.STANDALONE).getValue();
     config.setInt(DFS_CONTAINER_IPC_PORT, currentPort);
@@ -291,9 +290,9 @@ public void restartHddsDatanode(int i, boolean waitForDatanode)
     }
     String[] args = new String[]{};
     HddsDatanodeService service =
-        HddsDatanodeService.createHddsDatanodeService(args, config);
+        HddsDatanodeService.createHddsDatanodeService(args);
     hddsDatanodes.add(i, service);
-    service.start(null);
+    service.start(config);
     if (waitForDatanode) {
       // wait for the node to be identified as a healthy node again.
       waitForClusterToBeReady();
@@ -371,7 +370,7 @@ public void startScm() throws IOException {
   public void startHddsDatanodes() {
     hddsDatanodes.forEach((datanode) -> {
       datanode.setCertificateClient(getCAClient());
-      datanode.start(null);
+      datanode.start();
     });
   }
 
@@ -537,7 +536,7 @@ List<HddsDatanodeService> createHddsDatanodes(
       conf.setStrings(ScmConfigKeys.OZONE_SCM_NAMES, scmAddress);
       List<HddsDatanodeService> hddsDatanodes = new ArrayList<>();
       for (int i = 0; i < numOfDatanodes; i++) {
-        Configuration dnConf = new OzoneConfiguration(conf);
+        OzoneConfiguration dnConf = new OzoneConfiguration(conf);
         String datanodeBaseDir = path + "/datanode-" + Integer.toString(i);
         Path metaDir = Paths.get(datanodeBaseDir, "meta");
         Path dataDir = Paths.get(datanodeBaseDir, "data", "containers");
@@ -555,8 +554,10 @@ List<HddsDatanodeService> createHddsDatanodes(
         dnConf.set(OzoneConfigKeys.OZONE_CONTAINER_COPY_WORKDIR,
             wrokDir.toString());
 
-        hddsDatanodes.add(
-            HddsDatanodeService.createHddsDatanodeService(args, dnConf));
+        HddsDatanodeService datanode
+            = HddsDatanodeService.createHddsDatanodeService(args);
+        datanode.setConfiguration(dnConf);
+        hddsDatanodes.add(datanode);
       }
       return hddsDatanodes;
     }

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

@@ -270,7 +270,7 @@ public void testSCMSafeMode() throws Exception {
     assertFalse(logCapturer.getOutput().contains("SCM exiting safe mode."));
     assertTrue(scm.getCurrentContainerThreshold() == 0);
     for (HddsDatanodeService dn : cluster.getHddsDatanodes()) {
-      dn.start(null);
+      dn.start();
     }
     GenericTestUtils
         .waitFor(() -> scm.getCurrentContainerThreshold() == 1.0, 100, 20000);