[Branch,~linaro-validation/lava-dispatcher/trunk] Rev 626: Refactor handling of configuration directories

Message ID 20130613145420.2773.99143.launchpad@ackee.canonical.com
State Accepted
Headers show

Commit Message

Antonio Terceiro June 13, 2013, 2:54 p.m.
Merge authors:
  Antonio Terceiro (terceiro)
Related merge proposals:
  https://code.launchpad.net/~terceiro/lava-dispatcher/expose-config-dirs/+merge/168807
  proposed by: Antonio Terceiro (terceiro)
  review: Approve - Neil Williams (codehelp)
------------------------------------------------------------
revno: 626 [merge]
committer: Antonio Terceiro <antonio.terceiro@linaro.org>
branch nick: trunk
timestamp: Thu 2013-06-13 11:34:13 -0300
message:
  Refactor handling of configuration directories
  
    The configuration search path is now unconditionally set to:
  
    $HOME/.config/lava-dispatcher
    ${VIRTUAL_ENV}/etc/lava-dispatcher
    ${lava_dispatcher_root}/lava_dispatcher/default-config/lava-dispatcher
  
    If the VIRTUAL_ENV environment variable is not set, then it will read
    configuration from /etc/lava-dispatcher (as expected).
  
    - The search path is now exposed to client code by the
      lava_dispatcher.config.search_path() function
  
    - The path for *writing* configuration files is exported in the write_path()
      function.
  
    - Each individual directory in the search path can be accessed by the variables
      user_config_path, system_config_path and default_config_path.
  
    - Switched from /etc/xdg/lava-dispatcher to /etc/lava-dispatcher. Because of
      the previous state of the code, where a specific config_dir was always passed
      in and there was no way of overriding it, /etc/xdg/lava-dispatcher was never
      actually used by anybody, so the risk of this change is zero.
  
    - Removed passing around of configuration directory.
  
    - --config-dir switch in command line tools was kept and has the same behaviour
      as before. If a custom config dir is passed, both user and system-wide
      directories are ignored, and only the custom directory and the hardcoded
      defaults are used.
modified:
  doc/QUICKSTART
  lava/dispatcher/commands.py
  lava_dispatcher/config.py
  lava_dispatcher/context.py
  lava_dispatcher/downloader.py


--
lp:lava-dispatcher
https://code.launchpad.net/~linaro-validation/lava-dispatcher/trunk

You are subscribed to branch lp:lava-dispatcher.
To unsubscribe from this branch go to https://code.launchpad.net/~linaro-validation/lava-dispatcher/trunk/+edit-subscription

Patch

=== modified file 'doc/QUICKSTART'
--- doc/QUICKSTART	2012-05-22 17:52:40 +0000
+++ doc/QUICKSTART	2013-06-12 14:20:52 +0000
@@ -80,8 +80,9 @@ 
 Configuring the dispatcher
 ++++++++++++++++++++++++++
 
-The dispatcher looks for config values in ~/.config/lava-dispatcher/,
-then /etc/xdg/lava-dispatcher, then its own source tree.
+The dispatcher looks for config values in ~/.config/lava-dispatcher/, then
+/etc/lava-dispatcher (inside the current virtual environment if any, or
+/etc/lava-dispatcher itself otherwise), then its own source tree.
 
 To get started, you ned to make a directory for storing artefacts
 before they are downloaded to the device being tested

=== modified file 'lava/dispatcher/commands.py'
--- lava/dispatcher/commands.py	2013-01-23 22:28:49 +0000
+++ lava/dispatcher/commands.py	2013-06-11 20:27:05 +0000
@@ -1,3 +1,4 @@ 
+import argparse
 import json
 import logging
 import os
@@ -7,26 +8,24 @@ 
 from lava.tool.command import Command
 from lava.tool.errors import CommandError
 
+import lava_dispatcher.config
 from lava_dispatcher.config import get_config, get_device_config, get_devices
 from lava_dispatcher.job import LavaTestJob, validate_job_data
 
 
+class SetUserConfigDirAction(argparse.Action):
+    def __call__(self, parser, namespace, value, option_string=None):
+        lava_dispatcher.config.custom_config_path = value
+
+
 class DispatcherCommand(Command):
     @classmethod
     def register_arguments(cls, parser):
         super(DispatcherCommand, cls).register_arguments(parser)
-        # When we're working inside a virtual environment use venv-relative
-        # configuration directory. This works well with lava-deployment-tool
-        # and the directory layout it currently provides but will need to be
-        # changed for codeline support.
-        if "VIRTUAL_ENV" in os.environ:
-            default_config_dir = os.path.join(
-                os.environ["VIRTUAL_ENV"], "etc", "lava-dispatcher")
-        else:
-            default_config_dir = None
         parser.add_argument(
             "--config-dir",
-            default=default_config_dir,
+            default=None,
+            action=SetUserConfigDirAction,
             help="Configuration directory override (currently %(default)s")
 
 
@@ -35,7 +34,7 @@ 
     Lists all the configured devices in this LAVA instance.
     """
     def invoke(self):
-        for d in get_devices(self.args.config_dir):
+        for d in get_devices():
             print d.hostname
 
 
@@ -88,7 +87,7 @@ 
         FORMAT = '<LAVA_DISPATCHER>%(asctime)s %(levelname)s: %(message)s'
         DATEFMT = '%Y-%m-%d %I:%M:%S %p'
         logging.basicConfig(format=FORMAT, datefmt=DATEFMT)
-        config = get_config(self.args.config_dir)
+        config = get_config()
         logging.root.setLevel(config.logging_level)
 
         # Set process id if job-id was passed to dispatcher
@@ -139,7 +138,7 @@ 
     @property
     def device_config(self):
         try:
-            return get_device_config(self.args.device, self.args.config_dir)
+            return get_device_config(self.args.device)
         except Exception:
             raise CommandError("no such device: %s" % self.args.device)
 

=== modified file 'lava_dispatcher/config.py'
--- lava_dispatcher/config.py	2013-06-05 21:43:24 +0000
+++ lava_dispatcher/config.py	2013-06-13 14:33:49 +0000
@@ -152,31 +152,58 @@ 
 
 class DispatcherConfig(object):
 
-    def __init__(self, cp, config_dir):
+    def __init__(self, cp):
         self.cp = cp
-        self.config_dir = config_dir
 
     for option in DispatcherSchema().options():
         locals()[option.name] = OptionDescriptor(option.name)
 
 
-default_config_path = os.path.join(
-    os.path.dirname(__file__), 'default-config')
-
-
-def load_config_paths(name, config_dir):
-    if config_dir is None:
-        paths = [
-            os.path.join(path, name) for path in [
-                os.path.expanduser("~/.config"),
-                "/etc/xdg",
-                default_config_path]]
-    else:
-        paths = [config_dir, os.path.join(default_config_path, name)]
-    for path in paths:
-        if os.path.isdir(path):
-            yield path
-
+user_config_path = os.path.expanduser("~/.config/lava-dispatcher")
+
+if "VIRTUAL_ENV" in os.environ:
+    system_config_path = os.path.join(os.environ["VIRTUAL_ENV"],
+                                      "etc/lava-dispatcher")
+else:
+    system_config_path = "/etc/lava-dispatcher"
+
+deprecated_system_config_path = "/etc/xdg/lava-dispatcher"
+
+default_config_path = os.path.join(os.path.dirname(__file__),
+                                   'default-config/lava-dispatcher')
+
+custom_config_path = None
+
+def search_path():
+    if custom_config_path:
+        return [
+            custom_config_path,
+            default_config_path,
+        ]
+    else:
+        return [
+            user_config_path,
+            system_config_path,
+            default_config_path,
+        ]
+
+def write_path():
+    """
+    Returns the configuration directories where configuration files should be
+    written to.
+
+    Returns an array with a list of directories. Client tools should then write
+    any configuration files to the first directory in that list that is
+    writable by the user.
+    """
+    if custom_config_path:
+        return [custom_config_path]
+    else:
+        # Since usually you need to run the dispatcher as root, but lava-tool
+        # as a regular user, we give preference to writing to the system
+        # configuration to avoid the user writing config file to ~user, and the
+        # dispatcher looking for them at ~root.
+        return [system_config_path, user_config_path]
 
 def _read_into(path, cp):
     s = StringIO.StringIO()
@@ -186,7 +213,7 @@ 
     cp.readfp(s)
 
 
-def _get_config(name, config_dir, cp):
+def _get_config(name, cp):
     """Read a config file named name + '.conf'.
 
     This checks and loads files from the source tree, site wide location and
@@ -194,7 +221,7 @@ 
     settings which override source settings.
     """
     config_files = []
-    for directory in load_config_paths('lava-dispatcher', config_dir):
+    for directory in search_path():
         path = os.path.join(directory, '%s.conf' % name)
         if os.path.exists(path):
             config_files.append(path)
@@ -207,13 +234,13 @@ 
     return cp
 
 
-def get_config(config_dir):
+def get_config():
     cp = parser.SchemaConfigParser(DispatcherSchema())
-    _get_config("lava-dispatcher", config_dir, cp)
+    _get_config("lava-dispatcher", cp)
     valid, report = cp.is_valid(report=True)
     if not valid:
         logging.warning("dispatcher config is not valid:\n    %s", '\n    '.join(report))
-    return DispatcherConfig(cp, config_dir)
+    return DispatcherConfig(cp)
 
 
 def _hack_boot_options(scp):
@@ -243,19 +270,19 @@ 
     return scrubbed
 
 
-def get_device_config(name, config_dir):
+def get_device_config(name):
     # We read the device config once to get the device type, then we start
     # again and read device-defaults, device-types/$device-type and
     # devices/$device in that order.
     initial_config = ConfigParser()
-    _get_config("devices/%s" % name, config_dir, initial_config)
+    _get_config("devices/%s" % name, initial_config)
 
     real_device_config = parser.SchemaConfigParser(DeviceSchema())
-    _get_config("device-defaults", config_dir, real_device_config)
+    _get_config("device-defaults", real_device_config)
     _get_config(
         "device-types/%s" % initial_config.get('__main__', 'device_type'),
-        config_dir, real_device_config)
-    _get_config("devices/%s" % name, config_dir, real_device_config)
+        real_device_config)
+    _get_config("devices/%s" % name, real_device_config)
     real_device_config.set("__main__", "hostname", name)
     _hack_boot_options(real_device_config)
     valid, report = real_device_config.is_valid(report=True)
@@ -269,10 +296,19 @@ 
     return DeviceConfig(real_device_config)
 
 
-def get_devices(config_dir):
+def get_devices():
     devices = []
-    devices_dir = os.path.join(config_dir, 'devices')
-    for d in os.listdir(devices_dir):
-        d = os.path.splitext(d)[0]
-        devices.append(get_device_config(d, config_dir))
+    for config_dir in search_path():
+        devices_dir = os.path.join(config_dir, 'devices')
+        if os.path.isdir(devices_dir):
+            for d in os.listdir(devices_dir):
+                d = os.path.splitext(d)[0]
+                devices.append(get_device_config(d))
     return devices
+
+def get_config_file(config_file):
+    for config_dir in search_path():
+        config_file_path = os.path.join(config_dir, config_file)
+        if os.path.exists(config_file_path):
+            return config_file_path
+    return None

=== modified file 'lava_dispatcher/context.py'
--- lava_dispatcher/context.py	2013-04-02 16:16:18 +0000
+++ lava_dispatcher/context.py	2013-06-11 20:27:05 +0000
@@ -94,8 +94,7 @@ 
         self.job_data = job_data
         self.output = Outputter(output_dir)
         self.logfile_read = self.output.logfile_read
-        self.device_config = get_device_config(
-            target, dispatcher_config.config_dir)
+        self.device_config = get_device_config(target)
         self._client = TargetBasedClient(self, self.device_config)
         self.test_data = LavaTestData()
         self.oob_file = oob_file

=== modified file 'lava_dispatcher/downloader.py'
--- lava_dispatcher/downloader.py	2012-12-18 19:50:48 +0000
+++ lava_dispatcher/downloader.py	2013-06-11 20:27:05 +0000
@@ -33,6 +33,7 @@ 
 import zlib
 
 from tempfile import mkdtemp
+from lava_dispatcher.config import get_config_file
 from lava_dispatcher.utils import rmtree
 
 
@@ -122,8 +123,8 @@ 
     '''allows the downloader to override a URL so that something like:
      http://blah/ becomes file://localhost/blah
     '''
-    mappings = '%s/urlmappings.txt' % context.config.config_dir
-    if os.path.exists(mappings):
+    mappings = get_config_file('urlmappings.txt')
+    if mappings:
         newurl = url
         with open(mappings, 'r') as f:
             for line in f.readlines():