HDDS-1622. Use picocli for StorageContainerManager

Closes #918

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/SCMStarterInterface.java

@@ -0,0 +1,37 @@
+/**
+ * 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 regarding
+ * copyright ownership. The ASF licenses this file to you under the Apache
+ * License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a
+ * copy of the License at
+ *
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * <p>Unless required by applicable law or agreed to in writing, software
+ * distributed under the
+ * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
+ * CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdds.scm.server;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import java.io.IOException;
+
+/**
+ * This interface is used by the StorageContainerManager to allow the
+ * dependencies to be injected to the CLI class.
+ */
+public interface SCMStarterInterface {
+
+  void start(OzoneConfiguration conf) throws Exception;
+  boolean init(OzoneConfiguration conf, String clusterId)
+      throws IOException;
+  String generateClusterId();
+}

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManager.java

@@ -81,7 +81,6 @@ import org.apache.hadoop.hdds.security.x509.certificate.authority.DefaultCAServe
 import org.apache.hadoop.hdds.server.ServiceRuntimeInfoImpl;
 import org.apache.hadoop.hdds.server.events.EventPublisher;
 import org.apache.hadoop.hdds.server.events.EventQueue;
-import org.apache.hadoop.hdds.tracing.TracingUtil;
 import org.apache.hadoop.hdfs.DFSUtil;
 import org.apache.hadoop.io.IOUtils;
 import org.apache.hadoop.ipc.RPC;
@@ -90,7 +89,6 @@ import org.apache.hadoop.metrics2.util.MBeans;
 import org.apache.hadoop.ozone.OzoneConfigKeys;
 import org.apache.hadoop.ozone.OzoneSecurityUtil;
 import org.apache.hadoop.ozone.common.Storage.StorageState;
-import org.apache.hadoop.ozone.common.StorageInfo;
 import org.apache.hadoop.ozone.lease.LeaseManager;
 import org.apache.hadoop.ozone.lock.LockManager;
 import org.apache.hadoop.ozone.protocol.commands.RetriableDatanodeEventWatcher;
@@ -98,16 +96,13 @@ import org.apache.hadoop.security.SecurityUtil;
 import org.apache.hadoop.security.UserGroupInformation;
 import org.apache.hadoop.security.UserGroupInformation.AuthenticationMethod;
 import org.apache.hadoop.security.authentication.client.AuthenticationException;
-import org.apache.hadoop.util.GenericOptionsParser;
 import org.apache.hadoop.util.JvmPauseMonitor;
-import org.apache.hadoop.util.StringUtils;
 import org.apache.hadoop.utils.HddsVersionInfo;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import javax.management.ObjectName;
 import java.io.IOException;
-import java.io.PrintStream;
 import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.util.Collection;
@@ -120,7 +115,6 @@ import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_SCM_KERBEROS_KEYTAB_
 import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_SCM_KERBEROS_PRINCIPAL_KEY;
 import static org.apache.hadoop.hdds.scm.ScmConfigKeys.HDDS_SCM_WATCHER_TIMEOUT_DEFAULT;
 import static org.apache.hadoop.ozone.OzoneConfigKeys.OZONE_ENABLED;
-import static org.apache.hadoop.util.ExitUtil.terminate;
 
 /**
  * StorageContainerManager is the main entry point for the service that
@@ -140,19 +134,7 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl
 
   private static final Logger LOG = LoggerFactory
       .getLogger(StorageContainerManager.class);
-  private static final String USAGE =
+
-      "Usage: \n ozone scm [genericOptions] "
-          + "[ "
-          + StartupOption.INIT.getName()
-          + " [ "
-          + StartupOption.CLUSTERID.getName()
-          + " <cid> ] ]\n "
-          + "ozone scm [genericOptions] [ "
-          + StartupOption.GENCLUSTERID.getName()
-          + " ]\n "
-          + "ozone scm [ "
-          + StartupOption.HELP.getName()
-          + " ]\n";
   /**
    * SCM metrics.
    */
@@ -586,115 +568,23 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl
   }
 
   /**
-   * Main entry point for starting StorageContainerManager.
+   * Create an SCM instance based on the supplied configuration.
    *
-   * @param argv arguments
-   * @throws IOException if startup fails due to I/O error
-   */
-  public static void main(String[] argv) throws IOException {
-    if (DFSUtil.parseHelpArgument(argv, USAGE, System.out, true)) {
-      System.exit(0);
-    }
-    try {
-      TracingUtil.initTracing("StorageContainerManager");
-      OzoneConfiguration conf = new OzoneConfiguration();
-      GenericOptionsParser hParser = new GenericOptionsParser(conf, argv);
-      if (!hParser.isParseSuccessful()) {
-        System.err.println("USAGE: " + USAGE + "\n");
-        hParser.printGenericCommandUsage(System.err);
-        System.exit(1);
-      }
-      StorageContainerManager scm = createSCM(
-          hParser.getRemainingArgs(), conf, true);
-      if (scm != null) {
-        scm.start();
-        scm.join();
-      }
-    } catch (Throwable t) {
-      LOG.error("Failed to start the StorageContainerManager.", t);
-      terminate(1, t);
-    }
-  }
-
-  private static void printUsage(PrintStream out) {
-    out.println(USAGE + "\n");
-  }
-
-  /**
-   * Create an SCM 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
    * @return SCM instance
    * @throws IOException, AuthenticationException
    */
-  @VisibleForTesting
   public static StorageContainerManager createSCM(
-      String[] args, OzoneConfiguration conf)
+      OzoneConfiguration conf)
       throws IOException, AuthenticationException {
-    return createSCM(args, conf, false);
-  }
-
-  /**
-   * Create an SCM instance based on the supplied command-line arguments.
-   *
-   * @param args        command-line arguments.
-   * @param conf        HDDS configuration
-   * @param printBanner if true, then log a verbose startup message.
-   * @return SCM instance
-   * @throws IOException, AuthenticationException
-   */
-  private static StorageContainerManager createSCM(
-      String[] args,
-      OzoneConfiguration conf,
-      boolean printBanner)
-      throws IOException, AuthenticationException {
-    String[] argv = (args == null) ? new String[0] : args;
     if (!HddsUtils.isHddsEnabled(conf)) {
       System.err.println(
           "SCM cannot be started in secure mode or when " + OZONE_ENABLED + "" +
               " is set to false");
       System.exit(1);
     }
-    StartupOption startOpt = parseArguments(argv);
-    if (startOpt == null) {
-      printUsage(System.err);
-      terminate(1);
-      return null;
-    }
-    switch (startOpt) {
-    case INIT:
-      if (printBanner) {
-        StringUtils.startupShutdownMessage(StorageContainerManager.class, argv,
-            LOG);
-      }
-      terminate(scmInit(conf) ? 0 : 1);
-      return null;
-    case GENCLUSTERID:
-      if (printBanner) {
-        StringUtils.startupShutdownMessage(StorageContainerManager.class, argv,
-            LOG);
-      }
-      System.out.println("Generating new cluster id:");
-      System.out.println(StorageInfo.newClusterID());
-      terminate(0);
-      return null;
-    case HELP:
-      printUsage(System.err);
-      terminate(0);
-      return null;
-    default:
-      if (printBanner) {
-        StringUtils.startupShutdownMessage(StorageContainerManager.class, argv,
-            LOG);
-      }
     return new StorageContainerManager(conf);
   }
-  }
 
   /**
    * Routine to set up the Version info for StorageContainerManager.
@@ -703,12 +593,12 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl
    * @return true if SCM initialization is successful, false otherwise.
    * @throws IOException if init fails due to I/O error
    */
-  public static boolean scmInit(OzoneConfiguration conf) throws IOException {
+  public static boolean scmInit(OzoneConfiguration conf,
+      String clusterId) throws IOException {
     SCMStorageConfig scmStorageConfig = new SCMStorageConfig(conf);
     StorageState state = scmStorageConfig.getState();
     if (state != StorageState.INITIALIZED) {
       try {
-        String clusterId = StartupOption.INIT.getClusterId();
         if (clusterId != null && !clusterId.isEmpty()) {
           scmStorageConfig.setClusterId(clusterId);
         }
@@ -735,48 +625,6 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl
     }
   }
 
-  private static StartupOption parseArguments(String[] args) {
-    int argsLen = (args == null) ? 0 : args.length;
-    StartupOption startOpt = null;
-    if (argsLen == 0) {
-      startOpt = StartupOption.REGULAR;
-    }
-    for (int i = 0; i < argsLen; i++) {
-      String cmd = args[i];
-      if (StartupOption.INIT.getName().equalsIgnoreCase(cmd)) {
-        startOpt = StartupOption.INIT;
-        if (argsLen > 3) {
-          return null;
-        }
-        for (i = i + 1; i < argsLen; i++) {
-          if (args[i].equalsIgnoreCase(StartupOption.CLUSTERID.getName())) {
-            i++;
-            if (i < argsLen && !args[i].isEmpty()) {
-              startOpt.setClusterId(args[i]);
-            } else {
-              // if no cluster id specified or is empty string, return null
-              LOG.error(
-                  "Must specify a valid cluster ID after the "
-                      + StartupOption.CLUSTERID.getName()
-                      + " flag");
-              return null;
-            }
-          } else {
-            return null;
-          }
-        }
-      } else {
-        if (StartupOption.GENCLUSTERID.getName().equalsIgnoreCase(cmd)) {
-          if (argsLen > 1) {
-            return null;
-          }
-          startOpt = StartupOption.GENCLUSTERID;
-        }
-      }
-    }
-    return startOpt;
-  }
-
   /**
    * Initialize SCM metrics.
    */
@@ -1219,35 +1067,4 @@ public final class StorageContainerManager extends ServiceRuntimeInfoImpl
   public SCMMetadataStore getScmMetadataStore() {
     return scmMetadataStore;
   }
-  /**
-   * Startup options.
-   */
-  public enum StartupOption {
-    INIT("--init"),
-    CLUSTERID("--clusterid"),
-    GENCLUSTERID("--genclusterid"),
-    REGULAR("--regular"),
-    HELP("-help");
-
-    private final String name;
-    private String clusterId = null;
-
-    StartupOption(String arg) {
-      this.name = arg;
-    }
-
-    public String getClusterId() {
-      return clusterId;
-    }
-
-    public void setClusterId(String cid) {
-      if (cid != null && !cid.isEmpty()) {
-        clusterId = cid;
-      }
-    }
-
-    public String getName() {
-      return name;
-    }
-  }
 }

hadoop-hdds/server-scm/src/main/java/org/apache/hadoop/hdds/scm/server/StorageContainerManagerStarter.java

@@ -0,0 +1,153 @@
+/**
+ * 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 regarding
+ * copyright ownership. The ASF licenses this file to you under the Apache
+ * License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the
+ * License. You may obtain a
+ * copy of the License at
+ *
+ * <p>http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * <p>Unless required by applicable law or agreed to in writing, software
+ * distributed under the
+ * License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR
+ * CONDITIONS OF ANY KIND, either
+ * express or implied. See the License for the specific language governing
+ * permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdds.scm.server;
+
+import org.apache.hadoop.hdds.cli.GenericCli;
+import org.apache.hadoop.hdds.cli.HddsVersionProvider;
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.apache.hadoop.hdds.tracing.TracingUtil;
+import org.apache.hadoop.ozone.common.StorageInfo;
+import org.apache.hadoop.util.StringUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import picocli.CommandLine;
+import picocli.CommandLine.Command;
+
+import java.io.IOException;
+
+/**
+ * This class provides a command line interface to start the SCM
+ * using Picocli.
+ */
+
+@Command(name = "ozone scm",
+    hidden = true, description = "Start or initialize the scm server.",
+    versionProvider = HddsVersionProvider.class,
+    mixinStandardHelpOptions = true)
+public class StorageContainerManagerStarter extends GenericCli {
+
+  private OzoneConfiguration conf;
+  private SCMStarterInterface receiver;
+  private static final Logger LOG =
+      LoggerFactory.getLogger(StorageContainerManagerStarter.class);
+
+  public static void main(String[] args) throws Exception {
+    TracingUtil.initTracing("StorageContainerManager");
+    new StorageContainerManagerStarter(
+        new StorageContainerManagerStarter.SCMStarterHelper()).run(args);
+  }
+
+  public StorageContainerManagerStarter(SCMStarterInterface receiverObj) {
+    super();
+    receiver = receiverObj;
+  }
+
+  @Override
+  public Void call() throws Exception {
+    commonInit();
+    startScm();
+    return null;
+  }
+
+  /**
+   * This function implements a sub-command to generate a new
+   * cluster ID from the command line.
+   */
+  @CommandLine.Command(name = "--genclusterid",
+      customSynopsis = "ozone scm [global options] --genclusterid [options]",
+      hidden = false,
+      description = "Generate a new Cluster ID",
+      mixinStandardHelpOptions = true,
+      versionProvider = HddsVersionProvider.class)
+  public void generateClusterId() {
+    commonInit();
+    System.out.println("Generating new cluster id:");
+    System.out.println(receiver.generateClusterId());
+  }
+
+  /**
+   * This function implements a sub-command to allow the SCM to be
+   * initialized from the command line.
+   *
+   * @param clusterId - Cluster ID to use when initializing. If null,
+   *                  a random ID will be generated and used.
+   */
+  @CommandLine.Command(name = "--init",
+      customSynopsis = "ozone scm [global options] --init [options]",
+      hidden = false,
+      description = "Initialize the SCM if not already initialized",
+      mixinStandardHelpOptions = true,
+      versionProvider = HddsVersionProvider.class)
+  public void initScm(@CommandLine.Option(names = { "--clusterid" },
+      description = "Optional: The cluster id to use when formatting SCM",
+      paramLabel = "id") String clusterId)
+      throws Exception {
+    commonInit();
+    boolean result = receiver.init(conf, clusterId);
+    if (!result) {
+      throw new IOException("scm init failed");
+    }
+  }
+
+  /**
+   * This function is used by the command line to start the SCM.
+   */
+  private void startScm() throws Exception {
+    receiver.start(conf);
+  }
+
+  /**
+   * This function should be called by each command to ensure the configuration
+   * is set and print the startup banner message.
+   */
+  private void commonInit() {
+    conf = createOzoneConfiguration();
+
+    String[] originalArgs = getCmd().getParseResult().originalArgs()
+        .toArray(new String[0]);
+    StringUtils.startupShutdownMessage(StorageContainerManager.class,
+        originalArgs, LOG);
+  }
+
+  /**
+   * This static class wraps the external dependencies needed for this command
+   * to execute its tasks. This allows the dependency to be injected for unit
+   * testing.
+   */
+  static class SCMStarterHelper implements SCMStarterInterface {
+
+    public void start(OzoneConfiguration conf) throws Exception {
+      StorageContainerManager stm = StorageContainerManager.createSCM(conf);
+      stm.start();
+      stm.join();
+    }
+
+    public boolean init(OzoneConfiguration conf, String clusterId)
+        throws IOException{
+      return StorageContainerManager.scmInit(conf, clusterId);
+    }
+
+    public String generateClusterId() {
+      return StorageInfo.newClusterID();
+    }
+  }
+}

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/HddsTestUtils.java

@@ -91,7 +91,7 @@ public final class HddsTestUtils {
       // writes the version file properties
       scmStore.initialize();
     }
-    return StorageContainerManager.createSCM(null, conf);
+    return StorageContainerManager.createSCM(conf);
   }
 
   /**

hadoop-hdds/server-scm/src/test/java/org/apache/hadoop/hdds/scm/server/TestStorageContainerManagerStarter.java

@@ -0,0 +1,166 @@
+/**
+ * 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
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hdds.scm.server;
+
+import org.apache.hadoop.hdds.conf.OzoneConfiguration;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.Test;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.PrintStream;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import static org.junit.Assert.*;
+
+
+/**
+ * This class is used to test the StorageContainerManagerStarter using a mock
+ * class to avoid starting any services and hence just test the CLI component.
+ */
+public class TestStorageContainerManagerStarter {
+
+  private final ByteArrayOutputStream outContent = new ByteArrayOutputStream();
+  private final ByteArrayOutputStream errContent = new ByteArrayOutputStream();
+  private final PrintStream originalOut = System.out;
+  private final PrintStream originalErr = System.err;
+
+  private MockSCMStarter mock;
+
+  @Before
+  public void setUpStreams() {
+    System.setOut(new PrintStream(outContent));
+    System.setErr(new PrintStream(errContent));
+    mock = new MockSCMStarter();
+  }
+
+  @After
+  public void restoreStreams() {
+    System.setOut(originalOut);
+    System.setErr(originalErr);
+  }
+
+  @Test
+  public void testCallsStartWhenServerStarted() throws Exception {
+    executeCommand();
+    assertTrue(mock.startCalled);
+  }
+
+  @Test
+  public void testExceptionThrownWhenStartFails() throws Exception {
+    mock.throwOnStart = true;
+    try {
+      executeCommand();
+      fail("Exception show have been thrown");
+    } catch (Exception e) {
+      assertTrue(true);
+    }
+  }
+
+  @Test
+  public void testStartNotCalledWithInvalidParam() throws Exception {
+    executeCommand("--invalid");
+    assertFalse(mock.startCalled);
+  }
+
+  @Test
+  public void testPassingInitSwitchCallsInit() {
+    executeCommand("--init");
+    assertTrue(mock.initCalled);
+  }
+
+  @Test
+  public void testInitSwitchAcceptsClusterIdSSwitch() {
+    executeCommand("--init", "--clusterid=abcdefg");
+    assertEquals("abcdefg", mock.clusterId);
+  }
+
+  @Test
+  public void testInitSwitchWithInvalidParamDoesNotRun() {
+    executeCommand("--init", "--clusterid=abcdefg", "--invalid");
+    assertFalse(mock.initCalled);
+  }
+
+  @Test
+  public void testUnSuccessfulInitThrowsException() {
+    mock.throwOnInit = true;
+    try {
+      executeCommand("--init");
+      fail("Exception show have been thrown");
+    } catch (Exception e) {
+      assertTrue(true);
+    }
+  }
+
+  @Test
+  public void testGenClusterIdRunsGenerate() {
+    executeCommand("--genclusterid");
+    assertTrue(mock.generateCalled);
+  }
+
+  @Test
+  public void testGenClusterIdWithInvalidParamDoesNotRun() {
+    executeCommand("--genclusterid", "--invalid");
+    assertFalse(mock.generateCalled);
+  }
+
+  @Test
+  public void testUsagePrintedOnInvalidInput() {
+    executeCommand("--invalid");
+    Pattern p = Pattern.compile("^Unknown option:.*--invalid.*\nUsage");
+    Matcher m = p.matcher(errContent.toString());
+    assertTrue(m.find());
+  }
+
+  private void executeCommand(String... args) {
+    new StorageContainerManagerStarter(mock).execute(args);
+  }
+
+  static class MockSCMStarter implements SCMStarterInterface {
+
+    private boolean initStatus = true;
+    private boolean throwOnStart = false;
+    private boolean throwOnInit  = false;
+    private boolean startCalled = false;
+    private boolean initCalled = false;
+    private boolean generateCalled = false;
+    private String clusterId = null;
+
+    public void start(OzoneConfiguration conf) throws Exception {
+      if (throwOnStart) {
+        throw new Exception("Simulated error on start");
+      }
+      startCalled = true;
+    }
+
+    public boolean init(OzoneConfiguration conf, String cid)
+        throws IOException {
+      if (throwOnInit) {
+        throw new IOException("Simulated error on init");
+      }
+      initCalled = true;
+      clusterId = cid;
+      return initStatus;
+    }
+
+    public String generateClusterId() {
+      generateCalled = true;
+      return "static-cluster-id";
+    }
+  }
+}

hadoop-ozone/common/src/main/bin/ozone

@@ -144,7 +144,7 @@ function ozonecmd_case
     ;;
     scm)
       HADOOP_SUBCMD_SUPPORTDAEMONIZATION="true"
-      HADOOP_CLASSNAME='org.apache.hadoop.hdds.scm.server.StorageContainerManager'
+      HADOOP_CLASSNAME='org.apache.hadoop.hdds.scm.server.StorageContainerManagerStarter'
       hadoop_debug "Appending HDFS_STORAGECONTAINERMANAGER_OPTS onto HADOOP_OPTS"
       HDFS_STORAGECONTAINERMANAGER_OPTS="${HDFS_STORAGECONTAINERMANAGER_OPTS} -Dlog4j.configurationFile=${HADOOP_CONF_DIR}/scm-audit-log4j2.properties"
       HADOOP_OPTS="${HADOOP_OPTS} ${HDFS_STORAGECONTAINERMANAGER_OPTS}"

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

@@ -254,7 +254,7 @@ public class MiniOzoneClusterImpl implements MiniOzoneCluster {
       AuthenticationException {
     scm.stop();
     scm.join();
-    scm = StorageContainerManager.createSCM(null, conf);
+    scm = StorageContainerManager.createSCM(conf);
     scm.start();
     if (waitForDatanode) {
       waitForClusterToBeReady();
@@ -475,7 +475,7 @@ public class MiniOzoneClusterImpl implements MiniOzoneCluster {
       configureSCM();
       SCMStorageConfig scmStore = new SCMStorageConfig(conf);
       initializeScmStorage(scmStore);
-      return StorageContainerManager.createSCM(null, conf);
+      return StorageContainerManager.createSCM(conf);
     }
 
     private void initializeScmStorage(SCMStorageConfig scmStore)

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

@@ -260,7 +260,7 @@ public final class TestSecureOzoneCluster {
   public void testSecureScmStartupSuccess() throws Exception {
 
     initSCM();
-    scm = StorageContainerManager.createSCM(null, conf);
+    scm = StorageContainerManager.createSCM(conf);
     //Reads the SCM Info from SCM instance
     ScmInfo scmInfo = scm.getClientProtocolServer().getScmInfo();
     Assert.assertEquals(clusterId, scmInfo.getClusterId());
@@ -271,7 +271,7 @@ public final class TestSecureOzoneCluster {
   public void testSCMSecurityProtocol() throws Exception {
 
     initSCM();
-    scm = StorageContainerManager.createSCM(null, conf);
+    scm = StorageContainerManager.createSCM(conf);
     //Reads the SCM Info from SCM instance
     try {
       scm.start();
@@ -340,7 +340,7 @@ public final class TestSecureOzoneCluster {
     LambdaTestUtils.intercept(IOException.class,
         "Running in secure mode, but config doesn't have a keytab",
         () -> {
-          StorageContainerManager.createSCM(null, conf);
+          StorageContainerManager.createSCM(conf);
         });
 
     conf.set(ScmConfigKeys.HDDS_SCM_KERBEROS_PRINCIPAL_KEY,
@@ -349,7 +349,7 @@ public final class TestSecureOzoneCluster {
         "/etc/security/keytabs/scm.keytab");
 
     testCommonKerberosFailures(
-        () -> StorageContainerManager.createSCM(null, conf));
+        () -> StorageContainerManager.createSCM(conf));
 
   }
 
@@ -379,7 +379,7 @@ public final class TestSecureOzoneCluster {
   public void testSecureOMInitializationFailure() throws Exception {
     initSCM();
     // Create a secure SCM instance as om client will connect to it
-    scm = StorageContainerManager.createSCM(null, conf);
+    scm = StorageContainerManager.createSCM(conf);
     setupOm(conf);
     conf.set(OMConfigKeys.OZONE_OM_KERBEROS_PRINCIPAL_KEY,
         "non-existent-user@EXAMPLE.com");
@@ -395,7 +395,7 @@ public final class TestSecureOzoneCluster {
   public void testSecureOmInitializationSuccess() throws Exception {
     initSCM();
     // Create a secure SCM instance as om client will connect to it
-    scm = StorageContainerManager.createSCM(null, conf);
+    scm = StorageContainerManager.createSCM(conf);
     LogCapturer logs = LogCapturer.captureLogs(OzoneManager.LOG);
     GenericTestUtils.setLogLevel(OzoneManager.LOG, INFO);
 
@@ -719,7 +719,7 @@ public final class TestSecureOzoneCluster {
     omLogs.clearOutput();
     initSCM();
     try {
-      scm = StorageContainerManager.createSCM(null, conf);
+      scm = StorageContainerManager.createSCM(conf);
       scm.start();
       conf.setBoolean(OZONE_SECURITY_ENABLED_KEY, false);
       OMStorage omStore = new OMStorage(conf);
@@ -765,7 +765,7 @@ public final class TestSecureOzoneCluster {
     omLogs.clearOutput();
     initSCM();
     try {
-      scm = StorageContainerManager.createSCM(null, conf);
+      scm = StorageContainerManager.createSCM(conf);
       scm.start();
 
       OMStorage omStore = new OMStorage(conf);

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

@@ -64,7 +64,6 @@ import org.apache.hadoop.hdds.scm.node.NodeManager;
 import org.apache.hadoop.hdds.scm.server.SCMClientProtocolServer;
 import org.apache.hadoop.hdds.scm.server.SCMStorageConfig;
 import org.apache.hadoop.hdds.scm.server.StorageContainerManager;
-import org.apache.hadoop.hdds.scm.server.StorageContainerManager.StartupOption;
 import org.apache.hadoop.hdds.server.events.EventPublisher;
 import org.apache.hadoop.hdds.server.events.TypedEvent;
 import org.apache.hadoop.ozone.container.ContainerTestHelper;
@@ -76,7 +75,6 @@ import org.apache.hadoop.ozone.protocol.commands.DeleteBlocksCommand;
 import org.apache.hadoop.ozone.protocol.commands.SCMCommand;
 import org.apache.hadoop.security.authentication.client.AuthenticationException;
 import org.apache.hadoop.test.GenericTestUtils;
-import org.apache.hadoop.util.ExitUtil;
 import org.apache.hadoop.utils.HddsVersionInfo;
 import org.junit.Assert;
 import org.junit.Rule;
@@ -417,15 +415,13 @@ public class TestStorageContainerManager {
     Path scmPath = Paths.get(path, "scm-meta");
     conf.set(HddsConfigKeys.OZONE_METADATA_DIRS, scmPath.toString());
 
-    StartupOption.INIT.setClusterId("testClusterId");
     // This will initialize SCM
-    StorageContainerManager.scmInit(conf);
+    StorageContainerManager.scmInit(conf, "testClusterId");
 
     SCMStorageConfig scmStore = new SCMStorageConfig(conf);
     Assert.assertEquals(NodeType.SCM, scmStore.getNodeType());
     Assert.assertEquals("testClusterId", scmStore.getClusterID());
-    StartupOption.INIT.setClusterId("testClusterIdNew");
+    StorageContainerManager.scmInit(conf, "testClusterIdNew");
-    StorageContainerManager.scmInit(conf);
     Assert.assertEquals(NodeType.SCM, scmStore.getNodeType());
     Assert.assertEquals("testClusterId", scmStore.getClusterID());
   }
@@ -441,9 +437,8 @@ public class TestStorageContainerManager {
     MiniOzoneCluster cluster =
         MiniOzoneCluster.newBuilder(conf).setNumDatanodes(1).build();
     cluster.waitForClusterToBeReady();
-    StartupOption.INIT.setClusterId("testClusterId");
     // This will initialize SCM
-    StorageContainerManager.scmInit(conf);
+    StorageContainerManager.scmInit(conf, "testClusterId");
     SCMStorageConfig scmStore = new SCMStorageConfig(conf);
     Assert.assertEquals(NodeType.SCM, scmStore.getNodeType());
     Assert.assertNotEquals("testClusterId", scmStore.getClusterID());
@@ -462,20 +457,7 @@ public class TestStorageContainerManager {
     exception.expect(SCMException.class);
     exception.expectMessage(
         "SCM not initialized due to storage config failure");
-    StorageContainerManager.createSCM(null, conf);
+    StorageContainerManager.createSCM(conf);
-  }
-
-  @Test
-  public void testSCMInitializationReturnCode() throws IOException,
-      AuthenticationException {
-    ExitUtil.disableSystemExit();
-    OzoneConfiguration conf = new OzoneConfiguration();
-    conf.setBoolean(OzoneConfigKeys.OZONE_ENABLED, true);
-    // Set invalid args
-    String[] invalidArgs = {"--zxcvbnm"};
-    exception.expect(ExitUtil.ExitException.class);
-    exception.expectMessage("ExitException");
-    StorageContainerManager.createSCM(invalidArgs, conf);
   }
 
   @Test
@@ -493,7 +475,7 @@ public class TestStorageContainerManager {
     scmStore.setScmId(scmId);
     // writes the version file properties
     scmStore.initialize();
-    StorageContainerManager scm = StorageContainerManager.createSCM(null, conf);
+    StorageContainerManager scm = StorageContainerManager.createSCM(conf);
     //Reads the SCM Info from SCM instance
     ScmInfo scmInfo = scm.getClientProtocolServer().getScmInfo();
     Assert.assertEquals(clusterId, scmInfo.getClusterId());