Merge lp:~jtv/maas/bug-1238376 into lp:~maas-committers/maas/trunk

[Jeroen T. Vermeulen]

Keep the legacy ephemerals import script config for now.

This took a while to run through its paces. As per bug 1238376, users are bemused with the import_pxe_files config no longer being included as configuration for the ephemerals import script. Therefore in this branch I change it as follows:

1. No longer rewrite pserv.yaml with the combined config, and no longer move the import_ephemerals config out of the way. I kept the generic infrastructure for doing these things, but not the specific code.

2. Strip the now-inert items out of the import_ephemerals config, and add a notice about the file being obsolete. This means that upgrading users will get a notice about the change, giving them a fair chance of learning about the change. And it means that any further changes they make to either import_pxe_files or import_ephemerals will still be respected.

3. Document the new options in pserv.yaml. These options will still be respected, unless the legacy shell scripts set different values. It'd be nice to have this the other way around, but the config has no real difference between letting a setting empty to use the default, and deliberately setting it to its default value. At least with the legacy config we can tell whether a value has been set or not.

A packaging branch will follow, to add the new /etc/maas/import_ephemerals back into the package.

Jeroen

[Gavin Panella (allenap)]

Sad, but probably the right thing to do. Undoing badness takes time, and this is proof.

review: Approve

=== added file 'etc/maas/import_ephemerals'
--- etc/maas/import_ephemerals	1970-01-01 00:00:00 +0000
+++ etc/maas/import_ephemerals	2013-10-11 13:46:22 +0000
@@ -0,0 +1,14 @@
+# Legacy configuration file for the maas-import-ephemerals script.
+#
+# This file is obsolete, but the script will still read it for compatibility.
+# Configure /etc/maas/pserv.yaml by preference.
+
+## Include settings from import_pxe_files.
+[ ! -f /etc/maas/import_pxe_files ] || . /etc/maas/import_pxe_files
+
+# These options can be defined here, although for future compatibility they
+# should be set in pserv.yaml instead:
+
+#DATA_DIR="/var/lib/maas/ephemeral"
+#RELEASES="precise"
+#ARCHES="amd64/generic i386/generic"

=== modified file 'etc/maas/pserv.yaml'
--- etc/maas/pserv.yaml	2013-10-09 04:33:07 +0000
+++ etc/maas/pserv.yaml	2013-10-11 13:46:22 +0000
@@ -40,6 +40,29 @@
 
 ## Boot configuration.
 boot:
-  # ephemeral:
-  ## Directory containing ephemeral boot images, etc.
-  #   images_directory: /var/lib/maas/ephemeral
+  ## CPU architectures for which boot images should be downloaded from the
+  ## server, e.g. ['i386/generic', 'amd64/generic'].  MAAS needs these images
+  ## in order to boot nodes up.
+  ##
+  ## Leave this option out to download images for all architectures.
+  #
+  # architectures:
+
+  ## Settings for ephemeral boot images.  These images are used when
+  ## commissioning nodes, and during fast-path installation.
+  #
+  ephemeral:
+
+    ## Directory where ephemeral boot images and related state should be
+    ## stored.
+    #
+    #   images_directory: /var/lib/maas/ephemeral
+    images_directory: /var/lib/maas/ephemeral
+
+    ## Releases for which ephemeral images should be downloaded.
+    ## These images are quite large (about a quarter GB each), so you may want
+    ## to restrict these separately even if you do want the regular install
+    ## images for all releases.  Leave this out to download all currently
+    ## supported releases.
+    #
+    # releases:

=== modified file 'setup.py'
--- setup.py	2013-10-09 07:58:23 +0000
+++ setup.py	2013-10-11 13:46:22 +0000
@@ -69,6 +69,7 @@
              'etc/txlongpoll.yaml',
              'contrib/maas_local_celeryconfig.py',
              'contrib/maas_local_celeryconfig_cluster.py',
+             'etc/maas/import_ephemerals',
              'etc/maas/import_pxe_files',
              'contrib/maas-http.conf',
              'contrib/maas-cluster-http.conf',

=== modified file 'src/provisioningserver/import_images/ephemerals_script.py'
--- src/provisioningserver/import_images/ephemerals_script.py	2013-10-11 08:30:04 +0000
+++ src/provisioningserver/import_images/ephemerals_script.py	2013-10-11 13:46:22 +0000
@@ -33,9 +33,7 @@
 
 from provisioningserver.config import Config
 from provisioningserver.import_images.config import (
-    EPHEMERALS_LEGACY_CONFIG,
     merge_legacy_ephemerals_config,
-    retire_legacy_config,
     )
 from provisioningserver.import_images.tgt import (
     clean_up_info_file,
@@ -361,8 +359,7 @@
     except IOError as e:
         if e.errno != errno.ENOENT:
             raise
-        # Plod on with defaults, in case this is just a --help run.  If it's
-        # not, main() will fail anyway.
+        # Plod on with defaults.  There may be a legacy shell-script config.
         config = Config.get_defaults()
     # Merge legacy settings into our copy of the config.
     merge_legacy_ephemerals_config(config)
@@ -406,31 +403,11 @@
     return parser
 
 
-def convert_legacy_config():
-    """If appropriate, update config based on legacy shell-script config."""
-    config = Config.load_from_cache()
-    changed = merge_legacy_ephemerals_config(config)
-    if not changed:
-        return
-
-    logger.info(
-        "Updating configuration %s from legacy shell-script config %s.",
-        Config.DEFAULT_FILENAME, EPHEMERALS_LEGACY_CONFIG)
-    # Backup the old config.
-    Config.create_backup('legacy-script-migration')
-    # Save the new config.
-    Config.save(config)
-    # Now move the legacy config out of the way, so that we don't convert it
-    # a second time.
-    retire_legacy_config()
-
-
 def main(args):
     """Import ephemeral images.
 
     :param args: Command-line arguments, in parsed form.
     """
-    convert_legacy_config()
 
     def verify_signature(content, path):
         """Policy callback for Simplestreams: verify index signature."""

=== modified file 'src/provisioningserver/import_images/tests/test_ephemerals_script.py'
--- src/provisioningserver/import_images/tests/test_ephemerals_script.py	2013-10-09 11:35:14 +0000
+++ src/provisioningserver/import_images/tests/test_ephemerals_script.py	2013-10-11 13:46:22 +0000
@@ -27,10 +27,6 @@
 
 from fixtures import EnvironmentVariableFixture
 from maastesting.factory import factory
-from maastesting.utils import (
-    age_file,
-    get_write_time,
-    )
 from provisioningserver.config import Config
 from provisioningserver.import_images import (
     config as config_module,
@@ -38,7 +34,6 @@
     )
 from provisioningserver.import_images.ephemerals_script import (
     compose_filter,
-    convert_legacy_config,
     copy_file_by_glob,
     create_symlinked_image_dir,
     extract_image_tarball,
@@ -54,8 +49,6 @@
 from provisioningserver.utils import read_text_file
 from testtools.matchers import (
     FileContains,
-    FileExists,
-    Not,
     StartsWith,
     )
 
@@ -471,78 +464,12 @@
             original_boot_config,
             Config.load_from_cache()['boot'])
 
-
-class TestConvertLegacyConfig(PservTestCase):
-
-    def test_converts_legacy_config_if_old_config_available(self):
-        self.useFixture(ConfigFixture({'boot': {'ephemeral': {}}}))
-        data_dir = self.make_dir()
-        arches = [factory.make_name('arch')]
-        releases = [factory.make_name('rel')]
-        legacy_config = make_legacy_config(data_dir, arches, releases)
-        legacy_file = install_legacy_config(self, legacy_config)
-        initial_config = file(Config.DEFAULT_FILENAME).read()
-
-        convert_legacy_config()
-
-        self.assertEqual(
-            {
-                'architectures': arches,
-                'ephemeral': {
-                    'images_directory': data_dir,
-                    'releases': releases,
-                },
-            },
-            Config.load()['boot'])
-        # Legacy config file has been deleted.
-        self.assertThat(legacy_file, Not(FileExists()))
-        # A backup of the legacy config file has been kept.
-        self.assertThat(legacy_file + '.obsolete', FileContains(legacy_config))
-        # A copy of the initial config (i.e. the pserv config before
-        # migration) has been kept.
-        backup_name = Config._get_backup_name('legacy-script-migration')
-        self.assertThat(backup_name, FileContains(initial_config))
-
-    def test_does_nothing_without_old_config(self):
-        data_dir = self.make_dir()
-        arches = [factory.make_name('arch')]
-        releases = [factory.make_name('rel')]
-        config = self.useFixture(ConfigFixture(
-            {
-                'boot': {
-                    'architectures': arches,
-                    'ephemeral': {
-                        'images_directory': data_dir,
-                        'releases': releases,
-                    }
-                 }
-            }))
-        self.patch(config_module, 'parse_legacy_config').return_value = {}
-        age_file(config.filename, 600)
-        config_last_written = get_write_time(config.filename)
-
-        convert_legacy_config()
-
-        self.assertEqual(config_last_written, get_write_time(config.filename))
-        self.assertEqual(
-            {
-                'architectures': arches,
-                'ephemeral': {
-                    'images_directory': data_dir,
-                    'releases': releases,
-                },
-            },
-            Config.load()['boot'])
-
-    def test_is_idempotent(self):
-        self.useFixture(ConfigFixture({'boot': {'ephemeral': {}}}))
-        legacy_config = make_legacy_config()
-        legacy_file = install_legacy_config(self, legacy_config)
-        convert_legacy_config()
-        converted_config = Config.load()['boot']
-
-        convert_legacy_config()
-
-        self.assertEqual(converted_config, Config.load()['boot'])
-        self.assertThat(legacy_file, Not(FileExists()))
-        self.assertThat(legacy_file + '.obsolete', FileContains(legacy_config))
+    def test_uses_legacy_config(self):
+        data_dir = self.make_dir()
+        self.useFixture(ConfigFixture({}))
+        install_legacy_config(self, make_legacy_config(data_dir=data_dir))
+
+        parser = make_arg_parser(factory.getRandomString())
+
+        args = parser.parse_args('')
+        self.assertEqual(data_dir, args.output)