HDDS-12058. Use CommandLine out/err in GenericCli subclasses

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

@@ -17,6 +17,7 @@
 package org.apache.hadoop.hdds.cli;
 
 import java.io.IOException;
+import java.io.PrintWriter;
 import java.util.Map;
 
 import com.google.common.base.Strings;
@@ -87,9 +88,9 @@ protected void printError(Throwable error) {
     //message could be null in case of NPE. This is unexpected so we can
     //print out the stack trace.
     if (verbose || Strings.isNullOrEmpty(error.getMessage())) {
-      error.printStackTrace(System.err);
+      error.printStackTrace(cmd.getErr());
     } else {
-      System.err.println(error.getMessage().split("\n")[0]);
+      cmd.getErr().println(error.getMessage().split("\n")[0]);
     }
   }
 
@@ -114,4 +115,12 @@ public CommandLine getCmd() {
   public boolean isVerbose() {
     return verbose;
   }
+
+  protected PrintWriter out() {
+    return cmd.getOut();
+  }
+
+  protected PrintWriter err() {
+    return cmd.getErr();
+  }
 }

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

@@ -92,8 +92,8 @@ public Void call() throws Exception {
       versionProvider = HddsVersionProvider.class)
   public void generateClusterId() {
     commonInit();
-    System.out.println("Generating new cluster id:");
-    System.out.println(receiver.generateClusterId());
+    out().println("Generating new cluster id:");
+    out().println(receiver.generateClusterId());
   }
 
   /**

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/conf/OzoneGetConf.java

@@ -17,8 +17,6 @@
 
 package org.apache.hadoop.ozone.conf;
 
-import java.io.PrintStream;
-
 import org.apache.hadoop.hdds.cli.GenericCli;
 import org.apache.hadoop.hdds.cli.HddsVersionProvider;
 import org.apache.hadoop.hdds.conf.OzoneConfiguration;
@@ -45,31 +43,17 @@
         OzoneManagersCommandHandler.class
     })
 public class OzoneGetConf extends GenericCli {
-  private final PrintStream out; // Stream for printing command output
-  private final PrintStream err; // Stream for printing error
-  private OzoneConfiguration conf;
-
-  protected OzoneGetConf(OzoneConfiguration conf) {
-    this(conf, System.out, System.err);
-  }
-
-  protected OzoneGetConf(OzoneConfiguration conf, PrintStream out,
-      PrintStream err) {
-    this.conf = conf;
-    this.out = out;
-    this.err = err;
-  }
 
   void printError(String message) {
-    err.println(message);
+    err().println(message);
   }
 
   void printOut(String message) {
-    out.println(message);
+    out().println(message);
   }
 
   OzoneConfiguration getConf() {
-    return this.conf;
+    return getOzoneConf();
   }
 
   public static void main(String[] argv) {
@@ -79,8 +63,6 @@ public static void main(String[] argv) {
         .addAppender(new ConsoleAppender(new PatternLayout("%m%n")));
     Logger.getLogger(NativeCodeLoader.class).setLevel(Level.ERROR);
 
-    OzoneConfiguration conf = new OzoneConfiguration();
-    conf.addResource(new OzoneConfiguration());
-    new OzoneGetConf(conf).run(argv);
+    new OzoneGetConf().run(argv);
   }
 }

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/genconf/GenerateOzoneRequiredConfigurations.java

@@ -28,7 +28,6 @@
 import picocli.CommandLine.Command;
 import picocli.CommandLine.Option;
 import picocli.CommandLine.Parameters;
-import picocli.CommandLine.PicocliException;
 
 import javax.xml.bind.JAXBContext;
 import javax.xml.bind.JAXBException;
@@ -68,50 +67,25 @@ public final class GenerateOzoneRequiredConfigurations extends GenericCli implem
       "template, update Kerberos principal and keytab file before use.")
   private boolean genSecurityConf;
 
-  /**
-   * Entry point for using genconf tool.
-   *
-   * @param args
-   *
-   */
   public static void main(String[] args) throws Exception {
     new GenerateOzoneRequiredConfigurations().run(args);
   }
 
   @Override
   public Void call() throws Exception {
-    generateConfigurations(path, genSecurityConf);
+    generateConfigurations();
     return null;
   }
 
-  /**
-   * Generate ozone-site.xml at specified path.
-   * @param path
-   * @throws PicocliException
-   * @throws JAXBException
-   */
-  public static void generateConfigurations(String path) throws
-      PicocliException, JAXBException, IOException {
-    generateConfigurations(path, false);
-  }
-
-  /**
-   * Generate ozone-site.xml at specified path.
-   * @param path
-   * @param genSecurityConf
-   * @throws PicocliException
-   * @throws JAXBException
-   */
-  public static void generateConfigurations(String path,
-      boolean genSecurityConf) throws
-      PicocliException, JAXBException, IOException {
+  private void generateConfigurations() throws
+      JAXBException, IOException {
 
     if (!isValidPath(path)) {
-      throw new PicocliException("Invalid directory path.");
+      throw new IllegalArgumentException("Invalid directory path.");
     }
 
     if (!canWrite(path)) {
-      throw new PicocliException("Insufficient permission.");
+      throw new IllegalArgumentException("Insufficient permission.");
     }
 
     OzoneConfiguration oc = new OzoneConfiguration();
@@ -171,9 +145,9 @@ public static void generateConfigurations(String path,
       m.setProperty(Marshaller.JAXB_FORMATTED_OUTPUT, Boolean.TRUE);
       m.marshal(generatedConfig, output);
 
-      System.out.println("ozone-site.xml has been generated at " + path);
+      out().println("ozone-site.xml has been generated at " + path);
     } else {
-      System.out.printf("ozone-site.xml already exists at %s and " +
+      out().printf("ozone-site.xml already exists at %s and " +
           "will not be overwritten%n", path);
     }
 
@@ -182,21 +156,19 @@ public static void generateConfigurations(String path,
   /**
    * Check if the path is valid directory.
    *
-   * @param path
    * @return true, if path is valid directory, else return false
    */
   public static boolean isValidPath(String path) {
     try {
       return Files.isDirectory(Paths.get(path));
     } catch (InvalidPathException | NullPointerException ex) {
-      return Boolean.FALSE;
+      return false;
     }
   }
 
   /**
    * Check if user has permission to write in the specified path.
    *
-   * @param path
    * @return true, if the user has permission to write, else returns false
    */
   public static boolean canWrite(String path) {

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/Shell.java

@@ -109,7 +109,7 @@ public void printError(Throwable errorArg) {
     if (omException != null && !isVerbose()) {
       // In non-verbose mode, reformat OMExceptions as error messages to the
       // user.
-      System.err.println(String.format("%s %s", omException.getResult().name(),
+      err().println(String.format("%s %s", omException.getResult().name(),
               omException.getMessage()));
     } else {
       // Prints the stack trace when in verbose mode.

hadoop-ozone/tools/src/main/java/org/apache/hadoop/ozone/shell/checknative/CheckNative.java

@@ -57,10 +57,10 @@ public Void call() throws Exception {
         isalLoaded = true;
       }
     }
-    System.out.println("Native library checking:");
-    System.out.printf("hadoop:  %b %s%n", nativeHadoopLoaded,
+    out().println("Native library checking:");
+    out().printf("hadoop:  %b %s%n", nativeHadoopLoaded,
         hadoopLibraryName);
-    System.out.printf("ISA-L:   %b %s%n", isalLoaded, isalDetail);
+    out().printf("ISA-L:   %b %s%n", isalLoaded, isalDetail);
 
     // Attempt to load the rocks-tools lib
     boolean nativeRocksToolsLoaded = NativeLibraryLoader.getInstance().loadLibrary(
@@ -70,7 +70,7 @@ public Void call() throws Exception {
     if (nativeRocksToolsLoaded) {
       rocksToolsDetail = NativeLibraryLoader.getJniLibraryFileName();
     }
-    System.out.printf("rocks-tools: %b %s%n", nativeRocksToolsLoaded, rocksToolsDetail);
+    out().printf("rocks-tools: %b %s%n", nativeRocksToolsLoaded, rocksToolsDetail);
     return null;
   }
 }

hadoop-ozone/tools/src/test/java/org/apache/hadoop/ozone/conf/TestGetConfOptions.java

@@ -17,82 +17,68 @@
  */
 package org.apache.hadoop.ozone.conf;
 
-import org.apache.hadoop.hdds.conf.OzoneConfiguration;
 import org.apache.hadoop.hdds.scm.ScmConfigKeys;
+import org.apache.hadoop.hdds.utils.IOUtils;
 import org.apache.hadoop.ozone.om.OMConfigKeys;
-import static org.junit.jupiter.api.Assertions.assertEquals;
 
+import org.apache.ozone.test.GenericTestUtils;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Test;
 
-import java.io.ByteArrayOutputStream;
-import java.io.PrintStream;
-import java.io.UnsupportedEncodingException;
-
-import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 /**
  * Tests the ozone getconf command.
  */
 public class TestGetConfOptions {
-  private static OzoneConfiguration conf;
-  private static ByteArrayOutputStream bout;
-  private static PrintStream psBackup;
-  private static final String DEFAULT_ENCODING = UTF_8.name();
+  private static GenericTestUtils.PrintStreamCapturer out;
+  private static OzoneGetConf subject;
 
   @BeforeAll
-  public static void init() throws UnsupportedEncodingException {
-    conf = new OzoneConfiguration();
-    conf.set(OMConfigKeys.OZONE_OM_NODE_ID_KEY, "1");
-    conf.set(OMConfigKeys.OZONE_OM_SERVICE_IDS_KEY, "service1");
-    conf.set(ScmConfigKeys.OZONE_SCM_NAMES, "localhost");
-    psBackup = System.out;
-    bout = new ByteArrayOutputStream();
-    PrintStream psOut = new PrintStream(bout, false, DEFAULT_ENCODING);
-    System.setOut(psOut);
+  public static void init() {
+    out = GenericTestUtils.captureOut();
+    subject = new OzoneGetConf();
+    subject.getConf().set(OMConfigKeys.OZONE_OM_NODE_ID_KEY, "1");
+    subject.getConf().set(OMConfigKeys.OZONE_OM_SERVICE_IDS_KEY, "service1");
+    subject.getConf().set(ScmConfigKeys.OZONE_SCM_NAMES, "localhost");
   }
 
   @AfterEach
   public void setUp() {
-    bout.reset();
+    out.reset();
   }
 
   @AfterAll
   public static void tearDown() {
-    System.setOut(psBackup);
+    IOUtils.closeQuietly(out);
   }
 
   @Test
-  public void testGetConfWithTheOptionConfKey()
-      throws UnsupportedEncodingException {
-    new OzoneGetConf(conf)
-        .run(new String[] {"-confKey", ScmConfigKeys.OZONE_SCM_NAMES});
-    assertEquals("localhost\n", bout.toString(DEFAULT_ENCODING));
-    bout.reset();
-    new OzoneGetConf(conf)
-        .run(new String[] {"confKey", OMConfigKeys.OZONE_OM_NODE_ID_KEY});
-    assertEquals("1\n", bout.toString(DEFAULT_ENCODING));
+  public void testGetConfWithTheOptionConfKey() {
+    subject.run(new String[] {"-confKey", ScmConfigKeys.OZONE_SCM_NAMES});
+    assertEquals("localhost\n", out.get());
+    out.reset();
+    subject.run(new String[] {"confKey", OMConfigKeys.OZONE_OM_NODE_ID_KEY});
+    assertEquals("1\n", out.get());
   }
 
   @Test
-  public void testGetConfWithTheOptionStorageContainerManagers()
-      throws UnsupportedEncodingException {
-    new OzoneGetConf(conf).run(new String[] {"-storagecontainermanagers"});
-    assertEquals("localhost\n", bout.toString(DEFAULT_ENCODING));
-    bout.reset();
-    new OzoneGetConf(conf).run(new String[] {"storagecontainermanagers"});
-    assertEquals("localhost\n", bout.toString(DEFAULT_ENCODING));
+  public void testGetConfWithTheOptionStorageContainerManagers() {
+    subject.execute(new String[] {"-storagecontainermanagers"});
+    assertEquals("localhost\n", out.get());
+    out.reset();
+    subject.execute(new String[] {"storagecontainermanagers"});
+    assertEquals("localhost\n", out.get());
   }
 
   @Test
-  public void testGetConfWithTheOptionOzoneManagers()
-      throws UnsupportedEncodingException {
-    new OzoneGetConf(conf).run(new String[] {"-ozonemanagers"});
-    assertEquals("", bout.toString(DEFAULT_ENCODING));
-    bout.reset();
-    new OzoneGetConf(conf).run(new String[] {"ozonemanagers"});
-    assertEquals("", bout.toString(DEFAULT_ENCODING));
+  public void testGetConfWithTheOptionOzoneManagers() {
+    subject.execute(new String[] {"-ozonemanagers"});
+    assertEquals("", out.get());
+    out.reset();
+    subject.execute(new String[] {"ozonemanagers"});
+    assertEquals("", out.get());
   }
 }