updates to nova.flags to get help working better

Fixes some old bugs that were brought up on the mailing list.

First step towards moving flags into the places where they belong.

Also moves manager import into service's init so that we can get all the
dynamically loaded flags shortly after loading.

nova/flags.py

@@ -160,9 +160,45 @@ class StrWrapper(object):
         raise KeyError(name)
 
 
-FLAGS = FlagValues()
-gflags.FLAGS = FLAGS
-gflags.DEFINE_flag(gflags.HelpFlag(), FLAGS)
+# Copied from gflags with small mods to get the naming correct.
+# Originally gflags checks for the first module that is not gflags that is
+# in the call chain, we want to check for the first module that is not gflags
+# and not this module.
+def _GetCallingModule():
+    """Returns the name of the module that's calling into this module.
+
+    We generally use this function to get the name of the module calling a
+    DEFINE_foo... function.
+    """
+    # Walk down the stack to find the first globals dict that's not ours.
+    for depth in range(1, sys.getrecursionlimit()):
+        if not sys._getframe(depth).f_globals is globals():
+            module_name = __GetModuleName(sys._getframe(depth).f_globals)
+            if module_name == 'gflags':
+                continue
+            if module_name is not None:
+                return module_name
+    raise AssertionError("No module was found")
+
+
+# Copied from gflags because it is a private function
+def __GetModuleName(globals_dict):
+    """Given a globals dict, returns the name of the module that defines it.
+
+    Args:
+    globals_dict: A dictionary that should correspond to an environment
+      providing the values of the globals.
+
+    Returns:
+    A string (the name of the module) or None (if the module could not
+    be identified.
+    """
+    for name, module in sys.modules.iteritems():
+        if getattr(module, '__dict__', None) is globals_dict:
+            if name == '__main__':
+                return sys.argv[0]
+            return name
+    return None
 
 
 def _wrapper(func):
@@ -173,6 +209,11 @@ def _wrapper(func):
     return _wrapped
 
 
+FLAGS = FlagValues()
+gflags.FLAGS = FLAGS
+gflags._GetCallingModule = _GetCallingModule
+
+
 DEFINE = _wrapper(gflags.DEFINE)
 DEFINE_string = _wrapper(gflags.DEFINE_string)
 DEFINE_integer = _wrapper(gflags.DEFINE_integer)
@@ -185,8 +226,6 @@ DEFINE_spaceseplist = _wrapper(gflags.DEFINE_spaceseplist)
 DEFINE_multistring = _wrapper(gflags.DEFINE_multistring)
 DEFINE_multi_int = _wrapper(gflags.DEFINE_multi_int)
 DEFINE_flag = _wrapper(gflags.DEFINE_flag)
-
-
 HelpFlag = gflags.HelpFlag
 HelpshortFlag = gflags.HelpshortFlag
 HelpXMLFlag = gflags.HelpXMLFlag

nova/service.py

@@ -45,15 +45,10 @@ FLAGS = flags.FLAGS
 flags.DEFINE_integer('report_interval', 10,
                      'seconds between nodes reporting state to datastore',
                      lower_bound=1)
-
 flags.DEFINE_integer('periodic_interval', 60,
                      'seconds between running periodic tasks',
                      lower_bound=1)
 
-flags.DEFINE_flag(flags.HelpFlag())
-flags.DEFINE_flag(flags.HelpshortFlag())
-flags.DEFINE_flag(flags.HelpXMLFlag())
-
 
 class Service(object):
     """Base class for workers that run on hosts."""
@@ -64,6 +59,8 @@ class Service(object):
         self.binary = binary
         self.topic = topic
         self.manager_class_name = manager
+        manager_class = utils.import_class(self.manager_class_name)
+        self.manager = manager_class(host=self.host, *args, **kwargs)
         self.report_interval = report_interval
         self.periodic_interval = periodic_interval
         super(Service, self).__init__(*args, **kwargs)
@@ -71,9 +68,9 @@ class Service(object):
         self.timers = []
 
     def start(self):
-        manager_class = utils.import_class(self.manager_class_name)
-        self.manager = manager_class(host=self.host, *self.saved_args,
-                                                     **self.saved_kwargs)
+        vcs_string = version.version_string_with_vcs()
+        logging.audit(_("Starting %(topic)s node (version %(vcs_string)s)"),
+                      {'topic': self.topic, 'vcs_string': vcs_string})
         self.manager.init_host()
         self.model_disconnected = False
         ctxt = context.get_admin_context()
@@ -153,9 +150,6 @@ class Service(object):
             report_interval = FLAGS.report_interval
         if not periodic_interval:
             periodic_interval = FLAGS.periodic_interval
-        vcs_string = version.version_string_with_vcs()
-        logging.audit(_("Starting %(topic)s node (version %(vcs_string)s)")
-                % locals())
         service_obj = cls(host, binary, topic, manager,
                           report_interval, periodic_interval)
 
@@ -217,8 +211,19 @@ class Service(object):
 
 
 def serve(*services):
-    if not services:
-        services = [Service.create()]
+    try:
+        if not services:
+            services = [Service.create()]
+    except Exception:
+        logging.exception('in Service.create()')
+        raise
+    finally:
+        # After we've loaded up all our dynamic bits, check
+        # whether we should print help
+        flags.DEFINE_flag(flags.HelpFlag())
+        flags.DEFINE_flag(flags.HelpshortFlag())
+        flags.DEFINE_flag(flags.HelpXMLFlag())
+        FLAGS.ParseNewFlags()
 
     name = '_'.join(x.binary for x in services)
     logging.debug(_("Serving %s"), name)

nova/tests/test_service.py

@@ -50,13 +50,6 @@ class ExtendedService(service.Service):
 class ServiceManagerTestCase(test.TestCase):
     """Test cases for Services"""
 
-    def test_attribute_error_for_no_manager(self):
-        serv = service.Service('test',
-                               'test',
-                               'test',
-                               'nova.tests.test_service.FakeManager')
-        self.assertRaises(AttributeError, getattr, serv, 'test_method')
-
     def test_message_gets_to_manager(self):
         serv = service.Service('test',
                                'test',