launchpad-reviewers team mailing list archive

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

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/bug-1041158 into lp:maas.

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1041158 in MAAS: "Worker not authenticating to API in upload_leases"
  https://bugs.launchpad.net/maas/+bug/1041158

For more details, see:
https://code.launchpad.net/~jtv/maas/bug-1041158/+merge/121207

When the scheduled job for DHCP updates came to the worker, the worker tried but failed to upload its dhcp leases to the MAAS server.  Gavin figured out what was wrong.  It construed the wrong URL to post for, without the /api/<version> prefix.

This branch fixes that.  I added an API test to ensure that it really gets the right path now.  And I discovered that there was a similar test in the provisioning-server code, but since it didn't have access to django's reverse>(), it was built on the same wrong idea of the API path.  So I deleted that test in favour of the new one.


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

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-08-24 12:52:34 +0000
+++ src/maasserver/tests/test_api.py	2012-08-24 15:47:20 +0000
@@ -28,6 +28,7 @@
 import random
 import shutil
 
+from apiclient.maas_client import MAASClient
 from django.conf import settings
 from django.contrib.auth.models import AnonymousUser
 from django.core.urlresolvers import reverse
@@ -63,6 +64,7 @@
     get_auth_tokens,
     )
 from maasserver.preseed import compose_preseed_url
+from maasserver.refresh_worker import refresh_worker
 from maasserver.testing import (
     reload_object,
     reload_objects,
@@ -86,11 +88,13 @@
     NodeUserData,
     )
 from metadataserver.nodeinituser import get_node_init_user
+from mock import Mock
 from provisioningserver import (
     kernel_opts,
     tasks,
     )
 from provisioningserver.auth import get_recorded_nodegroup_name
+from provisioningserver.dhcp.leases import send_leases
 from provisioningserver.enum import (
     POWER_TYPE,
     POWER_TYPE_CHOICES,
@@ -2535,6 +2539,20 @@
             (response.status_code, response.content))
         self.assertEqual([], tasks.add_new_dhcp_host_map.calls)
 
+    def test_worker_calls_update_leases(self):
+        # In bug 1041158, the worker's upload_leases task tried to call
+        # the update_leases API at the wrong URL path.  It has the right
+        # path now.
+        nodegroup = factory.make_node_group()
+        refresh_worker(nodegroup)
+        self.patch(MAASClient, 'post', Mock())
+        leases = factory.make_random_leases()
+        send_leases(leases)
+        nodegroup_path = reverse('nodegroup_handler', args=[nodegroup.name])
+        nodegroup_path = nodegroup_path.decode('ascii').lstrip('/')
+        MAASClient.post.assert_called_once_with(
+            nodegroup_path, 'update_leases', leases=leases)
+
 
 class TestNodeGroupAPIAuth(APIv10TestMixin, TestCase):
     """Authorization tests for nodegroup API."""

=== modified file 'src/provisioningserver/dhcp/leases.py'
--- src/provisioningserver/dhcp/leases.py	2012-08-23 15:37:08 +0000
+++ src/provisioningserver/dhcp/leases.py	2012-08-24 15:47:20 +0000
@@ -129,7 +129,7 @@
             % ', '.join(list_missing_items(knowledge)))
         return
 
-    api_path = 'nodegroups/%s/' % knowledge['nodegroup_name']
+    api_path = 'api/1.0/nodegroups/%s/' % knowledge['nodegroup_name']
     oauth = MAASOAuth(*knowledge['api_credentials'])
     MAASClient(oauth, MAASDispatcher(), knowledge['maas_url']).post(
         api_path, 'update_leases', leases=leases)

=== modified file 'src/provisioningserver/dhcp/tests/test_leases.py'
--- src/provisioningserver/dhcp/tests/test_leases.py	2012-08-23 15:37:08 +0000
+++ src/provisioningserver/dhcp/tests/test_leases.py	2012-08-24 15:47:20 +0000
@@ -297,19 +297,6 @@
             (get_write_time(leases_file), {params['ip']: params['mac']}),
             parse_leases_file())
 
-    def test_send_leases_posts_to_API(self):
-        self.patch(Omshell, 'create', FakeMethod())
-        self.set_items_needed_for_lease_update()
-        nodegroup_name = self.set_nodegroup_name()
-        self.patch(MAASClient, 'post', FakeMethod())
-        leases = factory.make_random_leases()
-        send_leases(leases)
-        self.assertEqual([(
-                ('nodegroups/%s/' % nodegroup_name, 'update_leases'),
-                {'leases': leases},
-                )],
-            MAASClient.post.calls)
-
     def test_send_leases_does_nothing_without_maas_url(self):
         self.patch(MAASClient, 'post', FakeMethod())
         self.set_lease_state()