launchpad-reviewers team mailing list archive

[Raphaël Badin <raphael.badin@xxxxxxxxxxxxx>]

Raphaël Badin has proposed merging lp:~rvb/maas/ui-import-pxe-files into lp:maas with lp:~rvb/maas/api-maas-import-pxe-files-2 as a prerequisite.

Commit message:
Add UI to import PXE files for all the accepted cluster controllers.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~rvb/maas/ui-import-pxe-files/+merge/133100

The UI could probably be improved a bit (I think we should use a table similar to the 
'Users' table) but this is a start: http://people.canonical.com/~rvb/import-pxe.png

= Notes =

Even if the code is very small, I've extracted the content of 'import_pxe_files_accepted_clusters' from the API code so that the API and the view code can call the same method instead of duplicating the code.
-- 
https://code.launchpad.net/~rvb/maas/ui-import-pxe-files/+merge/133100
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~rvb/maas/ui-import-pxe-files into lp:maas.

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-11-06 16:55:22 +0000
+++ src/maasserver/api.py	2012-11-06 16:55:22 +0000
@@ -1199,13 +1199,10 @@
 
     @operation(idempotent=False)
     def import_pxe_files(self, request):
-        """Import the pxe files on all the accepted cluster controllers."""
+        """Import the PXE files on all the accepted cluster controllers."""
         if not request.user.is_superuser:
             raise PermissionDenied("That method is reserved to admin users.")
-        accepted_nodegroups = NodeGroup.objects.filter(
-            status=NODEGROUP_STATUS.ACCEPTED)
-        for nodegroup in accepted_nodegroups:
-            nodegroup.import_pxe_files()
+        NodeGroup.objects.import_pxe_files_accepted_clusters()
         return HttpResponse(
             "Import of PXE files started on all cluster controllers",
             status=httplib.OK)

=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2012-11-06 16:55:22 +0000
+++ src/maasserver/models/nodegroup.py	2012-11-06 16:55:22 +0000
@@ -133,6 +133,13 @@
         return self._mass_change_status(
             NODEGROUP_STATUS.PENDING, NODEGROUP_STATUS.ACCEPTED)
 
+    def import_pxe_files_accepted_clusters(self):
+        """Import the PXE files on all the accepted cluster controllers."""
+        accepted_nodegroups = NodeGroup.objects.filter(
+            status=NODEGROUP_STATUS.ACCEPTED)
+        for nodegroup in accepted_nodegroups:
+            nodegroup.import_pxe_files()
+
 
 NODEGROUP_CLUSTER_NAME_TEMPLATE = "Cluster %(uuid)s"
 

=== modified file 'src/maasserver/static/css/modifiers.css'
--- src/maasserver/static/css/modifiers.css	2012-10-04 05:13:15 +0000
+++ src/maasserver/static/css/modifiers.css	2012-11-06 16:55:22 +0000
@@ -65,3 +65,6 @@
 .pad-top-large {
     padding-top: 40px;
     }
+.warning {
+    color: #9F6000;
+    }

=== modified file 'src/maasserver/templates/maasserver/settings.html'
--- src/maasserver/templates/maasserver/settings.html	2012-10-04 07:50:22 +0000
+++ src/maasserver/templates/maasserver/settings.html	2012-11-06 16:55:22 +0000
@@ -70,7 +70,7 @@
       <div class="clear"></div>
     </div>
     <div class="divider"></div>
-    <div id="clusters" class="block size7 first">
+    <div id="clusters" class="block size11 first">
       {% include "maasserver/settings_cluster_listing.html" %}
       <div class="clear"></div>
     </div>

=== modified file 'src/maasserver/templates/maasserver/settings_cluster_listing.html'
--- src/maasserver/templates/maasserver/settings_cluster_listing.html	2012-10-04 07:43:24 +0000
+++ src/maasserver/templates/maasserver/settings_cluster_listing.html	2012-11-06 16:55:22 +0000
@@ -7,6 +7,15 @@
     {% endfor %}
   </tbody>
 </table>
+<div>
+  <form method="POST"
+        action="{% url 'settings' %}">
+    {% csrf_token %}
+    <input type="hidden" name="import_all_pxe_files" value="1" />
+    <input type="submit" class="button right" value="Import PXE files" />
+  </form>
+</div>
+<div class="clear"></div>
 {% if pending_clusters %}
 <h3 id="pending-clusters">Pending clusters</h3>
 <table class="list" id="pending-clusters-list">

=== modified file 'src/maasserver/templates/maasserver/settings_cluster_listing_row.html'
--- src/maasserver/templates/maasserver/settings_cluster_listing_row.html	2012-10-25 13:17:36 +0000
+++ src/maasserver/templates/maasserver/settings_cluster_listing_row.html	2012-11-06 16:55:22 +0000
@@ -1,6 +1,11 @@
 <tr class="cluster {{ cycle }}"
     id="{{ cluster.cluster_name }}">
-  <td>{{ cluster.cluster_name }}</td>
+  <td>
+    {{ cluster.cluster_name }}
+    {% if cluster.bootimage_set.count == 0 %}
+      <span class="warning">(Warning: this cluster has no PXE files.)</span>
+    {% endif %}
+  </td>
   <td>
     <a href="{% url 'cluster-edit' cluster.uuid %}"
        class="icon"

=== modified file 'src/maasserver/tests/test_nodegroup.py'
--- src/maasserver/tests/test_nodegroup.py	2012-11-06 16:55:22 +0000
+++ src/maasserver/tests/test_nodegroup.py	2012-11-06 16:55:22 +0000
@@ -247,6 +247,23 @@
             (unaffected_status, 0),
             (reload_object(nodegroup).status, changed_count))
 
+    def test_import_pxe_files_accepted_clusters_calls_tasks(self):
+        recorder = self.patch(nodegroup_module, 'import_pxe_files', Mock())
+        proxy = factory.make_name('proxy')
+        Config.objects.set_config('http_proxy', proxy)
+        accepted_nodegroups = [
+            factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED),
+            factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED),
+        ]
+        factory.make_node_group(status=NODEGROUP_STATUS.REJECTED)
+        factory.make_node_group(status=NODEGROUP_STATUS.PENDING)
+        NodeGroup.objects.import_pxe_files_accepted_clusters()
+        calls = [
+            call(queue=nodegroup.work_queue, kwargs={'http_proxy': proxy})
+            for nodegroup in accepted_nodegroups
+            ]
+        self.assertItemsEqual(calls, recorder.apply_async.call_args_list)
+
 
 class TestNodeGroup(TestCase):
 

=== modified file 'src/maasserver/tests/test_views_settings.py'
--- src/maasserver/tests/test_views_settings.py	2012-10-23 09:56:29 +0000
+++ src/maasserver/tests/test_views_settings.py	2012-11-06 16:55:22 +0000
@@ -25,6 +25,7 @@
     )
 from maasserver.models import (
     Config,
+    nodegroup as nodegroup_module,
     UserProfile,
     )
 from maasserver.testing import (
@@ -33,6 +34,7 @@
     )
 from maasserver.testing.factory import factory
 from maasserver.testing.testcase import AdminLoggedInTestCase
+from mock import call
 
 
 class SettingsTest(AdminLoggedInTestCase):
@@ -215,6 +217,21 @@
             [reload_object(nodegroup).status for nodegroup in nodegroups],
             [NODEGROUP_STATUS.REJECTED] * 2)
 
+    def test_settings_import_pxe_files_calls_tasks(self):
+        recorder = self.patch(nodegroup_module, 'import_pxe_files')
+        accepted_nodegroups = [
+            factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED),
+            factory.make_node_group(status=NODEGROUP_STATUS.ACCEPTED),
+        ]
+        response = self.client.post('/settings/', {'import_all_pxe_files': 1})
+        self.assertEqual(httplib.FOUND, response.status_code)
+        calls = [
+           call(queue=nodegroup.work_queue, kwargs={'http_proxy': None})
+           for nodegroup in accepted_nodegroups
+        ]
+        self.assertItemsEqual(calls, recorder.apply_async.call_args_list)
+
+
 # Settable attributes on User.
 user_attributes = [
     'email',

=== modified file 'src/maasserver/views/settings.py'
--- src/maasserver/views/settings.py	2012-10-03 15:48:11 +0000
+++ src/maasserver/views/settings.py	2012-11-06 16:55:22 +0000
@@ -189,6 +189,16 @@
         messages.info(request, "Rejected %d cluster(s)." % number)
         return HttpResponseRedirect(reverse('settings'))
 
+    # Import PXE files for all the accepted clusters.
+    if 'import_all_pxe_files' in request.POST:
+        NodeGroup.objects.import_pxe_files_accepted_clusters()
+        message = (
+            "Import of PXE files started on all cluster controllers.  "
+            "Importing the PXE files can be take a long time depending on "
+            "the available bandwidth.")
+        messages.info(request, message)
+        return HttpResponseRedirect(reverse('settings'))
+
     # Cluster listings:
     accepted_clusters = NodeGroup.objects.filter(
         status=NODEGROUP_STATUS.ACCEPTED).order_by('cluster_name')