Merge lp:~jtv/maas/bug-1042877 into lp:~maas-committers/maas/trunk

Proposed by Jeroen T. Vermeulen
Status: Merged
Approved by: Jeroen T. Vermeulen
Approved revision: no longer in the source branch.
Merged at revision: 949
Proposed branch: lp:~jtv/maas/bug-1042877
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 590 lines (+59/-130)
16 files modified
etc/pserv.yaml (+1/-1)
scripts/maas-import-pxe-files (+13/-31)
src/provisioningserver/config.py (+1/-1)
src/provisioningserver/pxe/config.commissioning.template (+4/-4)
src/provisioningserver/pxe/config.py (+5/-15)
src/provisioningserver/pxe/config.template (+2/-2)
src/provisioningserver/pxe/install_bootloader.py (+1/-1)
src/provisioningserver/pxe/install_image.py (+1/-1)
src/provisioningserver/pxe/tests/test_config.py (+7/-16)
src/provisioningserver/pxe/tests/test_install_image.py (+3/-3)
src/provisioningserver/pxe/tests/test_tftppath.py (+3/-3)
src/provisioningserver/pxe/tftppath.py (+3/-3)
src/provisioningserver/tests/test_config.py (+1/-1)
src/provisioningserver/tests/test_maas_import_pxe_files.py (+6/-23)
src/provisioningserver/tests/test_tftp.py (+7/-17)
src/provisioningserver/tftp.py (+1/-8)
To merge this branch: bzr merge lp:~jtv/maas/bug-1042877
Reviewer Review Type Date Requested Status
Gavin Panella (community) Approve
Review via email: mp+122049@code.launchpad.net

Commit message

Move our TFTP hierachy, and dispense with the bootpath (the "maas/" prefix in PXE TFTP paths).

Description of the change

I pre-imped this with Gavin, but also called in Daviey and Robie for details. A lot changes, and unfortunately it all more or less had to do so in one go:

 * Instead of re-using /var/lib/tftpboot/, create our own /var/lib/maas/tftp/.

 * No more "maas/" prefix: pxelinux.0, pxelinux.cfg/01-aa-bb-cc-dd-ee-ff, i386/generic/precise/install/kernel.

 * Don't download pxelinux.0; take just the i386 binary from syslinux.

 * Consistently get all pxelinux files from /usr/lib/syslinux; don't mix installed modules with downloaded loaders.

 * Drop bootpath and related complexity.

One small bit that I managed to leave for later: CURRENT_RELEASE should no longer be needed in maas-import-pxe-files. I'll remove that separately.

Rather than trawl through the MP diff, you may find it easier to read the commit log as a blow-by-blow account.

Jeroen

To post a comment you must log in.
Revision history for this message
Gavin Panella (allenap) wrote :

Bootiful, as they say in Norfolk (means: jolly spiffing).

[1]

-  # root: /var/lib/tftpboot
+  root: /var/lib/maas/tftp

The approach I've been taking with this file is to include the default
value, commented out. This is the expectation that Andres has now. So,
the value change here is fine, but it should still be commented. I see
that you've updated the ConfigTFTP.root default.

[2]

In tftp.py:

-        ^/?
...
+        ^/*

I think this should stay as zero-or-one rather than zero-or-more. The
latter speaks of slapdashery. The zero-or-one is there to accomodate
ambiguity in the TFTP spec, iirc.

review: Approve
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

Thanks for the reviews.

> [1]
>
> -  # root: /var/lib/tftpboot
> +  root: /var/lib/maas/tftp
>
> The approach I've been taking with this file is to include the default
> value, commented out. This is the expectation that Andres has now. So,
> the value change here is fine, but it should still be commented. I see
> that you've updated the ConfigTFTP.root default.

Okay: I commented this one out because it matches the (new) default.

> [2]
>
> In tftp.py:
>
> -        ^/?
> ...
> +        ^/*
>
> I think this should stay as zero-or-one rather than zero-or-more. The
> latter speaks of slapdashery. The zero-or-one is there to accomodate
> ambiguity in the TFTP spec, iirc.

On the other hand, you'd normally expect multiple consecutive slashes to be treated as a single slash. If not to satisfy general Unix sensibilities, then because it's what the standard Ubuntu tftpd does.

I agree that multiple slashes would be a bit suspicious, but when it comes to debugging, wouldn't a failure to find e.g. //pxelinux.0 be a massive red herring when it comes to debugging? I could imagine circumstances where it would be convenient to risk including an extra slash, and if that means that we end up normalizing slashes so as not to displease our own server, then all we've gained is some extra complexity to compensate for our own unwarranted strictness. Bear in mind that we want users to edit the templates that these paths occur in: we don't want to frustrate them with unexpected breakage. Be strict in what you generate and liberal in what you accept.

Conversely, in a case where a double leading slash is actually a sign of something wrong, you'd expect a file-not-found anyway. We don't even have items of the same name occurring at different depths in the tree.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'etc/pserv.yaml'
--- etc/pserv.yaml 2012-08-30 06:25:25 +0000
+++ etc/pserv.yaml 2012-08-30 16:38:18 +0000
@@ -31,7 +31,7 @@
31## TFTP configuration.31## TFTP configuration.
32#32#
33tftp:33tftp:
34 # root: /var/lib/tftpboot34 # root: /var/lib/maas/tftp
35 # port: 6935 # port: 69
36 port: 524436 port: 5244
37 ## The URL to be contacted to generate PXE configurations.37 ## The URL to be contacted to generate PXE configurations.
3838
=== modified file 'scripts/maas-import-pxe-files'
--- scripts/maas-import-pxe-files 2012-08-28 23:35:55 +0000
+++ scripts/maas-import-pxe-files 2012-08-30 16:38:18 +0000
@@ -6,7 +6,7 @@
6# pre-boot loader, kernels, and initrd images.6# pre-boot loader, kernels, and initrd images.
7#7#
8# This script downloads the required files into the TFTP home directory8# This script downloads the required files into the TFTP home directory
9# (by default, /var/lib/tftpboot). Run it with the necessarily privileges9# (by default, /var/lib/maas/tftp). Run it with the necessarily privileges
10# to write them there.10# to write them there.
1111
12# Exit immediately if a command exits with a non-zero status.12# Exit immediately if a command exits with a non-zero status.
@@ -64,31 +64,15 @@
64}64}
6565
6666
67# Download the pre-boot loader, pxelinux.0, for architecture $1. If67# Copy the pre-boot loader pxelinux.0, and modules we need, from the
68# successful, install it for netboot use. (Not all architectures need this68# installed syslinux version. Install it into the TFTP tree for
69# file, and there's rarely an urgent need for the very latest file, so if69# netbooting.
70# the download fails this function just skips it.)
71update_pre_boot_loader() {70update_pre_boot_loader() {
72 local arch=$171 for loader_file in pxelinux.0 chain.c32 ifcpu64.c32
73 local url=$(compose_installer_download_url $arch $CURRENT_RELEASE)72 do
74 if ! $DOWNLOAD $url/pxelinux.073 maas-provision install-pxe-bootloader \
75 then74 --loader="/usr/lib/syslinux/$loader_file"
76 echo "Could not download pre-boot loader for $arch; skipping."75 done
77 return
78 fi
79
80 # If the file has changed, move it into place (replacing any previous
81 # version). Otherwise, leave the filesystem alone.
82 if [ -f pxelinux.0 ]
83 then
84 # TODO: pxelinux.0 is downloaded but there's no reason why it can't
85 # come from the syslinux package like chain.c32 is.
86 maas-provision install-pxe-bootloader --loader='pxelinux.0'
87 maas-provision install-pxe-bootloader \
88 --loader='/usr/lib/syslinux/chain.c32'
89 maas-provision install-pxe-bootloader \
90 --loader='/usr/lib/syslinux/ifcpu64.c32'
91 fi
92}76}
9377
9478
@@ -122,14 +106,8 @@
122 echo "Downloading to temporary location $DOWNLOAD_DIR."106 echo "Downloading to temporary location $DOWNLOAD_DIR."
123 pushd -- $DOWNLOAD_DIR107 pushd -- $DOWNLOAD_DIR
124108
125 # All files we create here are public. The TFTP user will need to be
126 # able to read them.
127 umask a+r
128
129 for arch in $ARCHES109 for arch in $ARCHES
130 do110 do
131 update_pre_boot_loader $arch
132
133 for release in $RELEASES111 for release in $RELEASES
134 do112 do
135 update_install_files $arch $release113 update_install_files $arch $release
@@ -151,7 +129,11 @@
151129
152130
153main() {131main() {
132 # All files we create here are public. The TFTP user will need to be
133 # able to read them.
134 umask a+r
154135
136 update_pre_boot_loader
155 import_install_images137 import_install_images
156 import_ephemeral_images138 import_ephemeral_images
157}139}
158140
=== modified file 'src/provisioningserver/config.py'
--- src/provisioningserver/config.py 2012-08-30 06:25:25 +0000
+++ src/provisioningserver/config.py 2012-08-30 16:38:18 +0000
@@ -59,7 +59,7 @@
5959
60 if_key_missing = None60 if_key_missing = None
6161
62 root = String(if_missing="/var/lib/tftpboot")62 root = String(if_missing="/var/lib/maas/tftp")
63 port = Int(min=1, max=65535, if_missing=69)63 port = Int(min=1, max=65535, if_missing=69)
64 generator = String(if_missing=b"http://localhost/MAAS/api/1.0/pxeconfig/")64 generator = String(if_missing=b"http://localhost/MAAS/api/1.0/pxeconfig/")
6565
6666
=== modified file 'src/provisioningserver/pxe/config.commissioning.template'
--- src/provisioningserver/pxe/config.commissioning.template 2012-08-24 12:48:12 +0000
+++ src/provisioningserver/pxe/config.commissioning.template 2012-08-30 16:38:18 +0000
@@ -6,14 +6,14 @@
66
7LABEL amd647LABEL amd64
8 SAY Booting (amd64) under MAAS direction...8 SAY Booting (amd64) under MAAS direction...
9 KERNEL {{kernel_params(arch="amd64") | kernel_path | relative}}9 KERNEL {{kernel_params(arch="amd64") | kernel_path }}
10 INITRD {{kernel_params(arch="amd64") | initrd_path | relative}}10 INITRD {{kernel_params(arch="amd64") | initrd_path }}
11 APPEND {{kernel_params(arch="amd64") | kernel_command}}11 APPEND {{kernel_params(arch="amd64") | kernel_command}}
12 IPAPPEND 212 IPAPPEND 2
1313
14LABEL i38614LABEL i386
15 SAY Booting (i386) under MAAS direction...15 SAY Booting (i386) under MAAS direction...
16 KERNEL {{kernel_params(arch="i386") | kernel_path | relative}}16 KERNEL {{kernel_params(arch="i386") | kernel_path }}
17 INITRD {{kernel_params(arch="i386") | initrd_path | relative}}17 INITRD {{kernel_params(arch="i386") | initrd_path }}
18 APPEND {{kernel_params(arch="i386") | kernel_command}}18 APPEND {{kernel_params(arch="i386") | kernel_command}}
19 IPAPPEND 219 IPAPPEND 2
2020
=== modified file 'src/provisioningserver/pxe/config.py'
--- src/provisioningserver/pxe/config.py 2012-08-24 19:14:58 +0000
+++ src/provisioningserver/pxe/config.py 2012-08-30 16:38:18 +0000
@@ -23,7 +23,6 @@
23from errno import ENOENT23from errno import ENOENT
24from os import path24from os import path
2525
26import posixpath
27from provisioningserver.kernel_opts import compose_kernel_command_line_new26from provisioningserver.kernel_opts import compose_kernel_command_line_new
28from provisioningserver.pxe.tftppath import compose_image_path27from provisioningserver.pxe.tftppath import compose_image_path
29import tempita28import tempita
@@ -71,18 +70,14 @@
71 "No PXE template found in %r!" % template_dir)70 "No PXE template found in %r!" % template_dir)
7271
7372
74def render_pxe_config(bootpath, kernel_params, **extra):73def render_pxe_config(kernel_params, **extra):
75 """Render a PXE configuration file as a unicode string.74 """Render a PXE configuration file as a unicode string.
7675
77 :param bootpath: The directory path of `pxelinux.0`.
78 :param kernel_params: An instance of `KernelParameters`.76 :param kernel_params: An instance of `KernelParameters`.
79 :param extra: Allow for other arguments. This is a safety valve;77 :param extra: Allow for other arguments. This is a safety valve;
80 parameters generated in another component (for example, see78 parameters generated in another component (for example, see
81 `TFTPBackend.get_config_reader`) won't cause this to break.79 `TFTPBackend.get_config_reader`) won't cause this to break.
82 """80 """
83 if bootpath is None:
84 bootpath = ''
85
86 template = get_pxe_template(81 template = get_pxe_template(
87 kernel_params.purpose, kernel_params.arch,82 kernel_params.purpose, kernel_params.arch,
88 kernel_params.subarch)83 kernel_params.subarch)
@@ -95,24 +90,19 @@
95 params.arch, params.subarch,90 params.arch, params.subarch,
96 params.release, params.purpose)91 params.release, params.purpose)
9792
98 def initrd(params):93 def initrd_path(params):
99 return "%s/initrd.gz" % image_dir(params)94 return "%s/initrd.gz" % image_dir(params)
10095
101 def kernel(params):96 def kernel_path(params):
102 return "%s/linux" % image_dir(params)97 return "%s/linux" % image_dir(params)
10398
104 def kernel_command(params):99 def kernel_command(params):
105 return compose_kernel_command_line_new(params)100 return compose_kernel_command_line_new(params)
106101
107 def relative(path):
108 # Return `path` relative to `bootpath`.
109 return posixpath.relpath(path, start=bootpath)
110
111 namespace = {102 namespace = {
112 "initrd_path": initrd,103 "initrd_path": initrd_path,
113 "kernel_command": kernel_command,104 "kernel_command": kernel_command,
114 "kernel_params": kernel_params,105 "kernel_params": kernel_params,
115 "kernel_path": kernel,106 "kernel_path": kernel_path,
116 "relative": relative,
117 }107 }
118 return template.substitute(namespace)108 return template.substitute(namespace)
119109
=== modified file 'src/provisioningserver/pxe/config.template'
--- src/provisioningserver/pxe/config.template 2012-08-24 12:48:12 +0000
+++ src/provisioningserver/pxe/config.template 2012-08-30 16:38:18 +0000
@@ -2,7 +2,7 @@
22
3LABEL execute3LABEL execute
4 SAY Booting under MAAS direction...4 SAY Booting under MAAS direction...
5 KERNEL {{kernel_params | kernel_path | relative}}5 KERNEL {{kernel_params | kernel_path }}
6 INITRD {{kernel_params | initrd_path | relative}}6 INITRD {{kernel_params | initrd_path }}
7 APPEND {{kernel_params | kernel_command}}7 APPEND {{kernel_params | kernel_command}}
8 IPAPPEND 28 IPAPPEND 2
99
=== modified file 'src/provisioningserver/pxe/install_bootloader.py'
--- src/provisioningserver/pxe/install_bootloader.py 2012-08-17 14:28:27 +0000
+++ src/provisioningserver/pxe/install_bootloader.py 2012-08-30 16:38:18 +0000
@@ -30,7 +30,7 @@
30 """Locate a loader's destination, creating the directory if needed.30 """Locate a loader's destination, creating the directory if needed.
3131
32 :param tftproot: The root directory served up by the TFTP server,32 :param tftproot: The root directory served up by the TFTP server,
33 e.g. /var/lib/tftpboot/.33 e.g. /var/lib/maas/tftp/.
34 :return: Full path describing the directory that the installed loader34 :return: Full path describing the directory that the installed loader
35 should end up having.35 should end up having.
36 """36 """
3737
=== modified file 'src/provisioningserver/pxe/install_image.py'
--- src/provisioningserver/pxe/install_image.py 2012-07-23 10:24:39 +0000
+++ src/provisioningserver/pxe/install_image.py 2012-08-30 16:38:18 +0000
@@ -33,7 +33,7 @@
33 """Locate an image's destination. Create containing directory if needed.33 """Locate an image's destination. Create containing directory if needed.
3434
35 :param tftproot: The root directory served up by the TFTP server,35 :param tftproot: The root directory served up by the TFTP server,
36 e.g. /var/lib/tftpboot/.36 e.g. /var/lib/maas/tftp/.
37 :param arch: Main architecture to locate the destination for.37 :param arch: Main architecture to locate the destination for.
38 :param subarch: Sub-architecture of the main architecture.38 :param subarch: Sub-architecture of the main architecture.
39 :param release: OS release name, e.g. "precise".39 :param release: OS release name, e.g. "precise".
4040
=== modified file 'src/provisioningserver/pxe/tests/test_config.py'
--- src/provisioningserver/pxe/tests/test_config.py 2012-08-24 16:23:15 +0000
+++ src/provisioningserver/pxe/tests/test_config.py 2012-08-30 16:38:18 +0000
@@ -21,7 +21,6 @@
21from maastesting.matchers import ContainsAll21from maastesting.matchers import ContainsAll
22from maastesting.testcase import TestCase22from maastesting.testcase import TestCase
23import mock23import mock
24import posixpath
25from provisioningserver import kernel_opts24from provisioningserver import kernel_opts
26from provisioningserver.pxe import config25from provisioningserver.pxe import config
27from provisioningserver.pxe.config import render_pxe_config26from provisioningserver.pxe.config import render_pxe_config
@@ -150,9 +149,8 @@
150 def test_render(self):149 def test_render(self):
151 # Given the right configuration options, the PXE configuration is150 # Given the right configuration options, the PXE configuration is
152 # correctly rendered.151 # correctly rendered.
153 bootpath = factory.make_name("bootpath")
154 params = make_kernel_parameters()152 params = make_kernel_parameters()
155 output = render_pxe_config(bootpath=bootpath, kernel_params=params)153 output = render_pxe_config(kernel_params=params)
156 # The output is always a Unicode string.154 # The output is always a Unicode string.
157 self.assertThat(output, IsInstance(unicode))155 self.assertThat(output, IsInstance(unicode))
158 # The template has rendered without error. PXELINUX configurations156 # The template has rendered without error. PXELINUX configurations
@@ -162,7 +160,6 @@
162 image_dir = compose_image_path(160 image_dir = compose_image_path(
163 arch=params.arch, subarch=params.subarch,161 arch=params.arch, subarch=params.subarch,
164 release=params.release, purpose=params.purpose)162 release=params.release, purpose=params.purpose)
165 image_dir = posixpath.relpath(image_dir, bootpath)
166 self.assertThat(163 self.assertThat(
167 output, MatchesAll(164 output, MatchesAll(
168 MatchesRegex(165 MatchesRegex(
@@ -177,10 +174,7 @@
177174
178 def test_render_with_extra_arguments_does_not_affect_output(self):175 def test_render_with_extra_arguments_does_not_affect_output(self):
179 # render_pxe_config() allows any keyword arguments as a safety valve.176 # render_pxe_config() allows any keyword arguments as a safety valve.
180 options = {177 options = {"kernel_params": make_kernel_parameters()}
181 "bootpath": factory.make_name("bootpath"),
182 "kernel_params": make_kernel_parameters(),
183 }
184 # Capture the output before sprinking in some random options.178 # Capture the output before sprinking in some random options.
185 output_before = render_pxe_config(**options)179 output_before = render_pxe_config(**options)
186 # Sprinkle some magic in.180 # Sprinkle some magic in.
@@ -196,7 +190,6 @@
196 # If purpose is "local", the config.localboot.template should be190 # If purpose is "local", the config.localboot.template should be
197 # used.191 # used.
198 options = {192 options = {
199 "bootpath": factory.make_name("bootpath"),
200 "kernel_params":193 "kernel_params":
201 make_kernel_parameters()._replace(purpose="local"),194 make_kernel_parameters()._replace(purpose="local"),
202 }195 }
@@ -207,7 +200,6 @@
207 # Intel i386 is a special case and needs to use the chain.c32200 # Intel i386 is a special case and needs to use the chain.c32
208 # loader as the LOCALBOOT PXE directive is unreliable.201 # loader as the LOCALBOOT PXE directive is unreliable.
209 options = {202 options = {
210 "bootpath": factory.make_name("bootpath"),
211 "kernel_params": make_kernel_parameters()._replace(203 "kernel_params": make_kernel_parameters()._replace(
212 arch="i386", purpose="local"),204 arch="i386", purpose="local"),
213 }205 }
@@ -219,7 +211,6 @@
219 # Intel amd64 is a special case and needs to use the chain.c32211 # Intel amd64 is a special case and needs to use the chain.c32
220 # loader as the LOCALBOOT PXE directive is unreliable.212 # loader as the LOCALBOOT PXE directive is unreliable.
221 options = {213 options = {
222 "bootpath": factory.make_name("bootpath"),
223 "kernel_params": make_kernel_parameters()._replace(214 "kernel_params": make_kernel_parameters()._replace(
224 arch="amd64", purpose="local"),215 arch="amd64", purpose="local"),
225 }216 }
@@ -233,7 +224,6 @@
233 get_ephemeral_name = self.patch(kernel_opts, "get_ephemeral_name")224 get_ephemeral_name = self.patch(kernel_opts, "get_ephemeral_name")
234 get_ephemeral_name.return_value = factory.make_name("ephemeral")225 get_ephemeral_name.return_value = factory.make_name("ephemeral")
235 options = {226 options = {
236 "bootpath": factory.make_name("bootpath"),
237 "kernel_params": make_kernel_parameters()._replace(227 "kernel_params": make_kernel_parameters()._replace(
238 purpose="commissioning"),228 purpose="commissioning"),
239 }229 }
@@ -251,13 +241,14 @@
251 default_section["APPEND"].split())241 default_section["APPEND"].split())
252 # Both "i386" and "amd64" sections exist.242 # Both "i386" and "amd64" sections exist.
253 self.assertThat(config, ContainsAll(("i386", "amd64")))243 self.assertThat(config, ContainsAll(("i386", "amd64")))
254 # Each section defines KERNEL, INITRD, and APPEND settings, each244 # Each section defines KERNEL, INITRD, and APPEND settings. The
255 # containing paths referring to their architectures.245 # KERNEL and INITRD ones contain paths referring to their
246 # architectures.
256 for section_label in ("i386", "amd64"):247 for section_label in ("i386", "amd64"):
257 section = config[section_label]248 section = config[section_label]
258 self.assertThat(249 self.assertThat(
259 section, ContainsAll(("KERNEL", "INITRD", "APPEND")))250 section, ContainsAll(("KERNEL", "INITRD", "APPEND")))
260 contains_arch_path = Contains("/%s/" % section_label)251 contains_arch_path = StartsWith("%s/" % section_label)
261 self.assertThat(section["KERNEL"], contains_arch_path)252 self.assertThat(section["KERNEL"], contains_arch_path)
262 self.assertThat(section["INITRD"], contains_arch_path)253 self.assertThat(section["INITRD"], contains_arch_path)
263 self.assertThat(section["APPEND"], contains_arch_path)254 self.assertIn("APPEND", section)
264255
=== modified file 'src/provisioningserver/pxe/tests/test_install_image.py'
--- src/provisioningserver/pxe/tests/test_install_image.py 2012-07-23 13:25:15 +0000
+++ src/provisioningserver/pxe/tests/test_install_image.py 2012-08-30 16:38:18 +0000
@@ -79,13 +79,13 @@
79 def test_make_destination_follows_pxe_path_conventions(self):79 def test_make_destination_follows_pxe_path_conventions(self):
80 # The directory that make_destination returns follows the PXE80 # The directory that make_destination returns follows the PXE
81 # directory hierarchy specified for MAAS:81 # directory hierarchy specified for MAAS:
82 # /var/lib/tftproot/maas/<arch>/<subarch>/<release>/<purpose>82 # /var/lib/maas/tftp/<arch>/<subarch>/<release>/<purpose>
83 # (Where the /var/lib/tftproot/ part is configurable, so we83 # (Where the /var/lib/maas/tftp/ part is configurable, so we
84 # can test this without overwriting system files).84 # can test this without overwriting system files).
85 tftproot = self.make_dir()85 tftproot = self.make_dir()
86 arch, subarch, release, purpose = make_arch_subarch_release_purpose()86 arch, subarch, release, purpose = make_arch_subarch_release_purpose()
87 self.assertEqual(87 self.assertEqual(
88 os.path.join(tftproot, 'maas', arch, subarch, release, purpose),88 os.path.join(tftproot, arch, subarch, release, purpose),
89 make_destination(tftproot, arch, subarch, release, purpose))89 make_destination(tftproot, arch, subarch, release, purpose))
9090
91 def test_make_destination_creates_directory_if_not_present(self):91 def test_make_destination_creates_directory_if_not_present(self):
9292
=== modified file 'src/provisioningserver/pxe/tests/test_tftppath.py'
--- src/provisioningserver/pxe/tests/test_tftppath.py 2012-08-17 14:11:20 +0000
+++ src/provisioningserver/pxe/tests/test_tftppath.py 2012-08-30 16:38:18 +0000
@@ -41,7 +41,7 @@
41 def test_compose_config_path_follows_maas_pxe_directory_layout(self):41 def test_compose_config_path_follows_maas_pxe_directory_layout(self):
42 name = factory.make_name('config')42 name = factory.make_name('config')
43 self.assertEqual(43 self.assertEqual(
44 'maas/pxelinux.cfg/%02x-%s' % (ARP_HTYPE.ETHERNET, name),44 'pxelinux.cfg/%02x-%s' % (ARP_HTYPE.ETHERNET, name),
45 compose_config_path(name))45 compose_config_path(name))
4646
47 def test_compose_config_path_does_not_include_tftp_root(self):47 def test_compose_config_path_does_not_include_tftp_root(self):
@@ -56,7 +56,7 @@
56 release = factory.make_name('release')56 release = factory.make_name('release')
57 purpose = factory.make_name('purpose')57 purpose = factory.make_name('purpose')
58 self.assertEqual(58 self.assertEqual(
59 'maas/%s/%s/%s/%s' % (arch, subarch, release, purpose),59 '%s/%s/%s/%s' % (arch, subarch, release, purpose),
60 compose_image_path(arch, subarch, release, purpose))60 compose_image_path(arch, subarch, release, purpose))
6161
62 def test_compose_image_path_does_not_include_tftp_root(self):62 def test_compose_image_path_does_not_include_tftp_root(self):
@@ -69,7 +69,7 @@
69 Not(StartsWith(self.tftproot)))69 Not(StartsWith(self.tftproot)))
7070
71 def test_compose_bootloader_path_follows_maas_pxe_directory_layout(self):71 def test_compose_bootloader_path_follows_maas_pxe_directory_layout(self):
72 self.assertEqual('maas/pxelinux.0', compose_bootloader_path())72 self.assertEqual('pxelinux.0', compose_bootloader_path())
7373
74 def test_compose_bootloader_path_does_not_include_tftp_root(self):74 def test_compose_bootloader_path_does_not_include_tftp_root(self):
75 self.assertThat(75 self.assertThat(
7676
=== modified file 'src/provisioningserver/pxe/tftppath.py'
--- src/provisioningserver/pxe/tftppath.py 2012-08-30 06:25:25 +0000
+++ src/provisioningserver/pxe/tftppath.py 2012-08-30 16:38:18 +0000
@@ -29,7 +29,7 @@
29 simulate PXELINUX and don't actually load `pxelinux.0`, but use its path29 simulate PXELINUX and don't actually load `pxelinux.0`, but use its path
30 to figure out where configuration files are located.30 to figure out where configuration files are located.
31 """31 """
32 return "maas/pxelinux.0"32 return "pxelinux.0"
3333
3434
35# TODO: move this; it is now only used for testing.35# TODO: move this; it is now only used for testing.
@@ -48,7 +48,7 @@
48 # Not using os.path.join: this is a TFTP path, not a native path. Yes, in48 # Not using os.path.join: this is a TFTP path, not a native path. Yes, in
49 # practice for us they're the same. We always assume that the ARP HTYPE49 # practice for us they're the same. We always assume that the ARP HTYPE
50 # (hardware type) that PXELINUX sends is Ethernet.50 # (hardware type) that PXELINUX sends is Ethernet.
51 return "maas/pxelinux.cfg/{htype:02x}-{mac}".format(51 return "pxelinux.cfg/{htype:02x}-{mac}".format(
52 htype=ARP_HTYPE.ETHERNET, mac=mac)52 htype=ARP_HTYPE.ETHERNET, mac=mac)
5353
5454
@@ -66,7 +66,7 @@
66 :return: Path for the corresponding image directory (containing a66 :return: Path for the corresponding image directory (containing a
67 kernel and initrd) as exposed over TFTP.67 kernel and initrd) as exposed over TFTP.
68 """68 """
69 return '/'.join(['maas', arch, subarch, release, purpose])69 return '/'.join([arch, subarch, release, purpose])
7070
7171
72def locate_tftp_path(path, tftproot):72def locate_tftp_path(path, tftproot):
7373
=== modified file 'src/provisioningserver/tests/test_config.py'
--- src/provisioningserver/tests/test_config.py 2012-08-24 10:53:26 +0000
+++ src/provisioningserver/tests/test_config.py 2012-08-30 16:38:18 +0000
@@ -147,7 +147,7 @@
147 'tftp': {147 'tftp': {
148 'generator': 'http://localhost/MAAS/api/1.0/pxeconfig/',148 'generator': 'http://localhost/MAAS/api/1.0/pxeconfig/',
149 'port': 69,149 'port': 69,
150 'root': "/var/lib/tftpboot",150 'root': "/var/lib/maas/tftp",
151 },151 },
152 }152 }
153153
154154
=== modified file 'src/provisioningserver/tests/test_maas_import_pxe_files.py'
--- src/provisioningserver/tests/test_maas_import_pxe_files.py 2012-08-17 14:45:13 +0000
+++ src/provisioningserver/tests/test_maas_import_pxe_files.py 2012-08-30 16:38:18 +0000
@@ -23,11 +23,7 @@
23 )23 )
24from provisioningserver.pxe import tftppath24from provisioningserver.pxe import tftppath
25from provisioningserver.testing.config import ConfigFixture25from provisioningserver.testing.config import ConfigFixture
26from testtools.matchers import (26from testtools.matchers import FileContains
27 FileContains,
28 FileExists,
29 Not,
30 )
3127
3228
33def read_file(path, name):29def read_file(path, name):
@@ -97,7 +93,7 @@
97 archive = self.make_dir()93 archive = self.make_dir()
98 download = compose_download_dir(archive, arch, release)94 download = compose_download_dir(archive, arch, release)
99 os.makedirs(download)95 os.makedirs(download)
100 for filename in ['initrd.gz', 'linux', 'pxelinux.0']:96 for filename in ['initrd.gz', 'linux']:
101 factory.make_file(download, filename)97 factory.make_file(download, filename)
102 return archive98 return archive
10399
@@ -135,40 +131,27 @@
135 with open(os.devnull, 'wb') as dev_null:131 with open(os.devnull, 'wb') as dev_null:
136 check_call(script, env=env, stdout=dev_null)132 check_call(script, env=env, stdout=dev_null)
137133
138 def test_downloads_pre_boot_loader(self):134 def test_procures_pre_boot_loader(self):
139 arch = factory.make_name('arch')135 arch = factory.make_name('arch')
140 release = 'precise'136 release = 'precise'
141 archive = self.make_downloads(arch=arch, release=release)137 archive = self.make_downloads(arch=arch, release=release)
142 self.call_script(archive, self.tftproot, arch=arch, release=release)138 self.call_script(archive, self.tftproot, arch=arch, release=release)
143 tftp_path = compose_tftp_bootloader_path(self.tftproot)139 tftp_path = compose_tftp_bootloader_path(self.tftproot)
144 download_path = compose_download_dir(archive, arch, release)140 expected_contents = read_file('/usr/lib/syslinux', 'pxelinux.0')
145 expected_contents = read_file(download_path, 'pxelinux.0')
146 self.assertThat(tftp_path, FileContains(expected_contents))141 self.assertThat(tftp_path, FileContains(expected_contents))
147142
148 def test_ignores_missing_pre_boot_loader(self):
149 arch = factory.make_name('arch')
150 release = 'precise'
151 archive = self.make_downloads(arch=arch, release=release)
152 download_path = compose_download_dir(archive, arch, release)
153 os.remove(os.path.join(download_path, 'pxelinux.0'))
154 self.call_script(archive, self.tftproot, arch=arch, release=release)
155 tftp_path = compose_tftp_bootloader_path(self.tftproot)
156 self.assertThat(tftp_path, Not(FileExists()))
157
158 def test_updates_pre_boot_loader(self):143 def test_updates_pre_boot_loader(self):
159 arch = factory.make_name('arch')144 arch = factory.make_name('arch')
160 release = 'precise'145 release = 'precise'
161 tftp_path = compose_tftp_bootloader_path(self.tftproot)146 tftp_path = compose_tftp_bootloader_path(self.tftproot)
162 os.makedirs(os.path.dirname(tftp_path))
163 with open(tftp_path, 'w') as existing_file:147 with open(tftp_path, 'w') as existing_file:
164 existing_file.write(factory.getRandomString())148 existing_file.write(factory.getRandomString())
165 archive = self.make_downloads(arch=arch, release=release)149 archive = self.make_downloads(arch=arch, release=release)
166 self.call_script(archive, self.tftproot, arch=arch, release=release)150 self.call_script(archive, self.tftproot, arch=arch, release=release)
167 download_path = compose_download_dir(archive, arch, release)151 expected_contents = read_file('/usr/lib/syslinux', 'pxelinux.0')
168 expected_contents = read_file(download_path, 'pxelinux.0')
169 self.assertThat(tftp_path, FileContains(expected_contents))152 self.assertThat(tftp_path, FileContains(expected_contents))
170153
171 def test_downloads_install_image(self):154 def test_procures_install_image(self):
172 arch = factory.make_name('arch')155 arch = factory.make_name('arch')
173 release = 'precise'156 release = 'precise'
174 archive = self.make_downloads(arch=arch, release=release)157 archive = self.make_downloads(arch=arch, release=release)
175158
=== modified file 'src/provisioningserver/tests/test_tftp.py'
--- src/provisioningserver/tests/test_tftp.py 2012-08-27 12:00:44 +0000
+++ src/provisioningserver/tests/test_tftp.py 2012-08-30 16:38:18 +0000
@@ -70,10 +70,7 @@
70 The path is intended to match `re_config_file`, and the components are70 The path is intended to match `re_config_file`, and the components are
71 the expected groups from a match.71 the expected groups from a match.
72 """72 """
73 components = {73 components = {"mac": factory.getRandomMACAddress(b"-")}
74 "bootpath": b"maas", # Static.
75 "mac": factory.getRandomMACAddress(b"-"),
76 }
77 config_path = compose_config_path(components["mac"])74 config_path = compose_config_path(components["mac"])
78 return config_path, components75 return config_path, components
7976
@@ -115,17 +112,17 @@
115 mac = 'aa-bb-cc-dd-ee-ff'112 mac = 'aa-bb-cc-dd-ee-ff'
116 match = TFTPBackend.re_config_file.match('pxelinux.cfg/01-%s' % mac)113 match = TFTPBackend.re_config_file.match('pxelinux.cfg/01-%s' % mac)
117 self.assertIsNotNone(match)114 self.assertIsNotNone(match)
118 self.assertEqual({'mac': mac, 'bootpath': None}, match.groupdict())115 self.assertEqual({'mac': mac}, match.groupdict())
119116
120 def test_re_config_file_matches_pxelinux_cfg_with_leading_slash(self):117 def test_re_config_file_matches_pxelinux_cfg_with_leading_slash(self):
121 mac = 'aa-bb-cc-dd-ee-ff'118 mac = 'aa-bb-cc-dd-ee-ff'
122 match = TFTPBackend.re_config_file.match('/pxelinux.cfg/01-%s' % mac)119 match = TFTPBackend.re_config_file.match('/pxelinux.cfg/01-%s' % mac)
123 self.assertIsNotNone(match)120 self.assertIsNotNone(match)
124 self.assertEqual({'mac': mac, 'bootpath': None}, match.groupdict())121 self.assertEqual({'mac': mac}, match.groupdict())
125122
126 def test_re_config_file_does_not_match_non_config_file(self):123 def test_re_config_file_does_not_match_non_config_file(self):
127 self.assertIsNone(124 self.assertIsNone(
128 TFTPBackend.re_config_file.match('maas/pxelinux.cfg/kernel'))125 TFTPBackend.re_config_file.match('pxelinux.cfg/kernel'))
129126
130 def test_re_config_file_does_not_match_file_in_root(self):127 def test_re_config_file_does_not_match_file_in_root(self):
131 self.assertIsNone(128 self.assertIsNone(
@@ -153,18 +150,16 @@
153 def test_get_generator_url(self):150 def test_get_generator_url(self):
154 # get_generator_url() merges the parameters obtained from the request151 # get_generator_url() merges the parameters obtained from the request
155 # file path (arch, subarch, name) into the configured generator URL.152 # file path (arch, subarch, name) into the configured generator URL.
156 bootpath = factory.make_name("bootpath")
157 mac = factory.getRandomMACAddress(b"-")153 mac = factory.getRandomMACAddress(b"-")
158 dummy = factory.make_name("dummy").encode("ascii")154 dummy = factory.make_name("dummy").encode("ascii")
159 backend_url = b"http://example.com/?" + urlencode({b"dummy": dummy})155 backend_url = b"http://example.com/?" + urlencode({b"dummy": dummy})
160 backend = TFTPBackend(self.make_dir(), backend_url)156 backend = TFTPBackend(self.make_dir(), backend_url)
161 # params is an example of the parameters obtained from a request.157 # params is an example of the parameters obtained from a request.
162 params = {"bootpath": bootpath, "mac": mac}158 params = {"mac": mac}
163 generator_url = urlparse(backend.get_generator_url(params))159 generator_url = urlparse(backend.get_generator_url(params))
164 self.assertEqual("example.com", generator_url.hostname)160 self.assertEqual("example.com", generator_url.hostname)
165 query = parse_qsl(generator_url.query)161 query = parse_qsl(generator_url.query)
166 query_expected = [162 query_expected = [
167 ("bootpath", bootpath),
168 ("dummy", dummy),163 ("dummy", dummy),
169 ("mac", mac),164 ("mac", mac),
170 ]165 ]
@@ -200,9 +195,7 @@
200195
201 reader = yield backend.get_reader(config_path)196 reader = yield backend.get_reader(config_path)
202 output = reader.read(10000)197 output = reader.read(10000)
203 # The expected parameters include bootpath; this is extracted from the198 expected_params = dict(mac=mac)
204 # file path by re_config_file.
205 expected_params = dict(mac=mac, bootpath="maas")
206 observed_params = json.loads(output)199 observed_params = json.loads(output)
207 self.assertEqual(expected_params, observed_params)200 self.assertEqual(expected_params, observed_params)
208201
@@ -212,10 +205,7 @@
212 # `IReader` of a PXE configuration, rendered by `render_pxe_config`.205 # `IReader` of a PXE configuration, rendered by `render_pxe_config`.
213 backend = TFTPBackend(self.make_dir(), b"http://example.com/")206 backend = TFTPBackend(self.make_dir(), b"http://example.com/")
214 # Fake configuration parameters, as discovered from the file path.207 # Fake configuration parameters, as discovered from the file path.
215 fake_params = {208 fake_params = {"mac": factory.getRandomMACAddress(b"-")}
216 "bootpath": "maas",
217 "mac": factory.getRandomMACAddress(b"-"),
218 }
219 # Fake kernel configuration parameters, as returned from the API call.209 # Fake kernel configuration parameters, as returned from the API call.
220 fake_kernel_params = make_kernel_parameters()210 fake_kernel_params = make_kernel_parameters()
221211
222212
=== modified file 'src/provisioningserver/tftp.py'
--- src/provisioningserver/tftp.py 2012-08-24 18:34:57 +0000
+++ src/provisioningserver/tftp.py 2012-08-30 16:38:18 +0000
@@ -86,14 +86,7 @@
86 re_config_file = re.compile(86 re_config_file = re.compile(
87 r'''87 r'''
88 # Optional leading slash(es).88 # Optional leading slash(es).
89 ^/?89 ^/*
90 # Optional boot path prefix (separated from the rest by slash):
91 (?:
92 (?P<bootpath>
93 maas
94 )
95 /
96 )?
97 pxelinux[.]cfg # PXELINUX expects this.90 pxelinux[.]cfg # PXELINUX expects this.
98 /91 /
99 {htype:02x} # ARP HTYPE.92 {htype:02x} # ARP HTYPE.