airbag: T2186: improve information reported

4 files changed, 136 insertions, 104 deletions

diff --git a/python/vyos/airbag.py b/python/vyos/airbag.py
index a2e9de491..6698aa404 100644
--- a/python/vyos/airbag.py
+++ b/python/vyos/airbag.py
@@ -13,15 +13,14 @@
 # You should have received a copy of the GNU Lesser General Public
 # License along with this library.  If not, see <http://www.gnu.org/licenses/>.
 
-import os
 import sys
 from datetime import datetime
 
 from vyos import debug
 from vyos.config import Config
-from vyos.version import get_version
-from vyos.util import run
 from vyos.logger import syslog
+from vyos.version import get_version
+from vyos.version import get_full_version_data
 
 # we allow to disable the extra logging
 DISABLE = False
@@ -57,12 +56,14 @@ def bug_report(dtype, value, trace):
     sys.stdout.flush()
     sys.stderr.flush()
 
-    information = {
+    information = get_full_version_data()
+    trace = '\n'.join(format_exception(dtype, value, trace)).replace('\n\n','\n')
+
+    information.update({
         'date': datetime.now().strftime('%Y-%m-%d %H:%M:%S'),
-        'version': get_version(),
-        'trace': '\n'.join(format_exception(dtype, value, trace)),
+        'trace': trace,
         'instructions': COMMUNITY if 'rolling' in get_version() else SUPPORTED,
-    }
+    })
 
     sys.stdout.write(INTRO.format(**information))
     sys.stdout.flush()
@@ -125,8 +126,23 @@ if get_version() and insession:
 # if the key before the value has not time, syslog takes that as the source of the message
 
 FAULT = """\
-Fault Time: {date}
-VyOS image: {version}
+Report Time:      {date}
+Image Version:    VyOS {version}
+Release Train:    {release_train}
+
+Built by:         {built_by}
+Built on:         {built_on}
+Build UUID:       {build_uuid}
+Build Commit ID:  {build_git}
+
+Architecture:     {system_arch}
+Boot via:         {boot_via}
+System type:      {system_type}
+
+Hardware vendor:  {hardware_vendor}
+Hardware model:   {hardware_model}
+Hardware S/N:     {hardware_serial}
+Hardware UUID:    {hardware_uuid}
 
 {trace}
 """
@@ -134,13 +150,13 @@ VyOS image: {version}
 INTRO = """\
 VyOS had an issue completing a command.
 
-We are sorry that you encountered a problem with VyOS.
+We are sorry that you encountered a problem while using VyOS.
 There are a few things you can do to help us (and yourself):
 {instructions}
 
-PLEASE, when reporting, do include as much information as you can:
-- do not obfuscate any data (feel free to send us a private communication with
-  the extra information if your business policy is strict on information sharing)
+When reporting problems, please include as much information as possible:
+- do not obfuscate any data (feel free to contact us privately if your 
+  business policy requires it)
 - and include all the information presented below
 
 """
@@ -157,6 +173,8 @@ COMMUNITY = """\
 SUPPORTED = """\
 - Make sure you are running the latest stable version of VyOS
   the code is available at https://downloads.vyos.io/?dir=release/current
-- Contact us on our online help desk
+- Contact us using the online help desk
   https://support.vyos.io/
+- Join our community on slack where our users exchange help and advice
+  https://vyos.slack.com
 """.strip()
diff --git a/python/vyos/util.py b/python/vyos/util.py
index b1d95fbbf..7a1f39ff2 100644
--- a/python/vyos/util.py
+++ b/python/vyos/util.py
@@ -159,11 +159,36 @@ def call(command, flag='', shell=None, input=None, timeout=None, env=None,
     return code
 
 
-def read_file(path):
-    """ Read a file to string """
-    with open(path, 'r') as f:
-        data = f.read().strip()
-    return data
+def read_file(fname, defaultonfailure=None):
+    """
+    read the content of a file, stripping any end characters (space, newlines)
+    should defaultonfailure be not None, it is returned on failure to read
+    """
+    try:
+        """ Read a file to string """
+        with open(fname, 'r') as f:
+            data = f.read().strip()
+        return data
+    except Exception as e:
+        if defaultonfailure is not None:
+            return defaultonfailure
+        raise e
+
+
+def read_json(fname, defaultonfailure=None):
+    """
+    read and json decode the content of a file
+    should defaultonfailure be not None, it is returned on failure to read
+    """
+    import json
+    try:
+        with open(fname, 'r') as f:
+            data = json.load(f)
+        return data
+    except Exception as e:
+        if defaultonfailure is not None:
+            return defaultonfailure
+        raise e
 
 
 def chown(path, user, group):
diff --git a/python/vyos/version.py b/python/vyos/version.py
index d51a940d6..a524b36ea 100644
--- a/python/vyos/version.py
+++ b/python/vyos/version.py
@@ -34,9 +34,17 @@ import json
 
 import vyos.defaults
 
+from vyos.util import read_file
+from vyos.util import read_json
+from vyos.util import popen
+from vyos.util import run
+from vyos.util import DEVNULL
+
+
 version_file = os.path.join(vyos.defaults.directories['data'], 'version.json')
   
-def get_version_data(file=version_file):
+
+def get_version_data(fname=version_file):
     """
     Get complete version data
 
@@ -52,20 +60,50 @@ def get_version_data(file=version_file):
     is an implementation detail and may change in the future, while the interface
     of this module will stay the same.
     """
-    try:
-        with open(file, 'r') as f:
-            version_data = json.load(f)
-        return version_data
-    except FileNotFoundError:
-        return {}
+    return read_json(fname, {})
 
-def get_version(file=None):
+
+def get_version(fname=version_file):
     """
     Get the version number, or an empty string if it could not be determined
     """
-    version_data = None
-    if file:
-        version_data = get_version_data(file=file)
-    else:
-        version_data = get_version_data()
-    return version_data.get('version','')
+    return get_version_data(fname=fname).get('version', '')
+
+
+def get_full_version_data(fname=version_file):
+    version_data = get_version_data(fname)
+
+    # Get system architecture (well, kernel architecture rather)
+    version_data['system_arch'], _ = popen('uname -m', stderr=DEVNULL)
+
+    # Get hypervisor name, if any
+    try:
+        hypervisor, _ = popen('hvinfo', stderr=DEVNULL)
+        version_data['system_type'] = f"{hypervisor} guest"
+    except OSError:
+        # hvinfo returns 1 if it cannot detect any hypervisor
+        version_data['system_type'] = 'bare metal'
+
+    # Get boot type, it can be livecd, installed image, or, possible, a system installed
+    # via legacy "install system" mechanism
+    # In installed images, the squashfs image file is named after its image version,
+    # while on livecd it's just "filesystem.squashfs", that's how we tell a livecd boot
+    # from an installed image
+    boot_via = "installed image"
+    if run(""" grep -e '^overlay.*/filesystem.squashfs' /proc/mounts >/dev/null""") == 0:
+        boot_via = "livecd"
+    elif run(""" grep '^overlay /' /proc/mounts >/dev/null """) != 0:
+        boot_via = "legacy non-image installation"
+    version_data['boot_via'] = boot_via
+
+    # Get hardware details from DMI
+    dmi = '/sys/class/dmi/id'
+    version_data['hardware_vendor'] = read_file(dmi + '/sys_vendor', 'Unknown')
+    version_data['hardware_model'] = read_file(dmi +'/product_name','Unknown')
+
+    # These two assume script is run as root, normal users can't access those files
+    subsystem = '/sys/class/dmi/id/subsystem/id'
+    version_data['hardware_serial'] = read_file(subsystem + '/product_serial','Unknown')
+    version_data['hardware_uuid'] = read_file(subsystem + '/product_uuid', 'Unknown')
+
+    return version_data
diff --git a/src/op_mode/version.py b/src/op_mode/version.py
index 2791f5e5c..c335eefce 100755
--- a/src/op_mode/version.py
+++ b/src/op_mode/version.py
@@ -25,14 +25,13 @@ import sys
 import argparse
 import json
 
-import pystache
-
 import vyos.version
 import vyos.limericks
 
 from vyos.util import cmd
 from vyos.util import call
 from vyos.util import run
+from vyos.util import read_file
 from vyos.util import DEVNULL
 
 
@@ -41,90 +40,42 @@ parser.add_argument("-a", "--all", action="store_true", help="Include individual
 parser.add_argument("-f", "--funny", action="store_true", help="Add something funny to the output")
 parser.add_argument("-j", "--json", action="store_true", help="Produce JSON output")
 
-def read_file(name):
-    try:
-        with open (name, "r") as f:
-            data = f.read()
-        return data.strip()
-    except:
-        # This works since we only read /sys/class/* stuff
-        # with this function
-        return "Unknown"
 
 version_output_tmpl = """
-Version:          VyOS {{version}}
-Release Train:    {{release_train}}
+Version:          VyOS {version}
+Release Train:    {release_train}
 
-Built by:         {{built_by}}
-Built on:         {{built_on}}
-Build UUID:       {{build_uuid}}
-Build Commit ID:  {{build_git}}
+Built by:         {built_by}
+Built on:         {built_on}
+Build UUID:       {build_uuid}
+Build Commit ID:  {build_git}
 
-Architecture:     {{system_arch}}
-Boot via:         {{boot_via}}
-System type:      {{system_type}}
+Architecture:     {system_arch}
+Boot via:         {boot_via}
+System type:      {system_type}
 
-Hardware vendor:  {{hardware_vendor}}
-Hardware model:   {{hardware_model}}
-Hardware S/N:     {{hardware_serial}}
-Hardware UUID:    {{hardware_uuid}}
+Hardware vendor:  {hardware_vendor}
+Hardware model:   {hardware_model}
+Hardware S/N:     {hardware_serial}
+Hardware UUID:    {hardware_uuid}
 
 Copyright:        VyOS maintainers and contributors
-
 """
 
 if __name__ == '__main__':
     args = parser.parse_args()
 
-    version_data = vyos.version.get_version_data()
-
-    # Get system architecture (well, kernel architecture rather)
-    version_data['system_arch'] = cmd('uname -m')
-
-
-    # Get hypervisor name, if any
-    system_type = "bare metal"
-    try:
-        hypervisor = cmd('hvinfo',stderr=DEVNULL)
-        system_type = "{0} guest".format(hypervisor)
-    except OSError:
-        # hvinfo returns 1 if it cannot detect any hypervisor
-        pass
-    version_data['system_type'] = system_type
-
-
-    # Get boot type, it can be livecd, installed image, or, possible, a system installed
-    # via legacy "install system" mechanism
-    # In installed images, the squashfs image file is named after its image version,
-    # while on livecd it's just "filesystem.squashfs", that's how we tell a livecd boot
-    # from an installed image
-    boot_via = "installed image"
-    if run(""" grep -e '^overlay.*/filesystem.squashfs' /proc/mounts >/dev/null""") == 0:
-        boot_via = "livecd"
-    elif run(""" grep '^overlay /' /proc/mounts >/dev/null """) != 0:
-        boot_via = "legacy non-image installation"
-    version_data['boot_via'] = boot_via
-
-
-    # Get hardware details from DMI
-    version_data['hardware_vendor'] = read_file('/sys/class/dmi/id/sys_vendor')
-    version_data['hardware_model']  = read_file('/sys/class/dmi/id/product_name')
-
-    # These two assume script is run as root, normal users can't access those files
-    version_data['hardware_serial'] = read_file('/sys/class/dmi/id/subsystem/id/product_serial')
-    version_data['hardware_uuid']   = read_file('/sys/class/dmi/id/subsystem/id/product_uuid')
-
+    version_data = vyos.version.get_full_version_data()
 
     if args.json:
         print(json.dumps(version_data))
         sys.exit(0)
-    else:
-        output = pystache.render(version_output_tmpl, version_data).strip()
-        print(output)
 
-        if args.all:
-           print("Package versions:")
-           call("dpkg -l")
+    print(version_output_tmpl.format(**version_data).strip())
+
+    if args.all:
+        print("Package versions:")
+        call("dpkg -l")
 
-        if args.funny:
-            print(vyos.limericks.get_random())
+    if args.funny:
+        print(vyos.limericks.get_random())