launchpad-reviewers team mailing list archive

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

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/call-update-leases into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/call-update-leases/+merge/119077

Actually this requires lp:~jtv/maas/tell-worker-its-nodegroup-name to be landed, and some very minor conflicts to be resolved that it will have with that branch.  Adjacent new entries in __all__, that sort of thing.

In this branch you see a “missing link” being inserted: when a worker decides to send its DHCP leases to the MAAS server, it will now actually make the API call.  It probably won't actually work yet, since the worker does not know at the moment exactly where the MAAS API is to be found.  It's one item we can't just have the server send down using refresh_secrets, because a starting worker requests that refresh message through the very API that it does not yet know where to find.

Well, we could, if we wanted to.  We could just have the server send those refresh messages regularly, and the worker could just pick up the first one that came down the rabbit pipe and get to work.  It'd be exceedingly simple and robust in the face of changes and unreliable communications, just as long as the worker connects to the rabbit warren at some point.  But it would introduce a somewhat arbitrary delay, as well as another celerybeat or cron job — both of which have met with considerable controversy within the team.

Anyway.  For now I used a hard-coded default that could be made to work for a dev setup or for a production setup, but not both.  It's a horrible kludge and we'll want to fix it very soon, but we can argue over that once there are team members available to argue with.  I figure doing just enough to keep development going and forcing ourselves to plug the gap was better than going to the app-server config and thus establishing an unwanted temporary dependency that we might be tempted to extend before we got around to tearing it down again.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/call-update-leases/+merge/119077
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/call-update-leases into lp:maas.

=== modified file 'src/provisioningserver/auth.py'
--- src/provisioningserver/auth.py	2012-08-09 04:18:15 +0000
+++ src/provisioningserver/auth.py	2012-08-10 03:12:19 +0000
@@ -12,6 +12,7 @@
 __metaclass__ = type
 __all__ = [
     'get_recorded_api_credentials',
+    'locate_maas_api',
     'record_api_credentials',
     ]
 
@@ -21,6 +22,12 @@
 recorded_api_credentials = None
 
 
+def locate_maas_api():
+    """Return the base URL for the MAAS API."""
+# TODO: Configure this somehow.  What you see here is a placeholder.
+    return "http://localhost/MAAS/";
+
+
 def record_api_credentials(api_credentials):
     """Update the recorded API credentials.
 

=== modified file 'src/provisioningserver/dhcp/leases.py'
--- src/provisioningserver/dhcp/leases.py	2012-08-09 03:47:03 +0000
+++ src/provisioningserver/dhcp/leases.py	2012-08-10 03:12:19 +0000
@@ -37,7 +37,17 @@
     stat,
     )
 
+from apiclient.maas_client import (
+    MAASClient,
+    MAASDispatcher,
+    MAASOAuth,
+    )
 from celeryconfig import DHCP_LEASES_FILE
+from provisioningserver.auth import (
+    get_recorded_api_credentials,
+    get_recorded_nodegroup_name,
+    locate_maas_api,
+    )
 from provisioningserver.dhcp.leases_parser import parse_leases
 
 # Modification time on last-processed leases file.
@@ -146,6 +156,17 @@
 
 def send_leases(leases):
     """Send lease updates to the server API."""
+    api_credentials = get_recorded_api_credentials()
+    nodegroup_name = get_recorded_nodegroup_name()
+    if None in (api_credentials, nodegroup_name):
+        # The MAAS server hasn't sent us enough information for us to do
+        # this yet.  Leave it for another time.
+        return
+
+    api_path = 'nodegroups/%s/' % nodegroup_name
+    oauth = MAASOAuth(*get_recorded_api_credentials())
+    MAASClient(oauth, MAASDispatcher(), locate_maas_api()).post(
+        api_path, 'update_leases', leases=leases)
 
 
 def process_leases(timestamp, leases):

=== modified file 'src/provisioningserver/dhcp/tests/test_leases.py'
--- src/provisioningserver/dhcp/tests/test_leases.py	2012-08-09 03:05:38 +0000
+++ src/provisioningserver/dhcp/tests/test_leases.py	2012-08-10 03:12:19 +0000
@@ -18,6 +18,7 @@
     )
 from textwrap import dedent
 
+from apiclient.maas_client import MAASClient
 from maastesting.factory import factory
 from maastesting.fakemethod import FakeMethod
 from maastesting.testcase import TestCase
@@ -25,6 +26,10 @@
     age_file,
     get_write_time,
     )
+from provisioningserver.auth import (
+    record_api_credentials,
+    record_nodegroup_name,
+    )
 from provisioningserver.dhcp import leases as leases_module
 from provisioningserver.dhcp.leases import (
     check_lease_changes,
@@ -34,6 +39,7 @@
     record_lease_state,
     record_omapi_shared_key,
     register_new_leases,
+    send_leases,
     update_leases,
     upload_leases,
     )
@@ -163,21 +169,21 @@
 
     def test_update_leases_processes_leases_if_changed(self):
         record_lease_state(None, None)
-        send_leases = FakeMethod()
-        self.patch(leases_module, 'send_leases', send_leases)
+        fake_send_leases = FakeMethod()
+        self.patch(leases_module, 'send_leases', fake_send_leases)
         leases = self.make_lease()
         self.fake_leases_file(leases)
         self.patch(Omshell, 'create', FakeMethod())
         update_leases()
-        self.assertSequenceEqual([(leases, )], send_leases.extract_args())
+        self.assertSequenceEqual([(leases, )], fake_send_leases.extract_args())
 
     def test_update_leases_does_nothing_without_lease_changes(self):
-        send_leases = FakeMethod()
-        self.patch(leases_module, 'send_leases', send_leases)
+        fake_send_leases = FakeMethod()
+        self.patch(leases_module, 'send_leases', fake_send_leases)
         leases = self.make_lease()
         leases_file = self.fake_leases_file(leases)
         record_lease_state(get_write_time(leases_file), leases.copy())
-        self.assertSequenceEqual([], send_leases.calls)
+        self.assertSequenceEqual([], fake_send_leases.calls)
 
     def test_process_leases_records_update(self):
         record_lease_state(None, None)
@@ -232,13 +238,13 @@
         self.assertEqual([(ip, mac)], Omshell.create.extract_args())
 
     def test_upload_leases_processes_leases_unconditionally(self):
-        send_leases = FakeMethod()
+        fake_send_leases = FakeMethod()
         leases = self.make_lease()
         leases_file = self.fake_leases_file(leases)
         record_lease_state(get_write_time(leases_file), leases.copy())
-        self.patch(leases_module, 'send_leases', send_leases)
+        self.patch(leases_module, 'send_leases', fake_send_leases)
         upload_leases()
-        self.assertSequenceEqual([(leases, )], send_leases.extract_args())
+        self.assertSequenceEqual([(leases, )], fake_send_leases.extract_args())
 
     def test_parse_leases_file_parses_leases(self):
         params = {
@@ -315,3 +321,29 @@
         }
         register_new_leases(new_leases)
         self.assertEqual([], Omshell.create.calls)
+
+    def test_send_leases_posts_to_API(self):
+        self.patch(Omshell, 'create', FakeMethod())
+        record_api_credentials(
+            ':'.join(factory.getRandomString() for counter in range(3)))
+        nodegroup_name = factory.make_name('nodegroup')
+        record_nodegroup_name(nodegroup_name)
+        self.patch(MAASClient, 'post', FakeMethod())
+        leases = {
+            factory.getRandomIPAddress(): factory.getRandomMACAddress(),
+        }
+        send_leases(leases)
+        self.assertEqual([(
+                ('nodegroups/%s/' % nodegroup_name, 'update_leases'),
+                {'leases': leases},
+                )],
+            MAASClient.post.calls)
+
+    def test_send_leases_does_nothing_without_credentials(self):
+        record_api_credentials(None)
+        self.patch(MAASClient, 'post', FakeMethod())
+        leases = {
+            factory.getRandomIPAddress(): factory.getRandomMACAddress(),
+        }
+        send_leases(leases)
+        self.assertEqual([], MAASClient.post.calls)