← Back to team overview

launchpad-reviewers team mailing list archive

[Merge] lp:~jtv/maas/1.2-bug-1084507 into lp:maas/1.2

 

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

Commit message:
Backport trunk r1391: Pass cluster uuid to pxeconfig API call, so it knows which cluster a request comes from.

Requested reviews:
  MAAS Maintainers (maas-maintainers)
Related bugs:
  Bug #1084507 in MAAS: "When pserv requests the pxeconfig from the region, the context nodegroup is not detected correctly."
  https://bugs.launchpad.net/maas/+bug/1084507

For more details, see:
https://code.launchpad.net/~jtv/maas/1.2-bug-1084507/+merge/139273
-- 
https://code.launchpad.net/~jtv/maas/1.2-bug-1084507/+merge/139273
Your team MAAS Maintainers is requested to review the proposed merge of lp:~jtv/maas/1.2-bug-1084507 into lp:maas/1.2.
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2012-12-04 10:17:17 +0000
+++ src/maasserver/api.py	2012-12-11 17:51:26 +0000
@@ -1822,6 +1822,16 @@
     return macaddress.node if macaddress else None
 
 
+def find_nodegroup_for_pxeconfig_request(request):
+    """Find the nodegroup responsible for a `pxeconfig` request.
+
+    Looks for the `cluster_uuid` parameter in the request.  If there is
+    none, figures it out based on the requesting IP as a compatibility
+    measure.  In that case, the result may be incorrect.
+    """
+    return find_nodegroup(request)
+
+
 def pxeconfig(request):
     """Get the PXE configuration given a node's details.
 
@@ -1845,6 +1855,10 @@
     :param subarch: Subarchitecture name (in the pxelinux namespace).
     :param local: The IP address of the cluster controller.
     :param remote: The IP address of the booting node.
+    :param cluster_uuid: UUID of the cluster responsible for this node.
+        If omitted, the call will attempt to figure it out based on the
+        requesting IP address, for compatibility.  Passing `cluster_uuid`
+        is preferred.
     """
     node = get_node_from_mac_string(request.GET.get('mac', None))
 
@@ -1889,7 +1903,7 @@
             # 1-1 mapping.
             subarch = pxelinux_subarch
 
-        nodegroup = find_nodegroup(request)
+        nodegroup = find_nodegroup_for_pxeconfig_request(request)
         preseed_url = compose_enlistment_preseed_url(nodegroup=nodegroup)
         hostname = 'maas-enlist'
         domain = Config.objects.get_config('enlistment_domain')

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2012-12-04 10:17:17 +0000
+++ src/maasserver/tests/test_api.py	2012-12-11 17:51:26 +0000
@@ -59,6 +59,7 @@
     extract_constraints,
     extract_oauth_key,
     extract_oauth_key_from_auth_header,
+    find_nodegroup_for_pxeconfig_request,
     get_oauth_token,
     get_overrided_query_dict,
     store_node_power_parameters,
@@ -3436,7 +3437,7 @@
             self.get_pxeconfig(),
             ContainsAll(KernelParameters._fields))
 
-    def test_pxeconfig_returns_data_for_known_node(self):
+    def test_pxeconfig_returns_success_for_known_node(self):
         params = self.get_mac_params()
         response = self.client.get(reverse('pxeconfig'), params)
         self.assertEqual(httplib.OK, response.status_code)
@@ -3446,7 +3447,7 @@
         response = self.client.get(reverse('pxeconfig'), params)
         self.assertEqual(httplib.NO_CONTENT, response.status_code)
 
-    def test_pxeconfig_returns_data_for_detailed_but_unknown_node(self):
+    def test_pxeconfig_returns_success_for_detailed_but_unknown_node(self):
         architecture = factory.getRandomEnum(ARCHITECTURE)
         arch, subarch = architecture.split('/')
         params = dict(
@@ -3559,6 +3560,19 @@
             compose_preseed_url(node),
             json.loads(response.content)["preseed_url"])
 
+    def find_nodegroup_for_pxeconfig_request_uses_cluster_uuid(self):
+        # find_nodegroup_for_pxeconfig_request returns the nodegroup
+        # identified by the cluster_uuid parameter, if given.  It
+        # completely ignores the other node or request details, as shown
+        # here by passing a uuid for a different cluster.
+        params = self.get_mac_params()
+        nodegroup = factory.make_node_group()
+        params['cluster_uuid'] = nodegroup.uuid
+        request = RequestFactory.get(reverse('pxeconfig'), **params)
+        self.assertEqual(
+            nodegroup,
+            find_nodegroup_for_pxeconfig_request(request))
+
     def test_preseed_url_for_known_node_uses_nodegroup_maas_url(self):
         ng_url = 'http://%s' % factory.make_name('host')
         network = IPNetwork("10.1.1/24")

=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py	2012-10-30 10:25:55 +0000
+++ src/provisioningserver/tests/test_tftp.py	2012-12-11 17:51:26 +0000
@@ -23,6 +23,7 @@
 
 from maastesting.factory import factory
 from maastesting.testcase import TestCase
+from provisioningserver import tftp as tftp_module
 from provisioningserver.pxe.tftppath import compose_config_path
 from provisioningserver.tests.test_kernel_opts import make_kernel_parameters
 from provisioningserver.tftp import (
@@ -211,6 +212,9 @@
     def test_get_reader_config_file(self):
         # For paths matching re_config_file, TFTPBackend.get_reader() returns
         # a Deferred that will yield a BytesReader.
+        cluster_uuid = factory.getRandomUUID()
+        self.patch(tftp_module, 'get_cluster_uuid').return_value = (
+            cluster_uuid)
         mac = factory.getRandomMACAddress(b"-")
         config_path = compose_config_path(mac)
         backend = TFTPBackend(self.make_dir(), b"http://example.com/";)
@@ -240,6 +244,7 @@
             "mac": mac,
             "local": call_context["local"][0],  # address only.
             "remote": call_context["remote"][0],  # address only.
+            "cluster_uuid": cluster_uuid,
             }
         observed_params = json.loads(output)
         self.assertEqual(expected_params, observed_params)

=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py	2012-10-30 10:25:55 +0000
+++ src/provisioningserver/tftp.py	2012-12-11 17:51:26 +0000
@@ -28,6 +28,7 @@
 from provisioningserver.enum import ARP_HTYPE
 from provisioningserver.kernel_opts import KernelParameters
 from provisioningserver.pxe.config import render_pxe_config
+from provisioningserver.start_cluster_controller import get_cluster_uuid
 from provisioningserver.utils import deferred
 from tftp.backend import (
     FilesystemSynchronousBackend,
@@ -217,6 +218,10 @@
             params["local"] = local_host
             remote_host, remote_port = get("remote", (None, None))
             params["remote"] = remote_host
+            # XXX 2012-12-04 JeroenVermeulen bug=1086239: move
+            # get_cluster_uuid to a properly reusable location, and
+            # implement it without the celery import (and side effects).
+            params["cluster_uuid"] = get_cluster_uuid()
             d = self.get_config_reader(params)
             d.addErrback(self.get_page_errback, file_name)
             return d