Merge pull request #261 from elastic/minor-touch-up

Minor changes suggested in last review

README.md

@@ -76,6 +76,14 @@ As a first step the diagnostic will check the Github repo for the current releas
 
 ## Alternate Usages
 
+### Proxy Servers
+If the presence of a proxy server prevents the diagnostic from being run(generally due to running with the remote option on a client workstation) you can specify the settings with the parameters: 
+* --proxyHost
+* --proxyPort
+* --proxyUser
+* --proxyPassword 
+
+
 ### Customizing the output
 The `diag.yml` file in the `/lib/support-diagnostics-x.x.x` contains all the REST and system commands that will be run. These are tied to a particular version. You can extract this file and remove commands you do not wish to run as well as adding any that may not be currently included. Place this revised file in the directory containing the diagnostics.sh script and it will override the settings contained in the jar.
 

src/main/java/com/elastic/support/config/BaseInputs.java

@@ -10,10 +10,6 @@ public abstract class BaseInputs {
     protected JCommander jCommander;
     private static final Logger logger = LogManager.getLogger(BaseInputs.class);
 
-    public boolean validate(){
-        return true;
-    }
-
     public void parseInputs(String[] args){
         logger.info("Processing diagnosticInputs...");
         JCommander jc = new JCommander(this);

src/main/java/com/elastic/support/config/Constants.java

@@ -16,7 +16,7 @@ public class Constants {
    public static final int DEEFAULT_HTTPS_PORT = 443;
 
    public static final int LOGSTASH_PORT = 9600;
-   public static final String LOCAL_ADDRESSES = "127.0.0.1;localhost";
+   public static final String LOCAL_ADDRESSES = "127.0.0.1;localhost;::1";
 
    public static final String UTF8 = "UTF8";
 

src/main/java/com/elastic/support/config/DiagConfig.java

@@ -29,8 +29,6 @@ public DiagConfig(){
 
     public DiagConfig(Map configuration) {
 
-        super();
-
         this.configuration = configuration;
 
         githubSettings = (Map<String, String>) configuration.get("github-settings");

src/main/java/com/elastic/support/config/DiagnosticInputs.java

@@ -29,10 +29,6 @@ public class DiagnosticInputs extends BaseInputs {
     private String diagType = "standard";
     @Parameter(names = {"--ptp"}, description = "Insecure plain text password - allows you to input the password as a plain text argument for scripts. WARNING: Exposes passwords in clear text. Inherently insecure.")
     private String plainTextPassword = "";
-    @Parameter(names = {"--reps"}, description = "Number of times to execute the diagnostic. Use to create multiple runs at timed intervals.")
-    private int reps = 1;
-    @Parameter(names = {"--interval"}, description = "Timed interval in minutes at which to execute the diagnostic. Minimum interval is 10 minutes.")
-    private int interval = 600000;
     @Parameter(names = {"--scrub"}, description = "Set to true to use the scrub.yml dictionary to scrub logs and config files.  See README for more info.")
     private boolean scrubFiles = false;
     @Parameter(names = {"--noVerify"}, description = "Use this option to bypass hostname verification for certificate. This is inherently unsafe and NOT recommended.")
@@ -129,22 +125,6 @@ public void setPlainTextPassword(String plainTextPassword) {
         this.plainTextPassword = plainTextPassword;
     }
 
-    public int getReps() {
-        return reps;
-    }
-
-    public void setReps(int reps) {
-        this.reps = reps;
-    }
-
-    public int getInterval() {
-        return interval;
-    }
-
-    public void setInterval(int interval) {
-        this.interval = interval;
-    }
-
     public boolean isScrubFiles() {
         return scrubFiles;
     }
@@ -268,10 +248,6 @@ public boolean validate() {
             this.jCommander.usage();
             return false;
         }
-        return (validateAuth() && validateIntervals());
-    }
-
-    public boolean validateAuth() {
 
         if (StringUtils.isNotEmpty(this.plainTextPassword)) {
             this.password = plainTextPassword;
@@ -293,28 +269,6 @@ private boolean isAuthValid(String user, String password) {
                 (!StringUtils.isNotEmpty(password) || !StringUtils.isEmpty(user));
     }
 
-    public boolean validateIntervals() {
-
-        boolean repsOK = true, intervalOK = true;
-
-        if (this.getReps() > 1) {
-
-            if (this.getReps() > 6) {
-                logger.info("'--reps' specified [{}] exceed the maximum allowed [6].", this.getReps());
-                logger.info("Use --help for allowed values.");
-                repsOK = false;
-            }
-
-            if (this.getInterval() < 10) {
-                logger.info("Interval specificed is lower than the minium allowed.");
-                logger.info("Use --help for allowed values.");
-                intervalOK = false;
-            }
-        }
-
-        return (repsOK && intervalOK);
-
-    }
 
     @Override
     public String toString() {

src/main/java/com/elastic/support/diagnostics/chain/DiagnosticChainExec.java

@@ -1,5 +1,6 @@
 package com.elastic.support.diagnostics.chain;
 
+import com.elastic.support.util.SystemProperties;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 
@@ -17,8 +18,8 @@ public void runDiagnostic(DiagnosticContext context, List<String> chain) {
             diagnostic.execute(context);
 
         } catch (Exception e) {
-            logger.error("Error encountered running diagnostic. See logs for additional information.  Exiting application.", e);
-            throw new RuntimeException("DiagnosticService runtime error", e);
+            logger.log(SystemProperties.DIAG, "Error encountered running diagnostic. See logs for additional information.  Exiting application.", e);
+            throw new RuntimeException("DiagnosticService runtime error");
         }
 
     }

src/main/java/com/elastic/support/diagnostics/commands/BaseQueryCmd.java

@@ -14,18 +14,18 @@
 
 public abstract class BaseQueryCmd implements Command {
 
-    /**
+
+
+    private final Logger logger = LogManager.getLogger(BaseQueryCmd.class);
+
+    /*
      * This class has shared functionality for both the Elasticsearch and
      * Logstash based REST calls. It interates through set of endpoints from the
      * configuration and executes each. In all cases the results are written
      * directly to disk to a successful access. For some specialized configured
      * cases such as the node and shard calls, a failure will result in a reattempt
      * after the configured number of seconds.
      */
-
-    private final Logger logger = LogManager.getLogger(BaseQueryCmd.class);
-
-
     public RestCallManifest runQueries(RestClient restClient, Map<String, String> entries, String tempDir, DiagConfig diagConfig) {
 
         List<String> textExtensions = diagConfig.getTextFileExtensions();
@@ -34,7 +34,7 @@ public RestCallManifest runQueries(RestClient restClient, Map<String, String> en
         List<String> requireRetry = diagConfig.getRequireRetry();
         RestCallManifest restCallManifest = new RestCallManifest();
         Map<String, String> workEntries = new LinkedHashMap<>();
-        workEntries.putAll(entries);
+        //workEntries.putAll(entries);
 
         // We will go through a max of three tries attmmpting to get file output
         // or until there are no more entries to get.
@@ -45,7 +45,7 @@ public RestCallManifest runQueries(RestClient restClient, Map<String, String> en
         // still hasn't worked, write the error content in the result to the target file.
         for (int i = 1; i <= attempts; i++) {
             // If everything worked last pass, get out
-            if (workEntries.size() == 0) {
+            if (entries.size() == 0) {
                 break;
             }
 
@@ -61,17 +61,18 @@ public RestCallManifest runQueries(RestClient restClient, Map<String, String> en
             }
 
             restCallManifest.setRuns(i);
-            Set<String> keys = workEntries.keySet();
-            Set<String> removalEntries = new HashSet<>();
+            Iterator<Map.Entry<String,String>> iter = entries.entrySet().iterator();
 
-            for (String queryName : keys) {
-                String query = workEntries.get(queryName);
+            while (iter.hasNext()) {
+                Map.Entry<String,String> entry = iter.next();
+                String queryName = entry.getKey();
+                String query = entry.getValue();
                 String filename = buildFileName(queryName, tempDir, textExtensions);
                 logger.info("Running query:{} -  {}", queryName, query);
                 RestResult restResult = runQuery(filename, query, restClient);
                 // If it succeeded take it out of future work
                 if (restResult.getStatus() == 200) {
-                    removalEntries.add(queryName);
+                    iter.remove();
                     restCallManifest.setCallHistory(queryName, i, true);
                     logger.info("Results written to: {}", filename);
                 } else {
@@ -80,7 +81,7 @@ public RestCallManifest runQueries(RestClient restClient, Map<String, String> en
                     restCallManifest.setCallHistory(queryName, i, false);
 
                     if (!requireRetry.contains(queryName) || !isRetryable(restResult.getStatus())) {
-                        removalEntries.add(queryName);
+                        iter.remove();
                         restResult.toFile(filename);
                         logger.info("Call failed. Bypassing.");
 
@@ -95,12 +96,6 @@ public RestCallManifest runQueries(RestClient restClient, Map<String, String> en
                     }
                 }
             }
-
-            // if it was on the failed list remove it - do it this way to avoid modifying the
-            // Collection you're iterating through
-            for(String entry: removalEntries){
-                workEntries.remove(entry);
-            }
         }
 
         // Send back information on how many things didn't succeed and how many tries it took.

src/main/java/com/elastic/support/diagnostics/commands/SystemCallsCmd.java

@@ -62,12 +62,23 @@ public void execute(DiagnosticContext context) {
                     logger.log(SystemProperties.DIAG, e);
                     logger.info("Error creating container directory - bypassing Docker container calls.");
                 }
+                catch (Exception e){
+                    logger.log(SystemProperties.DIAG, e);
+                    logger.info("Unexpected error - bypassing Docker container calls.");
+                }
             }
         } else {
-            String calls = checkOS();
-            Map<String, String> osCmds = context.getDiagsConfig().getCommandMap(calls);
-            osCmds = preProcessOsCmds(osCmds, pid, SystemProperties.javaHome);
-            processCalls(tempDir, osCmds, pb);
+            try{
+                String calls = checkOS();
+                Map<String, String> osCmds = context.getDiagsConfig().getCommandMap(calls);
+                osCmds = preProcessOsCmds(osCmds, pid, SystemProperties.javaHome);
+                processCalls(tempDir, osCmds, pb);
+            }
+            catch (Exception e){
+                logger.log(SystemProperties.DIAG, e);
+                logger.info("Unexpected error - bypassing System calls.");
+            }
+
         }
 
     }
@@ -90,7 +101,7 @@ public String checkOS() {
             return "macOS";
         } else {
             logger.error("Failed to detect operating system!");
-            throw new RuntimeException("Unsupported OS");
+            throw new RuntimeException("Unsupported OS - " + osName);
         }
     }
 

src/main/java/com/elastic/support/scrub/ScrubApp.java

@@ -16,11 +16,6 @@ public static void main(String[] args) {
 
         ScrubInputs scrubInputs = new ScrubInputs();
         scrubInputs.parseInputs(args);
-        if(!scrubInputs.validate()){
-            logger.info("Exiting...");
-            System.exit(0);
-        }
-
         ScrubService scrub = new ScrubService(scrubInputs);
         scrub.exec();