Merge lp:~andreserl/maas/maas_tftppath_lp1042877 into lp:~maas-committers/maas/trunk

Proposed by Andres Rodriguez
Status: Merged
Approved by: Julian Edwards
Approved revision: no longer in the source branch.
Merged at revision: 940
Proposed branch: lp:~andreserl/maas/maas_tftppath_lp1042877
Merge into: lp:~maas-committers/maas/trunk
Diff against target: 56 lines (+5/-5)
3 files modified
etc/pserv.yaml (+1/-1)
src/provisioningserver/config.py (+1/-1)
src/provisioningserver/pxe/tftppath.py (+3/-3)
To merge this branch: bzr merge lp:~andreserl/maas/maas_tftppath_lp1042877
Reviewer Review Type Date Requested Status
Julian Edwards (community) Approve
Review via email: mp+121676@code.launchpad.net

Commit message

Fixes incorrect TFTP path to not prepend maas/

To post a comment you must log in.
Revision history for this message
Julian Edwards (julian-edwards) :
review: Approve
Revision history for this message
Jeroen T. Vermeulen (jtv) wrote :

This branch does not pass tests. Andres, did you run “make test” or “make check” before putting this up for review? Lots of failing tests in trunk now. :(

Revision history for this message
Julian Edwards (julian-edwards) wrote :

I'll take some blame, I should have checked before I approved.

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

I suspect the root problem is that different teams come into this with different operating procedures. We in Red Squad are used to running tests before & after any change, and running the full test suite at least before & after landing — but there's really no point doing that for packaging changes, changes to the client-side packages etc... which is the kind of work that the server team has been doing. In this case, the unfortunate circumstance was that Andres normally has no need to do this (and this _looked_ like a mere configuration change that might not affect tests) whereas Julian would normally expect the engineer to have ensured that tests run before review.

Also, our Jenkins setup must definitely be broken. We should have received emails screaming about this.

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

Meanwhile, if we're stepping away from the /maas prefix completely (are we?) then we should simplify it out of the regex in provisioningserver.tftp.TFTPBackend.re_config_file.

Revision history for this message
Gavin Panella (allenap) wrote :

On 30 Aug 2012 03:34, "Jeroen T. Vermeulen" <email address hidden> wrote:
>
> Meanwhile, if we're stepping away from the /maas prefix completely (are
we?) then we should simplify it out of the regex in
provisioningserver.tftp.TFTPBackend.re_config_file.

And bootpath also becomes obsolete (more so than it already is).

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-23 19:56:55 +0000
+++ etc/pserv.yaml 2012-08-28 18:26:18 +0000
@@ -31,7 +31,7 @@
31## TFTP configuration.31## TFTP configuration.
32#32#
33tftp:33tftp:
34 # root: /var/lib/tftpboot34 # root: /var/lib/tftpboot/maas
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 'src/provisioningserver/config.py'
--- src/provisioningserver/config.py 2012-08-24 10:53:26 +0000
+++ src/provisioningserver/config.py 2012-08-28 18:26: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/tftpboot/maas")
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/tftppath.py'
--- src/provisioningserver/pxe/tftppath.py 2012-08-17 14:11:20 +0000
+++ src/provisioningserver/pxe/tftppath.py 2012-08-28 18:26: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):