Code review comment for lp:~jtv/launchpad/bug-499405-translationtemplates-buildmanager

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

= Bug 499405 =

This branch implements a build manager for generating translation templates from a branch. It provides no real functionality on its own; rather it's part of an ongoing project. Pre-imp discussions were done IRL at the Wellington build-farm generalisation sprint.

A build manager is the piece of code that runs on a build slave to coordinate its work. It iterates through a finite-state machine describing the job to be done. An important thing to bear in mind is that this is not "Launchpad code": it runs on a slave machine that has neither the Launchpad modules nor access to the database.

For this particular job, the finite-state machine consists of 7 states:
1. INIT: Initialize.
2. UNPACK: Create a chroot directory by unpacking a chroot tarball.
3. MOUNT: Set up a realistic system environment inside the chroot.
4. UPDATE: Get the latest versions of the packages installed inside the chroot.
5. INSTALL: Add a few job-specific packages to the chroot.
6. GENERATE: Check out the branch and produce templates based on it.
7. CLEANUP: Return the slave to its original, clean state.

Of these, step 6 is the "payload." It invokes a separate script, also found in this branch, that checks out a branch and will eventually generate templates inside it. The script is run under a non-privileged user inside the slave's chroot, providing maximum insulation from untrusted code inside the branches.

The implementations for most of the other steps are inherited from the existing BuildSlave base class.

Testing the actual shell commands is rather hard, which is why other build managers seem to go largely untested. I did find a way to unit-test at least the iteration of the state machine: mock up the BuildManager method that normally forks off an action of the state machine and hands it to twistd. Instead of running the action and triggering a state transition in twistd, the test just logs and verifies the command that would have been run in a sub-process.

In order to make this testable, I had to make just one small change in the base class. It made changes to the filesystem in the current user's home directory. Instead, I made it record the current user's home directory in its constructor; the other methods now get the location of the home directory from that same field. This allows the test to substitute a temporary directory of its own.

I also added a manual test to kick off a templates generation build through XMLRPC. I'm told this can't be run as part of the normal test suite, but it can be run manually to see if anything breaks spectacularly. It's a blatant ripoff of a similar test that abentley implemented for his own build-manager class. You "make run" in one shell then while that's active, run the test script in another with one argument: the SHA1 of a chroot tarball that's in your local Librarian.

Finally, you'll notice a helper method makeTemporaryDirectory in the Launchpad version of the TestCase class. This breaks naming convention with the existing useTempDir, but matches the convention of both a similar method in bzr test cases and the naming style in the standard tempfile module. I did update useTempDir to make use of the new method.

Test:
{{{
./bin/test -vv -t generate_translation_templates -t translationtemplatesbuildmanager
}}}

No lint, as far as I can make out; but either my latest merge with devel or my ongoing system upgrade seems to have broken the make environment. Only minimal changes since then though.

Jeroen

« Back to merge proposal