launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

The proposal to merge lp:~jtv/maas/download-commissioning-script into lp:maas has been updated.

Description changed to:

This is a huge branch.  The work was done in collaboration with Raphael, who worked his arse off in the server sewers to get this tested and debugged.

The metadata service provides a tarball of commissioning scripts: the lshw script, plus any owner-provided commissioning scripts.  In this branch the main commissioning script learns to download and unpack that tarball, instead of writing the scripts as "here documents."

Ironically, those "here documents" are also needed and they got out of hand.  During development and testing we hit some problems that "make lint" would have detected if only the python weren't embedded in shell script; other problems would have stood out in unit tests, had the code been organized in a testable way; and yet other problems really did require manual testing.

And so this branch also splits up the big commissioning script, generating it from a tempita template (src/metadataserver/commissioning/user_data.template) and a directory of snippets (src/metadataserver/commissioning/snippets/*) for substitution into the template.  It normalizes the way we maintain this well-hidden python code.

Do not confuse these snippets with what's in the commissioning tarball: the snippets are just components that are needed to enable the download of that tarball.

Only one snippet is properly new code: maas_get.py.  I extracted a good portion of the old maas-signal into maas_api_helper.py, where maas_get.py could then re-use it.

Two snippets (maas_get.py and maas_signal.py) have different filenames in the codebase than they end up getting on the node (maas-get and maas-signal, respectively).  On the one hand, changing their names on the node might have upset existing habits, documentation, and scripting.  On the other hand, their names in the source tree had to make clear to our code maintenance tools that they were python.  The templating makes it easy to keep the two filenames separate.

You may wonder why I left the IPMI code and its configs in the template, instead of splitting those out as well.  The reason (besides this branch already being big enough!) is that those should probably become part of the downloaded tarball, not remain in the componentized main commissioning script.  The change would be easy, and make a lot of sense.  It would also reduce the size of the main commissioning script: I seem to remember hearing it should be no more than 16KB(*) and it's more than that already.

Jeroen

(*) That is Kilobytes, not Kibibytes.  There is a lot of confusion over this, because many people are unaware that a proper imperial byte consists of 8.192 bits.  They generally prefer to use the simpler, 8-bit "metric byte" instead.  A kilobyte is exactly 1,000 imperial 8.192-bit bytes, just as a kilometer is exactly 1,000 meters.  But due to the rounding error in conversion to metric bytes, a kilobyte needs 24 "leap bytes."  This raises yet more confusion over whether these leap bytes are metric or imperial bytes, and that is why there is so much miscommunication over gigabytes and terabytes in the hard-drive industry.  Are we all clear now?

For more details, see:
https://code.launchpad.net/~jtv/maas/download-commissioning-script/+merge/140422
-- 
https://code.launchpad.net/~jtv/maas/download-commissioning-script/+merge/140422
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/maas/download-commissioning-script into lp:maas.