HDDS-11851. Finer-grained subcommand interface for OzoneDebug and OzoneRepair

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

@@ -19,15 +19,13 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Map.Entry;
-import java.util.ServiceLoader;
 import java.util.concurrent.Callable;
 
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 
 import com.google.common.annotations.VisibleForTesting;
 import picocli.CommandLine;
-import picocli.CommandLine.Command;
 import picocli.CommandLine.ExitCode;
 import picocli.CommandLine.Model.CommandSpec;
 import picocli.CommandLine.Option;
@@ -67,26 +65,11 @@ public GenericCli(Class<?> type, CommandLine.IFactory factory) {
     });
 
     if (type != null) {
-      addSubcommands(cmd, type);
+      SubcommandWithParent.addSubcommands(getCmd(), type);
     }
-
     ExtensibleParentCommand.addSubcommands(cmd);
   }
 
-  private void addSubcommands(CommandLine cli, Class<?> type) {
-    ServiceLoader<SubcommandWithParent> registeredSubcommands =
-        ServiceLoader.load(SubcommandWithParent.class);
-    for (SubcommandWithParent subcommand : registeredSubcommands) {
-      if (subcommand.getParentType().equals(type)) {
-        final Command commandAnnotation =
-            subcommand.getClass().getAnnotation(Command.class);
-        CommandLine subcommandCommandLine = new CommandLine(subcommand, cli.getFactory());
-        addSubcommands(subcommandCommandLine, subcommand.getClass());
-        cli.addSubcommand(commandAnnotation.name(), subcommandCommandLine);
-      }
-    }
-  }
-
   /**
    * Handle the error when subcommand is required but not set.
    */

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

@@ -17,11 +17,32 @@
  */
 package org.apache.hadoop.hdds.cli;
 
+import picocli.CommandLine;
+
+import java.util.ServiceLoader;
+
 /**
- * Defineds parent command for SPI based subcommand registration.
+ * Defines parent command for SPI based subcommand registration.
+ * @deprecated use more specific interfaces
+ * @see ExtensibleParentCommand
  */
+@Deprecated
 public interface SubcommandWithParent {
 
+  static void addSubcommands(CommandLine cli, Class<?> type) {
+    ServiceLoader<SubcommandWithParent> registeredSubcommands =
+        ServiceLoader.load(SubcommandWithParent.class);
+    for (SubcommandWithParent subcommand : registeredSubcommands) {
+      if (subcommand.getParentType().equals(type)) {
+        final CommandLine.Command commandAnnotation =
+            subcommand.getClass().getAnnotation(CommandLine.Command.class);
+        CommandLine subcommandCommandLine = new CommandLine(subcommand);
+        addSubcommands(subcommandCommandLine, subcommand.getClass());
+        cli.addSubcommand(commandAnnotation.name(), subcommandCommandLine);
+      }
+    }
+  }
+
   /**
    * Java type of the parent command.
    */

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

@@ -0,0 +1,23 @@
+/*
+ * 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.cli;
+
+/** Marker interface for subcommands to be added to {@code OzoneDebug}. */
+public interface DebugSubcommand {
+  // marker
+}

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

@@ -0,0 +1,23 @@
+/*
+ * 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.cli;
+
+/** Marker interface for subcommands to be added to {@code OzoneRepair}. */
+public interface RepairSubcommand {
+  // marker
+}

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/debug/TestLDBCli.java

@@ -38,7 +38,6 @@
 import org.apache.hadoop.ozone.container.metadata.DatanodeSchemaThreeDBDefinition;
 import org.apache.hadoop.ozone.debug.ldb.DBScanner;
 import org.apache.hadoop.ozone.debug.ldb.RDBParser;
-import org.apache.hadoop.ozone.debug.ldb.ValueSchema;
 import org.apache.hadoop.ozone.om.helpers.OmKeyInfo;
 import org.apache.hadoop.ozone.om.request.OMRequestTestUtils;
 import jakarta.annotation.Nonnull;
@@ -103,8 +102,6 @@ public void setup() throws IOException {
     pstderr = new PrintWriter(stderr);
 
     cmd = new CommandLine(new RDBParser())
-        .addSubcommand(new DBScanner())
-        .addSubcommand(new ValueSchema())
         .setOut(pstdout)
         .setErr(pstderr);
 

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneDebugShell.java

@@ -36,7 +36,6 @@
 import org.apache.hadoop.ozone.client.OzoneClient;
 import org.apache.hadoop.ozone.client.OzoneClientFactory;
 import org.apache.hadoop.ozone.client.OzoneSnapshot;
-import org.apache.hadoop.ozone.debug.ldb.DBScanner;
 import org.apache.hadoop.ozone.debug.OzoneDebug;
 import org.apache.hadoop.ozone.debug.ldb.RDBParser;
 import org.apache.hadoop.ozone.om.OMConfigKeys;
@@ -149,7 +148,6 @@ public void testLdbCliForOzoneSnapshot() throws Exception {
     StringWriter stdout = new StringWriter();
     PrintWriter pstdout = new PrintWriter(stdout);
     CommandLine cmd = new CommandLine(new RDBParser())
-        .addSubcommand(new DBScanner())
         .setOut(pstdout);
     final String volumeName = UUID.randomUUID().toString();
     final String bucketName = UUID.randomUUID().toString();

hadoop-ozone/integration-test/src/test/java/org/apache/hadoop/ozone/shell/TestOzoneRepairShell.java

@@ -19,15 +19,10 @@
 
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.ozone.MiniOzoneCluster;
-import org.apache.hadoop.ozone.debug.ldb.DBScanner;
 import org.apache.hadoop.ozone.debug.ldb.RDBParser;
 import org.apache.hadoop.ozone.om.OMStorage;
 import org.apache.hadoop.ozone.repair.OzoneRepair;
 import org.apache.hadoop.ozone.repair.ldb.RDBRepair;
-import org.apache.hadoop.ozone.repair.ldb.TransactionInfoRepair;
-import org.apache.hadoop.ozone.repair.quota.QuotaRepair;
-import org.apache.hadoop.ozone.repair.quota.QuotaStatus;
-import org.apache.hadoop.ozone.repair.quota.QuotaTrigger;
 import org.apache.ozone.test.GenericTestUtils;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
@@ -89,7 +84,7 @@ public void reset() {
 
   @Test
   public void testUpdateTransactionInfoTable() throws Exception {
-    CommandLine cmd = new CommandLine(new RDBRepair()).addSubcommand(new TransactionInfoRepair());
+    CommandLine cmd = new CommandLine(new RDBRepair());
     String dbPath = new File(OMStorage.getOmDbDir(conf) + "/" + OM_DB_NAME).getPath();
 
     cluster.getOzoneManager().stop();
@@ -120,7 +115,7 @@ public void testUpdateTransactionInfoTable() throws Exception {
   }
 
   private String scanTransactionInfoTable(String dbPath) throws Exception {
-    CommandLine cmdDBScanner = new CommandLine(new RDBParser()).addSubcommand(new DBScanner());
+    CommandLine cmdDBScanner = new CommandLine(new RDBParser());
     String[] argsDBScanner =
         new String[] {"--db=" + dbPath, "scan", "--column_family", "transactionInfoTable"};
     cmdDBScanner.execute(argsDBScanner);
@@ -138,12 +133,11 @@ private String[] parseScanOutput(String output) throws IOException {
 
   @Test
   public void testQuotaRepair() throws Exception {
-    CommandLine cmd = new CommandLine(new OzoneRepair()).addSubcommand(new CommandLine(new QuotaRepair())
-        .addSubcommand(new QuotaStatus()).addSubcommand(new QuotaTrigger()));
+    CommandLine cmd = new OzoneRepair().getCmd();
 
     String[] args = new String[] {"quota", "status", "--service-host", conf.get(OZONE_OM_ADDRESS_KEY)};
     int exitCode = cmd.execute(args);
-    assertEquals(0, exitCode);
+    assertEquals(0, exitCode, err::toString);
     args = new String[] {"quota", "start", "--service-host", conf.get(OZONE_OM_ADDRESS_KEY)};
     exitCode = cmd.execute(args);
     assertEquals(0, exitCode);

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/CompactionLogDagPrinter.java

@@ -18,7 +18,7 @@
 
 package org.apache.hadoop.ozone.debug;
 
-import org.apache.hadoop.hdds.cli.SubcommandWithParent;
+import org.apache.hadoop.hdds.cli.DebugSubcommand;
 import org.apache.hadoop.ozone.client.OzoneClient;
 import org.apache.hadoop.ozone.shell.Handler;
 import org.apache.hadoop.ozone.shell.OzoneAddress;
@@ -35,9 +35,9 @@
     name = "print-log-dag",
     aliases = "pld",
     description = "Create an image of the current compaction log DAG in OM.")
-@MetaInfServices(SubcommandWithParent.class)
+@MetaInfServices(DebugSubcommand.class)
 public class CompactionLogDagPrinter extends Handler
-    implements SubcommandWithParent {
+    implements DebugSubcommand {
 
   @CommandLine.Option(names = {"-f", "--file-name-prefix"},
       description = "Prefix to be use in image file name. (optional)")
@@ -55,11 +55,6 @@ public class CompactionLogDagPrinter extends Handler
       defaultValue = "file_name")
   private String graphType;
 
-  @Override
-  public Class<?> getParentType() {
-    return OzoneDebug.class;
-  }
-
   @Override
   protected void execute(OzoneClient client, OzoneAddress address)
       throws IOException {

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/FindMissingPadding.java

@@ -17,7 +17,7 @@
  */
 package org.apache.hadoop.ozone.debug;
 
-import org.apache.hadoop.hdds.cli.SubcommandWithParent;
+import org.apache.hadoop.hdds.cli.DebugSubcommand;
 import org.apache.hadoop.hdds.client.ECReplicationConfig;
 import org.apache.hadoop.hdds.client.StandaloneReplicationConfig;
 import org.apache.hadoop.hdds.protocol.datanode.proto.ContainerProtos;
@@ -74,11 +74,8 @@
 @CommandLine.Command(name = "find-missing-padding",
     aliases = { "fmp" },
     description = "List all keys with any missing padding, optionally limited to a volume/bucket/key URI.")
-@MetaInfServices(SubcommandWithParent.class)
-public class FindMissingPadding extends Handler implements SubcommandWithParent {
-
-  @CommandLine.ParentCommand
-  private OzoneDebug parent;
+@MetaInfServices(DebugSubcommand.class)
+public class FindMissingPadding extends Handler implements DebugSubcommand {
 
   @CommandLine.Mixin
   private ScmOption scmOption;
@@ -100,11 +97,6 @@ protected OzoneAddress getAddress() throws OzoneClientException {
     return new OzoneAddress(uri);
   }
 
-  @Override
-  public Class<?> getParentType() {
-    return OzoneDebug.class;
-  }
-
   @Override
   protected void execute(OzoneClient ozoneClient, OzoneAddress address) throws IOException {
     findCandidateKeys(ozoneClient, address);

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/LeaseRecoverer.java

@@ -24,7 +24,7 @@
 import org.apache.hadoop.fs.FileSystem;
 import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.fs.LeaseRecoverable;
-import org.apache.hadoop.hdds.cli.SubcommandWithParent;
+import org.apache.hadoop.hdds.cli.DebugSubcommand;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 
 import org.kohsuke.MetaInfServices;
@@ -40,11 +40,8 @@
     customSynopsis = "ozone debug recover --path=<path>",
     description = "recover the lease of a specified file. Make sure to specify "
         + "file system scheme if ofs:// is not the default.")
-@MetaInfServices(SubcommandWithParent.class)
-public class LeaseRecoverer implements Callable<Void>, SubcommandWithParent {
-
-  @CommandLine.ParentCommand
-  private OzoneDebug parent;
+@MetaInfServices(DebugSubcommand.class)
+public class LeaseRecoverer implements Callable<Void>, DebugSubcommand {
 
   @Spec
   private CommandSpec spec;
@@ -62,11 +59,6 @@ public void setPath(String dbPath) {
     this.path = dbPath;
   }
 
-  @Override
-  public Class<?> getParentType() {
-    return OzoneDebug.class;
-  }
-
   @Override
   public Void call() throws Exception {
     OzoneConfiguration configuration = new OzoneConfiguration();

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/OzoneDebug.java

@@ -19,6 +19,8 @@
 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;
 
@@ -32,7 +34,7 @@
         description = "Developer tools for Ozone Debug operations",
         versionProvider = HddsVersionProvider.class,
         mixinStandardHelpOptions = true)
-public class OzoneDebug extends GenericCli {
+public class OzoneDebug extends GenericCli implements ExtensibleParentCommand {
 
   private OzoneConfiguration ozoneConf;
 
@@ -63,4 +65,9 @@ public static void main(String[] argv) throws Exception {
 
     new OzoneDebug().run(argv);
   }
+
+  @Override
+  public Class<?> subcommandType() {
+    return DebugSubcommand.class;
+  }
 }

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/debug/PrefixParser.java

@@ -26,7 +26,7 @@
 import java.util.concurrent.Callable;
 
 import java.nio.file.Path;
-import org.apache.hadoop.hdds.cli.SubcommandWithParent;
+import org.apache.hadoop.hdds.cli.DebugSubcommand;
 
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.utils.MetadataKeyFilters;
@@ -53,8 +53,8 @@
 @CommandLine.Command(
     name = "prefix",
     description = "Parse prefix contents")
-@MetaInfServices(SubcommandWithParent.class)
-public class PrefixParser implements Callable<Void>, SubcommandWithParent {
+@MetaInfServices(DebugSubcommand.class)
+public class PrefixParser implements Callable<Void>, DebugSubcommand {
 
   /**
    * Types to represent the level or path component type.
@@ -101,11 +101,6 @@ public void setDbPath(String dbPath) {
     this.dbPath = dbPath;
   }
 
-  @Override
-  public Class<?> getParentType() {
-    return OzoneDebug.class;
-  }
-
   @Override
   public Void call() throws Exception {
     parse(volume, bucket, dbPath, filePath);