ffdc: Simplify error messages

Tested: Error scenarios
- Missing CLI and env parm cases
- Missing combinations of CLI and env parm cases

Signed-off-by: Peter D  Phan <peterp@us.ibm.com>
Change-Id: I22be1fa0a8c947320c5a1bd4cda672290cfcfcf2
Signed-off-by: Peter D  Phan <peterp@us.ibm.com>
diff --git a/ffdc/collect_ffdc.py b/ffdc/collect_ffdc.py
index 775cd58..10f0b1c 100644
--- a/ffdc/collect_ffdc.py
+++ b/ffdc/collect_ffdc.py
@@ -1,7 +1,6 @@
 #!/usr/bin/env python
 
 r"""
-
 CLI FFDC Collector.
 """
 
@@ -11,7 +10,8 @@
 
 # ---------Set sys.path for cli command execution---------------------------------------
 # Absolute path to openbmc-test-automation/ffdc
-full_path = os.path.abspath(os.path.dirname(sys.argv[0])).split('ffdc')[0]
+abs_path = os.path.abspath(os.path.dirname(sys.argv[0]))
+full_path = abs_path.split('ffdc')[0]
 sys.path.append(full_path)
 # Walk path and append to sys.path
 for root, dirs, files in os.walk(full_path):
@@ -21,44 +21,65 @@
 from ffdc_collector import FFDCCollector
 
 
-@click.command()
-@click.option('-h', '--hostname', envvar='OPENBMC_HOST',
-              help="ip or hostname of the target [default: OPENBMC_HOST]")
+@click.command(context_settings=dict(help_option_names=['-h', '--help']))
+@click.option('-r', '--remote', envvar='OPENBMC_HOST',
+              help="Name/IP of the remote (targeting) host. [default: OPENBMC_HOST]")
 @click.option('-u', '--username', envvar='OPENBMC_USERNAME',
-              help="username on targeted system [default:  OPENBMC_USERNAME]")
+              help="User on the remote host with access to FFDC files.[default: OPENBMC_USERNAME]")
 @click.option('-p', '--password', envvar='OPENBMC_PASSWORD',
-              help="password for username on targeted system [default: OPENBMC_PASSWORD]")
-@click.option('-f', '--ffdc_config', default="ffdc_config.yaml",
-              show_default=True, help="YAML FFDC configuration file")
+              help="Password for user on remote host. [default: OPENBMC_PASSWORD]")
+@click.option('-f', '--ffdc_config', default=abs_path + "/ffdc_config.yaml",
+              show_default=True, help="YAML Configuration file listing commands and files for FFDC.")
 @click.option('-l', '--location', default="/tmp",
               show_default=True, help="Location to store collected FFDC data")
-def cli_ffdc(hostname, username, password, ffdc_config, location):
+def cli_ffdc(remote, username, password, ffdc_config, location):
     r"""
     Stand alone CLI to generate and collect FFDC from the selected target.
-
-        Description of argument(s):
-
-        hostname                Name/IP of the remote (targeting) host.
-        username                User on the remote host with access to FFDC files.
-        password                Password for user on remote host.
-        ffdc_config             Configuration file listing commands and files for FFDC.
-        location                Where to store collected FFDC.
-
     """
+
     click.echo("\n********** FFDC Starts **********")
 
-    thisFFDC = FFDCCollector(hostname, username, password, ffdc_config, location)
-    thisFFDC.collect_ffdc()
+    if input_options_ok(remote, username, password, ffdc_config):
+        thisFFDC = FFDCCollector(remote, username, password, ffdc_config, location)
+        thisFFDC.collect_ffdc()
 
-    if not thisFFDC.receive_file_list:
-        click.echo("\n\tFFDC Collection from " + hostname + " has failed\n\n.")
-    else:
-        click.echo(str("\t" + str(len(thisFFDC.receive_file_list)))
-                   + " files were retrieved from " + hostname)
-        click.echo("\tFiles are stored in " + thisFFDC.ffdc_dir_path + "\n\n")
+        if not thisFFDC.receive_file_list:
+            click.echo("\n\tFFDC Collection from " + remote + " has failed\n\n.")
+        else:
+            click.echo(str("\t" + str(len(thisFFDC.receive_file_list)))
+                       + " files were retrieved from " + remote)
+            click.echo("\tFiles are stored in " + thisFFDC.ffdc_dir_path + "\n\n")
 
     click.echo("\n********** FFDC Finishes **********\n\n")
 
 
+def input_options_ok(remote, username, password, ffdc_config):
+    r"""
+    Verify script options exist via CLI options or environment variables.
+    """
+
+    all_options_ok = True
+
+    if not remote:
+        all_options_ok = False
+        print("\
+        \n>>>>>\tERROR: Name/IP of the remote host is not specified in CLI options or env OPENBMC_HOST.")
+    if not username:
+        all_options_ok = False
+        print("\
+        \n>>>>>\tERROR: User on the remote host is not specified in CLI options or env OPENBMC_USERNAME.")
+    if not password:
+        all_options_ok = False
+        print("\
+        \n>>>>>\tERROR: Password for user on remote host is not specified in CLI options "
+              + "or env OPENBMC_PASSWORD.")
+    if not os.path.isfile(ffdc_config):
+        all_options_ok = False
+        print("\
+        \n>>>>>\tERROR: Config file %s is not found.  Please verify path and filename." % ffdc_config)
+
+    return all_options_ok
+
+
 if __name__ == '__main__':
     cli_ffdc()
diff --git a/ffdc/ffdc_collector.py b/ffdc/ffdc_collector.py
index 925ccfd..69c14e8 100644
--- a/ffdc/ffdc_collector.py
+++ b/ffdc/ffdc_collector.py
@@ -45,7 +45,7 @@
             self.receive_file_list = []
             self.target_type = ""
         else:
-            raise EnvironmentError("Python or python packages do not meet minimum version requirement.")
+            sys.exit(-1)
 
     def verify_script_env(self):
 
@@ -62,8 +62,9 @@
         print("\t{:<10}  {:>10}".format('click', click.__version__))
         print("\t{:<10}  {:>10}".format('paramiko', paramiko.__version__))
 
-        if eval(yaml.__version__.replace('.', ',')) < (5, 4):
-            print("\n\tERROR: PyYAML version 5.4 or higher is needed.")
+        if eval(yaml.__version__.replace('.', ',')) < (5, 4, 1):
+            print("\n\tERROR: Python or python packages do not meet minimum version requirement.")
+            print("\tERROR: PyYAML version 5.4.1 or higher is needed.\n")
             run_env_ok = False
 
         print("\t---- End script host environment ----")
@@ -187,7 +188,7 @@
 
             if not quiet:
                 if scp_result:
-                    print("\t\tSuccessfully copy from " + self.hostname + ':' + source_file_path + ".\n")
+                    print("\t\tSuccessfully copied from " + self.hostname + ':' + source_file_path + ".\n")
                 else:
                     print("\t\tFail to copy from " + self.hostname + ':' + source_file_path + ".\n")
             else:
diff --git a/ffdc/ssh_utility.py b/ffdc/ssh_utility.py
index 7957465..37b9314 100644
--- a/ffdc/ssh_utility.py
+++ b/ffdc/ssh_utility.py
@@ -1,9 +1,13 @@
 #!/usr/bin/env python
 
 import paramiko
-from paramiko.ssh_exception import AuthenticationException, NoValidConnectionsError, SSHException
+from paramiko.ssh_exception import AuthenticationException
+from paramiko.ssh_exception import NoValidConnectionsError
+from paramiko.ssh_exception import SSHException
+from paramiko.ssh_exception import BadHostKeyException
 from paramiko.buffered_pipe import PipeTimeout as PipeTimeout
 import scp
+import sys
 import socket
 from socket import timeout as SocketTimeout
 
@@ -19,9 +23,9 @@
         r"""
         Description of argument(s):
 
-        hostname                name/ip of the remote (targeting) host
-        username                user on the remote host with access to FFCD files
-        password                password for user on remote host
+        hostname        Name/IP of the remote (targeting) host
+        username        User on the remote host with access to FFCD files
+        password        Password for user on remote host
         """
 
         self.ssh_output = None
@@ -41,23 +45,18 @@
         try:
             # SSHClient to make connections to the remote server
             self.sshclient = paramiko.SSHClient()
-            # setting set_missing_host_key_policy() to allow any host.
+            # setting set_missing_host_key_policy() to allow any host
             self.sshclient.set_missing_host_key_policy(paramiko.AutoAddPolicy())
+            # pk=paramiko.RSAKey.from_private_key(open('~/.ssh_pub/id_rsa.pub'))
             # Connect to the server
             self.sshclient.connect(hostname=self.hostname,
                                    username=self.username,
                                    password=self.password)
 
-        except AuthenticationException:
-            print("\n>>>>>\tERROR: Authentication failed, please verify your login credentials")
-        except SSHException:
-            print("\n>>>>>\tERROR: Failures in SSH2 protocol negotiation or logic errors.")
-        except NoValidConnectionsError:
-            print('\n>>>>>\tERROR: No Valid SSH Connection after multiple attempts.')
-        except socket.error:
-            print("\n>>>>>\tERROR: SSH Connection refused.")
-        except Exception:
-            raise Exception("\n>>>>>\tERROR: Unexpected Exception.")
+        except (BadHostKeyException, AuthenticationException,
+                SSHException, NoValidConnectionsError, socket.error) as e:
+            print("\n>>>>>\tERROR: Unable to SSH to %s %s %s\n\n" % (self.hostname, e.__class__, e))
+            sys.exit(-1)
 
     def ssh_remoteclient_disconnect(self):
 
@@ -86,8 +85,9 @@
             response = stdout.readlines()
             return response
         except (paramiko.AuthenticationException, paramiko.SSHException,
-                paramiko.ChannelException) as ex:
-            print("\n>>>>>\tERROR: Remote command execution fails.")
+                paramiko.ChannelException) as e:
+            # Log command with error. Return to caller for next command, if any.
+            print("\n>>>>>\tERROR: Fail remote command %s %s %s\n\n" % (command, e.__class__, e))
 
     def scp_connection(self):
 
@@ -111,12 +111,9 @@
 
         try:
             self.scpclient.get(remote_file, local_file)
-        except scp.SCPException:
-            print("scp.SCPException scp %s from remotehost" % remote_file)
-            return False
-        except (SocketTimeout, PipeTimeout) as ex:
-            # Future enhancement: multiple retries on these exceptions due to bad ssh connection
-            print("Timeout scp %s from remotehost" % remote_file)
+        except (scp.SCPException, SocketTimeout, PipeTimeout) as e:
+            # Log command with error. Return to caller for next file, if any.
+            print("\n>>>>>\tERROR: Fail scp %s from remotehost %s %s\n\n" % (remote_file, e.__class__, e))
             return False
 
         # Return True for file accounting