launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #11755
[Merge] lp:~rvb/maas/use-worker-id into lp:maas
Raphaël Badin has proposed merging lp:~rvb/maas/use-worker-id into lp:maas with lp:~rvb/maas/nodegroup-uuid as a prerequisite.
Requested reviews:
MAAS Maintainers (maas-maintainers)
For more details, see:
https://code.launchpad.net/~rvb/maas/use-worker-id/+merge/123561
This branch uses the UUID introduce in the pre-req branch to identify the nodegroup everywhere.
= Pre-imp =
No real pre-imp for this but this is the logic follow-up branch after having introduced the nodegroup UUID.
= Notes =
More precisely this branch:
- exposes the UUID on the API
- uses the UUID to identify a nodegroup in the API
- replaces the usage of the nodegroup's name on the worker side with the usage of the nodegroup's UUID. This is fairly mecanical change. Note that this will have to be revisited once the UUID will be produced by the worker itself to be sent (via the API) to the region controller. But for now, this will do and everything will continue to work as it did before that change.
--
https://code.launchpad.net/~rvb/maas/use-worker-id/+merge/123561
Your team MAAS Maintainers is requested to review the proposed merge of lp:~rvb/maas/use-worker-id into lp:maas.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-09-03 04:59:58 +0000
+++ src/maasserver/api.py 2012-09-10 14:53:20 +0000
@@ -870,7 +870,7 @@
def read(self, request):
"""Index of node groups."""
return HttpResponse(sorted(
- [nodegroup.name for nodegroup in NodeGroup.objects.all()]))
+ [nodegroup.uuid for nodegroup in NodeGroup.objects.all()]))
@classmethod
def resource_uri(cls):
@@ -891,14 +891,14 @@
return HttpResponse("Sending worker refresh.", status=httplib.OK)
-def get_nodegroup_for_worker(request, nodegroup_name):
- """Get :class:`NodeGroup` by name, for access by its worker.
+def get_nodegroup_for_worker(request, uuid):
+ """Get :class:`NodeGroup` by uuid, for access by its worker.
This supports a nodegroup worker accessing its nodegroup object on
- the API. If the request is done by ayone but the worker for this
+ the API. If the request is done by anyone but the worker for this
particular nodegroup, the function raises :class:`PermissionDenied`.
"""
- nodegroup = get_object_or_404(NodeGroup, name=nodegroup_name)
+ nodegroup = get_object_or_404(NodeGroup, uuid=uuid)
try:
key = extract_oauth_key(request)
except Unauthorized as e:
@@ -906,7 +906,7 @@
if key != nodegroup.api_key:
raise PermissionDenied(
- "Only allowed for the %r worker." % nodegroup.name)
+ "Only allowed for the %r worker." % nodegroup.uuid)
return nodegroup
@@ -916,24 +916,24 @@
"""Node-group API."""
allowed_methods = ('GET', 'POST')
- fields = ('name', )
+ fields = ('name', 'uuid')
- def read(self, request, name):
+ def read(self, request, uuid):
"""GET a node group."""
- return get_object_or_404(NodeGroup, name=name)
+ return get_object_or_404(NodeGroup, uuid=uuid)
@classmethod
def resource_uri(cls, nodegroup=None):
if nodegroup is None:
- name = 'name'
+ uuid = 'uuid'
else:
- name = nodegroup.name
- return ('nodegroup_handler', [name])
+ uuid = nodegroup.uuid
+ return ('nodegroup_handler', [uuid])
@api_exported('POST')
- def update_leases(self, request, name):
+ def update_leases(self, request, uuid):
leases = get_mandatory_param(request.data, 'leases')
- nodegroup = get_nodegroup_for_worker(request, name)
+ nodegroup = get_nodegroup_for_worker(request, uuid)
leases = json.loads(leases)
new_leases = DHCPLease.objects.update_leases(nodegroup, leases)
if len(new_leases) > 0:
=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py 2012-09-10 14:53:20 +0000
+++ src/maasserver/models/nodegroup.py 2012-09-10 14:53:20 +0000
@@ -97,9 +97,9 @@
return master
- def get_by_natural_key(self, name):
- """For Django, a node group's name is a natural key."""
- return self.get(name=name)
+ def get_by_natural_key(self, uuid):
+ """For Django, a node group's uuid is a natural key."""
+ return self.get(uuid=uuid)
def refresh_workers(self):
"""Send refresh tasks to all node-group workers."""
=== modified file 'src/maasserver/refresh_worker.py'
--- src/maasserver/refresh_worker.py 2012-08-21 06:42:14 +0000
+++ src/maasserver/refresh_worker.py 2012-09-10 14:53:20 +0000
@@ -35,7 +35,7 @@
'api_credentials': convert_tuple_to_string(
get_creds_tuple(nodegroup.api_token)),
'maas_url': settings.DEFAULT_MAAS_URL,
- 'nodegroup_name': nodegroup.name,
+ 'nodegroup_uuid': nodegroup.uuid,
}
# XXX JeroenVermeulen 2012-08-21, bug=1039366: Route this to the
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-09-05 13:30:21 +0000
+++ src/maasserver/tests/test_api.py 2012-09-10 14:53:20 +0000
@@ -97,7 +97,7 @@
kernel_opts,
tasks,
)
-from provisioningserver.auth import get_recorded_nodegroup_name
+from provisioningserver.auth import get_recorded_nodegroup_uuid
from provisioningserver.dhcp.leases import send_leases
from provisioningserver.enum import (
POWER_TYPE,
@@ -2383,14 +2383,14 @@
nodegroup = factory.make_node_group()
response = self.client.get(reverse('nodegroups_handler'))
self.assertEqual(httplib.OK, response.status_code)
- self.assertIn(nodegroup.name, json.loads(response.content))
+ self.assertIn(nodegroup.uuid, json.loads(response.content))
def test_refresh_calls_refresh_worker(self):
nodegroup = factory.make_node_group()
response = self.client.post(
reverse('nodegroups_handler'), {'op': 'refresh_workers'})
self.assertEqual(httplib.OK, response.status_code)
- self.assertEqual(nodegroup.name, get_recorded_nodegroup_name())
+ self.assertEqual(nodegroup.uuid, get_recorded_nodegroup_uuid())
def test_refresh_does_not_return_secrets(self):
# The response from "refresh" contains only an innocuous
@@ -2427,16 +2427,16 @@
def test_reverse_points_to_nodegroup(self):
nodegroup = factory.make_node_group()
self.assertEqual(
- self.get_uri('nodegroups/%s/' % nodegroup.name),
- reverse('nodegroup_handler', args=[nodegroup.name]))
+ self.get_uri('nodegroups/%s/' % nodegroup.uuid),
+ reverse('nodegroup_handler', args=[nodegroup.uuid]))
def test_GET_returns_node_group(self):
nodegroup = factory.make_node_group()
response = self.client.get(
- reverse('nodegroup_handler', args=[nodegroup.name]))
+ reverse('nodegroup_handler', args=[nodegroup.uuid]))
self.assertEqual(httplib.OK, response.status_code)
self.assertEqual(
- nodegroup.name, json.loads(response.content).get('name'))
+ nodegroup.uuid, json.loads(response.content).get('uuid'))
def test_GET_returns_404_for_unknown_node_group(self):
response = self.client.get(
@@ -2449,7 +2449,7 @@
factory.make_dhcp_lease(nodegroup=nodegroup)
client = make_worker_client(nodegroup)
response = client.post(
- reverse('nodegroup_handler', args=[nodegroup.name]),
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
{
'op': 'update_leases',
'leases': json.dumps({}),
@@ -2465,7 +2465,7 @@
lease = factory.make_random_leases()
client = make_worker_client(nodegroup)
response = client.post(
- reverse('nodegroup_handler', args=[nodegroup.name]),
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
{
'op': 'update_leases',
'leases': json.dumps(lease),
@@ -2484,7 +2484,7 @@
lease = factory.make_random_leases()
client = make_worker_client(nodegroup)
response = client.post(
- reverse('nodegroup_handler', args=[nodegroup.name]),
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
{
'op': 'update_leases',
'leases': json.dumps(lease),
@@ -2503,7 +2503,7 @@
self.patch(Omshell, 'create', FakeMethod())
new_leases = factory.make_random_leases()
response = client.post(
- reverse('nodegroup_handler', args=[nodegroup.name]),
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
{
'op': 'update_leases',
'leases': json.dumps(new_leases),
@@ -2522,7 +2522,7 @@
client = make_worker_client(nodegroup)
self.patch(tasks, 'add_new_dhcp_host_map', FakeMethod())
response = client.post(
- reverse('nodegroup_handler', args=[nodegroup.name]),
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
{
'op': 'update_leases',
'leases': json.dumps(factory.make_random_leases()),
@@ -2541,7 +2541,8 @@
self.patch(MAASClient, 'post', Mock())
leases = factory.make_random_leases()
send_leases(leases)
- nodegroup_path = reverse('nodegroup_handler', args=[nodegroup.name])
+ nodegroup_path = reverse(
+ 'nodegroup_handler', args=[nodegroup.uuid])
nodegroup_path = nodegroup_path.decode('ascii').lstrip('/')
MAASClient.post.assert_called_once_with(
nodegroup_path, 'update_leases', leases=json.dumps(leases))
@@ -2559,14 +2560,14 @@
def test_nodegroup_requires_authentication(self):
nodegroup = factory.make_node_group()
response = self.client.get(
- reverse('nodegroup_handler', args=[nodegroup.name]))
+ reverse('nodegroup_handler', args=[nodegroup.uuid]))
self.assertEqual(httplib.UNAUTHORIZED, response.status_code)
def test_update_leases_works_for_nodegroup_worker(self):
nodegroup = factory.make_node_group()
client = make_worker_client(nodegroup)
response = client.post(
- reverse('nodegroup_handler', args=[nodegroup.name]),
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
{'op': 'update_leases', 'leases': json.dumps({})})
self.assertEqual(
httplib.OK, response.status_code,
@@ -2576,7 +2577,7 @@
nodegroup = factory.make_node_group()
self.log_in_as_normal_user()
response = self.client.post(
- reverse('nodegroup_handler', args=[nodegroup.name]),
+ reverse('nodegroup_handler', args=[nodegroup.uuid]),
{'op': 'update_leases', 'leases': json.dumps({})})
self.assertEqual(
httplib.FORBIDDEN, response.status_code,
@@ -2587,7 +2588,7 @@
about_nodegroup = factory.make_node_group()
client = make_worker_client(requesting_nodegroup)
response = client.post(
- reverse('nodegroup_handler', args=[about_nodegroup.name]),
+ reverse('nodegroup_handler', args=[about_nodegroup.uuid]),
{'op': 'update_leases', 'leases': json.dumps({})})
self.assertEqual(
httplib.FORBIDDEN, response.status_code,
=== modified file 'src/maasserver/tests/test_nodegroup.py'
--- src/maasserver/tests/test_nodegroup.py 2012-09-10 14:53:20 +0000
+++ src/maasserver/tests/test_nodegroup.py 2012-09-10 14:53:20 +0000
@@ -156,10 +156,11 @@
master.save()
self.assertEqual(ip, NodeGroup.objects.ensure_master().worker_ip)
- def test_get_by_natural_key_looks_up_by_name(self):
+ def test_get_by_natural_key_looks_up_by_uuid(self):
nodegroup = factory.make_node_group()
self.assertEqual(
- nodegroup, NodeGroup.objects.get_by_natural_key(nodegroup.name))
+ nodegroup,
+ NodeGroup.objects.get_by_natural_key(nodegroup.uuid))
def test_get_by_natural_key_will_not_return_other_nodegroup(self):
factory.make_node_group()
=== modified file 'src/maasserver/tests/test_refresh_worker.py'
--- src/maasserver/tests/test_refresh_worker.py 2012-08-20 11:15:44 +0000
+++ src/maasserver/tests/test_refresh_worker.py 2012-09-10 14:53:20 +0000
@@ -50,10 +50,10 @@
[(creds_string, )],
refresh_functions['api_credentials'].extract_args())
- def test_refreshes_nodegroup_name(self):
+ def test_refreshes_nodegroup_uuid(self):
refresh_functions = self.patch_refresh_functions()
nodegroup = factory.make_node_group()
refresh_worker(nodegroup)
self.assertEqual(
- [(nodegroup.name, )],
- refresh_functions['nodegroup_name'].extract_args())
+ [(nodegroup.uuid, )],
+ refresh_functions['nodegroup_uuid'].extract_args())
=== modified file 'src/maasserver/tests/test_start_up.py'
--- src/maasserver/tests/test_start_up.py 2012-08-23 07:57:12 +0000
+++ src/maasserver/tests/test_start_up.py 2012-09-10 14:53:20 +0000
@@ -64,11 +64,11 @@
def test_start_up_refreshes_workers(self):
patched_handlers = tasks.refresh_functions.copy()
- patched_handlers['nodegroup_name'] = Mock()
+ patched_handlers['nodegroup_uuid'] = Mock()
self.patch(tasks, 'refresh_functions', patched_handlers)
start_up.start_up()
- patched_handlers['nodegroup_name'].assert_called_once_with(
- NodeGroup.objects.ensure_master().name)
+ patched_handlers['nodegroup_uuid'].assert_called_once_with(
+ NodeGroup.objects.ensure_master().uuid)
def test_start_up_runs_in_exclusion(self):
called = Value('b', False)
=== modified file 'src/maasserver/urls_api.py'
--- src/maasserver/urls_api.py 2012-08-17 02:27:29 +0000
+++ src/maasserver/urls_api.py 2012-09-10 14:53:20 +0000
@@ -76,7 +76,7 @@
name='node_handler'),
url(r'nodes/$', nodes_handler, name='nodes_handler'),
url(
- r'nodegroups/(?P<name>[^/]+)/$',
+ r'nodegroups/(?P<uuid>[^/]+)/$',
nodegroup_handler, name='nodegroup_handler'),
url(r'files/$', files_handler, name='files_handler'),
url(r'account/$', account_handler, name='account_handler'),
=== modified file 'src/provisioningserver/auth.py'
--- src/provisioningserver/auth.py 2012-08-23 15:37:08 +0000
+++ src/provisioningserver/auth.py 2012-09-10 14:53:20 +0000
@@ -12,10 +12,10 @@
__metaclass__ = type
__all__ = [
'get_recorded_api_credentials',
- 'get_recorded_nodegroup_name',
+ 'get_recorded_nodegroup_uuid',
'get_recorded_maas_url',
'record_api_credentials',
- 'record_nodegroup_name',
+ 'record_nodegroup_uuid',
]
from apiclient.creds import convert_string_to_tuple
@@ -27,8 +27,8 @@
# Cache key for the API credentials as last sent by the server.
API_CREDENTIALS_CACHE_KEY = 'api_credentials'
-# Cache key for the name of the nodegroup that this worker manages.
-NODEGROUP_NAME_CACHE_KEY = 'nodegroup_name'
+# Cache key for the uuid of the nodegroup that this worker manages.
+NODEGROUP_UUID_CACHE_KEY = 'nodegroup_uuid'
def record_maas_url(maas_url):
@@ -65,14 +65,14 @@
return convert_string_to_tuple(credentials_string)
-def record_nodegroup_name(nodegroup_name):
- """Record the name of the nodegroup we manage, as sent by the server."""
- cache.cache.set(NODEGROUP_NAME_CACHE_KEY, nodegroup_name)
-
-
-def get_recorded_nodegroup_name():
- """Return the name of this worker's nodegroup, as sent by the server.
+def record_nodegroup_uuid(nodegroup_uuid):
+ """Record the uuid of the nodegroup we manage, as sent by the server."""
+ cache.cache.set(NODEGROUP_UUID_CACHE_KEY, nodegroup_uuid)
+
+
+def get_recorded_nodegroup_uuid():
+ """Return the uuid of this worker's nodegroup, as sent by the server.
If the server has not sent the name yet, returns None.
"""
- return cache.cache.get(NODEGROUP_NAME_CACHE_KEY)
+ return cache.cache.get(NODEGROUP_UUID_CACHE_KEY)
=== modified file 'src/provisioningserver/dhcp/leases.py'
--- src/provisioningserver/dhcp/leases.py 2012-08-27 07:12:44 +0000
+++ src/provisioningserver/dhcp/leases.py 2012-09-10 14:53:20 +0000
@@ -48,7 +48,7 @@
from provisioningserver.auth import (
get_recorded_api_credentials,
get_recorded_maas_url,
- get_recorded_nodegroup_name,
+ get_recorded_nodegroup_uuid,
)
from provisioningserver.dhcp.leases_parser import parse_leases
from provisioningserver.logging import task_logger
@@ -118,7 +118,7 @@
knowledge = {
'maas_url': get_recorded_maas_url(),
'api_credentials': get_recorded_api_credentials(),
- 'nodegroup_name': get_recorded_nodegroup_name(),
+ 'nodegroup_uuid': get_recorded_nodegroup_uuid(),
}
if None in knowledge.values():
# The MAAS server hasn't sent us enough information for us to do
@@ -130,7 +130,7 @@
% ', '.join(list_missing_items(knowledge)))
return
- api_path = 'api/1.0/nodegroups/%s/' % knowledge['nodegroup_name']
+ api_path = 'api/1.0/nodegroups/%s/' % knowledge['nodegroup_uuid']
oauth = MAASOAuth(*knowledge['api_credentials'])
MAASClient(oauth, MAASDispatcher(), knowledge['maas_url']).post(
api_path, 'update_leases', leases=json.dumps(leases))
=== modified file 'src/provisioningserver/dhcp/tests/test_leases.py'
--- src/provisioningserver/dhcp/tests/test_leases.py 2012-08-27 10:52:06 +0000
+++ src/provisioningserver/dhcp/tests/test_leases.py 2012-09-10 14:53:20 +0000
@@ -28,7 +28,7 @@
from provisioningserver import cache
from provisioningserver.auth import (
MAAS_URL_CACHE_KEY,
- NODEGROUP_NAME_CACHE_KEY,
+ NODEGROUP_UUID_CACHE_KEY,
)
from provisioningserver.dhcp import leases as leases_module
from provisioningserver.dhcp.leases import (
@@ -110,15 +110,15 @@
self.redirect_parser(leases_file)
return leases_file
- def set_nodegroup_name(self):
- """Set the recorded nodegroup name for the duration of this test."""
- name = factory.make_name('nodegroup')
- cache.cache.set(NODEGROUP_NAME_CACHE_KEY, name)
- return name
+ def set_nodegroup_uuid(self):
+ """Set the recorded nodegroup uuid for the duration of this test."""
+ uuid = factory.getRandomUUID()
+ cache.cache.set(NODEGROUP_UUID_CACHE_KEY, uuid)
+ return uuid
- def clear_nodegroup_name(self):
- """Clear the recorded nodegroup name."""
- cache.cache.set(NODEGROUP_NAME_CACHE_KEY, None)
+ def clear_nodegroup_uuid(self):
+ """Clear the recorded nodegroup uuid."""
+ cache.cache.set(NODEGROUP_UUID_CACHE_KEY, None)
def set_maas_url(self):
"""Set the recorded MAAS URL for the duration of this test."""
@@ -146,7 +146,7 @@
"""Set the recorded items required by `update_leases`."""
self.set_maas_url()
self.set_api_credentials()
- self.set_nodegroup_name()
+ self.set_nodegroup_uuid()
def set_lease_state(self, time=None, leases=None):
"""Set the recorded state of DHCP leases.
@@ -318,7 +318,7 @@
self.patch(MAASClient, 'post', FakeMethod())
self.set_lease_state()
self.set_items_needed_for_lease_update()
- self.clear_nodegroup_name()
+ self.clear_nodegroup_uuid()
leases = factory.make_random_leases()
send_leases(leases)
self.assertEqual([], MAASClient.post.calls)
=== modified file 'src/provisioningserver/tasks.py'
--- src/provisioningserver/tasks.py 2012-09-03 13:29:29 +0000
+++ src/provisioningserver/tasks.py 2012-09-10 14:53:20 +0000
@@ -35,7 +35,7 @@
from provisioningserver.auth import (
record_api_credentials,
record_maas_url,
- record_nodegroup_name,
+ record_nodegroup_uuid,
)
from provisioningserver.dhcp import config
from provisioningserver.dhcp.leases import upload_leases
@@ -54,7 +54,7 @@
refresh_functions = {
'api_credentials': record_api_credentials,
'maas_url': record_maas_url,
- 'nodegroup_name': record_nodegroup_name,
+ 'nodegroup_uuid': record_nodegroup_uuid,
}
@@ -92,7 +92,7 @@
:param api_credentials: A colon separated string containing this
worker's credentials for accessing the MAAS API: consumer key,
resource token, resource secret.
- :param nodegroup_name: The name of the node group that this worker
+ :param nodegroup_uuid: The uuid of the node group that this worker
manages.
"""
for key, value in kwargs.items():
=== modified file 'src/provisioningserver/tests/test_auth.py'
--- src/provisioningserver/tests/test_auth.py 2012-08-23 15:37:08 +0000
+++ src/provisioningserver/tests/test_auth.py 2012-09-10 14:53:20 +0000
@@ -46,7 +46,7 @@
def test_get_recorded_api_credentials_returns_None_without_creds(self):
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())
+ def test_get_recorded_nodegroup_uuid_vs_record_nodegroup_uuid(self):
+ nodegroup_uuid = factory.make_name('nodegroupuuid')
+ auth.record_nodegroup_uuid(nodegroup_uuid)
+ self.assertEqual(nodegroup_uuid, auth.get_recorded_nodegroup_uuid())
=== modified file 'src/provisioningserver/tests/test_tasks.py'
--- src/provisioningserver/tests/test_tasks.py 2012-09-05 06:38:49 +0000
+++ src/provisioningserver/tests/test_tasks.py 2012-09-10 14:53:20 +0000
@@ -112,10 +112,10 @@
api_credentials=convert_tuple_to_string(credentials))
self.assertEqual(credentials, auth.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, cache.cache.get('nodegroup_name'))
+ def test_updates_nodegroup_uuid(self):
+ nodegroup_uuid = factory.make_name('nodegroupuuid')
+ refresh_secrets(nodegroup_uuid=nodegroup_uuid)
+ self.assertEqual(nodegroup_uuid, cache.cache.get('nodegroup_uuid'))
class TestPowerTasks(PservTestCase):
