Merge pull request #454 from openvstorage/beaty_changes

Beauty fixes. Reorganize the folder structure

README.md

@@ -49,14 +49,14 @@ However if you do wish to use the Python interface:
 - All tests require a HCResults instance to be passed
 ```
 from ovs.extensions.healthcheck.result import HCResults
-from ovs.extensions.healthcheck.arakoon_hc import ArakoonHealthCheck
+from ovs.extensions.healthcheck.suites.arakoon_hc import ArakoonHealthCheck
 result = HCResults()
 ArakoonHealthCheck.check_collapse(result)
 ```
 If you wish to capture output: a named HCResults must be passed. This way a single result instance can capture all test outputs
 ```
 from ovs.extensions.healthcheck.result import HCResults
-from ovs.extensions.healthcheck.arakoon_hc import ArakoonHealthCheck
+from ovs.extensions.healthcheck.suites.arakoon_hc import ArakoonHealthCheck
 result = HCResults()
 ArakoonHealthCheck.check_collapse(result.HCResultCollector(result=result, test_name='collapse-test'))
 # Output

ovs/extensions/healthcheck/expose_to_cli.py

@@ -14,6 +14,11 @@
 # Open vStorage is distributed in the hope that it will be useful,
 # but WITHOUT ANY WARRANTY of any kind.
 
+
+"""
+Expose to CLI module. Depends on the python-click library
+"""
+
 from __future__ import absolute_import
 
 import os
@@ -29,10 +34,6 @@
 from ovs.extensions.storage.volatilefactory import VolatileFactory
 from ovs.extensions.healthcheck.logger import Logger
 
-"""
-Expose to CLI module. Depends on the python-click library
-"""
-
 # @todo Make it recursive. Current layout enforces SUBMODULE COMMAND, SUBMODULE, SUB, COMMAND is not possible
 
 
@@ -172,16 +173,19 @@ def discover():
                 for filename in filenames:
                     if not (filename.endswith('.py') and filename != '__init__.py'):
                         continue
-                    module_name = filename.replace('.py', '')
                     file_path = os.path.join(root, filename)
                     relative_path = os.path.relpath(file_path, path)
                     relative_module_name = ''.join(os.path.split(relative_path.replace('.py', '')))
                     # Import file, making it relative to the start path to avoid name collision.
                     # Without it, the module contents would be merged (eg. alba.py and testing/alba.py would be merged, overriding the path
                     # imp.load_source is different from importing. Therefore using the relative-joined name is safe
-                    mod = imp.load_source(relative_module_name, file_path)
+                    try:
+                        mod = imp.load_source(relative_module_name, file_path)
+                    except ImportError:
+                        cls.logger.exception('Unable to import module at {0}'.format(file_path))
+                        continue
                     for member_name, member_value in inspect.getmembers(mod):
-                        if not (inspect.isclass(member_value) and member_value.__module__ == module_name and 'object' in [base.__name__ for base in member_value.__bases__]):
+                        if not (inspect.isclass(member_value) and member_value.__module__ == relative_module_name and 'object' in [base.__name__ for base in member_value.__bases__]):
                             continue
                         for submember_name, submember_value in inspect.getmembers(member_value):
                             if not hasattr(submember_value, expose_to_cli.attribute):
@@ -308,7 +312,7 @@ class HealthcheckTerminatedException(KeyboardInterrupt):
     """
 
 
-class HealthcheckCLiContext(CLIContext):
+class HealthCheckCLiContext(CLIContext):
     """
     Context object which holds some information
     """
@@ -473,7 +477,7 @@ def run_methods_in_module(self, ctx):
             return
 
 
-class HealthcheckCLI(CLI):
+class HealthCheckCLI(CLI):
     """
     Click CLI which dynamically loads all possible commands
     """
@@ -494,7 +498,7 @@ def __init__(self, *args, **kwargs):
         Initializes a CLI instance
         Injects a healthcheck specific callback
         """
-        super(HealthcheckCLI, self).__init__(chain=True,
+        super(HealthCheckCLI, self).__init__(chain=True,
                                              invoke_without_command=True,
                                              result_callback=self.healthcheck_result_handler,
                                              *args, **kwargs)
@@ -513,7 +517,7 @@ def parse_args(self, ctx, args):
         if self.TO_JSON in args:
             args.remove(self.TO_JSON)
             args.insert(0, self.TO_JSON)
-        super(HealthcheckCLI, self).parse_args(ctx, args)
+        super(HealthCheckCLI, self).parse_args(ctx, args)
 
     def get_command(self, ctx, name):
         # type: (click.Context, str) -> HealthcheckAddonGroup
@@ -547,16 +551,16 @@ def healthcheck_result_handler(ctx, result, *args, **kwargs):
     def main(self, args=None, prog_name=None, complete_var=None, standalone_mode=False, **extra):
         # type: (list, str, bool, bool, **any) -> None
         try:
-            super(HealthcheckCLI, self).main(args, prog_name, complete_var, standalone_mode, **extra)
+            super(HealthCheckCLI, self).main(args, prog_name, complete_var, standalone_mode, **extra)
         except (HealthcheckTerminatedException, click.Abort, KeyboardInterrupt):
-            # Raised when an invoked command was abborted. The invoked command will output all results and then raise the exception
+            # Raised when an invoked command was aborted. The invoked command will output all results and then raise the exception
             pass
         except click.ClickException as e:
             e.show()
             sys.exit(e.exit_code)
 
 
-@click.group(cls=HealthcheckCLI)
+@click.group(cls=HealthCheckCLI)
 @click.option('--unattended', is_flag=True, help='Only output the results in a compact format')
 @click.option('--to-json', is_flag=True, help='Only output the results in a JSON format')
 @click.pass_context
@@ -568,11 +572,11 @@ def healthcheck_entry_point(ctx, unattended, to_json):
     # Will be the 'callback' method for the HealthcheckCLi instance
     # Provide a new instance of the results to collect all results within the complete healthcheck
     result_handler = HCResults(unattended=unattended, to_json=to_json)
-    ctx.obj = HealthcheckCLiContext(result_handler)
+    ctx.obj = HealthCheckCLiContext(result_handler)
     # When run with subcommand, it will fetch the command to execute
     if ctx.invoked_subcommand is None:
         # Invoked without sub command. Run all functions.
-        cli_instance = ctx.command  # type: HealthcheckCLI
+        cli_instance = ctx.command  # type: HealthCheckCLI
         for sub_command in cli_instance.list_commands(ctx):
             ctx.invoke(cli_instance.get_command(ctx, sub_command))
         return

ovs/extensions/healthcheck/suites/__init__.py

@@ -0,0 +1,19 @@
+# Copyright (C) 2018 iNuron NV
+#
+# This file is part of Open vStorage Open Source Edition (OSE),
+# as available from
+#
+#      http://www.openvstorage.org and
+#      http://www.openvstorage.com.
+#
+# This file is free software; you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License v3 (GNU AGPLv3)
+# as published by the Free Software Foundation, in version 3 as it comes
+# in the LICENSE.txt file of the Open vStorage OSE distribution.
+#
+# Open vStorage is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY of any kind.
+
+"""
+Suites module which contain all testing logic
+"""

ovs/extensions/healthcheck/alba.py → ovs/extensions/healthcheck/suites/alba.py

@@ -33,7 +33,7 @@
 from ovs.extensions.generic.system import System
 from ovs.extensions.healthcheck.config.error_codes import ErrorCodes
 from ovs.extensions.healthcheck.decorators import cluster_check
-from ovs.extensions.healthcheck.expose_to_cli import expose_to_cli, HealthcheckCLI
+from ovs.extensions.healthcheck.expose_to_cli import expose_to_cli, HealthCheckCLI
 from ovs.extensions.healthcheck.helpers.albacli import AlbaCLI
 from ovs.extensions.healthcheck.helpers.backend import BackendHelper
 from ovs.extensions.healthcheck.helpers.exceptions import AlbaException, AlbaTimeOutException,  ConfigNotMatchedException,\
@@ -148,7 +148,7 @@ def _check_backend_asds(cls, result_handler, asds, backend_name, config):
         return result
 
     @classmethod
-    @expose_to_cli(MODULE, 'proxy-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'proxy-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verifies that the proxies are able to perform basic operations',
                    short_help='Test if proxy basic operations')
     def check_if_proxies_work(cls, result_handler):
@@ -422,7 +422,7 @@ def _get_all_responding_backends(result_handler):
 
     @staticmethod
     @cluster_check
-    @expose_to_cli(MODULE, 'backend-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'backend-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verifies that the backends are still usable',
                    short_help='Test if backends are usable')
     def check_backends(result_handler):
@@ -484,7 +484,7 @@ def check_backends(result_handler):
 
     @staticmethod
     @cluster_check
-    @expose_to_cli(MODULE, 'disk-safety-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'disk-safety-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verifies that namespaces have enough safety',
                    short_help='Test if namespaces have enough safety')
     def check_disk_safety(result_handler):
@@ -593,7 +593,7 @@ def get_disk_safety(cls, result_handler):
 
     # @todo: incorporate asd-manager code to check the service
     @staticmethod
-    @expose_to_cli(MODULE, 'processes-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'processes-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verifies that the local Alba processes are still running',
                    short_help='Test if Alba processes are running')
     def check_alba_processes(result_handler):
@@ -620,7 +620,7 @@ def check_alba_processes(result_handler):
                                        code=ErrorCodes.alba_service_down)
 
     @staticmethod
-    @expose_to_cli(MODULE, 'proxy-port-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'proxy-port-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verifies that the proxies are still listening on their socket',
                    short_help='Test if proxies are still listening')
     def check_alba_proxy_ports(result_handler):
@@ -644,7 +644,7 @@ def check_alba_proxy_ports(result_handler):
 
     @classmethod
     @cluster_check
-    @expose_to_cli(MODULE, 'nsm-load-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'nsm-load-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verifies that the Namespace Managers are not overloaded',
                    short_help='Test if NSMs are not overloaded')
     def check_nsm_load(cls, result_handler):

ovs/extensions/healthcheck/arakoon.py → ovs/extensions/healthcheck/suites/arakoon.py

@@ -37,7 +37,7 @@
 from ovs_extensions.generic.toolbox import ExtensionsToolbox
 from ovs.extensions.healthcheck.decorators import cluster_check
 from ovs.extensions.healthcheck.config.error_codes import ErrorCodes
-from ovs.extensions.healthcheck.expose_to_cli import expose_to_cli, HealthcheckCLI
+from ovs.extensions.healthcheck.expose_to_cli import expose_to_cli, HealthCheckCLI
 from ovs.extensions.healthcheck.helpers.network import NetworkHelper
 from ovs.extensions.healthcheck.logger import Logger
 from ovs.extensions.services.servicefactory import ServiceFactory
@@ -89,7 +89,7 @@ def _get_arakoon_clusters(cls, result_handler):
 
     @classmethod
     @cluster_check
-    @expose_to_cli(MODULE, 'nodes-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'nodes-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verify if nodes are missing and if nodes are catching up to the master',
                    short_help='Test if there are nodes missing/catching up')
     @expose_to_cli.option('--max-transactions-behind', type=int, default=10, help='The number of transactions that a slave can be behind a master before logging a failure')
@@ -148,7 +148,7 @@ def check_node_status(cls, result_handler, max_transactions_behind=10):
 
     @classmethod
     @cluster_check
-    @expose_to_cli(MODULE, 'ports-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'ports-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verifies that the Arakoon clusters still respond to connections',
                    short_help='Test if Arakoons accepts connections')
     def check_arakoon_ports(cls, result_handler):
@@ -256,7 +256,7 @@ def _connection_worker(queue, result_handler):
 
     @classmethod
     @cluster_check
-    @expose_to_cli(MODULE, 'collapse-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'collapse-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verifies collapsing has occurred for all Arakoons',
                    short_help='Test if Arakoon collapsing is not failing')
     @expose_to_cli.option('--max-collapse-age', type=int, default=3, help='Maximum age in days for TLX')
@@ -327,7 +327,7 @@ def check_collapse(cls, result_handler, max_collapse_age=3, min_tlx_amount=10):
                         continue
                     if len(headdb_files) > 0:
                         headdb_size = sum([int(i[2]) for i in headdb_files])
-                        collapse_size_msg = 'Spare space for local collapse is '
+                        collapse_size_msg = 'Spare space for local collapse is'
                         if avail_size >= headdb_size * 4:
                             result_handler.success('{0} sufficient (n > 4x head.db size)'.format(collapse_size_msg))
                         elif avail_size >= headdb_size * 3:
@@ -456,7 +456,7 @@ def _collapse_worker(queue, clients, result_handler):
 
     @classmethod
     @cluster_check
-    @expose_to_cli(MODULE, 'integrity-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'integrity-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verifies that all Arakoon clusters are still responding to client calls',
                    short_help='Test if Arakoon clusters are still responding')
     def verify_integrity(cls, result_handler):
@@ -487,7 +487,7 @@ def verify_integrity(cls, result_handler):
 
     @classmethod
     @cluster_check
-    @expose_to_cli(MODULE, 'file-descriptors-test', HealthcheckCLI.ADDON_TYPE,
+    @expose_to_cli(MODULE, 'file-descriptors-test', HealthCheckCLI.ADDON_TYPE,
                    help='Verify the number of File Descriptors on every Arakoon does not exceed the limit',
                    short_help='Test if #FD does not exceed the limit')
     @expose_to_cli.option('--fd-limit', type=int, default=30, help='Threshold for the number number of tcp connections for which to start logging warnings')