HDDS-12012. Defer ozone repair prompt after subcommand validation

hadoop-ozone/integration-test/pom.xml

@@ -272,6 +272,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
       <artifactId>assertj-core</artifactId>
       <version>${assertj.version}</version>
     </dependency>
+    <dependency>
+      <groupId>com.github.stefanbirkner</groupId>
+      <artifactId>system-lambda</artifactId>
+      <scope>test</scope>
+    </dependency>
   </dependencies>
 
   <build>

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

@@ -34,7 +34,6 @@
 import org.apache.hadoop.ozone.om.OMStorage;
 import org.apache.hadoop.ozone.om.helpers.BucketLayout;
 import org.apache.hadoop.ozone.om.helpers.OmDirectoryInfo;
-import org.apache.hadoop.ozone.repair.OzoneRepair;
 import org.apache.ozone.test.GenericTestUtils;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -48,7 +47,6 @@
 import org.junit.jupiter.params.provider.ValueSource;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import picocli.CommandLine;
 
 import java.io.File;
 import java.io.IOException;
@@ -60,6 +58,7 @@
 import static org.apache.hadoop.ozone.OzoneConsts.OM_DB_NAME;
 import static org.apache.hadoop.ozone.OzoneConsts.OZONE_OFS_URI_SCHEME;
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY;
+import static org.apache.hadoop.ozone.shell.TestOzoneRepairShell.executeRepair;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -81,7 +80,6 @@ public class TestFSORepairTool {
   private static MiniOzoneCluster cluster;
   private static FileSystem fs;
   private static OzoneClient client;
-  private static CommandLine cmd;
   private static String dbPath;
   private static FSORepairTool.Report vol1Report;
   private static FSORepairTool.Report vol2Report;
@@ -104,7 +102,6 @@ public static void setup() throws Exception {
 
     out = GenericTestUtils.captureOut();
     err = GenericTestUtils.captureErr();
-    cmd = new OzoneRepair().getCmd();
     dbPath = new File(OMStorage.getOmDbDir(conf) + "/" + OM_DB_NAME).getPath();
 
     // Build multiple connected and disconnected trees
@@ -350,7 +347,7 @@ private int execute(boolean dryRun, String... args) {
     }
     argList.addAll(Arrays.asList(args));
 
-    return cmd.execute(argList.toArray(new String[0]));
+    return executeRepair(argList.toArray(new String[0]));
   }
 
   private <K, V> int countTableEntries(Table<K, V> table) throws Exception {

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

@@ -33,9 +33,11 @@
 import picocli.CommandLine;
 
 import java.io.File;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import static com.github.stefanbirkner.systemlambda.SystemLambda.withTextFromSystemIn;
 import static org.apache.hadoop.ozone.OzoneConsts.OM_DB_NAME;
 import static org.apache.hadoop.ozone.om.OMConfigKeys.OZONE_OM_ADDRESS_KEY;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -79,7 +81,6 @@ static void cleanup() {
 
   @Test
   public void testUpdateTransactionInfoTable() throws Exception {
-    CommandLine cmd = new OzoneRepair().getCmd();
     String dbPath = new File(OMStorage.getOmDbDir(conf) + "/" + OM_DB_NAME).getPath();
 
     cluster.getOzoneManager().stop();
@@ -89,7 +90,7 @@ public void testUpdateTransactionInfoTable() throws Exception {
 
     String testTerm = "1111";
     String testIndex = "1111";
-    int exitCode = cmd.execute("om", "update-transaction", "--db", dbPath, "--term", testTerm, "--index", testIndex);
+    int exitCode = executeRepair("om", "update-transaction", "--db", dbPath, "--term", testTerm, "--index", testIndex);
     assertEquals(0, exitCode, err);
     assertThat(out.get())
         .contains(
@@ -101,7 +102,7 @@ public void testUpdateTransactionInfoTable() throws Exception {
     String cmdOut2 = scanTransactionInfoTable(dbPath);
     assertThat(cmdOut2).contains(testTerm + "#" + testIndex);
 
-    cmd.execute("om", "update-transaction", "--db", dbPath, "--term",
+    executeRepair("om", "update-transaction", "--db", dbPath, "--term",
         originalHighestTermIndex[0], "--index", originalHighestTermIndex[1]);
     cluster.getOzoneManager().restart();
     try (OzoneClient ozoneClient = cluster.newClient()) {
@@ -130,7 +131,7 @@ public void testQuotaRepair() throws Exception {
 
     int exitCode = cmd.execute("quota", "status", "--service-host", conf.get(OZONE_OM_ADDRESS_KEY));
     assertEquals(0, exitCode, err);
-    exitCode = cmd.execute("quota", "start", "--service-host", conf.get(OZONE_OM_ADDRESS_KEY));
+    exitCode = executeRepair("quota", "start", "--service-host", conf.get(OZONE_OM_ADDRESS_KEY));
     assertEquals(0, exitCode, err);
     GenericTestUtils.waitFor(() -> {
       out.reset();
@@ -144,4 +145,23 @@ public void testQuotaRepair() throws Exception {
       return false;
     }, 1000, 10000);
   }
+
+  /**
+   * Executes "ozone repair" command, passing "y" to confirmation prompt.
+   * @param args arguments to pass to the command
+   * @return exit code
+   */
+  public static int executeRepair(String... args) {
+    final CommandLine cmd = new OzoneRepair().getCmd();
+    final AtomicInteger rc = new AtomicInteger();
+    try {
+      withTextFromSystemIn("y")
+          .execute(() -> rc.set(cmd.execute(args)));
+    } catch (RuntimeException e) {
+      throw e;
+    } catch (Exception e) {
+      throw new RuntimeException(e);
+    }
+    return rc.get();
+  }
 }

hadoop-ozone/tools/pom.xml

@@ -256,6 +256,11 @@ https://maven.apache.org/xsd/maven-4.0.0.xsd">
     </dependency>
 
     <!-- Test dependencies -->
+    <dependency>
+      <groupId>com.github.stefanbirkner</groupId>
+      <artifactId>system-lambda</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.apache.ozone</groupId>
       <artifactId>hdds-hadoop-dependency-test</artifactId>

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/repair/OzoneRepair.java

@@ -24,9 +24,6 @@
 import org.apache.hadoop.hdds.cli.RepairSubcommand;
 import picocli.CommandLine;
 
-import java.nio.charset.StandardCharsets;
-import java.util.Scanner;
-
 /**
  * Ozone Repair Command line tool.
  */
@@ -37,39 +34,10 @@
     mixinStandardHelpOptions = true)
 public class OzoneRepair extends GenericCli implements ExtensibleParentCommand {
 
-  public static final String WARNING_SYS_USER_MESSAGE =
-      "ATTENTION: Running as user %s. Make sure this is the same user used to run the Ozone process." +
-          " Are you sure you want to continue (y/N)? ";
-
   public static void main(String[] argv) {
     new OzoneRepair().run(argv);
   }
 
-  @Override
-  public int execute(String[] argv) {
-    if (argv.length == 0 || argv[0].equals("--help") || argv[0].equals("-h")) {
-      return super.execute(argv);
-    }
-
-    String currentUser = getSystemUserName();
-    if (!("y".equalsIgnoreCase(getConsoleReadLineWithFormat(currentUser)))) {
-      System.out.println("Aborting command.");
-      return 1;
-    }
-    System.out.println("Run as user: " + currentUser);
-
-    return super.execute(argv);
-  }
-
-  public  String getSystemUserName() {
-    return System.getProperty("user.name");
-  }
-
-  public  String getConsoleReadLineWithFormat(String currentUser) {
-    System.err.printf(WARNING_SYS_USER_MESSAGE, currentUser);
-    return (new Scanner(System.in, StandardCharsets.UTF_8.name())).nextLine().trim();
-  }
-
   @Override
   public Class<?> subcommandType() {
     return RepairSubcommand.class;

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/repair/RepairTool.java

@@ -21,11 +21,17 @@
 import picocli.CommandLine;
 
 import java.io.PrintWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.Scanner;
 import java.util.concurrent.Callable;
 
 /** Parent class for all actionable repair commands. */
 public abstract class RepairTool extends AbstractSubcommand implements Callable<Void> {
 
+  private static final String WARNING_SYS_USER_MESSAGE =
+      "ATTENTION: Running as user %s. Make sure this is the same user used to run the Ozone process." +
+          " Are you sure you want to continue (y/N)? ";
+
   @CommandLine.Option(names = {"--force"},
       description = "Use this flag if you want to bypass the check in false-positive cases.")
   private boolean force;
@@ -35,6 +41,7 @@ public abstract class RepairTool extends AbstractSubcommand implements Callable<
 
   @Override
   public final Void call() throws Exception {
+    confirmUser();
     execute();
     return null;
   }
@@ -84,4 +91,25 @@ private String formatMessage(String msg, Object[] args) {
     return msg;
   }
 
+  protected void confirmUser() {
+    final String currentUser = getSystemUserName();
+    final boolean confirmed = "y".equalsIgnoreCase(getConsoleReadLineWithFormat(currentUser));
+
+    if (!confirmed) {
+      throw new IllegalStateException("Aborting command.");
+    }
+
+    info("Run as user: " + currentUser);
+  }
+
+  private String getSystemUserName() {
+    return System.getProperty("user.name");
+  }
+
+  private String getConsoleReadLineWithFormat(String currentUser) {
+    err().printf(WARNING_SYS_USER_MESSAGE, currentUser);
+    return new Scanner(System.in, StandardCharsets.UTF_8.name())
+        .nextLine()
+        .trim();
+  }
 }

hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/repair/TestOzoneRepair.java

@@ -20,14 +20,20 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+import picocli.CommandLine;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
 import java.io.PrintStream;
+import java.util.List;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
+import static java.util.Arrays.asList;
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 
 /**
  * Tests the ozone repair command.
@@ -67,9 +73,9 @@ void testOzoneRepairWhenUserIsRemindedSystemUserAndDeclinesToProceed() throws Ex
     OzoneRepair ozoneRepair = new OzoneRepair();
     System.setIn(new ByteArrayInputStream("N".getBytes(DEFAULT_ENCODING)));
 
-    int res = ozoneRepair.execute(new String[]{"om", "fso-tree"});
-    assertEquals(1, res);
-    assertThat(out.toString(DEFAULT_ENCODING)).contains("Aborting command.");
+    int res = ozoneRepair.execute(new String[]{"om", "fso-tree", "--db", "/dev/null"});
+    assertThat(res).isNotEqualTo(CommandLine.ExitCode.OK);
+    assertThat(err.toString(DEFAULT_ENCODING)).contains("Aborting command.");
     // prompt should contain the current user name as well
     assertThat(err.toString(DEFAULT_ENCODING)).contains("ATTENTION: Running as user " + OZONE_USER);
   }
@@ -79,30 +85,30 @@ void testOzoneRepairWhenUserIsRemindedSystemUserAndAgreesToProceed() throws Exce
     OzoneRepair ozoneRepair = new OzoneRepair();
     System.setIn(new ByteArrayInputStream("y".getBytes(DEFAULT_ENCODING)));
 
-    ozoneRepair.execute(new String[]{"om", "fso-tree"});
+    ozoneRepair.execute(new String[]{"om", "fso-tree", "--db", "/dev/null"});
     assertThat(out.toString(DEFAULT_ENCODING)).contains("Run as user: " + OZONE_USER);
     // prompt should contain the current user name as well
     assertThat(err.toString(DEFAULT_ENCODING)).contains("ATTENTION: Running as user " + OZONE_USER);
   }
 
-  @Test
-  void testOzoneRepairSkipsPromptWhenNoSubcommandProvided() throws Exception {
-    OzoneRepair ozoneRepair = new OzoneRepair();
-
-    // when no argument is passed, prompt should not be displayed
-    ozoneRepair.execute(new String[]{});
-    assertThat(err.toString(DEFAULT_ENCODING)).doesNotContain("ATTENTION: Running as user " + OZONE_USER);
+  /** Arguments for which confirmation prompt should not be displayed. */
+  static List<List<String>> skipPromptParams() {
+    return asList(
+        emptyList(),
+        singletonList("om"),
+        asList("om", "fso-tree"),
+        asList("om", "fso-tree", "-h"),
+        asList("om", "fso-tree", "--help")
+    );
   }
 
-  @Test
-  void testOzoneRepairSkipsPromptWhenHelpFlagProvided() throws Exception {
+  @ParameterizedTest
+  @MethodSource("skipPromptParams")
+  void testSkipsPrompt(List<String> args) throws Exception {
     OzoneRepair ozoneRepair = new OzoneRepair();
 
-    // when --help or -h flag is passed, prompt should not be displayed
-    ozoneRepair.execute(new String[]{"--help"});
-    assertThat(err.toString(DEFAULT_ENCODING)).doesNotContain("ATTENTION: Running as user " + OZONE_USER);
+    ozoneRepair.execute(args.toArray(new String[0]));
 
-    ozoneRepair.execute(new String[]{"-h"});
     assertThat(err.toString(DEFAULT_ENCODING)).doesNotContain("ATTENTION: Running as user " + OZONE_USER);
   }
 

hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/repair/om/TestTransactionInfoRepair.java

@@ -33,6 +33,7 @@
 import org.rocksdb.RocksDBException;
 import picocli.CommandLine;
 
+import static com.github.stefanbirkner.systemlambda.SystemLambda.withTextFromSystemIn;
 import static org.apache.hadoop.ozone.OzoneConsts.TRANSACTION_INFO_KEY;
 import static org.apache.hadoop.ozone.om.OmMetadataManagerImpl.TRANSACTION_INFO_TABLE;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -69,7 +70,7 @@ void cleanup() {
   }
 
   @Test
-  public void testUpdateTransactionInfoTableSuccessful() {
+  public void testUpdateTransactionInfoTableSuccessful() throws Exception {
     ManagedRocksDB mdb = mockRockDB();
     testCommand(mdb, mock(ColumnFamilyHandle.class));
 
@@ -81,7 +82,7 @@ public void testUpdateTransactionInfoTableSuccessful() {
   }
 
   @Test
-  public void testCommandWhenTableNotInDBForGivenPath() {
+  public void testCommandWhenTableNotInDBForGivenPath() throws Exception {
     ManagedRocksDB mdb = mockRockDB();
     testCommand(mdb, null);
     assertThat(err.getOutput())
@@ -104,7 +105,7 @@ public void testCommandWhenFailToUpdateRocksDBForGivenPath() throws Exception {
   }
 
 
-  private void testCommand(ManagedRocksDB mdb, ColumnFamilyHandle columnFamilyHandle) {
+  private void testCommand(ManagedRocksDB mdb, ColumnFamilyHandle columnFamilyHandle) throws Exception {
     try (MockedStatic<ManagedRocksDB> mocked = mockStatic(ManagedRocksDB.class);
          MockedStatic<RocksDBUtils> mockUtil = mockStatic(RocksDBUtils.class)) {
       mocked.when(() -> ManagedRocksDB.open(anyString(), anyList(), anyList())).thenReturn(mdb);
@@ -125,12 +126,14 @@ private void testCommand(ManagedRocksDB mdb, ColumnFamilyHandle columnFamilyHand
           .thenReturn(transactionInfo2);
 
       CommandLine cli = new OzoneRepair().getCmd();
-      cli.execute(
-          "om",
-          "update-transaction",
-          "--db", DB_PATH,
-          "--term", String.valueOf(TEST_TERM),
-          "--index", String.valueOf(TEST_INDEX));
+      withTextFromSystemIn("y")
+          .execute(() -> cli.execute(
+              "om",
+              "update-transaction",
+              "--db", DB_PATH,
+              "--term", String.valueOf(TEST_TERM),
+              "--index", String.valueOf(TEST_INDEX)
+          ));
     }
   }