launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #15428
[Merge] lp:~allenap/maas/find-nodegroup-redux into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/find-nodegroup-redux into lp:maas.
Commit message:
Find the requester's cluster controller by also matching unmanaged interfaces.
This allows MAAS installations that use custom DHCP servers to be supported. In addition, multiple cluster controllers can have interfaces on the same network. At most one of these interfaces can be "managed"; any more and an error will result. If multiple non-managed interfaces exist the first one to have been recorded in MAAS wins, and its cluster controller is chosen.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/maas/find-nodegroup-redux/+merge/157238
--
https://code.launchpad.net/~allenap/maas/find-nodegroup-redux/+merge/157238
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/find-nodegroup-redux into lp:maas.
=== modified file 'src/maasserver/exceptions.py'
--- src/maasserver/exceptions.py 2012-10-24 16:29:02 +0000
+++ src/maasserver/exceptions.py 2013-04-04 21:58:21 +0000
@@ -17,6 +17,7 @@
"MAASAPIException",
"MAASAPINotFound",
"NodeStateViolation",
+ "NodeGroupMisconfiguration",
]
@@ -114,3 +115,12 @@
def make_http_response(self):
return HttpResponseRedirect(unicode(self))
+
+
+class NodeGroupMisconfiguration(MAASAPIException):
+ """Node Groups (aka Cluster Controllers) are misconfigured.
+
+ This might mean that more than one controller is marked as managing the
+ same network
+ """
+ api_error = httplib.CONFLICT
=== modified file 'src/maasserver/utils/__init__.py'
--- src/maasserver/utils/__init__.py 2013-02-14 10:32:22 +0000
+++ src/maasserver/utils/__init__.py 2013-04-04 21:58:21 +0000
@@ -29,6 +29,7 @@
from django.conf import settings
from django.core.urlresolvers import reverse
from maasserver.enum import NODEGROUPINTERFACE_MANAGEMENT
+from maasserver.exceptions import NodeGroupMisconfiguration
from maasserver.utils.orm import get_one
@@ -143,24 +144,47 @@
# Circular imports.
from maasserver.models import NodeGroup
ip_address = request.META['REMOTE_ADDR']
- if ip_address is not None:
- management_statuses = (
- NODEGROUPINTERFACE_MANAGEMENT.DHCP,
- NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS,
- )
+ if ip_address is None:
+ return None
+ else:
+ # Fetch nodegroups with interfaces in the requester's network,
+ # preferring those with managed networks first. The `NodeGroup`
+ # objects returned are annotated with the `management` field of the
+ # matching `NodeGroupInterface`. See https://docs.djangoproject.com
+ # /en/dev/topics/db/sql/#adding-annotations for this curious feature
+ # of Django's ORM.
query = NodeGroup.objects.raw("""
- SELECT *
- FROM maasserver_nodegroup
- WHERE id IN (
- SELECT nodegroup_id
- FROM maasserver_nodegroupinterface
- WHERE (inet %s & subnet_mask) = (ip & subnet_mask)
- AND management IN %s
- )
- """, [
- ip_address,
- management_statuses,
- ]
- )
- return get_one(query)
- return None
+ SELECT
+ ng.*,
+ ngi.management
+ FROM
+ maasserver_nodegroup AS ng,
+ maasserver_nodegroupinterface AS ngi
+ WHERE
+ ng.id = ngi.nodegroup_id
+ AND
+ (inet %s & ngi.subnet_mask) = (ngi.ip & ngi.subnet_mask)
+ ORDER BY
+ ngi.management DESC,
+ ng.id ASC
+ """, [ip_address])
+ nodegroups = list(query)
+ if len(nodegroups) == 0:
+ return None
+ elif len(nodegroups) == 1:
+ return nodegroups[0]
+ else:
+ # There are multiple matching nodegroups. Only zero or one may
+ # have a managed interface, otherwise it is a misconfiguration.
+ unmanaged = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
+ nodegroups_with_managed_interfaces = {
+ nodegroup.id for nodegroup in nodegroups
+ if nodegroup.management != unmanaged
+ }
+ if len(nodegroups_with_managed_interfaces) > 1:
+ raise NodeGroupMisconfiguration(
+ "Multiple clusters on the same network; only "
+ "one cluster may manage the network of which "
+ "%s is a member." % ip_address)
+ else:
+ return nodegroups[0]
=== modified file 'src/maasserver/utils/tests/test_utils.py'
--- src/maasserver/utils/tests/test_utils.py 2013-02-14 10:32:22 +0000
+++ src/maasserver/utils/tests/test_utils.py 2013-04-04 21:58:21 +0000
@@ -12,6 +12,7 @@
__metaclass__ = type
__all__ = []
+import httplib
from urllib import urlencode
from django.conf import settings
@@ -22,11 +23,8 @@
NODE_STATUS_CHOICES,
NODEGROUPINTERFACE_MANAGEMENT,
)
-from maasserver.models import (
- NodeGroup,
- nodegroupinterface,
- NodeGroupInterface,
- )
+from maasserver.exceptions import NodeGroupMisconfiguration
+from maasserver.models import NodeGroupInterface
from maasserver.testing.factory import factory
from maasserver.testing.testcase import TestCase as DjangoTestCase
from maasserver.utils import (
@@ -234,14 +232,6 @@
nodegroup,
find_nodegroup(get_request(ip)))
- def test_find_nodegroup_looks_up_only_configured_interfaces(self):
- network = IPNetwork("192.168.41.0/24")
- factory.make_node_group(
- network=network,
- management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED)
- ip = factory.getRandomIPInNetwork(network)
- self.assertIsNone(find_nodegroup(get_request(ip)))
-
def test_find_nodegroup_accepts_any_ip_in_nodegroup_subnet(self):
nodegroup = factory.make_node_group(
network=IPNetwork("192.168.41.0/24"))
@@ -253,21 +243,76 @@
self.assertIsNone(
find_nodegroup(get_request(factory.getRandomIPAddress())))
- def test_find_nodegroup_errors_if_multiple_matches(self):
- self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1)
- factory.make_node_group(network=IPNetwork("10/8"))
- factory.make_node_group(network=IPNetwork("10.1.1/24"))
- self.assertRaises(
- NodeGroup.MultipleObjectsReturned,
- find_nodegroup, get_request('10.1.1.2'))
-
def test_find_nodegroup_handles_multiple_matches_on_same_nodegroup(self):
- self.patch(nodegroupinterface, "MINIMUM_NETMASK_BITS", 1)
- nodegroup = factory.make_node_group(network=IPNetwork("10/8"))
+ nodegroup = factory.make_node_group(network=IPNetwork("10.0/16"))
NodeGroupInterface.objects.create(
- nodegroup=nodegroup, ip='10.0.0.2', subnet_mask='255.0.0.0',
+ nodegroup=nodegroup, ip='10.0.0.2', subnet_mask='255.255.0.0',
broadcast_ip='10.0.0.1', interface='eth71')
NodeGroupInterface.objects.create(
- nodegroup=nodegroup, ip='10.0.0.3', subnet_mask='255.0.0.0',
+ nodegroup=nodegroup, ip='10.0.0.3', subnet_mask='255.255.0.0',
broadcast_ip='10.0.0.2', interface='eth72')
self.assertEqual(nodegroup, find_nodegroup(get_request('10.0.0.9')))
+
+ #
+ # Finding a node's nodegroup (aka cluster controller) in a nutshell:
+ #
+ # when 1 managed interface on the network = choose this one
+ # when >1 managed interfaces on the network = misconfiguration
+ # when 1 unmanaged interface on a network = choose this one
+ # when >1 unmanaged interfaces on a network = choose any
+ #
+
+ def test_1_managed_interface(self):
+ nodegroup = factory.make_node_group(
+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS,
+ network=IPNetwork("192.168.41.0/24"))
+ self.assertEqual(
+ nodegroup, find_nodegroup(get_request('192.168.41.199')))
+
+ def test_1_managed_interface_and_1_unmanaged(self):
+ # The managed nodegroup is chosen in preference to the unmanaged
+ # nodegroup.
+ nodegroup = factory.make_node_group(
+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS,
+ network=IPNetwork("192.168.41.0/24"))
+ factory.make_node_group(
+ management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED,
+ network=IPNetwork("192.168.41.0/16"))
+ self.assertEqual(
+ nodegroup, find_nodegroup(get_request('192.168.41.199')))
+
+ def test_more_than_1_managed_interface(self):
+ factory.make_node_group(
+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS,
+ network=IPNetwork("192.168.41.0/16"))
+ factory.make_node_group(
+ management=NODEGROUPINTERFACE_MANAGEMENT.DHCP_AND_DNS,
+ network=IPNetwork("192.168.41.0/24"))
+ exception = self.assertRaises(
+ NodeGroupMisconfiguration,
+ find_nodegroup, get_request('192.168.41.199'))
+ self.assertEqual(
+ (httplib.CONFLICT,
+ "Multiple clusters on the same network; only "
+ "one cluster may manage the network of which "
+ "192.168.41.199 is a member."),
+ (exception.api_error,
+ "%s" % exception))
+
+ def test_1_unmanaged_interface(self):
+ nodegroup = factory.make_node_group(
+ management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED,
+ network=IPNetwork("192.168.41.0/24"))
+ self.assertEqual(
+ nodegroup, find_nodegroup(get_request('192.168.41.199')))
+
+ def test_more_than_1_unmanaged_interface(self):
+ nodegroup1 = factory.make_node_group(
+ management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED,
+ network=IPNetwork("192.168.41.0/16"))
+ nodegroup2 = factory.make_node_group(
+ management=NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED,
+ network=IPNetwork("192.168.41.0/24"))
+ self.assertIn(
+ find_nodegroup(get_request('192.168.41.199')),
+ {nodegroup1, nodegroup2})
