launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/no-maas-provisioning-settings into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jtv/maas/no-maas-provisioning-settings/+merge/121141

As discussed with Julian.  The Config object in provisioningserver now looks first for the environment variable, then falls back to ./etc/maas/pserv.yaml, and if all else fails, defaults to /etc/maas/pserv.yaml.  This means it's no longer necessary to set MAAS_PROVISIONING_SETTINGS when running the import scripts: it was only for the maas-provisioning commands' benefit anyway, and that now has smarter lookups.

You can still set the variable if you insist, and it will override the defaults.  But this should cover our more common use-cases.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/no-maas-provisioning-settings/+merge/121141
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/no-maas-provisioning-settings into lp:maas.

=== modified file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files	2012-08-17 14:34:14 +0000
+++ scripts/maas-import-pxe-files	2012-08-24 09:33:20 +0000
@@ -34,9 +34,6 @@
 # Supported architectures.
 ARCHES=${ARCHES:-amd64 i386}
 
-# Path to the provisioning configuration. Mandatory.
-MAAS_PROVISIONING_SETTINGS=${MAAS_PROVISIONING_SETTINGS?}
-
 # Command line to download a resource at a given URL into the current
 # directory.  A wget command line will work here, but curl will do as well.
 DOWNLOAD=${DOWNLOAD:-wget --no-verbose}

=== modified file 'src/provisioningserver/config.py'
--- src/provisioningserver/config.py	2012-08-23 19:56:55 +0000
+++ src/provisioningserver/config.py	2012-08-24 09:33:20 +0000
@@ -16,7 +16,7 @@
 
 from getpass import getuser
 from os import environ
-from os.path import abspath
+import os.path
 from threading import RLock
 
 from formencode import Schema
@@ -85,10 +85,17 @@
 
     def _get_default_filename(cls):
         # Get the configuration filename from the environment. Failing that,
-        # return a hard-coded default.
-        return environ.get(
-            "MAAS_PROVISIONING_SETTINGS",
-            "/etc/maas/pserv.yaml")
+        # look for the configuration in its default locations.
+        configured_location = environ.get("MAAS_PROVISIONING_SETTINGS")
+        if configured_location is not None:
+            return configured_location
+        for location in [os.path.curdir, '/']:
+            filename = os.path.join(location, 'etc', 'maas', 'pserv.yaml')
+            if os.path.isfile(filename):
+                return filename
+        # Oh well.  Just return the last filename we tried, and let the
+        # caller report that that file was not found.
+        return filename
 
     def _set_default_filename(cls, filename):
         # Set the configuration filename in the environment.
@@ -143,7 +150,7 @@
         """
         if filename is None:
             filename = cls.DEFAULT_FILENAME
-        filename = abspath(filename)
+        filename = os.path.abspath(filename)
         with cls._cache_lock:
             if filename not in cls._cache:
                 with open(filename, "rb") as stream:

=== modified file 'src/provisioningserver/tests/test_config.py'
--- src/provisioningserver/tests/test_config.py	2012-08-23 19:56:55 +0000
+++ src/provisioningserver/tests/test_config.py	2012-08-24 09:33:20 +0000
@@ -74,15 +74,39 @@
         fixture = EnvironmentVariableFixture("MAAS_PROVISIONING_SETTINGS")
         self.useFixture(fixture)
 
-    def test_get_with_environment_empty(self):
-        self.assertEqual("/etc/maas/pserv.yaml", Config.DEFAULT_FILENAME)
-
-    def test_get_with_environment_set(self):
-        dummy_filename = factory.make_name("config")
-        fixture = EnvironmentVariableFixture(
-            "MAAS_PROVISIONING_SETTINGS", dummy_filename)
-        self.useFixture(fixture)
-        self.assertEqual(dummy_filename, Config.DEFAULT_FILENAME)
+    def make_current_dir(self):
+        """Create a directory and pretend it's the current one."""
+        local_dir = self.make_dir()
+        # This is enough to fool the config-searching logic.
+        self.patch(os.path, 'curdir', local_dir)
+        return local_dir
+
+    def make_local_config(self):
+        """Set up a local configuration file."""
+        file_location = os.path.join(self.make_current_dir(), 'etc', 'maas')
+        os.makedirs(file_location)
+        return factory.make_file(file_location, name='pserv.yaml')
+
+    def test_gets_filename_from_environment(self):
+        dummy_filename = factory.make_name("config")
+        self.useFixture(EnvironmentVariableFixture(
+            "MAAS_PROVISIONING_SETTINGS", dummy_filename))
+        self.assertEqual(dummy_filename, Config.DEFAULT_FILENAME)
+
+    def test_gets_local_config_file(self):
+        local_config = self.make_local_config()
+        self.assertEqual(local_config, Config.DEFAULT_FILENAME)
+
+    def test_environment_overrides_filesystem(self):
+        dummy_filename = factory.make_name("config")
+        self.useFixture(EnvironmentVariableFixture(
+            "MAAS_PROVISIONING_SETTINGS", dummy_filename))
+        self.make_local_config()
+        self.assertEqual(dummy_filename, Config.DEFAULT_FILENAME)
+
+    def test_defaults_to_global_config(self):
+        self.make_current_dir()
+        self.assertEqual('/etc/maas/pserv.yaml', Config.DEFAULT_FILENAME)
 
     def test_set(self):
         dummy_filename = factory.make_name("config")
@@ -90,6 +114,7 @@
         self.assertEqual(dummy_filename, Config.DEFAULT_FILENAME)
 
     def test_delete(self):
+        self.make_current_dir()
         Config.DEFAULT_FILENAME = factory.make_name("config")
         del Config.DEFAULT_FILENAME
         # The filename reverts; see test_get_with_environment_empty.

=== modified file 'src/provisioningserver/utils.py'
--- src/provisioningserver/utils.py	2012-08-16 13:07:40 +0000
+++ src/provisioningserver/utils.py	2012-08-24 09:33:20 +0000
@@ -284,7 +284,8 @@
 
     The `--config-file` option defaults to the value of
     `MAAS_PROVISIONING_SETTINGS` in the process's environment, otherwise
-    `/etc/maas/pserv.yaml`.
+    `etc/maas/pserv.yaml` relative to the current directory or if that does
+    not exist, `/etc/maas/pserv.yaml`.
     """
 
     def __init__(self, description):