Address review comments.

LuQQiu · Dec 21, 2017 · f92346f · f92346f
1 parent 39d444a
commit f92346f
Show file tree

Hide file tree

Showing 3 changed files with 12 additions and 25 deletions.
diff --git a/core/server/common/src/main/java/alluxio/cli/validation/AbstractValidationTask.java b/core/server/common/src/main/java/alluxio/cli/validation/AbstractValidationTask.java
@@ -20,17 +20,11 @@
  * Abstract class for validation environment.
  */
 public abstract class AbstractValidationTask implements ValidationTask {
-  /**
-   * {@inheritDoc}
-   */
   @Override
   public List<Option> getOptionList() {
     return new ArrayList<>();
   }
 
-  /**
-   * {@inheritDoc}
-   */
   @Override
   public boolean shouldSkip() {
     return false;

diff --git a/core/server/common/src/main/java/alluxio/cli/validation/HdfsValidationTask.java b/core/server/common/src/main/java/alluxio/cli/validation/HdfsValidationTask.java
@@ -39,9 +39,6 @@ public class HdfsValidationTask extends AbstractValidationTask {
    */
   public HdfsValidationTask() {}
 
-  /**
-   * {@inheritDoc}
-   */
   @Override
   public List<Option> getOptionList() {
     List<Option> opts = new ArrayList<>();
@@ -61,16 +58,12 @@ public boolean validate(Map<String, String> optionsMap) {
     return true;
   }
 
-  /**
-   * {@inheritDoc}
-   */
   @Override
   public boolean shouldSkip() {
     String scheme =
         new AlluxioURI(Configuration.get(PropertyKey.MASTER_MOUNT_TABLE_ROOT_UFS)).getScheme();
     if (scheme == null || !scheme.startsWith("hdfs")) {
-      System.err.format("Root underFS is %s, not HDFS. Skipping validation for HDFS properties.%n",
-          scheme);
+      System.err.format("Root underFS is not HDFS. Skipping validation for HDFS properties.%n");
       return true;
     }
     return false;
@@ -85,7 +78,7 @@ private boolean validateHdfsSettingParity(Map<String, String> optionsMap) {
     }
     if (serverHadoopConfDirPath == null) {
       System.err.println("Path to server-side hadoop configuration unspecified,"
-          + " skipping validation for HDFS properties.%n");
+          + " skipping validation for HDFS properties.");
       return true;
     }
     String serverCoreSiteFilePath = PathUtils.concatPath(serverHadoopConfDirPath, "/core-site.xml");
@@ -115,12 +108,12 @@ private boolean validateHdfsSettingParity(Map<String, String> optionsMap) {
     }
     if (clientCoreSiteFilePath == null || clientCoreSiteFilePath.isEmpty()) {
       System.err.println("Cannot locate the client-side core-site.xml,"
-          + " skipping validation for HDFS properties.%n");
+          + " skipping validation for HDFS properties.");
       return true;
     }
     if (clientHdfsSiteFilePath == null || clientHdfsSiteFilePath.isEmpty()) {
       System.err.println("Cannot locate the client-side hdfs-site.xml,"
-          + " skipping validation for HDFS properties.%n");
+          + " skipping validation for HDFS properties.");
       return true;
     }
     return compareConfigurations(clientCoreSiteFilePath, serverCoreSiteFilePath)

diff --git a/core/server/common/src/main/java/alluxio/cli/validation/SecureHdfsValidationTask.java b/core/server/common/src/main/java/alluxio/cli/validation/SecureHdfsValidationTask.java
@@ -46,6 +46,8 @@ public final class SecureHdfsValidationTask extends HdfsValidationTask {
       PRINCIPAL_MAP_WORKER_KEY, PropertyKey.WORKER_KEYTAB_FILE);
 
   private final String mProcess;
+  private PropertyKey mPrincipalProperty;
+  private PropertyKey mKeytabProperty;
 
   /**
    * Constructor of {@link SecureHdfsValidationTask}.
@@ -54,6 +56,8 @@ public final class SecureHdfsValidationTask extends HdfsValidationTask {
    */
   public SecureHdfsValidationTask(String process) {
     mProcess = process.toLowerCase();
+    mPrincipalProperty = PRINCIPAL_MAP.get(mProcess);
+    mKeytabProperty = KEYTAB_MAP.get(mProcess);
   }
 
   @Override
@@ -65,23 +69,19 @@ public boolean validate(Map<String, String> optionsMap) {
       return false;
     }
     if (!validatePrincipalLogin()) {
-      System.err.format("Principal login test failed.%n");
       return false;
     }
     return true;
   }
 
-  /**
-   * {@inheritDoc}
-   */
   @Override
   public boolean shouldSkip() {
     if (super.shouldSkip()) {
       return true;
     }
     String principal = null;
-    if (Configuration.containsKey(PRINCIPAL_MAP.get(mProcess))) {
-      principal = Configuration.get(PRINCIPAL_MAP.get(mProcess));
+    if (Configuration.containsKey(mPrincipalProperty)) {
+      principal = Configuration.get(mPrincipalProperty);
     }
     if (principal == null || principal.isEmpty()) {
       System.err.format("Skip validation for secure HDFS. %s is not specified.%n",
@@ -93,7 +93,7 @@ public boolean shouldSkip() {
 
   private boolean validatePrincipalLogin() {
     // Check whether can login with specified principal and keytab
-    String principal = Configuration.get(PRINCIPAL_MAP.get(mProcess));
+    String principal = Configuration.get(mPrincipalProperty);
     Matcher matchPrincipal = PRINCIPAL_PATTERN.matcher(principal);
     if (!matchPrincipal.matches()) {
       System.err.format("Principal %s is not in the right format.%n", principal);
@@ -104,7 +104,7 @@ private boolean validatePrincipalLogin() {
     String realm = matchPrincipal.group("realm");
 
     // Login with principal and keytab
-    String keytab = Configuration.get(KEYTAB_MAP.get(mProcess));
+    String keytab = Configuration.get(mKeytabProperty);
     int exitVal =
         Utils.getResultFromProcess(new String[] {"kinit", "-kt", keytab, principal}).getExitValue();
     if (exitVal != 0) {