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

[Blake Rouse]

Fix the import boot images message to hide once boot resources exist.

[Jason Hobbs (jason-hobbs)]

Looks fine but I think the warning message should change a bit - comment below.

review: Approve

[Blake Rouse (blake-rouse)]

Thanks for the review.

I have fixed the requested items.

=== modified file 'src/maasserver/components.py'
--- src/maasserver/components.py	2013-10-07 09:12:40 +0000
+++ src/maasserver/components.py	2014-09-24 17:54:53 +0000
@@ -17,9 +17,14 @@
     "get_persistent_error",
     "get_persistent_errors",
     "register_persistent_error",
+    "show_missing_boot_image_error",
+    "hide_missing_boot_image_error",
     ]
 
+from textwrap import dedent
+
 from django.utils.safestring import mark_safe
+from maasserver.enum import COMPONENT
 from maasserver.models import ComponentError
 from maasserver.utils.orm import get_one
 
@@ -55,3 +60,18 @@
     """Return list of current persistent error messages."""
     return sorted(
         mark_safe(err.error) for err in ComponentError.objects.all())
+
+
+def show_missing_boot_image_error():
+    """Show missing boot images error on the UI."""
+    warning = dedent("""\
+        Boot image import process not started. Nodes will not be able to
+        provision without boot images. Start the boot images import process to
+        resolve this issue.
+        """)
+    register_persistent_error(COMPONENT.IMPORT_PXE_FILES, warning)
+
+
+def hide_missing_boot_image_error():
+    """Hide missing boot images error on the UI."""
+    discard_persistent_error(COMPONENT.IMPORT_PXE_FILES)

=== modified file 'src/maasserver/models/bootresource.py'
--- src/maasserver/models/bootresource.py	2014-09-17 13:17:42 +0000
+++ src/maasserver/models/bootresource.py	2014-09-24 17:54:53 +0000
@@ -22,6 +22,10 @@
     IntegerField,
     Manager,
     )
+from django.db.models.signals import (
+    post_delete,
+    post_save,
+    )
 from maasserver import DefaultMeta
 from maasserver.enum import (
     BOOT_RESOURCE_TYPE,
@@ -117,7 +121,9 @@
         arches = set()
         for resource in self.all():
             resource_set = resource.get_latest_set()
-            if resource_set.commissionable and resource_set.installable:
+            if (resource_set is not None and
+                    resource_set.commissionable and
+                    resource_set.installable):
                 arches.add(resource.architecture)
                 if 'subarches' in resource.extra:
                     arch, _ = resource.split_arch()
@@ -305,3 +311,22 @@
             return False
         subarches = self.extra['subarches'].split(',')
         return subarch in subarches
+
+
+def update_boot_image_error(sender, instance, **kwargs):
+    """Show or hide missing boot image error on the UI, depending on if any
+    boot resources exist.
+    """
+    # Avoid circular import
+    from maasserver.components import (
+        hide_missing_boot_image_error,
+        show_missing_boot_image_error,
+        )
+    if BootResource.objects.all().exists():
+        hide_missing_boot_image_error()
+    else:
+        show_missing_boot_image_error()
+
+
+post_delete.connect(update_boot_image_error, BootResource)
+post_save.connect(update_boot_image_error, BootResource)

=== modified file 'src/maasserver/models/tests/test_bootresource.py'
--- src/maasserver/models/tests/test_bootresource.py	2014-09-17 13:18:15 +0000
+++ src/maasserver/models/tests/test_bootresource.py	2014-09-24 17:54:53 +0000
@@ -23,12 +23,14 @@
     BOOT_RESOURCE_FILE_TYPE,
     BOOT_RESOURCE_TYPE,
     BOOT_RESOURCE_TYPE_CHOICES_DICT,
+    COMPONENT,
     )
 from maasserver.models import bootresource
 from maasserver.models.bootresource import (
     BootResource,
     RTYPE_REQUIRING_OS_SERIES_NAME,
     )
+from maasserver.models.component_error import ComponentError
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import MAASServerTestCase
 
@@ -430,3 +432,13 @@
             extra={'subarches': ','.join(subarches)})
         self.assertFalse(
             resource.supports_subarch(factory.make_name('subarch')))
+
+    def test_shows_and_hides_missing_boot_image_error(self):
+        resource = factory.make_BootResource()
+        self.assertFalse(
+            ComponentError.objects.filter(
+                component=COMPONENT.IMPORT_PXE_FILES).exists())
+        resource.delete()
+        self.assertTrue(
+            ComponentError.objects.filter(
+                component=COMPONENT.IMPORT_PXE_FILES).exists())

=== modified file 'src/maasserver/start_up.py'
--- src/maasserver/start_up.py	2014-09-23 19:54:04 +0000
+++ src/maasserver/start_up.py	2014-09-24 17:54:53 +0000
@@ -16,8 +16,6 @@
     'start_up'
     ]
 
-from textwrap import dedent
-
 from django.db import (
     connection,
     transaction,
@@ -28,11 +26,10 @@
     )
 from maasserver.bootresources import ensure_boot_source_definition
 from maasserver.components import (
-    get_persistent_error,
-    register_persistent_error,
+    hide_missing_boot_image_error,
+    show_missing_boot_image_error,
     )
 from maasserver.dns.config import write_full_dns_config
-from maasserver.enum import COMPONENT
 from maasserver.fields import register_mac_type
 from maasserver.models import (
     BootResource,
@@ -63,18 +60,6 @@
     eventloop.start().wait(10)
 
 
-def update_boot_resource_error():
-    import_script = COMPONENT.IMPORT_PXE_FILES
-    have_resources = BootResource.objects.all().exists()
-    if not have_resources and get_persistent_error(import_script) is None:
-        warning = dedent("""\
-            No boot images are available. Nodes will not be able to provision
-            without boot images. Start the boot images import process to
-            resolve this issue.
-            """)
-        register_persistent_error(import_script, warning)
-
-
 def inner_start_up():
     """Startup jobs that must run serialized w.r.t. other starting servers."""
     # Register our MAC data type with psycopg.
@@ -95,5 +80,8 @@
     # Regenerate MAAS's DNS configuration.  This should be reentrant, really.
     write_full_dns_config(reload_retry=True)
 
-    # Check whether we have boot images yet.
-    update_boot_resource_error()
+    # Check whether we have boot resources yet.
+    if not BootResource.objects.all().exists():
+        show_missing_boot_image_error()
+    else:
+        hide_missing_boot_image_error()

=== modified file 'src/maasserver/tests/test_start_up.py'
--- src/maasserver/tests/test_start_up.py	2014-09-23 21:21:43 +0000
+++ src/maasserver/tests/test_start_up.py	2014-09-24 17:54:53 +0000
@@ -19,11 +19,7 @@
     locks,
     start_up,
     )
-from maasserver.components import (
-    discard_persistent_error,
-    register_persistent_error,
-    )
-from maasserver.enum import COMPONENT
+from maasserver.components import hide_missing_boot_image_error
 from maasserver.models import (
     BootSource,
     NodeGroup,
@@ -37,12 +33,8 @@
     MockCalledOnceWith,
     MockCalledWith,
     )
-from mock import ANY
 from testresources import FixtureResource
-from testtools.matchers import (
-    HasLength,
-    Not,
-    )
+from testtools.matchers import HasLength
 
 
 class LockChecker:
@@ -120,35 +112,14 @@
     def test__warns_about_missing_boot_resources(self):
         # If no boot resources have been created, then the user has not
         # performed the import process.
-        discard_persistent_error(COMPONENT.IMPORT_PXE_FILES)
-        recorder = self.patch(start_up, 'register_persistent_error')
-
+        hide_missing_boot_image_error()
+        recorder = self.patch(start_up, 'show_missing_boot_image_error')
         start_up.inner_start_up()
-
-        self.assertThat(
-            recorder,
-            MockCalledWith(COMPONENT.IMPORT_PXE_FILES, ANY))
+        self.assertThat(recorder, MockCalledWith())
 
     def test__does_not_warn_if_boot_resources_are_known(self):
         # If boot resources are known, there is no warning.
         factory.make_BootResource()
-        recorder = self.patch(start_up, 'register_persistent_error')
-
-        start_up.inner_start_up()
-
-        self.assertThat(
-            recorder,
-            Not(MockCalledWith(COMPONENT.IMPORT_PXE_FILES, ANY)))
-
-    def test__does_not_warn_if_already_warning(self):
-        # If there already is a warning about missing boot resources, it will
-        # not be replaced.
-        register_persistent_error(
-            COMPONENT.IMPORT_PXE_FILES, factory.make_string())
-        recorder = self.patch(start_up, 'register_persistent_error')
-
-        start_up.inner_start_up()
-
-        self.assertThat(
-            recorder,
-            Not(MockCalledWith(COMPONENT.IMPORT_PXE_FILES, ANY)))
+        recorder = self.patch(start_up, 'hide_missing_boot_image_error')
+        start_up.inner_start_up()
+        self.assertThat(recorder, MockCalledWith())