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

[Blake Rouse]

Set the correct boot source selections and start importing of boot resources if the cluster already has boot images.

[Andres Rodriguez (andreserl)]

Should we consider that we need to import the same arches / releases as
those that have already been imported previously? I think we should.
On Apr 3, 2015 15:06, "Blake Rouse" <email address hidden> wrote:

> Blake Rouse has proposed merging lp:~blake-rouse/maas/fix-1439359 into
> lp:maas.
>
> Commit message:
> Automatically start importing of boot resources if the cluster already has
> boot images.
>
> Requested reviews:
> MAAS Maintainers (maas-maintainers)
> Related bugs:
> Bug #1439359 in MAAS: "When upgrading to MAAS 1.7 from MAAS 1.5, MAAS
> should trigger the image import automatically."
> https://bugs.launchpad.net/maas/+bug/1439359
>
> For more details, see:
> https://code.launchpad.net/~blake-rouse/maas/fix-1439359/+merge/255206
> --
> You are subscribed to branch lp:maas.
>
> === modified file 'src/maasserver/bootresources.py'
> --- src/maasserver/bootresources.py 2015-03-25 15:33:23 +0000
> +++ src/maasserver/bootresources.py 2015-04-03 19:06:06 +0000
> @@ -1027,6 +1027,13 @@
> images_link = absolute_url_reverse('images')
> boot_images_locally = list_boot_images()
> if len(boot_images_locally) > 0:
> + # Start the import process for the user, since the cluster
> + # already has images. Even though the cluster is usable
> the
> + # region will not be usable until it has boot images as
> well.
> + import_resources()
> +
> + # Show the warning as well. This will do away once the
> images
> + # have been imported.
> warning = (
> "Your cluster currently has boot images, but your
> region "
> "does not. Nodes will not be able to provision until
> you "
>
> === modified file 'src/maasserver/tests/test_bootresources.py'
> --- src/maasserver/tests/test_bootresources.py 2015-03-25 15:33:23 +0000
> +++ src/maasserver/tests/test_bootresources.py 2015-04-03 19:06:06 +0000
> @@ -1268,6 +1268,7 @@
> utils_module.settings, 'DEFAULT_MAAS_URL', 'http://%s' %
> abs_path)
> list_boot_images = self.patch(bootresources, "list_boot_images")
> list_boot_images.return_value = [make_rpc_boot_image()]
> + self.patch(bootresources, "import_resources")
>
> service = bootresources.ImportResourcesProgressService()
> service.check_boot_images()
> @@ -1284,6 +1285,18 @@
> normalise_whitespace(error_expected % images_link),
> error_observed)
>
> + def
> test__starts_import_if_boot_images_exists_on_cluster_not_region(self):
> + abs_path = "/%s/%s/" % (factory.make_string(),
> factory.make_string())
> + self.patch(
> + utils_module.settings, 'DEFAULT_MAAS_URL', 'http://%s' %
> abs_path)
> + list_boot_images = self.patch(bootresources, "list_boot_images")
> + list_boot_images.return_value = [make_rpc_boot_image()]
> + import_resources = self.patch(bootresources, "import_resources")
> +
> + service = bootresources.ImportResourcesProgressService()
> + service.check_boot_images()
> + self.assertThat(import_resourc...

[Andres Rodriguez (andreserl)]

lgtm!

review: Approve

[Andres Rodriguez (andreserl)]

lgtm! I only have one comment, not a blocker though.

review: Approve

=== modified file 'src/maasserver/start_up.py'
--- src/maasserver/start_up.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/start_up.py	2015-04-07 17:43:48 +0000
@@ -25,10 +25,18 @@
     locks,
     security,
 )
-from maasserver.bootresources import ensure_boot_source_definition
+from maasserver.bootresources import (
+    ensure_boot_source_definition,
+    import_resources,
+)
 from maasserver.dns.config import dns_update_all_zones
 from maasserver.fields import register_mac_type
-from maasserver.models import NodeGroup
+from maasserver.models import (
+    BootResource,
+    BootSource,
+    BootSourceSelection,
+    NodeGroup,
+)
 from maasserver.triggers import register_all_triggers
 from maasserver.utils import synchronised
 from maasserver.utils.orm import (
@@ -36,6 +44,7 @@
     transactional,
 )
 from provisioningserver.logger import get_maas_logger
+from provisioningserver.rpc.boot_images import list_boot_images
 from provisioningserver.upgrade_cluster import create_gnupg_home
 
 
@@ -90,6 +99,59 @@
             break
 
 
+def start_import_on_upgrade():
+    """Starts importing `BootResource`s on upgrade from MAAS where the boot
+    images where only stored on the clusters."""
+    # Do nothing, because `BootResource`s already exist.
+    if BootResource.objects.exists():
+        return
+
+    # Do nothing if the cluster on the machine does not have
+    # boot images present.
+    boot_images = list_boot_images()
+    if len(boot_images) == 0:
+        return
+
+    # Build the selections that need to be set based on the images
+    # that live on the cluster.
+    osystems = dict()
+    for image in boot_images:
+        osystem = image["osystem"]
+        if osystem not in osystems:
+            osystems[osystem] = {
+                "arches": set(),
+                "releases": set(),
+                "labels": set(),
+                }
+        osystems[osystem]["arches"].add(
+            image["architecture"])
+        osystems[osystem]["releases"].add(
+            image["release"])
+        osystems[osystem]["labels"].add(
+            image["label"])
+
+    # We have no way to truly know which boot source this came
+    # from, but since this should only occur on upgrade we
+    # take the first source, which will be the default source and
+    # apply the selection to that source.
+    boot_source = BootSource.objects.first()
+
+    # Clear all current selections and create the new selections
+    # based on the information retrieved from list_boot_images.
+    boot_source.bootsourceselection_set.all().delete()
+    for osystem, options in osystems.items():
+        for release in options["releases"]:
+            BootSourceSelection.objects.create(
+                boot_source=boot_source, os=osystem,
+                release=release, arches=list(options["arches"]),
+                subarches=["*"], labels=list(options["labels"]))
+
+    # Start the import process for the user, since the cluster
+    # already has images. Even though the cluster is usable the
+    # region will not be usable until it has boot images as well.
+    import_resources()
+
+
 @transactional
 @synchronised(locks.startup)
 def inner_start_up():
@@ -109,6 +171,9 @@
     # If there are no boot-source definitions yet, create defaults.
     ensure_boot_source_definition()
 
+    # Start import on upgrade if needed.
+    start_import_on_upgrade()
+
     # Register all of the triggers.
     register_all_triggers()
 

=== modified file 'src/maasserver/tests/test_start_up.py'
--- src/maasserver/tests/test_start_up.py	2015-03-25 15:33:23 +0000
+++ src/maasserver/tests/test_start_up.py	2015-04-07 17:43:48 +0000
@@ -19,20 +19,27 @@
     locks,
     start_up,
 )
+from maasserver.bootresources import ensure_boot_source_definition
+from maasserver.clusterrpc.testing.boot_images import make_rpc_boot_image
 from maasserver.models import (
     BootSource,
+    BootSourceSelection,
     NodeGroup,
 )
 from maasserver.testing.eventloop import RegionEventLoopFixture
+from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
-from maastesting.factory import factory
 from maastesting.fakemethod import FakeMethod
 from maastesting.matchers import (
     MockCalledOnceWith,
     MockCallsMatch,
+    MockNotCalled,
 )
 from mock import call
-from testtools.matchers import HasLength
+from testtools.matchers import (
+    Equals,
+    HasLength,
+)
 
 
 class LockChecker:
@@ -90,6 +97,51 @@
         self.expectThat(sleep, MockCalledOnceWith(10.0))
 
 
+class TestStartImportOnUpgrade(MAASServerTestCase):
+    """Tests for the `start_import_on_upgrade` function."""
+
+    def setUp(self):
+        super(TestStartImportOnUpgrade, self).setUp()
+        ensure_boot_source_definition()
+        self.mock_import_resources = self.patch(start_up, 'import_resources')
+
+    def test__does_nothing_if_boot_resources_exist(self):
+        mock_list_boot_images = self.patch(start_up, 'list_boot_images')
+        factory.make_BootResource()
+        start_up.start_import_on_upgrade()
+        self.assertThat(mock_list_boot_images, MockNotCalled())
+
+    def test__does_nothing_if_list_boot_images_is_empty(self):
+        self.patch(start_up, 'list_boot_images').return_value = []
+        start_up.start_import_on_upgrade()
+        self.assertThat(self.mock_import_resources, MockNotCalled())
+
+    def test__calls_import_resources(self):
+        self.patch(start_up, 'list_boot_images').return_value = [
+            make_rpc_boot_image(),
+            ]
+        start_up.start_import_on_upgrade()
+        self.assertThat(self.mock_import_resources, MockCalledOnceWith())
+
+    def test__sets_source_selections_based_on_boot_images(self):
+        boot_images = [
+            make_rpc_boot_image()
+            for _ in range(3)
+            ]
+        self.patch(start_up, 'list_boot_images').return_value = boot_images
+        start_up.start_import_on_upgrade()
+
+        boot_source = BootSource.objects.first()
+        for image in boot_images:
+            selection = BootSourceSelection.objects.get(
+                boot_source=boot_source, os=image["osystem"],
+                release=image["release"])
+            self.assertIsNotNone(selection)
+            self.expectThat(selection.arches, Equals([image["architecture"]]))
+            self.expectThat(selection.subarches, Equals(["*"]))
+            self.expectThat(selection.labels, Equals([image["label"]]))
+
+
 class TestInnerStartUp(MAASServerTestCase):
 
     """Tests for the actual work done in `inner_start_up`."""
@@ -127,3 +179,8 @@
         self.assertItemsEqual([], BootSource.objects.all())
         start_up.inner_start_up()
         self.assertThat(BootSource.objects.all(), HasLength(1))
+
+    def test__calls_start_import_on_upgrade(self):
+        mock_start_import = self.patch(start_up, 'start_import_on_upgrade')
+        start_up.inner_start_up()
+        self.expectThat(mock_start_import, MockCalledOnceWith())