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

[Andres Rodriguez]

Fast Path Installer

[Julian Edwards (julian-edwards)]

<bigjools> I am trying to be constructive on the review but you're going to hate me, sorry :(
<roaksoax> hehe :)

Did you guys have any pre-implementation chat with one of us before starting this? I can't approve this as it stands for quite a few fundamental architectural reasons, and also it's an 1100+ line diff without a single test change. I did previously say I won't accept any more branches that have untested code, especially untested walls of shell :(

More below.

25 === added file 'contrib/preseeds_v2/preseed_xinstall'
26 --- contrib/preseeds_v2/preseed_xinstall 1970-01-01 00:00:00 +0000
27 +++ contrib/preseeds_v2/preseed_xinstall 2013-04-17 16:47:25 +0000
[snip]
36 +preseed = subprocess.check_output(["/usr/share/maas/xinstall/make-userdata-maas",
37 + "--apt-proxy="+proxy,
38 + "--dpkg-selections="+preseed_data,
39 + "--finished-url="+node_disable_pxe_url,
40 + "--finished-url-data="+node_disable_pxe_data,
41 + "http://"+cluster_host+"/MAAS/static/images/"+node.architecture+"/"+node.distro_series+"/xinstall/root.tar.gz",
42 + "/dev/sda"])
43 +}}
44 +{{preseed}}

This scares the bejesus out of me. This is just a workaround so that the shell script make-userdata-maas can be used instead of doing it properly in Python, and tested. It has completely sidestepped all of the code that exists already in src/maasserver/preseed.py and src/maasserver/compose_preseed.py that calculates preseed_data based on the node state.

It is extremely fragile and it is likely to break, horribly, in many ways, as soon as someone changes anything elsewhere.

At the very least, this template should not be calling out to the shell script. If you can't re-write it in Python and test it in the time available, it should be called from the existing Python code that generates preseeds and returned in preseed_data for the template. It can then be tested so we know the right preseed is getting returned for FPI.

1045 === modified file 'src/maasserver/api.py'
1061 === modified file 'src/maasserver/models/nodegroup.py'
1077 === modified file 'src/maasserver/preseed.py'
1096 === modified file 'src/provisioningserver/kernel_opts.py'
1135 === modified file 'src/provisioningserver/pxe/install_image.py'

The changes in all these files need tests. I expect some tests are broken as a result but I can't run the test suite at the moment as I get this:

$ make build
bin/buildout install database
Traceback (most recent call last):
  File "bin/buildout", line 5, in <module>
    from pkg_resources import load_entry_point
  File "/home/ed/canonical/maas/sandbox/local/lib/python2.7/site-packages/distribute-0.6.24-py2.7.egg/pkg_resources.py", line 16, in <module>
    import sys, os, zipimport, time, re, imp, types
  File "/home/ed/canonical/maas/sandbox/lib/python2.7/re.py", line 105, in <module>
    import sre_compile
  File "/home/ed/canonical/maas/sandbox/lib/python2.7/sre_compile.py", line 14, in <module>
    import sre_parse
  File "/home/ed/canonical/maas/sandbox/lib/python2.7/sre_parse.py", line 17, in <module>
    from sre_constants import *
  File "/home/ed/canonical/maas/sandbox/lib/python2.7/sre_constants.py", line 18, in <module>
    from _sre impo...

[Julian Edwards (julian-edwards)]

So based on the conversation I just had with Andres he explained that the FPI script stuff here is also going to be used on the CD installer so they wanted to share the code.

The maas-make-userdata script is used to generate a template, and the intention is that in the future people should be able to pass extra scripts to run in preinst and postinst time in the installer.

I am strongly of the opinion that that template should be static in MAAS and any data that it requires comes from MAAS (it does now anyway - it's just indirect!). MAAS's templating system will handle this requirement with ease and vastly reduce this complexity. It is fragile and will be very very hard to debug problems.

[Julian Edwards (julian-edwards)]

To fix the devel env, you need to merge trunk and then "make distclean".

The following tests are all broken in your branch:

======================================================================
ERROR: maasserver.tests.test_preseed.TestPreseedMethods.test_get_preseed_returns_default_preseed
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/ed/canonical/maas/sandbox/src/maasserver/tests/test_preseed.py", line 525, in test_get_preseed_returns_default_preseed
    preseed = get_preseed(node)
  File "/home/ed/canonical/maas/sandbox/src/maasserver/preseed.py", line 80, in get_preseed
    release=node.get_distro_series())
  File "/home/ed/canonical/maas/sandbox/src/maasserver/preseed.py", line 306, in render_preseed
    return template.substitute(**context)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 177, in substitute
    result = self._interpret_inherit(result, defs, inherit, ns)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 204, in _interpret_inherit
    return templ.substitute(ns)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 173, in substitute
    result, defs, inherit = self._interpret(ns)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 184, in _interpret
    self._interpret_codes(self._parsed, ns, out=parts, defs=defs)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 212, in _interpret_codes
    self._interpret_code(item, ns, out, defs)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 218, in _interpret_code
    self._exec(code[2], ns, pos)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 312, in _exec
    exec code in self.default_namespace, ns
  File "<string>", line 12, in <module>
TypeError: coercing to Unicode: need string or buffer, NoneType found at line 1 column 3 in file /home/ed/canonical/maas/sandbox/contrib/preseeds_v2/preseed_xinstall

======================================================================
ERROR: maasserver.tests.test_preseed.TestPreseedProxy.test_preseed_uses_configured_proxy
----------------------------------------------------------------------
_StringException: Traceback (most recent call last):
  File "/home/ed/canonical/maas/sandbox/src/maasserver/tests/test_preseed.py", line 513, in test_preseed_uses_configured_proxy
    factory.make_node(), PRESEED_TYPE.DEFAULT, "precise")
  File "/home/ed/canonical/maas/sandbox/src/maasserver/preseed.py", line 306, in render_preseed
    return template.substitute(**context)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 177, in substitute
    result = self._interpret_inherit(result, defs, inherit, ns)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 204, in _interpret_inherit
    return templ.substitute(ns)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 173, in substitute
    result, defs, inherit = self._interpret(ns)
  File "/usr/lib/python2.7/dist-packages/tempita/__init__.py", line 184, in _interpret
    self._interpret_codes(self._parsed, ns, out=parts, defs=defs)
  File "/usr...

[Scott Moser (smoser)]

> <bigjools> I am trying to be constructive on the review but you're going to
> hate me, sorry :(
> <roaksoax> hehe :)
>
> Did you guys have any pre-implementation chat with one of us before starting
> this? I can't approve this as it stands for quite a few fundamental
> architectural reasons, and also it's an 1100+ line diff without a single test
> change. I did previously say I won't accept any more branches that have
> untested code, especially untested walls of shell :(
>
> More below.
>
> 25 === added file 'contrib/preseeds_v2/preseed_xinstall'
> 26 --- contrib/preseeds_v2/preseed_xinstall 1970-01-01 00:00:00
> +0000
> 27 +++ contrib/preseeds_v2/preseed_xinstall 2013-04-17 16:47:25
> +0000
> [snip]
> 36 +preseed = subprocess.check_output(["/usr/share/maas/xinstall/make-
> userdata-maas",
> 37 + "--apt-proxy="+proxy,
> 38 + "--dpkg-selections="+preseed_data,
> 39 + "--finished-url="+node_disable_pxe_url,
> 40 + "--finished-url-data="+node_disable_pxe_data,
> 41 + "http://"+cluster_host+"/MAAS/static/images/"+node.architecture+"
> /"+node.distro_series+"/xinstall/root.tar.gz",
> 42 + "/dev/sda"])
> 43 +}}
> 44 +{{preseed}}
>
> This scares the bejesus out of me. This is just a workaround so that the
> shell script make-userdata-maas can be used instead of doing it properly in
> Python, and tested. It has completely sidestepped all of the code that exists

"doing it properly in python" is a general problem that maas has. Maas knows too much. The installer (xinstall) is not maas. The proper way to do this is to not have the xinstall code in maas at all, and have maas know an interface to it. We've pushed xinstall here as a temporary landing location.

What you have above is a sane way to interface between maas an an installer.

We've explicitly put this is a preseed as these templates are sold as user-configurable. Some configuration may need to be done by an end user, and this is how they do that. Making maas invoke that code internally means a user has to change maas python code to make a change.

> already in src/maasserver/preseed.py and src/maasserver/compose_preseed.py
> that calculates preseed_data based on the node state.
>
> It is extremely fragile and it is likely to break, horribly, in many ways, as
> soon as someone changes anything elsewhere.

We have 2 components. yes, one component can break another component. A change to python can break maas. A change to glibc can break python. a change to the kernel can break glibc. Thats *good* design.

> At the very least, this template should not be calling out to the shell
> script. If you can't re-write it in Python and test it in the time available,
> it should be called from the existing Python code that generates preseeds and
> returned in preseed_data for the template. It can then be tested so we know
> the right preseed is getting returned for FPI.
>
>
> 1045 === modified file 'src/maasserver/api.py'
> 1061 === modified file 'src/maasserver/models/nodegroup.py'
> 1077 === modified file 'src/maasserver/preseed.py'
> 1096 === modified file 'src/provisionings...

[Julian Edwards (julian-edwards)]

Hi Scott,

On 19/04/13 03:09, Scott Moser wrote:
> "doing it properly in python" is a general problem that maas has. Maas knows too much. The installer (xinstall) is not maas. The proper way to do this is to not have the xinstall code in maas at all, and have maas know an interface to it. We've pushed xinstall here as a temporary landing location.

I don't think maas itself knows too much - all the installer specific
stuff is in templates, exactly where it should be. However I would love
for the xinstall code to not be in MAAS too. Fortunately there's an
easy solution, we just use the template feature that MAAS already has.

> What you have above is a sane way to interface between maas an an installer.

With respect, in no way do I think that is sane and you'll get the same
response from the rest of the guys in my team. I've explained already
where it's faults are and how to do it better, so I won't do so again.

> We've explicitly put this is a preseed as these templates are sold as user-configurable. Some configuration may need to be done by an end user, and this is how they do that. Making maas invoke that code internally means a user has to change maas python code to make a change.

Right now with this branch you have a situation like this:
- Maas renders a preseed template, passing in some data to it
- The template shells out to a script, passing it the data, that
generates another template

This can be vastly simplified as:
 - Maas renders a template, passing in some data to it

where the template is the one that the script previously generated.

The same configuration and flexibility is still there, without the
intermediate step. The template system we chose is very powerful.

>
>> already in src/maasserver/preseed.py and src/maasserver/compose_preseed.py
>> that calculates preseed_data based on the node state.
>>
>> It is extremely fragile and it is likely to break, horribly, in many ways, as
>> soon as someone changes anything elsewhere.
>
> We have 2 components. yes, one component can break another component. A change to python can break maas. A change to glibc can break python. a change to the kernel can break glibc. Thats *good* design.

I think that's a strawman. It's not good to have things break. You're
also falsely assuming the likelihood of Python/glibc/kernel breaking is
of the same likelihood as a change here. This change is fragile and it
will break easily.

> Its an installer. It should be separate. Please just review this as if that code was not there. We'll replace or move it at a later date.

The fact that it is here and you know it should be separate is enough of
a warning that this approach is wrong.

>> I realise that you need to get *something* in raring before FF tomorrow.
>> Last-minute rushed changes are never good, but if you can at least add tests
>> and make the change to the preseed template as I suggested then this could
>> land with a view to improving more later.
>
> I really prefer to use user-editable and modifable template files than maas python files for the interface between 'xinstall' and maas.

I was never suggesting modifiable python files. The existing preseed
setup is tot...

[Gavin Panella (allenap)]

On 18 April 2013 18:09, Scott Moser <email address hidden> wrote:
...
> "doing it properly in python" is a general problem that maas has.
> Maas knows too much.  The installer (xinstall) is not maas.  The
> proper way to do this is to not have the xinstall code in maas at
> all, and have maas know an interface to it.  We've pushed xinstall
> here as a temporary landing location.

I say more later, but if it's not MAAS then it shouldn't land in MAAS;
it should exist as a patch in the packaging.

...
>> It is extremely fragile and it is likely to break, horribly, in
>> many ways, as soon as someone changes anything elsewhere.
>
> We have 2 components.  yes, one component can break another
> component.  A change to python can break maas.  A change to glibc
> can break python. a change to the kernel can break glibc.  Thats
> *good* design.

It's a nitpick, but that's not good design, that's just a property of
interrelating systems.

...
> Its an installer. It should be separate.  Please just review this as
> if that code was not there.  We'll replace or move it at a later
> date.

There's no one actively driving MAAS development at this very moment,
but I guess we - the Red Squad - still feel the most responsible, and
we still care about the thing we've created. Our criteria for landing
a change to any product that we're working on is: does this improve
the product? This change does not.

It satisfies a short-term goal, that of getting something to work for
Raring, but it's a horrible change: it contains a moderately complex
program written in shell-script (meaning it's now highly complex); the
script has been cargo-culted from somewhere else, and so can diverge
unchecked; paths have been hard-coded into the template; there are no
tests; and so on.

If MAAS's limitations have meant that this was all that was possible
in the time available, then more time needs to be made available:
changing MAAS *needs* to be part of the engineering effort.

This change should *not* land on trunk. Previous changes to trunk in
the same vein as this - especially those without tests - should
probably not have landed either. This change should be carried as a
patch in the packaging.

>
>> I realise that you need to get *something* in raring before FF
>> tomorrow.  Last-minute rushed changes are never good, but if you
>> can at least add tests and make the change to the preseed template
>> as I suggested then this could land with a view to improving more
>> later.
>
> I really prefer to use user-editable and modifable template files
> than maas python files for the interface between 'xinstall' and
> maas.

Fwiw, the template files are user-editable. There's meant to be an
inheritance/override mechanism so that the templates can be installed
by the package but selectively customised in, say, /etc/maas. I'm not
sure if that's effective in practice right now, but that's the
intention. Throwing that away instead of improving it - especially by
replacing it with untested shell-script! - is not doing anyone any
favours.

If you really, definitely want to land large amounts of shell-script
to MAAS, it needs to be tested. We can help by integratiing, say,
ShUnit <http://shunit.s...

review: Disapprove

[Scott Moser (smoser)]

We have 2 completely separate components here:
 a.) maas
 b.) a fast path installer

There is a problem in that the fast path installer doesn't "live" anywhere. That will be addressed at a later date, and would then be addressed here by removal of any 'xinstall' reference in maas. We'll re-submit this branch with such a removal.

Maas will interface with this fast path installer via popen. Essentially running a command like:
  make-userdata-maas "--apt-proxy={{proxy}}" \
     "--dpkg-selections={{dpkg_set_selections_here}}" \
     "--finished-url={{node_disable_pxe_url}}" \
     "--finished-url-data={{node_disable_pxe_data}}" \
     "{{url_of_thing_to_install}}" "{{target_disk}}"

Having maas invoke that command itself, and making the output available in a preseed variable named 'xinstall_data' which is then rendered in a preseed wholly with '{{xinstall_data}}' is pointlessly complex and not modifiable by the end user.

[Julian Edwards (julian-edwards)]

The installer should not live in MAAS upstream code, that is certain. Make a separate package for it and pull it in if you need it.

The problem with temporary fixes is that they become permanent. Especially if they go out in a release - look at how long it's taken to remove Cobbler from 12.04.

>Having maas invoke that command itself, and making the output available in a preseed variable named 'xinstall_data'
>which is then rendered in a preseed wholly with '{{xinstall_data}}' is pointlessly complex and not modifiable by the
>end user.

Yet this is exactly what you are doing via the template that shells out!

If you want FPI to land in MAAS you need to add the FPI template that it needs and populate it with data from MAAS, as both Gavin and I have said above. I don't understand why a separate script is needed.

review: Disapprove

[Julian Edwards (julian-edwards)]

Much better without the xinstall stuff in here. It's obviously incomplete but I have no fundamental objections any more. Provided the xinstall preseed is sane in the future, it's looking ok.

As you said, you need tests for all the changes here. Some other comments below:

70 +Usage() {
71 +cat <<EOF
72 + Usage: ${0##*/} uec-tarball [output]
73 +EOF

Can you put a description in here so people know what this script is doing. Also, *sadface* another Wall of Shell™
:(

201 def compose_purpose_opts(params):
202 """Return the list of the purpose-specific kernel options."""
203 - if params.purpose == "commissioning":
204 + if params.purpose == "commissioning" or params.purpose == "xinstall":

Woa, really? Why is this needed?

[Andres Rodriguez (andreserl)]

Hi Julian,

Thanks for the review. We will be shipping the xinstall stuff in packaging so won't really be missing anything. I'll however provide a sample preseed to be merged on trunk.

Now, as far as the latter, the 'xinstall' uses exactly the same kernel arguments as the ones the 'commissioning' step uses, with the exception of 'ds=nocloud-net', which is being added in the template file. If you believe is better to add this additional parameter on the code above, I'm also fine doing that.

224 + APPEND ds=nocloud-net {{kernel_params(arch="amd64") | kernel_command}}

Cheers.

[Julian Edwards (julian-edwards)]

On 04/05/13 00:51, Andres Rodriguez wrote:
> Hi Julian,
>
> Thanks for the review. We will be shipping the xinstall stuff in packaging so won't really be missing anything. I'll however provide a sample preseed to be merged on trunk.

How can the "sample" one diverge from the xinstall one? And why would
they ever be different?

>
> Now, as far as the latter, the 'xinstall' uses exactly the same kernel arguments as the ones the 'commissioning' step uses, with the exception of 'ds=nocloud-net', which is being added in the template file. If you believe is better to add this additional parameter on the code above, I'm also fine doing that.
>
> 224 + APPEND ds=nocloud-net {{kernel_params(arch="amd64") | kernel_command}}
>
> Cheers.
>

How are you defining the tag that indicates to use the FPI? Because
tags support extra kernel options and that's where ds=nocloud-net should
be. Once it's in tag.kernel_opts, everything will DTRT. Having custom
code is not the right way here.

Cheers.

[Andres Rodriguez (andreserl)]

Hi Julian,

The ds=nocloud-net is a required kernel option for xinstall and should not be set when creating/assigning the tag. Since the 'xinstall' is another purpose, aside from commissioning and install, all the required kernel opts should be within MAAS as it is for the other purposes. I have updated the branch for it.

Thanks

[Andres Rodriguez (andreserl)]

Tests are available in:

https://code.launchpad.net/~andreserl/maas/trunk-fpi-tests/+merge/163052

[Julian Edwards (julian-edwards)]

Ok fair enough on the tag. I guess you want to disassociate the notion of FPI from *how* FPI works (the xinstaller)?

Why did you add the tests in a separate branch? They should be in here!

review: Needs Fixing

[Julian Edwards (julian-edwards)]

Thanks for the tests! Ok let's review:

154 if node.netboot:
155 - return "install"
156 + if node.should_use_traditional_installer():
157 + return "install"
158 + else:
159 + return "xinstall"

There's no test for this change, can you add one please.

171 + def get_any_interface(self):

This needs its own test please.

206 - def test_preseed_context_null_cluster_host_if_unmanaged(self):
207 + def test_preseed_context_not_null_cluster_host_if_unmanaged(self):
208 # If the nodegroup has no managed interface recorded, which is
209 # possible in the data model but would be a bit weird, the
210 # cluster_host context variable is present, but None.
211 @@ -378,7 +378,7 @@
212 interface.management = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
213 interface.save()
214 context = get_preseed_context(release, nodegroup)
215 - self.assertIsNone(context["cluster_host"])
216 + self.assertIsNotNone(context["cluster_host"])

This should check for the actual expected interface rather than it not being None. Also, the test's comment is now wrong, fix that please!

243 + kernel_params = []
244 # These are kernel parameters read by the ephemeral environment.
245 tname = "%s:%s" % (ISCSI_TARGET_NAME_PREFIX,
246 get_ephemeral_name(params.release, params.arch))
247 - return [
248 + if params.purpose == "xinstall":
249 + kernel_params += ["ds=nocloud-net"]
250 + kernel_params += [

You can simplify this code a bit.

Remove the "kernel_params = []" bit, and assign straight to kernel_params instead of the first +=. Then you can add these lines:

    if params.purpose == "xinstall":
        kernel_params.append("ds=nocloud-net")

Much simpler!

274 + SAY Booting (amd64) under MAAS direction...
275 + SAY {{kernel_params(arch="amd64") | kernel_command}}
276 + KERNEL {{kernel_params(arch="amd64") | kernel_path }}
277 + INITRD {{kernel_params(arch="amd64") | initrd_path }}
278 + APPEND {{kernel_params(arch="amd64") | kernel_command}}
279 + IPAPPEND 2
280 +
281 +LABEL i386
282 + SAY Booting (i386) under MAAS direction...
283 + SAY {{kernel_params(arch="i386") | kernel_command}}

Since the "SAY" gets evaluated regardless of the LABEL it's executing, this will repeat 4 lots of SAY. I think we can just make it dump the kernel params as information, just the once.

341 + def test_render_pxe_config_for_xinstall(self):
342 + # The xinstall config uses an extra PXELINUX module to auto
343 + # select between i386 and amd64.
344 + get_ephemeral_name = self.patch(kernel_opts, "get_ephemeral_name")
345 + get_ephemeral_name.return_value = factory.make_name("ephemeral")
346 + options = {
347 + "kernel_params":
348 + make_kernel_parameters(purpose="xinstall"),

.. [snip]...

Given that this new test is identical to test_render_pxe_config_for_commissioning except for one piece of data, let's introduce a test scenario into it.

I've done that for you, so just revert your changes t...

[Andres Rodriguez (andreserl)]

Hi Julian,

I have addressed all of your comments but the SAY one. SO something like?

LABEL execute
  SAY Booting under MAAS direction...
  SAY {{kernel_params() | kernel_command}}
  KERNEL ifcpu64.c32
  APPEND amd64 -- i386

[Gavin Panella (allenap)]

On 9 May 2013 10:04, Andres Rodriguez <email address hidden> wrote:
> Hi Julian,
>
> I have addressed all of your comments but the SAY one. SO something like?
>
> LABEL execute
> SAY Booting under MAAS direction...
> SAY {{kernel_params() | kernel_command}}

These SAY commands are processed as the file is parsed, not when the
particular label is chosen. You can't use SAY to show which label was
actually chosen.

For example:

  SAY hello
  LABEL foo
    SAY foo
  LABEL bar
    SAY bar

will echo:

  hello
  foo
  bar

regardless.

[Julian Edwards (julian-edwards)]

Andres, looks great! I'll echo what Gavin said about SAY. Just put them only at the TOP level like this:

SAY Booting under MAAS direction...
SAY {{kernel_params() | kernel_command}}

review: Approve

[MAAS Lander (maas-lander)]

No commit message specified.

[MAAS Lander (maas-lander)]

Voting does not meet specified criteria. Required: Approve >= 1, Disapprove == 0. Got: 1 Approve, 1 Disapprove.

[Gavin Panella (allenap)]

review: Abstain

[Andres Rodriguez (andreserl)]

review: Approve

[Andres Rodriguez (andreserl)]

review: Approve

[MAAS Lander (maas-lander)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/maas-merger-trunk/255/console reported an error when processing this lp:~andreserl/maas/trunk-fpi branch.
Not merging it.

[Andres Rodriguez (andreserl)]

The branch has been rebased against latest trunk.

[MAAS Lander (maas-lander)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/maas-merger-trunk/275/console reported an error when processing this lp:~andreserl/maas/trunk-fpi branch.
Not merging it.

=== added file 'contrib/maas-cluster-http.conf'
--- contrib/maas-cluster-http.conf	1970-01-01 00:00:00 +0000
+++ contrib/maas-cluster-http.conf	2013-05-11 18:43:26 +0000
@@ -0,0 +1,6 @@
+# Server static files for tftp images as FPI
+# installer needs them
+Alias /MAAS/static/images/ /var/lib/maas/tftp/
+<Directory /var/lib/maas/tftp/>
+    SetHandler None
+</Directory>

=== modified file 'contrib/preseeds_v2/generic'
--- contrib/preseeds_v2/generic	2012-11-14 10:32:25 +0000
+++ contrib/preseeds_v2/generic	2013-05-11 18:43:26 +0000
@@ -1,4 +1,8 @@
+{{if node.should_use_traditional_installer() }}
 {{inherit "preseed_master"}}
+{{else}}
+{{inherit "preseed_xinstall"}}
+{{endif}}
 
 {{def proxy}}
 d-i     mirror/country string manual

=== added file 'contrib/preseeds_v2/preseed_xinstall'
--- contrib/preseeds_v2/preseed_xinstall	1970-01-01 00:00:00 +0000
+++ contrib/preseeds_v2/preseed_xinstall	2013-05-11 18:43:26 +0000
@@ -0,0 +1,1 @@
+# Disabled by default

=== modified file 'scripts/maas-import-ephemerals'
--- scripts/maas-import-ephemerals	2012-11-23 11:00:04 +0000
+++ scripts/maas-import-ephemerals	2013-05-11 18:43:26 +0000
@@ -239,6 +239,9 @@
         tar -Sxzf - -C  "$exdir" < "$tarball" ||
         { error "failed to extract tarball from $furl"; return 1; }
 
+    # Create root disk
+    uec2roottar "$tarball" || { error "failed to create root image"; return 1; }
+
     # Look for our files in the extracted tarball.
     local x="" img="" kernel="" initrd=""
     for x in "$exdir/"*.img; do
@@ -342,12 +345,14 @@
             return 1
         copy_first_available "$src/initrd.gz" "$src/initrd" "$tmpdir/initrd.gz" ||
             return 1
+        copy_first_available "$src/dist-root.tar.gz" "" "$tmpdir/root.tar.gz" ||
+            return 1
     fi
 
     local cmd out=""
     cmd=( maas-provision install-pxe-image
           "--arch=$arch" "--subarch=$subarch" "--release=$release"
-          --purpose="commissioning" --image="$tmpdir" )
+          --purpose="commissioning" --image="$tmpdir" --symlink="xinstall")
     debug 2 "${cmd[@]}"
     out=$("${cmd[@]}" 2>&1) ||
         { error "cmd failed:" "${cmd[@]}"; error "$out"; return 1; }

=== added file 'scripts/uec2roottar'
--- scripts/uec2roottar	1970-01-01 00:00:00 +0000
+++ scripts/uec2roottar	2013-05-11 18:43:26 +0000
@@ -0,0 +1,56 @@
+#!/bin/sh
+
+TEMP_D=""
+MP=""
+
+Usage() {
+cat <<EOF
+   ${0##*/} converts an ubuntu ephemeral image into a root
+   disk image tarball.
+
+   Usage: ${0##*/} uec-tarball [output]
+EOF
+}
+error() { echo "$(date -R):" "$@" 1>&2; }
+fail() { [ $# -eq 0 ] || error "$@"; exit 1; }
+cleanup() { 
+  [ -z "$MP" ] || umount "$MP"
+  [ -z "$TEMP_D" -o ! -d "$TEMP_D" ] || rm -Rf "${TEMP_D}";
+}
+
+[ "$1" = "-h" -o "$1" = "--help" ] && { Usage; exit 0; }
+[ $# -eq 1 -o $# -eq 2 ] || { Usage 1>&2; exit 1; }
+[ -f "$1" ] || { Usage 1>&2; error "$1: not a file"; exit 1; }
+
+[ "$(id -u)" = "0" ] || fail "must be root"
+
+TEMP_D=$(mktemp -d "${TMPDIR:-/tmp}/${0##*/}.XXXXXX") || fail "failed mktemp"
+trap cleanup EXIT
+
+uec="$1"
+output=${2}
+
+[ -n "$output" ] || output="${uec%.tar.gz}-root.tar.gz"; 
+error "converting ${uec} to ${output}"
+
+error "extracting *.img from ${uec}"
+tar -C ${TEMP_D} --wildcards "*.img" -Sxvzf "$uec" ||
+   fail "failed extract $uec"
+
+img=""
+for cand in "${TEMP_D}/"*.img; do
+   [ -z "$img" ] || fail "multiple .img files in $uec"
+   [ -f "$cand" ] && img="$cand"
+done
+
+[ -n "$img" ] || fail "failed to find image in $uec"
+
+mkdir "${TEMP_D}/mp" && mount -o ro "$img" "${TEMP_D}/mp" &&
+  MP="$TEMP_D/mp" || fail "failed to mount $img"
+
+error "copying contents of ${img#${TEMP_D}/} in ${uec} to ${output}"
+tar -C "$MP" -cpSzf "${output}" --numeric-owner . ||
+  fail "failed to create ${output}"
+
+error "finished. wrote to ${output}"
+

=== modified file 'setup.py'
--- setup.py	2012-12-18 17:06:43 +0000
+++ setup.py	2013-05-11 18:43:26 +0000
@@ -62,6 +62,7 @@
              'etc/maas/import_ephemerals',
              'etc/maas/import_pxe_files',
              'contrib/maas-http.conf',
+             'contrib/maas-cluster-http.conf',
              'contrib/maas_local_settings.py']),
         ('/usr/share/maas',
             ['contrib/wsgi.py',
@@ -73,10 +74,13 @@
              'contrib/preseeds_v2/enlist',
              'contrib/preseeds_v2/generic',
              'contrib/preseeds_v2/enlist_userdata',
+             'contrib/preseeds_v2/preseed_xinstall',
              'contrib/preseeds_v2/preseed_master']),
         ('/usr/sbin',
             ['scripts/maas-import-ephemerals',
              'scripts/maas-import-pxe-files']),
+        ('/usr/bin',
+            ['scripts/uec2roottar']),
     ],
 
     install_requires=[

=== modified file 'src/maasserver/api.py'
--- src/maasserver/api.py	2013-05-08 23:03:51 +0000
+++ src/maasserver/api.py	2013-05-11 18:43:26 +0000
@@ -1753,7 +1753,10 @@
     elif node.status == NODE_STATUS.ALLOCATED:
         # Install the node if netboot is enabled, otherwise boot locally.
         if node.netboot:
-            return "install"
+            if node.should_use_traditional_installer():
+                return "install"
+            else:
+                return "xinstall"
         else:
             return "local"  # TODO: Investigate.
     else:

=== modified file 'src/maasserver/models/nodegroup.py'
--- src/maasserver/models/nodegroup.py	2012-11-23 09:54:47 +0000
+++ src/maasserver/models/nodegroup.py	2013-05-11 18:43:26 +0000
@@ -204,6 +204,11 @@
             self.api_key = api_token.key
         return super(NodeGroup, self).save(*args, **kwargs)
 
+    def get_any_interface(self):
+        for interface in self.nodegroupinterface_set.all():
+            return interface
+        return None
+
     def get_managed_interface(self):
         """Return the interface for which MAAS managed the DHCP service.
 

=== modified file 'src/maasserver/preseed.py'
--- src/maasserver/preseed.py	2013-04-08 19:56:00 +0000
+++ src/maasserver/preseed.py	2013-05-11 18:43:26 +0000
@@ -232,7 +232,13 @@
     else:
         base_url = nodegroup.maas_url
         cluster_if = nodegroup.get_managed_interface()
-        cluster_host = None if cluster_if is None else cluster_if.ip
+        any_cluster_if = nodegroup.get_any_interface()
+        cluster_host = None
+        if cluster_if is None:
+            if any_cluster_if is not None:
+                cluster_host = any_cluster_if.ip
+        else:
+            cluster_host = cluster_if.ip
     return {
         'main_archive_hostname': main_archive_hostname,
         'main_archive_directory': main_archive_directory,

=== modified file 'src/maasserver/tests/test_api.py'
--- src/maasserver/tests/test_api.py	2013-05-08 23:03:51 +0000
+++ src/maasserver/tests/test_api.py	2013-05-11 18:43:26 +0000
@@ -3776,11 +3776,14 @@
             ("poweroff", {"status": NODE_STATUS.READY}),
             ("poweroff", {"status": NODE_STATUS.RESERVED}),
             ("install", {"status": NODE_STATUS.ALLOCATED, "netboot": True}),
+            ("xinstall", {"status": NODE_STATUS.ALLOCATED, "netboot": True}),
             ("local", {"status": NODE_STATUS.ALLOCATED, "netboot": False}),
             ("poweroff", {"status": NODE_STATUS.RETIRED}),
             ]
         node = factory.make_node()
         for purpose, parameters in options:
+            if purpose == "xinstall":
+                node.use_fastpath_installer()
             for name, value in parameters.items():
                 setattr(node, name, value)
             self.assertEqual(purpose, api.get_boot_purpose(node))

=== modified file 'src/maasserver/tests/test_preseed.py'
--- src/maasserver/tests/test_preseed.py	2013-02-21 23:32:20 +0000
+++ src/maasserver/tests/test_preseed.py	2013-05-11 18:43:26 +0000
@@ -368,17 +368,19 @@
             nodegroup.get_managed_interface().ip,
             context["cluster_host"])
 
-    def test_preseed_context_null_cluster_host_if_unmanaged(self):
-        # If the nodegroup has no managed interface recorded, which is
-        # possible in the data model but would be a bit weird, the
-        # cluster_host context variable is present, but None.
+    def test_preseed_context_cluster_host_if_unmanaged(self):
+        # If the nodegroup has no managed interface recorded, the cluster_host
+        # context variable is still present and derived from the nodegroup.
         release = factory.getRandomString()
         nodegroup = factory.make_node_group(maas_url=factory.getRandomString())
         for interface in nodegroup.nodegroupinterface_set.all():
             interface.management = NODEGROUPINTERFACE_MANAGEMENT.UNMANAGED
             interface.save()
         context = get_preseed_context(release, nodegroup)
-        self.assertIsNone(context["cluster_host"])
+        self.assertIsNotNone(context["cluster_host"])
+        self.assertEqual(
+            nodegroup.get_any_interface().ip,
+            context["cluster_host"])
 
     def test_preseed_context_null_cluster_host_if_does_not_exist(self):
         # If there's no nodegroup, the cluster_host context variable is
@@ -525,6 +527,12 @@
         preseed = get_preseed(node)
         self.assertIn('preseed/late_command', preseed)
 
+    def test_get_preseed_returns_xinstall_preseed(self):
+        node = factory.make_node()
+        node.use_fastpath_installer()
+        preseed = get_preseed(node)
+        self.assertIn('# Disabled by default', preseed)
+
     def test_get_enlist_preseed_returns_enlist_preseed(self):
         preseed = get_enlist_preseed()
         self.assertTrue(preseed.startswith('#cloud-config'))

=== modified file 'src/provisioningserver/kernel_opts.py'
--- src/provisioningserver/kernel_opts.py	2013-02-01 01:07:25 +0000
+++ src/provisioningserver/kernel_opts.py	2013-05-11 18:43:26 +0000
@@ -124,11 +124,11 @@
 
 def compose_purpose_opts(params):
     """Return the list of the purpose-specific kernel options."""
-    if params.purpose == "commissioning":
+    if params.purpose == "commissioning" or params.purpose == "xinstall":
         # These are kernel parameters read by the ephemeral environment.
         tname = "%s:%s" % (ISCSI_TARGET_NAME_PREFIX,
                            get_ephemeral_name(params.release, params.arch))
-        return [
+        kernel_params = [
             # Read by the open-iscsi initramfs code.
             "iscsi_target_name=%s" % tname,
             "iscsi_target_ip=%s" % params.fs_host,
@@ -145,6 +145,9 @@
             # Read by cloud-init.
             "cloud-config-url=%s" % params.preseed_url,
             ]
+        if params.purpose == "xinstall":
+            kernel_params.append("ds=nocloud-net")
+        return kernel_params
     else:
         # These are options used by the Debian Installer.
         return [

=== added file 'src/provisioningserver/pxe/config.xinstall.template'
--- src/provisioningserver/pxe/config.xinstall.template	1970-01-01 00:00:00 +0000
+++ src/provisioningserver/pxe/config.xinstall.template	2013-05-11 18:43:26 +0000
@@ -0,0 +1,20 @@
+DEFAULT execute
+
+SAY Booting under MAAS direction...
+SAY {{kernel_params() | kernel_command}}
+
+LABEL execute
+  KERNEL ifcpu64.c32
+  APPEND amd64 -- i386
+
+LABEL amd64
+  KERNEL {{kernel_params(arch="amd64") | kernel_path }}
+  INITRD {{kernel_params(arch="amd64") | initrd_path }}
+  APPEND {{kernel_params(arch="amd64") | kernel_command}}
+  IPAPPEND 2
+
+LABEL i386
+  KERNEL {{kernel_params(arch="i386") | kernel_path }}
+  INITRD {{kernel_params(arch="i386") | initrd_path }}
+  APPEND {{kernel_params(arch="i386") | kernel_command}}
+  IPAPPEND 2

=== modified file 'src/provisioningserver/pxe/install_image.py'
--- src/provisioningserver/pxe/install_image.py	2012-08-31 17:21:01 +0000
+++ src/provisioningserver/pxe/install_image.py	2013-05-11 18:43:26 +0000
@@ -70,7 +70,7 @@
         return False
 
 
-def install_dir(new, old):
+def install_dir(new, old, symlink=None):
     """Install directory `new`, replacing directory `old` if it exists.
 
     This works as atomically as possible, but isn't entirely.  Moreover,
@@ -121,6 +121,13 @@
     # Now delete the old image directory at leisure.
     rmtree('%s.old' % old, ignore_errors=True)
 
+    # Symlink the new image directory to 'symlink'.
+    if symlink is not None:
+        sdest = "%s/%s" % (os.path.dirname(old), symlink)
+        if os.path.exists(sdest) or os.path.islink(sdest):
+            os.unlink(sdest)
+        os.symlink(old, sdest)
+
 
 def add_arguments(parser):
     parser.add_argument(
@@ -138,6 +145,9 @@
     parser.add_argument(
         '--image', dest='image', default=None,
         help="Netboot image directory, containing kernel & initrd.")
+    parser.add_argument(
+        '--symlink', dest='symlink', default=None,
+        help="Destination directory to symlink the installed images to.")
 
 
 def run(args):
@@ -154,5 +164,5 @@
         tftproot, args.arch, args.subarch, args.release, args.purpose)
     if not are_identical_dirs(destination, args.image):
         # Image has changed.  Move the new version into place.
-        install_dir(args.image, destination)
+        install_dir(args.image, destination, args.symlink)
     rmtree(args.image, ignore_errors=True)

=== modified file 'src/provisioningserver/pxe/tests/test_config.py'
--- src/provisioningserver/pxe/tests/test_config.py	2012-09-19 13:51:15 +0000
+++ src/provisioningserver/pxe/tests/test_config.py	2013-05-11 18:43:26 +0000
@@ -223,14 +223,22 @@
         self.assertIn("chain.c32", output)
         self.assertNotIn("LOCALBOOT", output)
 
-    def test_render_pxe_config_for_commissioning(self):
+class TestRenderPXEConfigScenarios(TestCase):
+    """Tests for `provisioningserver.pxe.config.render_pxe_config`."""
+
+    scenarios = [
+        ("commissioning", dict(purpose="commissioning")),
+        ("xinstall", dict(purpose="xinstall")),
+        ]
+
+    def test_render_pxe_config_scenarios(self):
         # The commissioning config uses an extra PXELINUX module to auto
         # select between i386 and amd64.
         get_ephemeral_name = self.patch(kernel_opts, "get_ephemeral_name")
         get_ephemeral_name.return_value = factory.make_name("ephemeral")
         options = {
             "kernel_params":
-                make_kernel_parameters(purpose="commissioning"),
+                make_kernel_parameters(purpose=self.purpose),
             }
         output = render_pxe_config(**options)
         config = parse_pxe_config(output)

=== modified file 'src/provisioningserver/pxe/tests/test_install_image.py'
--- src/provisioningserver/pxe/tests/test_install_image.py	2012-08-31 15:41:18 +0000
+++ src/provisioningserver/pxe/tests/test_install_image.py	2013-05-11 18:43:26 +0000
@@ -209,3 +209,38 @@
         self.assertEqual(
             "rw-r--r--",
             target_dir.child("image").getPermissions().shorthand())
+
+    def test_install_dir_moves_dir_into_place_with_symlink(self):
+        download_image = os.path.join(self.make_dir(), 'download-image')
+        published_image = os.path.join(self.make_dir(), 'published-image')
+        base_path = os.path.dirname(published_image)
+        symlink_dest = 'xinstall'
+        contents = factory.getRandomString()
+        os.makedirs(download_image)
+        sample_file = factory.make_file(download_image, contents=contents)
+        install_dir(download_image, published_image, symlink_dest)
+        self.assertThat(
+            os.path.join(published_image, os.path.basename(sample_file)),
+            FileContains(contents))
+        self.assertThat(
+            os.path.join(base_path, symlink_dest, os.path.basename(sample_file)),
+            FileContains(contents))
+
+    def test_install_dir_replaces_existing_dir_with_symlink(self):
+        download_image = os.path.join(self.make_dir(), 'download-image')
+        published_image = os.path.join(self.make_dir(), 'published-image')
+        base_path = os.path.dirname(published_image)
+        symlink_dest = 'xinstall'
+        os.makedirs(download_image)
+        sample_file = factory.make_file(download_image)
+        os.makedirs(published_image)
+        os.symlink(published_image, os.path.join(base_path, symlink_dest))
+        obsolete_file = factory.make_file(published_image)
+        install_dir(download_image, published_image, symlink_dest)
+        self.assertThat(
+            os.path.join(published_image, os.path.basename(sample_file)),
+            FileExists())
+        self.assertThat(obsolete_file, Not(FileExists()))
+        self.assertThat(
+            os.path.join(base_path, symlink_dest, os.path.basename(sample_file)),
+            FileExists())

=== modified file 'src/provisioningserver/tests/test_kernel_opts.py'
--- src/provisioningserver/tests/test_kernel_opts.py	2013-02-01 01:07:25 +0000
+++ src/provisioningserver/tests/test_kernel_opts.py	2013-05-11 18:43:26 +0000
@@ -118,6 +118,22 @@
             "netcfg/choose_interface=auto",
             compose_kernel_command_line(params))
 
+    def test_xinstall_compose_kernel_command_line_inc_purpose_opts(self):
+        # The result of compose_kernel_command_line includes the purpose
+        # options for a non "xinstall" node.
+        get_ephemeral_name = self.patch(kernel_opts, "get_ephemeral_name")
+        get_ephemeral_name.return_value = "RELEASE-ARCH"
+        params = make_kernel_parameters(purpose="xinstall")
+        cmdline = compose_kernel_command_line(params)
+        self.assertThat(
+            cmdline,
+            ContainsAll([
+                "ds=nocloud-net",
+                "root=/dev/disk/by-path/ip-",
+                "iscsi_initiator=",
+                "overlayroot=tmpfs",
+                "ip=::::%s:BOOTIF" % params.hostname]))
+
     def test_commissioning_compose_kernel_command_line_inc_purpose_opts(self):
         # The result of compose_kernel_command_line includes the purpose
         # options for a non "commissioning" node.
@@ -147,8 +163,8 @@
         self.assertNotIn(cmdline, "None")
 
     def test_compose_kernel_command_line_inc_common_opts(self):
-        # Test that some kernel arguments appear on both commissioning
-        # and install command lines.
+        # Test that some kernel arguments appear on commissioning, install
+        # and xinstall command lines.
         get_ephemeral_name = self.patch(kernel_opts, "get_ephemeral_name")
         get_ephemeral_name.return_value = "RELEASE-ARCH"
         expected = ["nomodeset"]
@@ -159,6 +175,11 @@
         self.assertThat(cmdline, ContainsAll(expected))
 
         params = make_kernel_parameters(
+            purpose="xinstall", arch="i386")
+        cmdline = compose_kernel_command_line(params)
+        self.assertThat(cmdline, ContainsAll(expected))
+
+        params = make_kernel_parameters(
             purpose="install", arch="i386")
         cmdline = compose_kernel_command_line(params)
         self.assertThat(cmdline, ContainsAll(expected))
@@ -182,6 +203,23 @@
         factory.make_file(
             ephemeral_dir, name='info', contents=ephemeral_info)
 
+    def test_compose_kernel_command_line_inc_purpose_opts_xinstall_node(self):
+        # The result of compose_kernel_command_line includes the purpose
+        # options for a "xinstall" node.
+        ephemeral_name = factory.make_name("ephemeral")
+        params = make_kernel_parameters(purpose="xinstall")
+        self.create_ephemeral_info(
+            ephemeral_name, params.arch, params.release)
+        self.assertThat(
+            compose_kernel_command_line(params),
+            ContainsAll([
+                "ds=nocloud-net",
+                "iscsi_target_name=%s:%s" % (
+                    ISCSI_TARGET_NAME_PREFIX, ephemeral_name),
+                "iscsi_target_port=3260",
+                "iscsi_target_ip=%s" % params.fs_host,
+                ]))
+
     def test_compose_kernel_command_line_inc_purpose_opts_comm_node(self):
         # The result of compose_kernel_command_line includes the purpose
         # options for a "commissioning" node.