HDDS-11852. Reduce duplication in some GenericCli subclasses (apache#…

…7553)
aswinshakil · Dec 13, 2024 · 008f9a6 · 008f9a6
1 parent 7a46080
commit 008f9a6
Show file tree

Hide file tree

Showing 15 changed files with 36 additions and 191 deletions.
diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/GenericCli.java
@@ -16,6 +16,7 @@
  */
 package org.apache.hadoop.hdds.cli;
 
+import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -25,6 +26,7 @@
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 
 import com.google.common.annotations.VisibleForTesting;
+import org.apache.hadoop.security.UserGroupInformation;
 import picocli.CommandLine;
 import picocli.CommandLine.ExitCode;
 import picocli.CommandLine.Model.CommandSpec;
@@ -48,25 +50,20 @@ public class GenericCli implements Callable<Void>, GenericParentCommand {
   private String configurationPath;
 
   private final CommandLine cmd;
+  private OzoneConfiguration conf;
+  private UserGroupInformation user;
 
   public GenericCli() {
-    this(null);
+    this(CommandLine.defaultFactory());
   }
 
-  public GenericCli(Class<?> type) {
-    this(type, CommandLine.defaultFactory());
-  }
-
-  public GenericCli(Class<?> type, CommandLine.IFactory factory) {
+  public GenericCli(CommandLine.IFactory factory) {
     cmd = new CommandLine(this, factory);
     cmd.setExecutionExceptionHandler((ex, commandLine, parseResult) -> {
       printError(ex);
       return EXECUTION_ERROR_EXIT_CODE;
     });
 
-    if (type != null) {
-      SubcommandWithParent.addSubcommands(getCmd(), type);
-    }
     ExtensibleParentCommand.addSubcommands(cmd);
   }
 
@@ -122,6 +119,20 @@ public OzoneConfiguration createOzoneConfiguration() {
     return ozoneConf;
   }
 
+  public OzoneConfiguration getOzoneConf() {
+    if (conf == null) {
+      conf = createOzoneConfiguration();
+    }
+    return conf;
+  }
+
+  public UserGroupInformation getUser() throws IOException {
+    if (user == null) {
+      user = UserGroupInformation.getCurrentUser();
+    }
+    return user;
+  }
+
   @VisibleForTesting
   public picocli.CommandLine getCmd() {
     return cmd;

diff --git a/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/SubcommandWithParent.java b/hadoop-hdds/common/src/main/java/org/apache/hadoop/hdds/cli/SubcommandWithParent.java
diff --git a/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/cli/OzoneAdmin.java b/hadoop-hdds/tools/src/main/java/org/apache/hadoop/hdds/cli/OzoneAdmin.java
@@ -17,11 +17,7 @@
  */
 package org.apache.hadoop.hdds.cli;
 
-import com.google.common.annotations.VisibleForTesting;
-import java.io.IOException;
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.tracing.TracingUtil;
-import org.apache.hadoop.security.UserGroupInformation;
 
 import picocli.CommandLine;
 
@@ -35,38 +31,6 @@
     mixinStandardHelpOptions = true)
 public class OzoneAdmin extends GenericCli implements ExtensibleParentCommand {
 
-  private OzoneConfiguration ozoneConf;
-
-  private UserGroupInformation user;
-
-  public OzoneAdmin() {
-    super();
-  }
-
-  @VisibleForTesting
-  public OzoneAdmin(OzoneConfiguration conf) {
-    ozoneConf = conf;
-  }
-
-  public OzoneConfiguration getOzoneConf() {
-    if (ozoneConf == null) {
-      ozoneConf = createOzoneConfiguration();
-    }
-    return ozoneConf;
-  }
-
-  public UserGroupInformation getUser() throws IOException {
-    if (user == null) {
-      user = UserGroupInformation.getCurrentUser();
-    }
-    return user;
-  }
-
-  /**
-   * Main for the Ozone Admin shell Command handling.
-   *
-   * @param argv - System Args Strings[]
-   */
   public static void main(String[] argv) {
     new OzoneAdmin().run(argv);
   }

diff --git a/hadoop-ozone/insight/src/main/java/org/apache/hadoop/ozone/insight/Insight.java b/hadoop-ozone/insight/src/main/java/org/apache/hadoop/ozone/insight/Insight.java
@@ -34,10 +34,6 @@
     mixinStandardHelpOptions = true)
 public class Insight extends GenericCli {
 
-  public Insight() {
-    super(Insight.class);
-  }
-
   public static void main(String[] args) throws Exception {
     new Insight().run(args);
   }

diff --git a/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokensCLI.java b/hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/TestBlockTokensCLI.java
@@ -103,7 +103,8 @@ public final class TestBlockTokensCLI {
 
   @BeforeAll
   public static void init() throws Exception {
-    conf = new OzoneConfiguration();
+    ozoneAdmin = new OzoneAdmin();
+    conf = ozoneAdmin.getOzoneConf();
     conf.set(OZONE_SCM_CLIENT_ADDRESS_KEY, "localhost");
 
     ExitUtils.disableSystemExit();
@@ -117,7 +118,6 @@ public static void init() throws Exception {
     setSecretKeysConfig();
     startCluster();
     client = cluster.newClient();
-    ozoneAdmin = new OzoneAdmin(conf);
   }
 
   @AfterAll

diff --git a/...zone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestNSSummaryAdmin.java b/...zone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestNSSummaryAdmin.java
@@ -58,7 +58,8 @@ public class TestNSSummaryAdmin extends StandardOutputTestBase {
 
   @BeforeAll
   public static void init() throws Exception {
-    conf = new OzoneConfiguration();
+    ozoneAdmin = new OzoneAdmin();
+    conf = ozoneAdmin.getOzoneConf();
     OMRequestTestUtils.configureFSOptimizedPaths(conf, true);
     conf.set(OZONE_RECON_ADDRESS_KEY, "localhost:9888");
     cluster = MiniOzoneCluster.newBuilder(conf)
@@ -67,9 +68,6 @@ public static void init() throws Exception {
     client = cluster.newClient();
     store = client.getObjectStore();
 
-    // Client uses server conf for this test
-    ozoneAdmin = new OzoneAdmin(conf);
-
     volumeName = UUID.randomUUID().toString();
     bucketOBS = UUID.randomUUID().toString();
     bucketFSO = UUID.randomUUID().toString();

diff --git a/...one/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneDebugShell.java b/...one/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneDebugShell.java
@@ -83,6 +83,7 @@ public class TestOzoneDebugShell {
 
   private static MiniOzoneCluster cluster = null;
   private static OzoneClient client;
+  private static OzoneDebug ozoneDebugShell;
 
   private static OzoneConfiguration conf = null;
 
@@ -100,7 +101,8 @@ protected static void startCluster() throws Exception {
 
   @BeforeAll
   public static void init() throws Exception {
-    conf = new OzoneConfiguration();
+    ozoneDebugShell = new OzoneDebug();
+    conf = ozoneDebugShell.getOzoneConf();
     conf.setTimeDuration(OZONE_SCM_HEARTBEAT_PROCESS_INTERVAL,
         100, TimeUnit.MILLISECONDS);
     conf.setTimeDuration(HDDS_HEARTBEAT_INTERVAL, 1, SECONDS);
@@ -206,7 +208,6 @@ private int runChunkInfoCommand(String volumeName, String bucketName,
         getSetConfStringFromConf(OMConfigKeys.OZONE_OM_ADDRESS_KEY),
         "chunkinfo", bucketPath + Path.SEPARATOR + keyName };
 
-    OzoneDebug ozoneDebugShell = new OzoneDebug(conf);
     int exitCode = ozoneDebugShell.execute(args);
     return exitCode;
   }
@@ -218,7 +219,6 @@ private int runChunkInfoAndVerifyPaths(String volumeName, String bucketName,
     String[] args = new String[] {
         getSetConfStringFromConf(OMConfigKeys.OZONE_OM_ADDRESS_KEY),
         "chunkinfo", bucketPath + Path.SEPARATOR + keyName };
-    OzoneDebug ozoneDebugShell = new OzoneDebug(conf);
     int exitCode = 1;
     try (GenericTestUtils.SystemOutCapturer capture = new GenericTestUtils
         .SystemOutCapturer()) {

diff --git a/...-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java b/...-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneShellHA.java
@@ -223,7 +223,7 @@ public static void shutdown() {
   @BeforeEach
   public void setup() throws UnsupportedEncodingException {
     ozoneShell = new OzoneShell();
-    ozoneAdminShell = new OzoneAdmin(ozoneConfiguration);
+    ozoneAdminShell = new OzoneAdmin();
     System.setOut(new PrintStream(out, false, DEFAULT_ENCODING));
     System.setErr(new PrintStream(err, false, DEFAULT_ENCODING));
   }

diff --git a/...ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestReconfigShell.java b/...ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestReconfigShell.java
@@ -67,7 +67,8 @@ public class TestReconfigShell {
    */
   @BeforeAll
   public static void setup() throws Exception {
-    OzoneConfiguration conf = new OzoneConfiguration();
+    ozoneAdmin = new OzoneAdmin();
+    OzoneConfiguration conf = ozoneAdmin.getOzoneConf();
     conf.setTimeDuration(OZONE_SCM_STALENODE_INTERVAL, 3, TimeUnit.SECONDS);
     String omServiceId = UUID.randomUUID().toString();
     cluster = MiniOzoneCluster.newHABuilder(conf)
@@ -77,7 +78,6 @@ public static void setup() throws Exception {
         .setNumDatanodes(DATANODE_COUNT)
         .build();
     cluster.waitForClusterToBeReady();
-    ozoneAdmin = new OzoneAdmin(cluster.getConf());
     ozoneManager = cluster.getOzoneManager();
     storageContainerManager = cluster.getStorageContainerManager();
     datanodeServices = cluster.getHddsDatanodes();

diff --git a/...gration-test/src/test/java/org/apache/hadoop/ozone/shell/TestTransferLeadershipShell.java b/...gration-test/src/test/java/org/apache/hadoop/ozone/shell/TestTransferLeadershipShell.java
@@ -52,6 +52,7 @@ public class TestTransferLeadershipShell {
   private String scmServiceId;
   private int numOfOMs = 3;
   private int numOfSCMs = 3;
+  private OzoneAdmin ozoneAdmin;
 
   private static final long SNAPSHOT_THRESHOLD = 5;
 
@@ -62,7 +63,8 @@ public class TestTransferLeadershipShell {
    */
   @BeforeAll
   public void init() throws Exception {
-    conf = new OzoneConfiguration();
+    ozoneAdmin = new OzoneAdmin();
+    conf = ozoneAdmin.getOzoneConf();
     omServiceId = "om-service-test1";
     scmServiceId = "scm-service-test1";
     conf.setLong(ScmConfigKeys.OZONE_SCM_HA_RATIS_SNAPSHOT_THRESHOLD,
@@ -95,7 +97,6 @@ public void testOmTransfer() throws Exception {
     omList.remove(oldLeader);
     OzoneManager newLeader = omList.get(0);
     cluster.waitForClusterToBeReady();
-    OzoneAdmin ozoneAdmin = new OzoneAdmin(conf);
     String[] args1 = {"om", "transfer", "-n", newLeader.getOMNodeId()};
     ozoneAdmin.execute(args1);
     Thread.sleep(3000);
@@ -119,7 +120,6 @@ public void testScmTransfer() throws Exception {
     scmList.remove(oldLeader);
     StorageContainerManager newLeader = scmList.get(0);
 
-    OzoneAdmin ozoneAdmin = new OzoneAdmin(conf);
     String[] args1 = {"scm", "transfer", "-n", newLeader.getScmId()};
     ozoneAdmin.execute(args1);
     cluster.waitForClusterToBeReady();

diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/OzoneDebug.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/OzoneDebug.java
@@ -18,13 +18,11 @@
 
 package org.apache.hadoop.ozone.debug;
 
-import com.google.common.annotations.VisibleForTesting;
 import org.apache.hadoop.hdds.cli.DebugSubcommand;
 import org.apache.hadoop.hdds.cli.ExtensibleParentCommand;
 import org.apache.hadoop.hdds.cli.GenericCli;
 import org.apache.hadoop.hdds.cli.HddsVersionProvider;
 
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import picocli.CommandLine;
 
 /**
@@ -36,33 +34,7 @@
         mixinStandardHelpOptions = true)
 public class OzoneDebug extends GenericCli implements ExtensibleParentCommand {
 
-  private OzoneConfiguration ozoneConf;
-
-  public OzoneDebug() {
-    super(OzoneDebug.class);
-  }
-
-  @VisibleForTesting
-  public OzoneDebug(OzoneConfiguration configuration) {
-    super(OzoneDebug.class);
-    this.ozoneConf = configuration;
-  }
-
-  public OzoneConfiguration getOzoneConf() {
-    if (ozoneConf == null) {
-      ozoneConf = createOzoneConfiguration();
-    }
-    return ozoneConf;
-  }
-
-  /**
-     * Main for the Ozone Debug shell Command handling.
-     *
-     * @param argv - System Args Strings[]
-     * @throws Exception
-     */
-  public static void main(String[] argv) throws Exception {
-
+  public static void main(String[] argv) {
     new OzoneDebug().run(argv);
   }
 

diff --git a/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/Freon.java b/hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/freon/Freon.java
@@ -84,10 +84,6 @@ public class Freon extends GenericCli {
 
   public static final Logger LOG = LoggerFactory.getLogger(Freon.class);
 
-  public Freon() {
-    super(Freon.class);
-  }
-
   @Option(names = "--server",
       description = "Enable internal http server to provide metric "
           + "and profile endpoint")