launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #09645
[Merge] lp:~allenap/maas/dynamic-tftp into lp:maas
Gavin Panella has proposed merging lp:~allenap/maas/dynamic-tftp into lp:maas with lp:~allenap/maas/import-python-tx-tftp as a prerequisite.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
For more details, see:
https://code.launchpad.net/~allenap/maas/dynamic-tftp/+merge/113641
Integrated python-tx-tftp into the pserv daemon, bringing up a dynamic
TFTP daemon on port 5244 by default. It's not fully configured yet,
but once up you can test it with:
$ make start
$ mkdir tmp && cd tmp
$ tftp localhost 5244 -c get setup.py
$ diff setup.py ../setup.py
$ tftp localhost 5244 -c get maas/i386/generic/pxelinux.cfg/fred
$ cat fred
There is more work to be done to get this fully working both in
python-tx-tftp (its timeout handling is a bit odd for one, though it
appears to operate fine) and in maas (paths are probably not all
correct; more work may need to be done getting the correct MAC from
the file requested), but this is a good point to land.
--
https://code.launchpad.net/~allenap/maas/dynamic-tftp/+merge/113641
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/maas/dynamic-tftp into lp:maas.
=== modified file 'Makefile'
--- Makefile 2012-06-26 16:49:03 +0000
+++ Makefile 2012-07-05 20:37:22 +0000
@@ -88,7 +88,7 @@
bin/test.maastesting
bin/test.pserv
-lint: sources = contrib setup.py src templates twisted utilities
+lint: sources = $(wildcard *.py contrib/*.py) src templates twisted utilities
lint: bin/flake8
@find $(sources) -name '*.py' ! -path '*/migrations/*' \
-print0 | xargs -r0 bin/flake8
=== modified file 'buildout.cfg'
--- buildout.cfg 2012-06-15 15:31:42 +0000
+++ buildout.cfg 2012-07-05 20:37:22 +0000
@@ -131,7 +131,9 @@
entry-points =
maas-provision=provisioningserver.__main__:main
twistd.pserv=twisted.scripts.twistd:run
-extra-paths = ${common:extra-paths}
+extra-paths =
+ ${common:extra-paths}
+ contrib/python-tx-tftp
scripts =
maas-provision
twistd.pserv
@@ -146,7 +148,7 @@
test.pserv=nose.core:TestProgram
initialization =
sys.argv[1:1] = ["--where=src/provisioningserver"]
-extra-paths = ${common:extra-paths}
+extra-paths = ${pserv:extra-paths}
scripts =
test.pserv
=== modified file 'etc/pserv.yaml'
--- etc/pserv.yaml 2012-04-18 15:36:43 +0000
+++ etc/pserv.yaml 2012-07-05 20:37:22 +0000
@@ -56,3 +56,11 @@
url: http://local.cobbler.dev/cobbler_api
username: cobbler
password: xcobbler
+
+## TFTP configuration.
+#
+tftp:
+ # root: <current directory>
+ # port: 5244
+ ## The URL to be contacted to generate PXE configurations.
+ # generator: http://localhost:5243/api/1.0/pxeconfig
=== modified file 'src/provisioningserver/plugin.py'
--- src/provisioningserver/plugin.py 2012-06-22 09:48:17 +0000
+++ src/provisioningserver/plugin.py 2012-07-05 20:37:22 +0000
@@ -13,6 +13,7 @@
__all__ = []
from getpass import getuser
+from os import getcwd
from formencode import Schema
from formencode.validators import (
@@ -28,6 +29,9 @@
LogService,
OOPSService,
)
+from provisioningserver.tftp import TFTPBackend
+from tftp.protocol import TFTP
+from twisted.application import internet
from twisted.application.internet import (
TCPClient,
TCPServer,
@@ -141,6 +145,19 @@
password = String(if_missing=b"test")
+class ConfigTFTP(Schema):
+ """Configuration validator for the TFTP service."""
+
+ if_key_missing = None
+
+ root = String(if_missing=getcwd())
+ port = Int(min=1, max=65535, if_missing=5244)
+ generator = URL(
+ add_http=True, require_tld=False,
+ if_missing=b"http://localhost:5243/api/1.0/pxeconfig";,
+ )
+
+
class Config(Schema):
"""Configuration validator."""
@@ -154,6 +171,7 @@
oops = ConfigOops
broker = ConfigBroker
cobbler = ConfigCobbler
+ tftp = ConfigTFTP
@classmethod
def parse(cls, stream):
@@ -243,6 +261,14 @@
client_service.setName("amqp")
return client_service
+ 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
+
def makeService(self, options):
"""Construct a service."""
services = MultiService()
@@ -269,4 +295,7 @@
site_service = self._makeSiteService(papi_root, config)
site_service.setServiceParent(services)
+ tftp_service = self._makeTFTPService(config["tftp"])
+ tftp_service.setServiceParent(services)
+
return services
=== modified file 'src/provisioningserver/tests/test_plugin.py'
--- src/provisioningserver/tests/test_plugin.py 2012-06-22 09:48:17 +0000
+++ src/provisioningserver/tests/test_plugin.py 2012-07-05 20:37:22 +0000
@@ -32,6 +32,7 @@
SingleUsernamePasswordChecker,
)
from provisioningserver.testing.fakecobbler import make_fake_cobbler_session
+from provisioningserver.tftp import TFTPBackend
from testtools.deferredruntest import (
assert_fails_with,
AsynchronousDeferredRunTest,
@@ -40,7 +41,11 @@
MatchesException,
Raises,
)
-from twisted.application.internet import TCPServer
+from tftp.protocol import TFTP
+from twisted.application.internet import (
+ TCPServer,
+ UDPServer,
+ )
from twisted.application.service import MultiService
from twisted.cred.credentials import UsernamePassword
from twisted.cred.error import UnauthorizedLogin
@@ -78,6 +83,11 @@
'directory': '',
'reporter': '',
},
+ 'tftp': {
+ 'generator': 'http://localhost:5243/api/1.0/pxeconfig',
+ 'port': 5244,
+ 'root': os.getcwd(),
+ },
'interface': '127.0.0.1',
'port': 5241,
'username': getuser(),
@@ -176,7 +186,7 @@
service = service_maker.makeService(options)
self.assertIsInstance(service, MultiService)
self.assertSequenceEqual(
- ["log", "oops", "site"],
+ ["log", "oops", "site", "tftp"],
sorted(service.namedServices))
self.assertEqual(
len(service.namedServices), len(service.services),
@@ -194,7 +204,7 @@
service = service_maker.makeService(options)
self.assertIsInstance(service, MultiService)
self.assertSequenceEqual(
- ["amqp", "log", "oops", "site"],
+ ["amqp", "log", "oops", "site", "tftp"],
sorted(service.namedServices))
self.assertEqual(
len(service.namedServices), len(service.services),
@@ -268,6 +278,31 @@
# The request has not been authorized.
self.assertEqual(httplib.UNAUTHORIZED, request.responseCode)
+ def test_tftp_service(self):
+ # A TFTP service is configured and added to the top-level service.
+ config = {
+ "tftp": {
+ "generator": "http://candlemass/solitude";,
+ "root": self.tempdir,
+ "port": factory.getRandomPort(),
+ },
+ }
+ options = Options()
+ 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()))
+
class TestSingleUsernamePasswordChecker(TestCase):
"""Tests for `SingleUsernamePasswordChecker`."""
=== added file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py 1970-01-01 00:00:00 +0000
+++ src/provisioningserver/tests/test_tftp.py 2012-07-05 20:37:22 +0000
@@ -0,0 +1,136 @@
+# Copyright 2005-2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Tests for the maastftp Twisted plugin."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = []
+
+from os import path
+from urllib import urlencode
+from urlparse import (
+ parse_qsl,
+ urlparse,
+ )
+
+from maastesting.factory import factory
+from maastesting.testcase import TestCase
+from provisioningserver.pxe.tftppath import compose_config_path
+from provisioningserver.tftp import (
+ BytesReader,
+ TFTPBackend,
+ )
+from testtools.deferredruntest import AsynchronousDeferredRunTest
+from tftp.backend import IReader
+from twisted.internet.defer import (
+ inlineCallbacks,
+ succeed,
+ )
+from zope.interface.verify import verifyObject
+
+
+class TestBytesReader(TestCase):
+ """Tests for `provisioningserver.tftp.BytesReader`."""
+
+ def test_interfaces(self):
+ reader = BytesReader(b"")
+ self.addCleanup(reader.finish)
+ verifyObject(IReader, reader)
+
+ def test_read(self):
+ data = factory.getRandomString(size=10).encode("ascii")
+ reader = BytesReader(data)
+ self.addCleanup(reader.finish)
+ self.assertEqual(data[:7], reader.read(7))
+ self.assertEqual(data[7:], reader.read(7))
+ self.assertEqual(b"", reader.read(7))
+
+ def test_finish(self):
+ reader = BytesReader(b"1234")
+ reader.finish()
+ self.assertRaises(ValueError, reader.read, 1)
+
+
+class TestTFTPBackend(TestCase):
+ """Tests for `provisioningserver.tftp.TFTPBackend`."""
+
+ run_tests_with = AsynchronousDeferredRunTest.make_factory(timeout=5)
+
+ def test_re_config_file(self):
+ # The regular expression for extracting components of the file path is
+ # compatible with the PXE config path generator.
+ regex = TFTPBackend.re_config_file
+ for iteration in range(10):
+ args = {
+ "arch": factory.getRandomString(),
+ "subarch": factory.getRandomString(),
+ "name": factory.getRandomString(),
+ }
+ config_path = compose_config_path(**args)
+ # Remove leading slash from config path; the TFTP server does not
+ # include them in paths.
+ config_path = config_path.lstrip("/")
+ match = regex.match(config_path)
+ self.assertIsNotNone(match, config_path)
+ self.assertEqual(args, match.groupdict())
+
+ def test_init(self):
+ temp_dir = self.make_dir()
+ generator_url = "http://%s.example.com/%s"; % (
+ factory.getRandomString(), factory.getRandomString())
+ backend = TFTPBackend(temp_dir, generator_url)
+ self.assertEqual((True, False), (backend.can_read, backend.can_write))
+ self.assertEqual(temp_dir, backend.base.path)
+ self.assertEqual(generator_url, backend.generator_url.geturl())
+
+ def test_get_reader_regular_file(self):
+ # TFTPBackend.get_reader() returns a regular FilesystemReader for
+ # paths not matching re_config_file.
+ data = factory.getRandomString().encode("ascii")
+ temp_dir = self.make_dir()
+ temp_file = path.join(temp_dir, "example")
+ with open(temp_file, "wb") as stream:
+ stream.write(data)
+ backend = TFTPBackend(temp_dir, "http://nowhere.example.com/";)
+ reader = backend.get_reader("example")
+ self.addCleanup(reader.finish)
+ self.assertEqual(len(data), reader.size)
+ self.assertEqual(data, reader.read(len(data)))
+ self.assertEqual(b"", reader.read(1))
+
+ @inlineCallbacks
+ def test_get_reader_config_file(self):
+ # TFTPBackend.get_reader() returns a BytesReader for paths matching
+ # re_config_file.
+ arch = factory.getRandomString().encode("ascii")
+ subarch = factory.getRandomString().encode("ascii")
+ name = factory.getRandomString().encode("ascii")
+ kernelimage = factory.getRandomString().encode("ascii")
+ menutitle = factory.getRandomString().encode("ascii")
+ append = factory.getRandomString().encode("ascii")
+ backend_url = "http://example.com/?"; + urlencode(
+ {b"kernelimage": kernelimage, b"menutitle": menutitle,
+ b"append": append})
+ config_path = compose_config_path(arch, subarch, name)
+ backend = TFTPBackend(self.make_dir(), backend_url)
+ backend.get_page = succeed # Return the URL, via a Deferred.
+ reader = yield backend.get_reader(config_path.lstrip("/"))
+ self.addCleanup(reader.finish)
+ self.assertIsInstance(reader, BytesReader)
+ url = reader.read(1000)
+ query = parse_qsl(urlparse(url).query)
+ query_expected = [
+ ("append", append),
+ ("kernelimage", kernelimage),
+ ("arch", arch),
+ ("subarch", subarch),
+ ("menutitle", menutitle),
+ ("name", name),
+ ]
+ self.assertItemsEqual(query_expected, sorted(query))
=== added file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py 1970-01-01 00:00:00 +0000
+++ src/provisioningserver/tftp.py 2012-07-05 20:37:22 +0000
@@ -0,0 +1,82 @@
+# Copyright 2012 Canonical Ltd. This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Twisted Application Plugin for the MAAS TFTP server."""
+
+from __future__ import (
+ absolute_import,
+ print_function,
+ unicode_literals,
+ )
+
+__metaclass__ = type
+__all__ = [
+ "TFTPBackend",
+ ]
+
+from io import BytesIO
+import re
+from urllib import urlencode
+from urlparse import (
+ parse_qsl,
+ urlparse,
+ )
+
+from tftp.backend import (
+ FilesystemSynchronousBackend,
+ IReader,
+ )
+from twisted.web.client import getPage
+from zope.interface import implementer
+
+
+@implementer(IReader)
+class BytesReader:
+
+ def __init__(self, data):
+ super(BytesReader, self).__init__()
+ self.buffer = BytesIO(data)
+ self.size = len(data)
+
+ def read(self, size):
+ return self.buffer.read(size)
+
+ def finish(self):
+ self.buffer.close()
+
+
+class TFTPBackend(FilesystemSynchronousBackend):
+
+ get_page = staticmethod(getPage)
+
+ re_config_file = re.compile(
+ r'^maas/(?P<arch>[^/]+)/(?P<subarch>[^/]+)/'
+ r'pxelinux[.]cfg/(?P<name>[^/]+)$')
+
+ def __init__(self, base_path, generator_url):
+ super(TFTPBackend, self).__init__(
+ base_path, can_read=True, can_write=False)
+ self.generator_url = urlparse(generator_url)
+
+ def get_reader(self, file_name):
+ config_file_match = self.re_config_file.match(file_name)
+ if config_file_match is None:
+ return super(TFTPBackend, self).get_reader(file_name)
+ else:
+ # TODO: update query defaults.
+ query = {
+ b"menutitle": b"",
+ b"kernelimage": b"",
+ b"append": b"",
+ }
+ # Merge parameters from the generator URL.
+ query.update(parse_qsl(self.generator_url.query))
+ # Merge parameters obtained from the request.
+ query.update(config_file_match.groupdict())
+ # Merge updated query into the generator URL.
+ url = self.generator_url._replace(query=urlencode(query))
+ # TODO: do something more intelligent with unicode URLs here; see
+ # maas_client._ascii_url() for inspiration.
+ d = self.get_page(url.geturl().encode("ascii"))
+ d.addCallback(BytesReader)
+ return d
=== modified file 'twisted/plugins/maasps.py'
--- twisted/plugins/maasps.py 2012-04-16 10:00:51 +0000
+++ twisted/plugins/maasps.py 2012-07-05 20:37:22 +0000
@@ -13,10 +13,12 @@
__all__ = []
-from provisioningserver.plugin import ProvisioningServiceMaker
-
-# Construct objects which *provide* the relevant interfaces. The name of
-# these variables is irrelevant, as long as there are *some* names bound
-# to providers of IPlugin and IServiceMaker.
-
-service = ProvisioningServiceMaker("maas-pserv", "...") # TODO: finish
+try:
+ from provisioningserver.plugin import ProvisioningServiceMaker
+except ImportError:
+ pass # Ignore.
+else:
+ # Construct objects which *provide* the relevant interfaces. The name of
+ # these variables is irrelevant, as long as there are *some* names bound
+ # to providers of IPlugin and IServiceMaker.
+ service = ProvisioningServiceMaker("maas-pserv", "...") # TODO: finish
