Merge lp:~jtv/maas-test/main-breakup into lp:maas-test

[Jeroen T. Vermeulen]

Break up main() a bit, make it testable, use exceptions sensibly.

This was getting to a size where carrying on Go-style was just not feasible any more.

I introduced a dedicated exception for "this is a mode of failure that we know can happen, and which should terminate the program in a known, managed, helpful way." I hope that was not presumptuous of me. If we wanted to get fancy, we might have exception classes for this instead of a return-codes enum, but that's not furthering the goal of this branch. The goal of this branch is to let me add yet another startup check without everything getting out of hand, and without unleashing sheer patching hell just to be able to exercise my new code in tests.

Jeroen

[Raphaël Badin (rvb)]

review: Approve

=== modified file 'maastest/main.py'
--- maastest/main.py	2014-01-09 14:21:17 +0000
+++ maastest/main.py	2014-01-10 09:34:51 +0000
@@ -1,4 +1,4 @@
-# Copyright 2013 Canonical Ltd.  This software is licensed under the
+# Copyright 2014 Canonical Ltd.  This software is licensed under the
 # GNU Affero General Public License version 3 (see the file LICENSE).
 
 """Test whether or not a node is compatible with MAAS."""
@@ -33,28 +33,55 @@
     UNEXPECTED_DHCP_SERVER = 4
 
 
-def main(argv=None):
-    from maastest.parser import prepare_parser
-    args = prepare_parser().parse_args(argv)
-
-    from maastest import utils
-    import logging
-    # We tie everything onto one output stream so that we can capture
-    # things for reporting.
-    output_stream = utils.CachingOutputStream(sys.stdout)
-    logging.basicConfig(
-        level=logging.DEBUG, format='%(asctime)s %(levelname)s %(message)s',
-        stream=output_stream)
-
+class ProgramFailure(Exception):
+    """A known kind of program failure.
+
+    These are fatal, program-level failures that we know how to present to the
+    user, including which error code to return.
+
+    :ivar return_code: Return code, from `RETURN_CODES`, which the program
+        should return to the shell.
+    """
+    def __init__(self, return_code, message):
+        super(ProgramFailure, self).__init__(message)
+        self.return_code = return_code
+
+
+def check_for_root():
+    """Verify that the program is being run as root.
+
+    :raise ProgramFailure: if the program is not running as root.
+    """
+    # Importing locally for responsiveness.
     import os
     if os.geteuid() != 0:
-        logging.error("This test must be run as root.  Use sudo.")
-        return RETURN_CODES.NOT_ROOT
-
+        raise ProgramFailure(
+            RETURN_CODES.NOT_ROOT, "This test must be run as root.  Use sudo.")
+
+
+def check_for_virtualisation_support():
+    """Verify that the system supports efficient virtual machines.
+
+    :raise ProgramFailure: if the CPU lacks virtualisation support, or support
+        has been disabled in firmware.
+    """
+    # Importing locally for responsiveness.
     from maastest import utils
     if not utils.check_kvm_ok():
-        logging.error("Unable to continue without KVM extensions.")
-        return RETURN_CODES.NO_KVM_EXTENSIONS
+        raise ProgramFailure(
+            RETURN_CODES.NO_KVM_EXTENSIONS,
+            "Unable to continue without KVM extensions.")
+
+
+def check_against_virtual_host():
+    """Warn if the program is running in a virtual machine.
+
+    For acceptable performance, maas-test should be run on a physical system.
+    This function warns, but does not fail, if the system is virtual.
+    """
+    # Importing locally for responsiveness.
+    import logging
+    from maastest import utils
     virt_type = utils.virtualization_type()
     if virt_type is not None:
         logging.info(
@@ -63,16 +90,49 @@
             "Running maas-test on this machine will result in significantly "
             "poorer performance than on physical hardware.")
 
+
+def check_against_dhcp_servers(interface):
+    """Make sure there are no unexpected DHCP servers on the testing network.
+
+    :param interface: Network interface to the testing network.  This will
+        send out a DHCP discovery request on that interface.
+    :raise ProgramFailure: if any DHCP servers were detected.
+    """
+    # Importing locally for responsiveness.
     from maastest.detect_dhcp import probe_dhcp
-    dhcp_servers = probe_dhcp(args.interface)
+    dhcp_servers = probe_dhcp(interface)
     if len(dhcp_servers) > 0:
-        logging.error(
+        raise ProgramFailure(
+            RETURN_CODES.UNEXPECTED_DHCP_SERVER,
             "DHCP server(s) detected on %s: %s. "
             "Pass an interface that is connected to the testing network. "
             "Ensure that the testing network connects only to the node and "
-            "its BMC, and has no DHCP service running.",
-            args.interface, ', '.join(sorted(dhcp_servers)))
-        return RETURN_CODES.UNEXPECTED_DHCP_SERVER
+            "its BMC, and has no DHCP service running."
+            % (interface, ', '.join(sorted(dhcp_servers))))
+
+
+def main(argv=None):
+    from maastest.parser import prepare_parser
+    args = prepare_parser().parse_args(argv)
+
+    # Importing locally for responsiveness.
+    from maastest import utils
+    import logging
+    # We tie everything onto one output stream so that we can capture
+    # things for reporting.
+    output_stream = utils.CachingOutputStream(sys.stdout)
+    logging.basicConfig(
+        level=logging.DEBUG, format='%(asctime)s %(levelname)s %(message)s',
+        stream=output_stream)
+
+    try:
+        check_for_root()
+        check_for_virtualisation_support()
+        check_against_virtual_host()
+        check_against_dhcp_servers(args.interface)
+    except ProgramFailure as e:
+        logging.error(e)
+        return e.return_code
 
     from maastest.cases import TestOneNode