launchpad-reviewers team mailing list archive

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

Jeroen T. Vermeulen has proposed merging lp:~jtv/maas/tell-worker-its-nodegroup-name into lp:maas.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)

For more details, see:
https://code.launchpad.net/~jtv/maas/tell-worker-its-nodegroup-name/+merge/118923

We need this in order to hook up send_leases to the update_leases API call: the API call needs to be made at the right nodegroup, and the worker currently does not know where that is.

I ran my approach by Julian while he was too tired and preoccupied to defend himself, so let's call that a pre-imp.  He agreed that we don't want to make the API call respond differently depending on which worker-user API key is being used to make the request.  We did that for the metadata service, but as he points out, that was just because we were imitating EC2's API.  Maybe he was compos mentis after all.

I used the refresh_secrets mechanism (the name may want changing) since we already have it at hand.  It's not ideal to have the name sent asynchronously, but since we already have a card on the board for bootstrapping workers to request this information, we might as well add the node-group name there.  It also makes it easy to change a node group's name, without disasters ensuing if the worker happens to be unavailable, and I like to think it avoids complications in error handling during bootstrapping of node groups as well.

Oh, and using the existing mechanism is of course just incredibly easy!  It looks as if the mechanism may have to support multi-processing as well as multi-threading, but that will be a uniform change and shouldn't introduce any incremental complications as we add refreshable items.


Jeroen
-- 
https://code.launchpad.net/~jtv/maas/tell-worker-its-nodegroup-name/+merge/118923
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/tell-worker-its-nodegroup-name 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-09 10:40:27 +0000
@@ -12,7 +12,9 @@
 __metaclass__ = type
 __all__ = [
     'get_recorded_api_credentials',
+    'get_recorded_nodegroup_name',
     'record_api_credentials',
+    'record_nodegroup_name',
     ]
 
 # API credentials as last sent by the server.  The worker uses these
@@ -21,6 +23,11 @@
 recorded_api_credentials = None
 
 
+# The name of the nodegroup that this worker manages.
+# Shared between threads.
+recorded_nodegroup_name = None
+
+
 def record_api_credentials(api_credentials):
     """Update the recorded API credentials.
 
@@ -44,3 +51,17 @@
         return None
     else:
         return tuple(credentials_string.split(':'))
+
+
+def record_nodegroup_name(nodegroup_name):
+    """Record the name of the nodegroup we manage, as sent by the server."""
+    global recorded_nodegroup_name
+    recorded_nodegroup_name = nodegroup_name
+
+
+def get_recorded_nodegroup_name():
+    """Return the name of this worker's nodegroup, as sent by the server.
+
+    If the server has not sent the name yet, returns None.
+    """
+    return recorded_nodegroup_name

=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py	2012-08-09 04:18:15 +0000
+++ src/provisioningserver/tasks.py	2012-08-09 10:40:27 +0000
@@ -29,7 +29,10 @@
 
 from celery.task import task
 from celeryconfig import DHCP_CONFIG_FILE
-from provisioningserver.auth import record_api_credentials
+from provisioningserver.auth import (
+    record_api_credentials,
+    record_nodegroup_name,
+    )
 from provisioningserver.dhcp import (
     config,
     leases,
@@ -50,6 +53,7 @@
 refresh_functions = {
     'api_credentials': record_api_credentials,
     'omapi_shared_key': leases.record_omapi_shared_key,
+    'nodegroup_name': record_nodegroup_name,
 }
 
 
@@ -84,8 +88,13 @@
     To help catch simple programming mistakes, passing an unknown argument
     will result in an assertion failure.
 
+    :param api_credentials: A colon separated string containing this
+        worker's credentials for accessing the MAAS API: consumer key,
+        resource token, resource secret.
     :param omapi_shared_key: Shared key for working with the worker's
         DHCP server.
+    :param nodegroup_name: The name of the node group that this worker
+        manages.
     """
     for key, value in kwargs.items():
         assert key in refresh_functions, "Unknown refresh item: %s" % key

=== modified file 'src/provisioningserver/tests/test_auth.py'
--- src/provisioningserver/tests/test_auth.py	2012-08-09 04:18:15 +0000
+++ src/provisioningserver/tests/test_auth.py	2012-08-09 10:40:27 +0000
@@ -46,3 +46,8 @@
     def test_get_recorded_api_credentials_returns_None_without_creds(self):
         auth.record_api_credentials(None)
         self.assertIsNone(auth.get_recorded_api_credentials())
+
+    def test_get_recorded_nodegroup_name_vs_record_nodegroup_name(self):
+        nodegroup_name = factory.make_name('nodegroup')
+        auth.record_nodegroup_name(nodegroup_name)
+        self.assertEqual(nodegroup_name, auth.get_recorded_nodegroup_name())

=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py	2012-08-09 04:18:15 +0000
+++ src/provisioningserver/tests/test_tasks.py	2012-08-09 10:40:27 +0000
@@ -23,7 +23,10 @@
 from maastesting.testcase import TestCase
 from netaddr import IPNetwork
 from provisioningserver import tasks
-from provisioningserver.auth import get_recorded_api_credentials
+from provisioningserver.auth import (
+    get_recorded_api_credentials,
+    get_recorded_nodegroup_name,
+    )
 from provisioningserver.dhcp import leases
 from provisioningserver.dns.config import (
     conf,
@@ -109,6 +112,11 @@
         refresh_secrets(api_credentials=':'.join(credentials))
         self.assertEqual(credentials, get_recorded_api_credentials())
 
+    def test_updates_nodegroup_name(self):
+        nodegroup_name = factory.make_name('nodegroup')
+        refresh_secrets(nodegroup_name=nodegroup_name)
+        self.assertEqual(nodegroup_name, get_recorded_nodegroup_name())
+
     def test_updates_omapi_shared_key(self):
         self.patch(leases, 'recorded_omapi_shared_key', None)
         key = factory.make_name('omapi-shared-key')