launchpad-reviewers team mailing list archive

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

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1025582-ui into lp:maas with lp:~jtv/maas/bug-1025582-api as a prerequisite.

Requested reviews:
  MAAS Maintainers (maas-maintainers)

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1025582-ui/+merge/124116

See commit message.  Pre-imp with Julian.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/bug-1025582-ui/+merge/124116
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/bug-1025582-ui into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-09-13 07:34:27 +0000
+++ src/maasserver/api.py	2012-09-13 07:34:27 +0000
@@ -104,11 +104,19 @@
 from docutils import core
 from formencode import validators
 from formencode.validators import Invalid
+<<<<<<< TREE
 from maasserver.apidoc import (
     describe_handler,
     find_api_handlers,
     generate_api_docs,
     )
+=======
+from maasserver.components import (
+    COMPONENT,
+    discard_persistent_error,
+    register_persistent_error,
+    )
+>>>>>>> MERGE-SOURCE
 from maasserver.enum import (
     ARCHITECTURE,
     NODE_PERMISSION,
@@ -1181,11 +1189,25 @@
         """
         get_nodegroup_for_worker(request, 'master')
         images = json.loads(get_mandatory_param(request.data, 'images'))
+
         for image in images:
             BootImage.objects.register_image(
                 architecture=image['architecture'],
                 subarchitecture=image.get('subarchitecture', 'generic'),
                 release=image['release'],
                 purpose=image['purpose'])
+
+        if len(images) == 0:
+            warning = dedent("""\
+                No boot images have been imported yet.  Either the
+                maas-import-pxe-files script has not run yet, or it failed.
+
+                Try running it manually.  If it succeeds, this message should
+                go away within 5 minutes.
+                """)
+            register_persistent_error(COMPONENT.IMPORT_PXE_FILES, warning)
+        else:
+            discard_persistent_error(COMPONENT.IMPORT_PXE_FILES)
+
         return HttpResponse("Images noted.")
 >>>>>>> MERGE-SOURCE

=== modified file 'src/maasserver/models/bootimage.py'
--- src/maasserver/models/bootimage.py	2012-09-13 03:51:58 +0000
+++ src/maasserver/models/bootimage.py	2012-09-13 07:34:27 +0000
@@ -87,3 +87,11 @@
 
     # Boot purpose (e.g. "commissioning" or "install") that the image is for.
     purpose = CharField(max_length=255, blank=False, editable=False)
+
+    def __repr__(self):
+        return "<BootImage %s/%s-%s-%s>" % (
+            self.architecture,
+            self.subarchitecture,
+            self.release,
+            self.purpose,
+            )

=== modified file 'src/maasserver/start_up.py'
--- src/maasserver/start_up.py	2012-08-23 04:46:14 +0000
+++ src/maasserver/start_up.py	2012-09-13 07:34:27 +0000
@@ -15,10 +15,19 @@
     ]
 
 
+from textwrap import dedent
+
 from lockfile import FileLock
+from maasserver.components import (
+    COMPONENT,
+    register_persistent_error,
+    )
 from maasserver.dns import write_full_dns_config
 from maasserver.maasavahi import setup_maas_avahi_service
-from maasserver.models import NodeGroup
+from maasserver.models import (
+    BootImage,
+    NodeGroup,
+    )
 
 # Lock file used to prevent concurrent runs of the start_up() method.
 LOCK_FILE_NAME = '/run/lock/' + __name__
@@ -63,3 +72,16 @@
 
     # Send secrets etc. to workers.
     NodeGroup.objects.refresh_workers()
+
+    # Check whether we have boot images yet.
+    if not BootImage.objects.all().exists():
+        warning = dedent("""\
+            No boot images have been registered yet.  This may mean that the
+            maas-import-pxe-files script has not been run yet, or that it
+            failed; alternatively, there may be a communication problem
+            between the master worker and the MAAS server.
+
+            Try running maas-import-pxe-files manually.  If it succeeds, this
+            message should disappear within 5 minutes.
+            """)
+        register_persistent_error(COMPONENT.IMPORT_PXE_FILES, warning)

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-09-13 07:34:27 +0000
+++ src/maasserver/tests/test_api.py	2012-09-13 07:34:27 +0000
@@ -34,7 +34,10 @@
 from django.core.urlresolvers import reverse
 from django.http import QueryDict
 from fixtures import Fixture
-from maasserver import api
+from maasserver import (
+    api,
+    )
+from maasserver.components import COMPONENT
 from maasserver.api import (
     extract_constraints,
     extract_oauth_key,
@@ -2626,6 +2629,14 @@
 
 class TestBootImagesAPI(APITestCase):
 
+    def make_boot_image_params(self):
+        return {
+            'architecture': factory.make_name('architecture'),
+            'subarchitecture': factory.make_name('subarchitecture'),
+            'release': factory.make_name('release'),
+            'purpose': factory.make_name('purpose'),
+        }
+
     def report_images(self, images, client=None):
         if client is None:
             client = self.client
@@ -2651,12 +2662,7 @@
         self.assertEqual(httplib.FORBIDDEN, response.status_code)
 
     def test_report_boot_images_stores_images(self):
-        image = {
-            'architecture': factory.make_name('architecture'),
-            'subarchitecture': factory.make_name('subarchitecture'),
-            'release': factory.make_name('release'),
-            'purpose': factory.make_name('purpose'),
-        }
+        image = self.make_boot_image_params()
         client = make_worker_client(NodeGroup.objects.ensure_master())
         response = self.report_images([image], client=client)
         self.assertEqual(
@@ -2666,16 +2672,41 @@
             BootImage.objects.have_image(**image))
 
     def test_report_boot_images_ignores_unknown_image_properties(self):
-        image = {
-            'architecture': factory.make_name('architecture'),
-            'subarchitecture': factory.make_name('subarchitecture'),
-            'release': factory.make_name('release'),
-            'purpose': factory.make_name('purpose'),
-            'nonesuch': factory.make_name('nonesuch'),
-        }
+        image = self.make_boot_image_params()
+        image['nonesuch'] = factory.make_name('nonesuch'),
         client = make_worker_client(NodeGroup.objects.ensure_master())
         response = self.report_images([image], client=client)
         self.assertEqual(
             (httplib.OK, "Images noted."),
             (response.status_code, response.content))
+
+    def test_report_boot_images_warns_if_no_images_found(self):
+        recorder = self.patch(api, 'register_persistent_error')
+        client = make_worker_client(NodeGroup.objects.ensure_master())
+
+        response = self.report_images([], client=client)
+        self.assertEqual(
+            (httplib.OK, "Images noted."),
+            (response.status_code, response.content))
+
+        self.assertIn(
+            COMPONENT.IMPORT_PXE_FILES,
+            [args[0][0] for args in recorder.call_args_list])
+
+    def test_report_boot_images_removes_warning_if_images_found(self):
+        self.patch(api, 'register_persistent_error')
+        self.patch(api, 'discard_persistent_error')
+        client = make_worker_client(NodeGroup.objects.ensure_master())
+
+        response = self.report_images(
+            [self.make_boot_image_params()], client=client)
+        self.assertEqual(
+            (httplib.OK, "Images noted."),
+            (response.status_code, response.content))
+
+        self.assertItemsEqual(
+            [],
+            api.register_persistent_error.call_args_list)
+        api.discard_persistent_error.assert_called_once_with(
+            COMPONENT.IMPORT_PXE_FILES)
 >>>>>>> MERGE-SOURCE

=== modified file 'src/maasserver/tests/test_start_up.py'
--- src/maasserver/tests/test_start_up.py	2012-09-10 14:27:00 +0000
+++ src/maasserver/tests/test_start_up.py	2012-09-13 07:34:27 +0000
@@ -21,8 +21,15 @@
     FileLock,
     LockTimeout,
     )
-from maasserver import start_up
-from maasserver.models.nodegroup import NodeGroup
+from maasserver import (
+    components,
+    start_up,
+    )
+from maasserver.models import (
+    BootImage,
+    NodeGroup,
+    )
+from maasserver.testing.factory import factory
 from maastesting.celery import CeleryFixture
 from maastesting.fakemethod import FakeMethod
 from maastesting.testcase import TestCase
@@ -97,3 +104,29 @@
 
         self.assertRaises(LockTimeout, start_up.start_up)
         self.assertEqual(0, recorder.call_count)
+
+    def test_start_up_warns_about_missing_boot_images(self):
+        # If no boot images have been registered yet, that may mean that
+        # the import script has not been successfully run yet, or that
+        # the master worker is having trouble reporting its images.  And
+        # so start_up registers a persistent warning about this.
+        BootImage.objects.all().delete()
+        recorder = self.patch(start_up, 'register_persistent_error')
+
+        start_up.start_up()
+
+        self.assertIn(
+            components.COMPONENT.IMPORT_PXE_FILES,
+            [args[0][0] for args in recorder.call_args_list])
+
+    def test_start_up_does_not_warn_if_boot_images_are_known(self):
+        # If boot images are known, there is no warning about the import
+        # script.
+        factory.make_boot_image()
+        recorder = self.patch(start_up, 'register_persistent_error')
+
+        start_up.start_up()
+
+        self.assertNotIn(
+            components.COMPONENT.IMPORT_PXE_FILES,
+            [args[0][0] for args in recorder.call_args_list])

=== modified file 'src/provisioningserver/tests/test_maas_import_pxe_files.py'
--- src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-08-30 13:09:12 +0000
+++ src/provisioningserver/tests/test_maas_import_pxe_files.py	2012-09-13 07:34:27 +0000
@@ -98,7 +98,7 @@
         return archive
 
     def call_script(self, archive_dir, tftproot, arch=None, release=None):
-        """Call the maas-download-pxe-files script with given settings.
+        """Call the maas-import-pxe-files script with given settings.
 
         The ARCHIVE URL and TFTPROOT path must be set, or the script will try
         to download from the Ubuntu server and store into the system's real