Merge lp:~blake-rouse/maas/fix-1341001 into lp:~maas-committers/maas/trunk

[Blake Rouse]

Re-write maas.tgt on cluster upgrade, when moving from 1.5 to 1.6.

[Gavin Panella (allenap)]

Looks good, one comment in the diff.

Is there a reason to choose this approach rather than running an import? Would the ideal actually be to run this migration *and then* run an import too?

review: Approve

[Blake Rouse (blake-rouse)]

From 1.5 to 1.6 the removal of bootresources.yaml and hwe kernel imports by default. The import would take a while, making the upgrade a very long process.

This approach allows the user to keep all imported files, and allow them to still work with 1.6.

[Gavin Panella (allenap)]

> From 1.5 to 1.6 the removal of bootresources.yaml and hwe kernel
> imports by default. The import would take a while, making the upgrade
> a very long process.
>
> This approach allows the user to keep all imported files, and allow
> them to still work with 1.6.

Thanks for the explanations :)

=== modified file 'src/provisioningserver/import_images/boot_resources.py'
--- src/provisioningserver/import_images/boot_resources.py	2014-06-09 19:58:06 +0000
+++ src/provisioningserver/import_images/boot_resources.py	2014-07-14 14:30:58 +0000
@@ -159,14 +159,30 @@
     os.symlink(latest_snapshot, symlink_path)
 
 
-def write_snapshot_metadata(snapshot, meta_file_content, targets_conf,
-                            targets_conf_content):
-    """Write "meta" file and tgt config for `snapshot`."""
+def write_snapshot_metadata(snapshot, meta_file_content):
+    """Write "maas.meta" file."""
     meta_file = os.path.join(snapshot, 'maas.meta')
     atomic_write(meta_file_content, meta_file, mode=0644)
+
+
+def write_targets_conf(snapshot):
+    """Write "maas.tgt" file."""
+    targets_conf = os.path.join(snapshot, 'maas.tgt')
+    targets_conf_content = compose_targets_conf(snapshot)
     atomic_write(targets_conf_content, targets_conf, mode=0644)
 
 
+def update_targets_conf(snapshot):
+    """Runs tgt-admin to update the new targets from "maas.tgt"."""
+    targets_conf = os.path.join(snapshot, 'maas.tgt')
+    call_and_check([
+        'sudo',
+        '/usr/sbin/tgt-admin',
+        '--conf', targets_conf,
+        '--update', 'ALL',
+        ])
+
+
 def read_sources(sources_yaml):
     """Read boot resources config file.
 
@@ -228,25 +244,18 @@
 
     snapshot_path = download_all_boot_resources(
         sources, storage, product_mapping)
-    targets_conf = os.path.join(snapshot_path, 'maas.tgt')
-
-    targets_conf_content = compose_targets_conf(snapshot_path)
-
-    logger.info("Writing metadata and updating iSCSI targets.")
-    write_snapshot_metadata(
-        snapshot_path, meta_file_content, targets_conf, targets_conf_content)
-    call_and_check([
-        'sudo',
-        '/usr/sbin/tgt-admin',
-        '--conf', targets_conf,
-        '--update', 'ALL',
-        ])
+
+    logger.info("Writing metadata and iSCSI targets.")
+    write_snapshot_metadata(snapshot_path, meta_file_content)
+    write_targets_conf(snapshot_path)
 
     logger.info("Installing boot images snapshot %s.", snapshot_path)
     install_boot_loaders(snapshot_path)
 
     # If we got here, all went well.  This is now truly the "current" snapshot.
     update_current_symlink(storage, snapshot_path)
+    logger.info("Updating iSCSI targets.")
+    update_targets_conf(snapshot_path)
     logger.info("Import done.")
 
 

=== modified file 'src/provisioningserver/import_images/tests/test_boot_resources.py'
--- src/provisioningserver/import_images/tests/test_boot_resources.py	2014-06-20 13:38:39 +0000
+++ src/provisioningserver/import_images/tests/test_boot_resources.py	2014-07-14 14:30:58 +0000
@@ -424,6 +424,8 @@
         self.patch(boot_resources, 'install_boot_loaders')
         self.patch(boot_resources, 'update_current_symlink')
         self.patch(boot_resources, 'write_snapshot_metadata')
+        self.patch(boot_resources, 'write_targets_conf')
+        self.patch(boot_resources, 'update_targets_conf')
 
         fake_write_all_keyrings = self.patch(
             boot_resources, 'write_all_keyrings')

=== modified file 'src/provisioningserver/tests/test_upgrade_cluster.py'
--- src/provisioningserver/tests/test_upgrade_cluster.py	2014-07-08 13:39:40 +0000
+++ src/provisioningserver/tests/test_upgrade_cluster.py	2014-07-14 14:30:58 +0000
@@ -254,6 +254,7 @@
             os.makedirs(
                 os.path.join(
                     storage_dir, 'current', arch, subarch, release, label))
+        self.patch(upgrade_cluster, 'update_targets_conf')
         upgrade_cluster.migrate_architectures_into_ubuntu_directory()
         self.assertItemsEqual(
             arches,
@@ -279,6 +280,7 @@
                 factory.make_name('subarch'),
                 factory.make_name('release'),
                 factory.make_name('label')))
+        self.patch(upgrade_cluster, 'update_targets_conf')
         upgrade_cluster.migrate_architectures_into_ubuntu_directory()
         self.assertItemsEqual(
             [move_arch],
@@ -308,8 +310,39 @@
                 factory.make_name('subarch'),
                 factory.make_name('release'),
                 factory.make_name('label')))
-
+        self.patch(upgrade_cluster, 'update_targets_conf')
         upgrade_cluster.migrate_architectures_into_ubuntu_directory()
         self.assertItemsEqual(
             [move_arch],
             list_subdirs(os.path.join(storage_dir, 'current', 'ubuntu')))
+
+    def setup_working_migration_scenario(self):
+        storage_dir = self.make_dir()
+        self.configure_storage(storage_dir)
+        arches = [factory.make_name('arch') for _ in range(3)]
+        subarches = [factory.make_name('subarch') for _ in range(3)]
+        releases = [factory.make_name('release') for _ in range(3)]
+        labels = [factory.make_name('label') for _ in range(3)]
+        for arch, subarch, release, label in product(
+                arches, subarches, releases, labels):
+            os.makedirs(
+                os.path.join(
+                    storage_dir, 'current', arch, subarch, release, label))
+        return storage_dir
+
+    def test__calls_write_targets_conf_with_current_dir(self):
+        storage_dir = self.setup_working_migration_scenario()
+        mock_write = self.patch(upgrade_cluster, 'write_targets_conf')
+        self.patch(upgrade_cluster, 'update_targets_conf')
+        upgrade_cluster.migrate_architectures_into_ubuntu_directory()
+        self.assertThat(
+            mock_write,
+            MockCalledOnceWith(os.path.join(storage_dir, 'current')))
+
+    def test__calls_update_targets_conf_with_current_dir(self):
+        storage_dir = self.setup_working_migration_scenario()
+        mock_update = self.patch(upgrade_cluster, 'update_targets_conf')
+        upgrade_cluster.migrate_architectures_into_ubuntu_directory()
+        self.assertThat(
+            mock_update,
+            MockCalledOnceWith(os.path.join(storage_dir, 'current')))

=== modified file 'src/provisioningserver/upgrade_cluster.py'
--- src/provisioningserver/upgrade_cluster.py	2014-07-08 13:39:40 +0000
+++ src/provisioningserver/upgrade_cluster.py	2014-07-14 14:30:58 +0000
@@ -45,7 +45,10 @@
     drill_down,
     list_subdirs,
     )
-
+from provisioningserver.import_images.boot_resources import (
+    write_targets_conf,
+    update_targets_conf,
+    )
 
 logger = getLogger(__name__)
 
@@ -184,14 +187,20 @@
     # Extract the only top directories (arch) from the paths, as we only need
     # its name to move into the new 'ubuntu' folder.
     arches = {arch for arch, _, _, _ in paths}
+    if len(arches) == 0:
+        return
 
     # Create the ubuntu directory and move the archiecture folders under that
     # directory.
-    if len(arches) > 0:
-        ubuntu_dir = os.path.join(current_dir, 'ubuntu')
-        os.mkdir(ubuntu_dir)
-        for arch in arches:
-            shutil.move(os.path.join(current_dir, arch), ubuntu_dir)
+    ubuntu_dir = os.path.join(current_dir, 'ubuntu')
+    os.mkdir(ubuntu_dir)
+    for arch in arches:
+        shutil.move(os.path.join(current_dir, arch), ubuntu_dir)
+
+    # Re-write the maas.tgt to point to the new location for the ubuntu boot
+    # resources.
+    write_targets_conf(current_dir)
+    update_targets_conf(current_dir)
 
 
 # Upgrade hooks, from oldest to newest.  The hooks are callables, taking no