launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13911
[Merge] lp:~allenap/maas/one-tftp-server-per-interface into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/one-tftp-server-per-interface into lp:maas.
Commit message:
Start a TFTP server for each IPv4 address configured on a cluster controller's interfaces.
Previously there was a single server on all interfaces, which made it tough to figure out on which interface a particular datagram was received.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1068843 in MAAS: "maas-cluster-controller doesn't have images for provisioning"
https://bugs.launchpad.net/maas/+bug/1068843
For more details, see:
https://code.launchpad.net/~allenap/maas/one-tftp-server-per-interface/+merge/132252
--
https://code.launchpad.net/~allenap/maas/one-tftp-server-per-interface/+merge/132252
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/one-tftp-server-per-interface into lp:maas.
=== modified file 'src/provisioningserver/plugin.py'
--- src/provisioningserver/plugin.py 2012-08-16 08:07:06 +0000
+++ src/provisioningserver/plugin.py 2012-10-31 08:29:24 +0000
@@ -19,6 +19,7 @@
OOPSService,
)
from provisioningserver.tftp import TFTPBackend
+from provisioningserver.utils import get_all_interface_addresses
from tftp.protocol import TFTP
from twisted.application import internet
from twisted.application.internet import (
@@ -150,10 +151,17 @@
def _makeTFTPService(self, tftp_config):
"""Create the dynamic TFTP service."""
backend = TFTPBackend(tftp_config["root"], tftp_config["generator"])
- factory = TFTP(backend)
- tftp_service = internet.UDPServer(tftp_config["port"], factory)
- tftp_service.setName("tftp")
- return tftp_service
+ # Create a UDP server individually for each discovered network
+ # interface, so that we can detect the interface via which we have
+ # received a datagram.
+ tftp_services = MultiService()
+ tftp_services.setName("tftp")
+ for address in get_all_interface_addresses():
+ tftp_service = internet.UDPServer(
+ tftp_config["port"], TFTP(backend), interface=address)
+ tftp_service.setName(address)
+ tftp_service.setServiceParent(tftp_services)
+ return tftp_services
def makeService(self, options):
"""Construct a service."""
=== modified file 'src/provisioningserver/tests/test_plugin.py'
--- src/provisioningserver/tests/test_plugin.py 2012-08-16 10:50:16 +0000
+++ src/provisioningserver/tests/test_plugin.py 2012-10-31 08:29:24 +0000
@@ -17,6 +17,7 @@
from maastesting.factory import factory
from maastesting.testcase import TestCase
+from provisioningserver import plugin
from provisioningserver.plugin import (
Options,
ProvisioningRealm,
@@ -120,6 +121,13 @@
def test_tftp_service(self):
# A TFTP service is configured and added to the top-level service.
+ interfaces = [
+ factory.getRandomIPAddress(),
+ factory.getRandomIPAddress(),
+ ]
+ self.patch(
+ plugin, "get_all_interface_addresses",
+ lambda: interfaces)
config = {
"tftp": {
"generator": "http://candlemass/solitude";,
@@ -131,17 +139,23 @@
options["config-file"] = self.write_config(config)
service_maker = ProvisioningServiceMaker("Harry", "Hill")
service = service_maker.makeService(options)
- tftp_service = service.getServiceNamed("tftp")
- self.assertIsInstance(tftp_service, UDPServer)
- port, protocol = tftp_service.args
- self.assertEqual(config["tftp"]["port"], port)
- self.assertIsInstance(protocol, TFTP)
- self.assertIsInstance(protocol.backend, TFTPBackend)
- self.assertEqual(
- (config["tftp"]["root"],
- config["tftp"]["generator"]),
- (protocol.backend.base.path,
- protocol.backend.generator_url.geturl()))
+ tftp_services = service.getServiceNamed("tftp")
+ # The "tftp" service is a multi-service containing UDP servers for
+ # each interface defined by get_all_interface_addresses().
+ self.assertIsInstance(tftp_services, MultiService)
+ for interface in interfaces:
+ tftp_service = tftp_services.getServiceNamed(interface)
+ self.assertIsInstance(tftp_service, UDPServer)
+ port, protocol = tftp_service.args
+ self.assertEqual(config["tftp"]["port"], port)
+ self.assertIsInstance(protocol, TFTP)
+ self.assertIsInstance(protocol.backend, TFTPBackend)
+ self.assertEqual({"interface": interface}, tftp_service.kwargs)
+ self.assertEqual(
+ (config["tftp"]["root"],
+ config["tftp"]["generator"]),
+ (protocol.backend.base.path,
+ protocol.backend.generator_url.geturl()))
class TestSingleUsernamePasswordChecker(TestCase):
=== modified file 'src/provisioningserver/tests/test_utils.py'
--- src/provisioningserver/tests/test_utils.py 2012-09-25 05:21:24 +0000
+++ src/provisioningserver/tests/test_utils.py 2012-10-31 08:29:24 +0000
@@ -17,6 +17,7 @@
Namespace,
)
import os
+import netifaces
from random import randint
import stat
import StringIO
@@ -37,6 +38,7 @@
from mock import Mock
import provisioningserver
from provisioningserver.utils import (
+ get_all_interface_addresses,
ActionScript,
atomic_write,
AtomicWriteScript,
@@ -59,6 +61,58 @@
from testtools.testcase import ExpectedException
+class TestInterfaceFunctions(TestCase):
+ """Tests for functions relating to network interfaces."""
+
+ example_interfaces = {
+ 'eth0': {
+ 17: [{'addr': '00:1d:ba:86:aa:fe',
+ 'broadcast': 'ff:ff:ff:ff:ff:ff'}],
+ },
+ 'lo': {
+ 2: [{'addr': '127.0.0.1',
+ 'netmask': '255.0.0.0',
+ 'peer': '127.0.0.1'}],
+ 10: [{'addr': '::1',
+ 'netmask': 'ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff'}],
+ 17: [{'addr': '00:00:00:00:00:00',
+ 'peer': '00:00:00:00:00:00'}],
+ },
+ 'lxcbr0': {
+ 2: [{'addr': '10.0.3.1',
+ 'broadcast': '10.0.3.255',
+ 'netmask': '255.255.255.0'}],
+ 10: [{'addr': 'fe80::9894:6fff:fe8b:22%lxcbr0',
+ 'netmask': 'ffff:ffff:ffff:ffff::'}],
+ 17: [{'addr': '9a:94:6f:8b:00:22',
+ 'broadcast': 'ff:ff:ff:ff:ff:ff'}]},
+ 'tun0': {
+ 2: [{'addr': '10.99.244.250',
+ 'netmask': '255.255.255.255',
+ 'peer': '10.99.244.249'}],
+ },
+ 'wlan0': {
+ 2: [{'addr': '10.155.1.159',
+ 'broadcast': '10.155.31.255',
+ 'netmask': '255.255.224.0'}],
+ 10: [{'addr': 'fe80::221:5dff:fe85:d2e4%wlan0',
+ 'netmask': 'ffff:ffff:ffff:ffff::'}],
+ 17: [{'addr': '00:21:5d:85:d2:e4',
+ 'broadcast': 'ff:ff:ff:ff:ff:ff'}],
+ },
+ }
+
+ def test_get_all_interface_addresses(self):
+ # get_all_interface_addresses() returns the IPv4 addresses associated
+ # with each of the network devices present on the system, as reported
+ # by netifaces. IPv6 is ignored.
+ self.patch(netifaces, "interfaces", self.example_interfaces.keys)
+ self.patch(netifaces, "ifaddresses", self.example_interfaces.get)
+ self.assertEqual(
+ ["127.0.0.1", "10.0.3.1", "10.99.244.250", "10.155.1.159"],
+ list(get_all_interface_addresses()))
+
+
class TestSafe(TestCase):
"""Test `Safe`."""
=== modified file 'src/provisioningserver/utils.py'
--- src/provisioningserver/utils.py 2012-10-11 10:47:57 +0000
+++ src/provisioningserver/utils.py 2012-10-31 08:29:24 +0000
@@ -26,6 +26,7 @@
from argparse import ArgumentParser
import errno
from functools import wraps
+import netifaces
import os
from os import fdopen
from pipes import quote
@@ -460,3 +461,13 @@
atomic_write(
content, args.filename, overwrite=not args.no_overwrite,
mode=mode)
+
+
+def get_all_interface_addresses():
+ """For each network interface, yield its IPv4 address."""
+ for interface in netifaces.interfaces():
+ addresses = netifaces.ifaddresses(interface)
+ if netifaces.AF_INET in addresses:
+ for inet_address in addresses[netifaces.AF_INET]:
+ if "addr" in inet_address:
+ yield inet_address["addr"]
