launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #13878
[Merge] lp:~allenap/maas/pass-addresses-to-tftp-backend into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/pass-addresses-to-tftp-backend into lp:maas.
Commit message:
Use the node-facing address of the cluster controller as the iSCSI host.
The cluster and node addresses are passed over to the pxeconfig view.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/maas/pass-addresses-to-tftp-backend/+merge/132015
--
https://code.launchpad.net/~allenap/maas/pass-addresses-to-tftp-backend/+merge/132015
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/pass-addresses-to-tftp-backend into lp:maas.
=== modified file 'contrib/python-tx-tftp/tftp/protocol.py'
--- contrib/python-tx-tftp/tftp/protocol.py 2012-07-05 14:36:27 +0000
+++ contrib/python-tx-tftp/tftp/protocol.py 2012-10-30 08:03:21 +0000
@@ -12,6 +12,7 @@
from twisted.internet.defer import inlineCallbacks, returnValue
from twisted.internet.protocol import DatagramProtocol
from twisted.python import log
+from twisted.python.context import call
class TFTP(DatagramProtocol):
@@ -48,11 +49,22 @@
@inlineCallbacks
def _startSession(self, datagram, addr, mode):
+ # Set up a call context so that we can pass extra arbitrary
+ # information to interested backends without adding extra call
+ # arguments, or switching to using a request object, for example.
+ context = {}
+ if self.transport is not None:
+ # Add the local and remote addresses to the call context.
+ local = self.transport.getHost()
+ context["local"] = local.host, local.port
+ context["remote"] = addr
try:
if datagram.opcode == OP_WRQ:
- fs_interface = yield self.backend.get_writer(datagram.filename)
+ fs_interface = yield call(
+ context, self.backend.get_writer, datagram.filename)
elif datagram.opcode == OP_RRQ:
- fs_interface = yield self.backend.get_reader(datagram.filename)
+ fs_interface = yield call(
+ context, self.backend.get_reader, datagram.filename)
except Unsupported, e:
self.transport.write(ERRORDatagram.from_code(ERR_ILLEGAL_OP,
str(e)).to_wire(), addr)
=== modified file 'contrib/python-tx-tftp/tftp/test/test_protocol.py'
--- contrib/python-tx-tftp/tftp/test/test_protocol.py 2012-07-25 19:59:37 +0000
+++ contrib/python-tx-tftp/tftp/test/test_protocol.py 2012-10-30 08:03:21 +0000
@@ -11,9 +11,11 @@
from tftp.netascii import NetasciiReceiverProxy, NetasciiSenderProxy
from tftp.protocol import TFTP
from twisted.internet import reactor
-from twisted.internet.defer import Deferred
+from twisted.internet.address import IPv4Address
+from twisted.internet.defer import Deferred, inlineCallbacks
from twisted.internet.protocol import DatagramProtocol
from twisted.internet.task import Clock
+from twisted.python import context
from twisted.python.filepath import FilePath
from twisted.test.proto_helpers import StringTransport
from twisted.trial import unittest
@@ -50,7 +52,8 @@
def setUp(self):
self.clock = Clock()
- self.transport = FakeTransport(hostAddress=('127.0.0.1', self.port))
+ self.transport = FakeTransport(
+ hostAddress=IPv4Address('UDP', '127.0.0.1', self.port))
def test_malformed_datagram(self):
tftp = TFTP(BackendFactory(), _clock=self.clock)
@@ -247,3 +250,71 @@
self.clock.advance(1)
self.assertTrue(d.called)
self.assertTrue(IWriter.providedBy(d.result.backend))
+
+
+class CapturedContext(Exception):
+ """A donkey, to carry the call context back up the stack."""
+
+ def __init__(self, args, names):
+ super(CapturedContext, self).__init__(*args)
+ self.context = {name: context.get(name) for name in names}
+
+
+class ContextCapturingBackend(object):
+ """A fake `IBackend` that raises `CapturedContext`.
+
+ Calling `get_reader` or `get_writer` raises a `CapturedContext` exception,
+ which captures the values of the call context for the given `names`.
+ """
+
+ def __init__(self, *names):
+ self.names = names
+
+ def get_reader(self, file_name):
+ raise CapturedContext(("get_reader", file_name), self.names)
+
+ def get_writer(self, file_name):
+ raise CapturedContext(("get_writer", file_name), self.names)
+
+
+class HostTransport(object):
+ """A fake `ITransport` that only responds to `getHost`."""
+
+ def __init__(self, host):
+ self.host = host
+
+ def getHost(self):
+ return IPv4Address("UDP", *self.host)
+
+
+class BackendCallingContext(unittest.TestCase):
+
+ def setUp(self):
+ super(BackendCallingContext, self).setUp()
+ self.backend = ContextCapturingBackend("local", "remote")
+ self.tftp = TFTP(self.backend)
+ self.tftp.transport = HostTransport(("12.34.56.78", 1234))
+
+ @inlineCallbacks
+ def test_context_rrq(self):
+ rrq_datagram = RRQDatagram('nonempty', 'NetASCiI', {})
+ rrq_addr = ('127.0.0.1', 1069)
+ error = yield self.assertFailure(
+ self.tftp._startSession(rrq_datagram, rrq_addr, "octet"),
+ CapturedContext)
+ self.assertEqual(("get_reader", rrq_datagram.filename), error.args)
+ self.assertEqual(
+ {"local": self.tftp.transport.host, "remote": rrq_addr},
+ error.context)
+
+ @inlineCallbacks
+ def test_context_wrq(self):
+ wrq_datagram = WRQDatagram('nonempty', 'NetASCiI', {})
+ wrq_addr = ('127.0.0.1', 1069)
+ error = yield self.assertFailure(
+ self.tftp._startSession(wrq_datagram, wrq_addr, "octet"),
+ CapturedContext)
+ self.assertEqual(("get_writer", wrq_datagram.filename), error.args)
+ self.assertEqual(
+ {"local": self.tftp.transport.host, "remote": wrq_addr},
+ error.context)
=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py 2012-10-26 10:56:06 +0000
+++ src/maasserver/api.py 2012-10-30 08:03:21 +0000
@@ -1792,6 +1792,8 @@
:param arch: Architecture name (in the pxelinux namespace, eg. 'arm' not
'armhf').
: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.
"""
node = get_node_from_mac_string(request.GET.get('mac', None))
@@ -1846,11 +1848,12 @@
purpose = get_boot_purpose(node)
server_address = get_maas_facing_server_address()
+ cluster_address = get_mandatory_param(request.GET, "local")
params = KernelParameters(
arch=arch, subarch=subarch, release=series, purpose=purpose,
hostname=hostname, domain=domain, preseed_url=preseed_url,
- log_host=server_address, fs_host=server_address)
+ log_host=server_address, fs_host=cluster_address)
return HttpResponse(
json.dumps(params._asdict()),
=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py 2012-10-26 10:56:06 +0000
+++ src/maasserver/tests/test_api.py 2012-10-30 08:03:21 +0000
@@ -3202,11 +3202,16 @@
class TestPXEConfigAPI(AnonAPITestCase):
+ def get_default_params(self):
+ return {
+ "local": factory.getRandomIPAddress(),
+ "remote": factory.getRandomIPAddress(),
+ }
+
def get_mac_params(self):
- return {'mac': factory.make_mac_address().mac_address}
-
- def get_default_params(self):
- return dict()
+ params = self.get_default_params()
+ params['mac'] = factory.make_mac_address().mac_address
+ return params
def get_pxeconfig(self, params=None):
"""Make a request to `pxeconfig`, and return its response dict."""
@@ -3240,7 +3245,8 @@
ContainsAll(KernelParameters._fields))
def test_pxeconfig_returns_data_for_known_node(self):
- response = self.client.get(reverse('pxeconfig'), self.get_mac_params())
+ params = self.get_mac_params()
+ response = self.client.get(reverse('pxeconfig'), params)
self.assertEqual(httplib.OK, response.status_code)
def test_pxeconfig_returns_no_content_for_unknown_node(self):
@@ -3252,6 +3258,7 @@
architecture = factory.getRandomEnum(ARCHITECTURE)
arch, subarch = architecture.split('/')
params = dict(
+ self.get_default_params(),
mac=factory.getRandomMACAddress(delimiter=b'-'),
arch=arch,
subarch=subarch)
@@ -3280,15 +3287,19 @@
full_hostname = '.'.join([host, domain])
node = factory.make_node(hostname=full_hostname)
mac = factory.make_mac_address(node=node)
- pxe_config = self.get_pxeconfig(params={'mac': mac.mac_address})
+ params = self.get_default_params()
+ params['mac'] = mac.mac_address
+ pxe_config = self.get_pxeconfig(params)
self.assertEqual(host, pxe_config.get('hostname'))
self.assertNotIn(domain, pxe_config.values())
def test_pxeconfig_uses_nodegroup_domain_for_node(self):
mac = factory.make_mac_address()
+ params = self.get_default_params()
+ params['mac'] = mac
self.assertEqual(
mac.node.nodegroup.name,
- self.get_pxeconfig({'mac': mac.mac_address}).get('domain'))
+ self.get_pxeconfig(params).get('domain'))
def get_without_param(self, param):
"""Request a `pxeconfig()` response, but omit `param` from request."""
@@ -3353,6 +3364,13 @@
fake_boot_purpose,
json.loads(response.content)["purpose"])
+ def test_pxeconfig_returns_fs_host_as_cluster_controller(self):
+ # The kernel parameter `fs_host` points to the cluster controller
+ # address, which is passed over within the `local` parameter.
+ params = self.get_default_params()
+ kernel_params = KernelParameters(**self.get_pxeconfig(params))
+ self.assertEqual(params["local"], kernel_params.fs_host)
+
class TestNodeGroupsAPI(APIv10TestMixin, MultipleUsersScenarios, TestCase):
scenarios = [
=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py 2012-10-02 10:53:22 +0000
+++ src/provisioningserver/tests/test_tftp.py 2012-10-30 08:03:21 +0000
@@ -35,6 +35,7 @@
inlineCallbacks,
succeed,
)
+from twisted.python import context
from zope.interface.verify import verifyObject
@@ -213,6 +214,16 @@
mac = factory.getRandomMACAddress(b"-")
config_path = compose_config_path(mac)
backend = TFTPBackend(self.make_dir(), b"http://example.com/";)
+ # python-tx-tftp sets up call context so that backends can discover
+ # more about the environment in which they're running.
+ call_context = {
+ "local": (
+ factory.getRandomIPAddress(),
+ factory.getRandomPort()),
+ "remote": (
+ factory.getRandomIPAddress(),
+ factory.getRandomPort()),
+ }
@partial(self.patch, backend, "get_config_reader")
def get_config_reader(params):
@@ -220,9 +231,16 @@
params_json_reader = BytesReader(params_json)
return succeed(params_json_reader)
- reader = yield backend.get_reader(config_path)
+ reader = yield context.call(
+ call_context, backend.get_reader, config_path)
output = reader.read(10000)
- expected_params = dict(mac=mac)
+ # The addresses provided by python-tx-tftp in the call context are
+ # passed over the wire as address:port strings.
+ expected_params = {
+ "mac": mac,
+ "local": call_context["local"][0], # address only.
+ "remote": call_context["remote"][0], # address only.
+ }
observed_params = json.loads(output)
self.assertEqual(expected_params, observed_params)
=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py 2012-10-03 08:50:17 +0000
+++ src/provisioningserver/tftp.py 2012-10-30 08:03:21 +0000
@@ -34,6 +34,7 @@
IReader,
)
from tftp.errors import FileNotFound
+from twisted.python.context import get
from twisted.web.client import getPage
import twisted.web.error
from zope.interface import implementer
@@ -211,6 +212,13 @@
for key, value in config_file_match.groupdict().items()
if value is not None
}
+ # Send the local and remote endpoint addresses.
+ local_host, local_port = get("local", (None, None))
+ if local_host is not None:
+ params["local"] = local_host
+ remote_host, remote_port = get("remote", (None, None))
+ if remote_host is not None:
+ params["remote"] = remote_host
d = self.get_config_reader(params)
d.addErrback(self.get_page_errback, file_name)
return d
