Merge lp:~mwhudson/launchpad/ami-from-scratch into lp:launchpad

[Michael Hudson-Doyle]

[Michael Hudson-Doyle (mwhudson)]

Hi jml,

I hit back with an unsolicited ec2 branch :-)

This one started with thinking about preparing an image for public consumption. I don't really know what files are lurking in unexpected places on the ec2test instances we currently use, so I started to think about building images from a blank hardy image such as those on alestic.com. Eventually, I came up with this approach, which hides all the setup of an instance in the EC2Instance object, and gives a way of saying that an image will come up 'blank' and needs completely setting up. It also changes to assuming a sudo account exists on a pre set-up image rather than creating a user on each run.

As a side effect, you could say 'ec2 test -m based-on:<id of some blank karmic ami>' to run all the tests in a karmic image, which is nice.

I made a couple of images with --public and got wgrant to test them (successfully). I shouldn't make the image public until this branch is about ready to land as while the trunk ec2 script works with the new image, it's rather slow (lots and lots of stuff gets rsynced off devpad).

Because I spent so long going back and forth on ways to do this, a fair bit of drive by clean up happened en route. I hope it doesn't muddle things too much.

Cheers,
mwh

[Jonathan Lange (jml)]

On Thu, Sep 24, 2009 at 8:30 AM, Michael Hudson
<email address hidden> wrote:
> Michael Hudson has proposed merging lp:~mwhudson/launchpad/ami-from-scratch into lp:launchpad/devel.
>
>    Requested reviews:
>    Jonathan Lange (jml)
>
>
> Hi jml,
>
> I hit back with an unsolicited ec2 branch :-)
>

Wuu.

> This one started with thinking about preparing an image for public consumption.  I don't really know what files are lurking in unexpected places on the ec2test instances we currently use, so I started to think about building images from a blank hardy image such as those on alestic.com.  Eventually, I came up with this approach, which hides all the setup of an instance in the EC2Instance object, and gives a way of saying that an image will come up 'blank' and needs completely setting up.  It also changes to assuming a sudo account exists on a pre set-up image rather than creating a user on each run.
>

By "pre set-up image", you mean these "blank" images, right?

> As a side effect, you could say 'ec2 test -m based-on:<id of some blank karmic ami>' to run all the tests in a karmic image, which is nice.
>

That *is* nice.

> I made a couple of images with --public and got wgrant to test them (successfully).  I shouldn't make the image public until this branch is about ready to land as while the trunk ec2 script works with the new image, it's rather slow (lots and lots of stuff gets rsynced off devpad).
>

Why will the public image be slow while trunk ec2 works with the new
image. I feel like I'm missing a couple of steps.

> Because I spent so long going back and forth on ways to do this, a fair bit of drive by clean up happened en route.  I hope it doesn't muddle things too much.
>

I'm sure I'll appreciate the drive-by cleanup.

...

OK, I'm done reviewing the branch. I've got a lot of questions,
because there's a lot I don't know. Overall the branch looks good, the
code seems much more solid, despite the fact that you're adding
functionality.

I would, however, like to see your answers before the branch lands.

> Cheers,
> mwh
> --
> https://code.launchpad.net/~mwhudson/launchpad/ami-from-scratch/+merge/12331
> You are requested to review the proposed merge of lp:~mwhudson/launchpad/ami-from-scratch into lp:launchpad/devel.
>
> === modified file 'lib/devscripts/ec2test/builtins.py'
> --- lib/devscripts/ec2test/builtins.py  2009-09-18 05:57:43 +0000
> +++ lib/devscripts/ec2test/builtins.py  2009-09-24 07:30:35 +0000
> @@ -95,6 +95,7 @@
>           'changes in your download cache, you must explicitly choose to '
>           'include or ignore the changes.'))
>
> +
>  postmortem_option = Option(
>     'postmortem', short_name='p',
>     help=('Drop to interactive prompt after the test and before shutting '
> @@ -308,7 +309,7 @@
>         postmortem = True
>         shutdown = True
>         instance.set_up_and_run(
> -            postmortem, shutdown, self.run_server, runner,
> +            postmortem, shutdown, self.run_server, runner, instance,
>             demo_network_string)
>

"instance" is a parameter of set_up_and_run? That's a bit weird. I wonder...

So I still don't know the motivation for this change, but I wonder if
set_u...

[Jonathan Lange (jml)]

As per previous comment.

review: Needs Information

[Michael Hudson-Doyle (mwhudson)]

Jonathan Lange wrote:
> On Thu, Sep 24, 2009 at 8:30 AM, Michael Hudson
> <email address hidden> wrote:
>> Michael Hudson has proposed merging lp:~mwhudson/launchpad/ami-from-scratch into lp:launchpad/devel.
>>
>> Requested reviews:
>> Jonathan Lange (jml)
>>
>>
>> Hi jml,
>>
>> I hit back with an unsolicited ec2 branch :-)
>>
>
> Wuu.
>
>> This one started with thinking about preparing an image for public consumption. I don't really know what files are lurking in unexpected places on the ec2test instances we currently use, so I started to think about building images from a blank hardy image such as those on alestic.com. Eventually, I came up with this approach, which hides all the setup of an instance in the EC2Instance object, and gives a way of saying that an image will come up 'blank' and needs completely setting up. It also changes to assuming a sudo account exists on a pre set-up image rather than creating a user on each run.
>>
>
> By "pre set-up image", you mean these "blank" images, right?

No, I mean an image that has been "prepared", as in, "not from scratch".

>> As a side effect, you could say 'ec2 test -m based-on:<id of some blank karmic ami>' to run all the tests in a karmic image, which is nice.
>>
>
> That *is* nice.
>
>> I made a couple of images with --public and got wgrant to test them (successfully). I shouldn't make the image public until this branch is about ready to land as while the trunk ec2 script works with the new image, it's rather slow (lots and lots of stuff gets rsynced off devpad).
>>
>
> Why will the public image be slow while trunk ec2 works with the new
> image. I feel like I'm missing a couple of steps.

Mostly because there is a lot of crap in
/code/rocketfuel-built/sourcecode that's no longer specified in
sourcedeps.conf and so the rsync that the trunk ec2 test takes ages.

>> Because I spent so long going back and forth on ways to do this, a fair bit of drive by clean up happened en route. I hope it doesn't muddle things too much.
>>
>
> I'm sure I'll appreciate the drive-by cleanup.
>
> ...
>
> OK, I'm done reviewing the branch. I've got a lot of questions,
> because there's a lot I don't know. Overall the branch looks good, the
> code seems much more solid, despite the fact that you're adding
> functionality.

Yeah, I'm happy with the changes overall.

> I would, however, like to see your answers before the branch lands.

OK.

>> Cheers,
>> mwh
>> --
>> https://code.launchpad.net/~mwhudson/launchpad/ami-from-scratch/+merge/12331
>> You are requested to review the proposed merge of lp:~mwhudson/launchpad/ami-from-scratch into lp:launchpad/devel.
>>
>> === modified file 'lib/devscripts/ec2test/builtins.py'
>> --- lib/devscripts/ec2test/builtins.py 2009-09-18 05:57:43 +0000
>> +++ lib/devscripts/ec2test/builtins.py 2009-09-24 07:30:35 +0000
>> @@ -95,6 +95,7 @@
>> 'changes in your download cache, you must explicitly choose to '
>> 'include or ignore the changes.'))
>>
>> +
>> postmortem_option = Option(
>> 'postmortem', short_name='p',
>> help=('Drop to interactive prompt after the test and before shutting '
>> @@ -308,7 +309,7 @@
>> po...

[Jonathan Lange (jml)]

Thanks. Can you send me the interdiff of your changes when they're done?

One day, Launchpad will do this for us...

jml

review: Approve

[Michael Hudson-Doyle (mwhudson)]

Michael Hudson wrote:

>>> user_connection.run_with_ssh_agent(
>>> 'bzr pull -d /var/launchpad/download-cache lp:lp-source-dependencies')
>>> + user_connection.run_with_ssh_agent(
>>> + "/var/launchpad/test/utilities/update-sourcecode "
>>> + "/var/launchpad/sourcecode")
>>> + if public:
>>> + user_connection.perform(
>>> + 'rm -rf /var/launchpad/sourcecode/shipit '
>>> + '/var/launchpad/sourcecode/canonical-identity-provider')
>>> + user_connection.perform(
>>> + 'rm -rf .ssh/known_hosts .bazaar .bzr.log')
>> This duplicates knowledge from update-sourcecode. Could we perhaps
>> instead change update-sourcecode to have an option to not even try to
>> get the private branches, and then remove this removal logic?
>>
>> Actually, this would be a worthwhile change even for day-to-day use of
>> update-sourcecode.
>
> Hm, yeah, that would be more reliable. I'll change that.

I did this in a separate branch, and got Tim to review it. So this
change has happened, but mostly isn't in the interdiff I'm about to
attach to this mail.

Cheers,
mwh

=== modified file 'lib/devscripts/ec2test/builtins.py'
--- lib/devscripts/ec2test/builtins.py	2009-09-27 00:27:17 +0000
+++ lib/devscripts/ec2test/builtins.py	2009-09-27 20:39:13 +0000
@@ -95,6 +95,7 @@
           'changes in your download cache, you must explicitly choose to '
           'include or ignore the changes.'))
 
+
 postmortem_option = Option(
     'postmortem', short_name='p',
     help=('Drop to interactive prompt after the test and before shutting '
@@ -308,7 +309,7 @@
         postmortem = True
         shutdown = True
         instance.set_up_and_run(
-            postmortem, shutdown, self.run_server, runner,
+            postmortem, shutdown, self.run_server, runner, instance,
             demo_network_string)
 
     def run_server(self, runner, instance, demo_network_string):
@@ -335,7 +336,6 @@
             "\n\n")
 
 
-
 class cmd_update_image(EC2Command):
     """Make a new AMI."""
 
@@ -349,12 +349,17 @@
             help=('Run this command (with an ssh agent) on the image before '
                   'running the default update steps.  Can be passed more than '
                   'once, the commands will be run in the order specified.')),
+        Option(
+            'public',
+            help=('Remove proprietary code from the sourcecode directory '
+                  'before bundling.')),
         ]
 
     takes_args = ['ami_name']
 
     def run(self, ami_name, machine=None, instance_type='m1.large',
-            debug=False, postmortem=False, extra_update_image_command=[]):
+            debug=False, postmortem=False, extra_update_image_command=[],
+            public=False):
         if debug:
             pdb.set_trace()
 
@@ -367,19 +372,18 @@
 
         instance.set_up_and_run(
             postmortem, True, self.update_image, instance,
-            extra_update_image_command, ami_name, credentials)
+            extra_update_image_command, ami_name, credentials, public)
 
     def update_image(self, instance, extra_update_image_command, ami_name,
-                     credentials):
+                     credentials, public):
         """Bring the image up to date.
 
         The steps we take are:
 
          * run any commands specified with --extra-update-image-command
-         * update sourcecode via rsync.
+         * update sourcecode
          * update the launchpad branch to the tip of the trunk branch.
          * update the copy of the download-cache.
-         * remove the user account.
          * bundle the image
 
         :param instance: `EC2Instance` to operate on.
@@ -387,25 +391,27 @@
             instance in addition to the usual ones.
         :param ami_name: The name to give the created AMI.
         :param credentials: An `EC2Credentials` object.
+        :param public: If true, remove proprietary code from the sourcecode
+            directory before bundling.
         """
-        user_connection = instance.connect_as_user()
+        user_connection = instance.connect()
         user_connection.perform('bzr launchpad-login %(launchpad-login)s')
         for cmd in extra_update_image_command:
             user_connection.run_with_ssh_agent(cmd)
         user_connection.run_with_ssh_agent(
-            "rsync -avp --partial --delete "
-            "--filter='P *.o' --filter='P *.pyc' --filter='P *.so' "
-            "devpad.canonical.com:/code/rocketfuel-built/launchpad/sourcecode/* "
-            "/var/launchpad/sourcecode/")
-        user_connection.run_with_ssh_agent(
             'bzr pull -d /var/launchpad/test ' + TRUNK_BRANCH)
         user_connection.run_with_ssh_agent(
             'bzr pull -d /var/launchpad/download-cache lp:lp-source-dependencies')
+        if public:
+            update_sourcecode_options = '--public-only'
+        else:
+            update_sourcecode_options = ''
+        user_connection.run_with_ssh_agent(
+            "/var/launchpad/test/utilities/update-sourcecode "
+            "/var/launchpad/sourcecode" + update_sourcecode_options)
+        user_connection.perform(
+            'rm -rf .ssh/known_hosts .bazaar .bzr.log')
         user_connection.close()
-        root_connection = instance.connect_as_root()
-        root_connection.perform(
-            'deluser --remove-home %(USER)s', ignore_failure=True)
-        root_connection.close()
         instance.bundle(ami_name, credentials)
 
 

=== modified file 'lib/devscripts/ec2test/instance.py'
--- lib/devscripts/ec2test/instance.py	2009-09-27 00:27:17 +0000
+++ lib/devscripts/ec2test/instance.py	2009-09-27 20:39:13 +0000
@@ -23,7 +23,6 @@
 
 import paramiko
 
-from devscripts.ec2test.sshconfig import SSHConfig
 from devscripts.ec2test.credentials import EC2Credentials
 
 
@@ -56,19 +55,126 @@
     return user_key
 
 
+# Commands to run to turn a blank image into one usable for the rest of the
+# ec2 functionality.  They come in two parts, one set that need to be run as
+# root and another that should be run as the 'ec2test' user.
+
+from_scratch_root = """
+# From 'help set':
+# -x  Print commands and their arguments as they are executed.
+# -e  Exit immediately if a command exits with a non-zero status.
+set -xe
+
+sed -ie 's/main universe/main universe multiverse/' /etc/apt/sources.list
+
+. /etc/lsb-release
+
+cat >> /etc/apt/sources.list << EOF
+deb http://ppa.launchpad.net/launchpad/ubuntu $DISTRIB_CODENAME main
+deb http://ppa.launchpad.net/bzr/ubuntu $DISTRIB_CODENAME main
+deb http://ppa.launchpad.net/bzr-beta-ppa/ubuntu $DISTRIB_CODENAME main
+EOF
+
+# This next part is cribbed from rocketfuel-setup
+dev_host() {
+  sed -i \"s/^127.0.0.88.*$/&\ ${hostname}/\" /etc/hosts
+}
+
+echo 'Adding development hosts on local machine'
+echo '
+# Launchpad virtual domains. This should be on one line.
+127.0.0.88      launchpad.dev
+' >> /etc/hosts
+
+declare -a hostnames
+hostnames=$(cat <<EOF
+    answers.launchpad.dev
+    api.launchpad.dev
+    bazaar-internal.launchpad.dev
+    beta.launchpad.dev
+    blueprints.launchpad.dev
+    bugs.launchpad.dev
+    code.launchpad.dev
+    feeds.launchpad.dev
+    id.launchpad.dev
+    keyserver.launchpad.dev
+    lists.launchpad.dev
+    openid.launchpad.dev
+    ppa.launchpad.dev
+    private-ppa.launchpad.dev
+    shipit.edubuntu.dev
+    shipit.kubuntu.dev
+    shipit.ubuntu.dev
+    translations.launchpad.dev
+    xmlrpc-private.launchpad.dev
+    xmlrpc.launchpad.dev
+EOF
+    )
+
+for hostname in $hostnames; do
+  dev_host;
+done
+
+echo '
+127.0.0.99      bazaar.launchpad.dev
+' >> /etc/hosts
+
+# Add the keys for the three PPAs added to sources.list above.
+apt-key adv --recv-keys --keyserver pool.sks-keyservers.net 2af499cb24ac5f65461405572d1ffb6c0a5174af
+apt-key adv --recv-keys --keyserver pool.sks-keyservers.net ece2800bacf028b31ee3657cd702bf6b8c6c1efd
+apt-key adv --recv-keys --keyserver pool.sks-keyservers.net cbede690576d1e4e813f6bb3ebaf723d37b19b80
+
+aptitude update
+aptitude -y full-upgrade
+
+apt-get -y install launchpad-developer-dependencies apache2 apache2-mpm-worker
+
+# Creat the ec2test user, give them passwordless sudo.
+adduser --gecos "" --disabled-password ec2test
+echo 'ec2test\tALL=(ALL) NOPASSWD: ALL' >> /etc/sudoers
+
+mkdir /home/ec2test/.ssh
+cat > /home/ec2test/.ssh/config << EOF
+CheckHostIP no
+StrictHostKeyChecking no
+EOF
+
+mkdir /var/launchpad
+chown -R ec2test:ec2test /var/www /var/launchpad /home/ec2test/
+"""
+
+
+from_scratch_ec2test = """
+# From 'help set':
+# -x  Print commands and their arguments as they are executed.
+# -e  Exit immediately if a command exits with a non-zero status.
+set -xe
+
+bzr launchpad-login %(launchpad-login)s
+bzr init-repo --2a /var/launchpad
+bzr branch lp:~launchpad-pqm/launchpad/devel /var/launchpad/test
+bzr branch --standalone lp:lp-source-dependencies /var/launchpad/download-cache
+mkdir /var/launchpad/sourcecode
+/var/launchpad/test/utilities/update-sourcecode /var/launchpad/sourcecode
+"""
+
+
 class EC2Instance:
     """A single EC2 instance."""
 
     @classmethod
-    def make(cls, name, instance_type, machine_id,
-             demo_networks=None, credentials=None):
+    def make(cls, name, instance_type, machine_id, demo_networks=None,
+             credentials=None):
         """Construct an `EC2Instance`.
 
         :param name: The name to use for the key pair and security group for
             the instance.
         :param instance_type: One of the AVAILABLE_INSTANCE_TYPES.
         :param machine_id: The AMI to use, or None to do the usual regexp
-            matching.
+            matching.  If you put 'based-on:' before the AMI id, it is assumed
+            that the id specifies a blank image that should be made into one
+            suitable for the other ec2 functions (see `from_scratch_root` and
+            `from_scratch_ec2test` above).
         :param demo_networks: A list of networks to add to the security group
             to allow access to the instance.
         :param credentials: An `EC2Credentials` object.
@@ -76,6 +182,10 @@
         if instance_type not in AVAILABLE_INSTANCE_TYPES:
             raise ValueError('unknown instance_type %s' % (instance_type,))
 
+        # We call this here so that it has a chance to complain before the
+        # instance is started (which can take some time).
+        user_key = get_user_key()
+
         if credentials is None:
             credentials = EC2Credentials.load_from_file()
 
@@ -91,6 +201,12 @@
         # generate it.
         account.delete_previous_key_pair()
 
+        if machine_id and machine_id.startswith('based-on:'):
+            from_scratch = True
+            machine_id = machine_id[len('based-on:'):]
+        else:
+            from_scratch = False
+
         # get the image
         image = account.acquire_image(machine_id)
 
@@ -102,12 +218,11 @@
         vals['launchpad-login'] = login
 
         return EC2Instance(
-            name, image, instance_type, demo_networks, account, vals)
-
-    # XXX: JonathanLange 2009-05-31: Separate out demo server
+            name, image, instance_type, demo_networks, account, vals,
+            from_scratch, user_key)
 
     def __init__(self, name, image, instance_type, demo_networks, account,
-                 vals):
+                 vals, from_scratch, user_key):
         self._name = name
         self._image = image
         self._account = account
@@ -115,6 +230,8 @@
         self._demo_networks = demo_networks
         self._boto_instance = None
         self._vals = vals
+        self._from_scratch = from_scratch
+        self._user_key = user_key
 
     def log(self, msg):
         """Log a message on stdout, flushing afterwards."""
@@ -148,6 +265,7 @@
                      (elapsed // 60, elapsed % 60))
             self._output = self._boto_instance.get_console_output()
             self.log(self._output.output)
+            self._ec2test_user_has_keys = False
         else:
             raise BzrCommandError(
                 'failed to start: %s\n' % self._boto_instance.state)
@@ -170,22 +288,21 @@
             return None
         return self._boto_instance.public_dns_name
 
-    def _connect(self, user, use_agent):
+    def _connect(self, username):
         """Connect to the instance as `user`. """
         ssh = paramiko.SSHClient()
         ssh.set_missing_host_key_policy(AcceptAllPolicy())
-        connect_args = {'username': user}
-        if not use_agent:
-            connect_args.update({
-                'pkey': self.private_key,
-                'allow_agent': False,
-                'look_for_keys': False,
-                })
+        connect_args = {
+            'username': username,
+            'pkey': self.private_key,
+            'allow_agent': False,
+            'look_for_keys': False,
+            }
         for count in range(10):
             try:
                 ssh.connect(self.hostname, **connect_args)
             except (socket.error, paramiko.AuthenticationException), e:
-                self.log('_connect: %r' % (e,))
+                self.log('_connect: %r\n' % (e,))
                 if count < 9:
                     time.sleep(5)
                     self.log('retrying...')
@@ -193,83 +310,72 @@
                     raise
             else:
                 break
-        return EC2InstanceConnection(self, user, ssh)
-
-    def connect_as_root(self):
-        return self._connect('root', False)
-
-    def connect_as_user(self):
-        return self._connect(self._vals['USER'], True)
-
-    def set_up_user(self, user_key):
-        """Set up an account named after the local user."""
-        root_connection = self.connect_as_root()
-        as_root = root_connection.perform
-        if self._vals['USER'] == 'gary':
-            # This helps gary debug problems others are having by removing
-            # much of the initial setup used to work on the original image.
-            as_root('deluser --remove-home gary', ignore_failure=True)
-        # Let root perform sudo without a password.
-        as_root('echo "root\tALL=NOPASSWD: ALL" >> /etc/sudoers')
-        # Add the user.
-        as_root('adduser --gecos "" --disabled-password %(USER)s')
-        # Give user sudo without password.
-        as_root('echo "%(USER)s\tALL=(ALL) NOPASSWD: ALL" >> /etc/sudoers')
-        # Update the system.
-        as_root('aptitude update')
-        as_root('aptitude -y full-upgrade')
-        # Set up ssh for user
-        # Make user's .ssh directory
-        as_root('sudo -u %(USER)s mkdir /home/%(USER)s/.ssh')
-        root_sftp = root_connection.ssh.open_sftp()
-        remote_ssh_dir = '/home/%(USER)s/.ssh' % self._vals
-        # Create config file
-        self.log('Creating %s/config\n' % (remote_ssh_dir,))
-        ssh_config_file_name = os.path.join(
-            self._vals['HOME'], '.ssh', 'config')
-        ssh_config_source = open(ssh_config_file_name)
-        config = SSHConfig()
-        config.parse(ssh_config_source)
-        ssh_config_source.close()
-        ssh_config_dest = root_sftp.open("%s/config" % remote_ssh_dir, 'w')
-        ssh_config_dest.write('CheckHostIP no\n')
-        ssh_config_dest.write('StrictHostKeyChecking no\n')
-        for hostname in ('devpad.canonical.com', 'chinstrap.canonical.com'):
-            ssh_config_dest.write('Host %s\n' % (hostname,))
-            data = config.lookup(hostname)
-            for key in ('hostname', 'gssapiauthentication', 'proxycommand',
-                        'user', 'forwardagent'):
-                value = data.get(key)
-                if value is not None:
-                    ssh_config_dest.write('    %s %s\n' % (key, value))
-        ssh_config_dest.close()
-        # create authorized_keys
-        self.log('Setting up %s/authorized_keys\n' % remote_ssh_dir)
-        authorized_keys_file = root_sftp.open(
-            "%s/authorized_keys" % remote_ssh_dir, 'w')
+        return EC2InstanceConnection(self, username, ssh)
+
+    def _upload_local_key(self, conn, remote_filename):
+        """Upload a key from the local user's agent to `remote_filename`.
+
+        The key will be uploaded in a format suitable for
+        ~/.ssh/authorized_keys.
+        """
+        authorized_keys_file = conn.sftp.open(remote_filename, 'w')
         authorized_keys_file.write(
-            "%s %s\n" % (user_key.get_name(), user_key.get_base64()))
+            "%s %s\n" % (self._user_key.get_name(), self._user_key.get_base64()))
         authorized_keys_file.close()
-        root_sftp.close()
-        # Chown and chmod the .ssh directory and contents that we just
-        # created.
-        as_root('chown -R %(USER)s:%(USER)s /home/%(USER)s/')
-        as_root('chmod 644 /home/%(USER)s/.ssh/*')
-        self.log(
-            'You can now use ssh -A %s to log in the instance.\n' %
-            self.hostname)
-        # What follows is somewhat ec2test specfic.
-        # give the user permission to do whatever in /var/www
-        as_root('chown -R %(USER)s:%(USER)s /var/www')
-        # Make /var/launchpad owned by user.
-        as_root('chown -R %(USER)s:%(USER)s /var/launchpad')
-        # Clean out left-overs from the instance image.
-        as_root('rm -fr /var/tmp/*')
-        root_connection.close()
+
+    def _ensure_ec2test_user_has_keys(self, connection=None):
+        """Make sure that we can connect over ssh as the 'ec2test' user.
+
+        We add both the key that was used to start the instance (so
+        _connect('ec2test') works and a key from the locally running ssh agent
+        (so EC2InstanceConnection.run_with_ssh_agent works).
+        """
+        if not self._ec2test_user_has_keys:
+            if connection is None:
+                connection = self._connect('root')
+                our_connection = True
+            else:
+                our_connection = False
+            self._upload_local_key(connection, 'local_key')
+            connection.perform(
+                'cat /root/.ssh/authorized_keys local_key '
+                '> /home/ec2test/.ssh/authorized_keys && rm local_key')
+            connection.perform('chown -R ec2test:ec2test /home/ec2test/')
+            connection.perform('chmod 644 /home/ec2test/.ssh/*')
+            if our_connection:
+                connection.close()
+            self.log(
+                'You can now use ssh -A ec2test@%s to log in the instance.\n' %
+                self.hostname)
+            self._ec2test_user_has_keys = True
+
+    def connect(self):
+        """Connect to the instance as a user with passwordless sudo.
+
+        This may involve first connecting as root and adding SSH keys to the
+        user's account, and in the case of a from scratch image, it will do a
+        lot of set up.
+        """
+        if self._from_scratch:
+            root_connection = self._connect('root')
+            self._upload_local_key(root_connection, 'local_key')
+            root_connection.perform(
+                'cat local_key >> ~/.ssh/authorized_keys && rm local_key')
+            root_connection.run_script(from_scratch_root % self._vals)
+            self._ensure_ec2test_user_has_keys(root_connection)
+            root_connection.close()
+            conn = self._connect('ec2test')
+            conn.run_script(from_scratch_ec2test % self._vals)
+            self._from_scratch = False
+            return conn
+        self._ensure_ec2test_user_has_keys()
+        return self._connect('ec2test')
 
     def set_up_and_run(self, postmortem, shutdown, func, *args, **kw):
-        """Start, set up a user account, run `func` and then maybe shut down.
+        """Start, run `func` and then maybe shut down.
 
+        :param config: A dictionary specifying details of how the instance
+            should be run:
         :param postmortem: If true, any exceptions will be caught and an
             interactive session run to allow debugging the problem.
         :param shutdown: If true, shut down the instance after `func` and
@@ -279,10 +385,8 @@
         :param args: Passed to `func`.
         :param kw: Passed to `func`.
         """
-        user_key = get_user_key()
-        self.start()
         try:
-            self.set_up_user(user_key)
+            self.start()
             try:
                 return func(*args, **kw)
             except Exception:
@@ -309,7 +413,6 @@
             finally:
                 if shutdown:
                     self.shutdown()
-        
 
     def _copy_single_file(self, sftp, local_path, remote_dir):
         """Copy `local_path` to `remote_dir` on this instance.
@@ -333,7 +436,6 @@
         :param sftp: A paramiko SFTP object.
         """
         remote_ec2_dir = '/mnt/ec2'
-        sftp.mkdir(remote_ec2_dir)
         remote_pk = self._copy_single_file(
             sftp, self.local_pk, remote_ec2_dir)
         remote_cert = self._copy_single_file(
@@ -379,20 +481,21 @@
         :param name: The name-to-be of the new AMI.
         :param credentials: An `EC2Credentials` object.
         """
-        root_connection = self.connect_as_root()
-        sftp = root_connection.ssh.open_sftp()
-
-        remote_pk, remote_cert =  self.copy_key_and_certificate_to_image(sftp)
-
-        sftp.close()
+        connection = self.connect()
+        connection.perform('rm -f .ssh/authorized_keys')
+        connection.perform('sudo mkdir /mnt/ec2')
+        connection.perform('sudo chown $USER:$USER /mnt/ec2')
+
+        remote_pk, remote_cert =  self.copy_key_and_certificate_to_image(
+            connection.sftp)
 
         bundle_dir = os.path.join('/mnt', name)
 
-        root_connection.perform('mkdir ' + bundle_dir)
-        root_connection.perform(' '.join([
-            'ec2-bundle-vol',
+        connection.perform('sudo mkdir ' + bundle_dir)
+        connection.perform(' '.join([
+            'sudo ec2-bundle-vol',
             '-d %s' % bundle_dir,
-            '-b',   # Set batch-mode, which doesn't use prompts.
+            '--batch',   # Set batch-mode, which doesn't use prompts.
             '-k %s' % remote_pk,
             '-c %s' % remote_cert,
             '-u %s' % self.aws_user,
@@ -404,18 +507,17 @@
 
         # Best check that the manifest actually exists though.
         test = 'test -f %s' % manifest
-        root_connection.perform(test)
+        connection.perform(test)
 
-        root_connection.perform(' '.join([
-            'ec2-upload-bundle',
+        connection.perform(' '.join([
+            'sudo ec2-upload-bundle',
             '-b %s' % name,
             '-m %s' % manifest,
             '-a %s' % credentials.identifier,
             '-s %s' % credentials.secret,
             ]))
 
-        sftp.close()
-        root_connection.close()
+        connection.close()
 
         # This is invoked locally.
         mfilename = os.path.basename(manifest)
@@ -438,9 +540,16 @@
     """An ssh connection to an `EC2Instance`."""
 
     def __init__(self, instance, username, ssh):
-        self.instance = instance
-        self.username = username
-        self.ssh = ssh
+        self._instance = instance
+        self._username = username
+        self._ssh = ssh
+        self._sftp = None
+
+    @property
+    def sftp(self):
+        if self._sftp is None:
+            self._sftp = self._ssh.open_sftp()
+        return self._sftp
 
     def perform(self, cmd, ignore_failure=False, out=None):
         """Perform 'cmd' on server.
@@ -449,10 +558,11 @@
             statuses.
         :param out: A stream to write the output of the remote command to.
         """
-        cmd = cmd % self.instance._vals
-        self.instance.log(
-            '%s@%s$ %s\n' % (self.username, self.instance._boto_instance.id, cmd))
-        session = self.ssh.get_transport().open_session()
+        cmd = cmd % self._instance._vals
+        self._instance.log(
+            '%s@%s$ %s\n'
+            % (self._username, self._instance._boto_instance.id, cmd))
+        session = self._ssh.get_transport().open_session()
         session.exec_command(cmd)
         session.shutdown_write()
         while 1:
@@ -488,9 +598,11 @@
         Use this to run commands that require local SSH credentials. For
         example, getting private branches from Launchpad.
         """
-        cmd = cmd % self.instance._vals
-        self.instance.log('%s@%s$ %s\n' % (self.username, self.instance._boto_instance.id, cmd))
-        call = ['ssh', '-A', self.instance.hostname,
+        cmd = cmd % self._instance._vals
+        self._instance.log(
+            '%s@%s$ %s\n'
+            % (self._username, self._instance._boto_instance.id, cmd))
+        call = ['ssh', '-A', self._username + '@' + self._instance.hostname,
                '-o', 'CheckHostIP no',
                '-o', 'StrictHostKeyChecking no',
                '-o', 'UserKnownHostsFile ~/.ec2/known_hosts',
@@ -500,6 +612,25 @@
             raise RuntimeError('Command failed: %s' % (cmd,))
         return res
 
+    def run_script(self, script_text):
+        """Upload `script_text` to the instance and run it with bash."""
+        script = self.sftp.open('script.sh', 'w')
+        script.write(script_text)
+        script.close()
+        self.run_with_ssh_agent('/bin/bash script.sh')
+        # At least for mwhudson, the paramiko connection often drops while the
+        # script is running.  Reconnect just in case.
+        self.reconnect()
+        self.perform('rm script.sh')
+
+    def reconnect(self):
+        """Close the connection and reopen it."""
+        self.close()
+        self._ssh = self._instance._connect(self._username)._ssh
+
     def close(self):
-        self.ssh.close()
-        self.ssh = None
+        if self._sftp is not None:
+            self._sftp.close()
+            self._sftp = None
+        self._ssh.close()
+        self._ssh = None

=== removed file 'lib/devscripts/ec2test/sshconfig.py'
--- lib/devscripts/ec2test/sshconfig.py	2009-09-27 01:51:50 +0000
+++ lib/devscripts/ec2test/sshconfig.py	1970-01-01 00:00:00 +0000
@@ -1,95 +0,0 @@
-#############################################################################
-# Modified from paramiko.config.  The change should be pushed upstream.
-# Our fork supports Host lines with more than one host.
-
-import fnmatch
-
-class SSHConfig (object):
-    """
-    Representation of config information as stored in the format used by
-    OpenSSH.  Queries can be made via L{lookup}.  The format is described in
-    OpenSSH's C{ssh_config} man page.  This class is provided primarily as a
-    convenience to posix users (since the OpenSSH format is a de-facto
-    standard on posix) but should work fine on Windows too.
-
-    @since: 1.6
-    """
-
-    def __init__(self):
-        """
-        Create a new OpenSSH config object.
-        """
-        self._config = [ { 'host': '*' } ]
-
-    def parse(self, file_obj):
-        """
-        Read an OpenSSH config from the given file object.
-
-        @param file_obj: a file-like object to read the config file from
-        @type file_obj: file
-        """
-        configs = [self._config[0]]
-        for line in file_obj:
-            line = line.rstrip('\n').lstrip()
-            if (line == '') or (line[0] == '#'):
-                continue
-            if '=' in line:
-                key, value = line.split('=', 1)
-                key = key.strip().lower()
-            else:
-                # find first whitespace, and split there
-                i = 0
-                while (i < len(line)) and not line[i].isspace():
-                    i += 1
-                if i == len(line):
-                    raise Exception('Unparsable line: %r' % line)
-                key = line[:i].lower()
-                value = line[i:].lstrip()
-
-            if key == 'host':
-                del configs[:]
-                # the value may be multiple hosts, space-delimited
-                for host in value.split():
-                    # do we have a pre-existing host config to append to?
-                    matches = [c for c in self._config if c['host'] == host]
-                    if len(matches) > 0:
-                        configs.append(matches[0])
-                    else:
-                        config = { 'host': host }
-                        self._config.append(config)
-                        configs.append(config)
-            else:
-                for config in configs:
-                    config[key] = value
-
-    def lookup(self, hostname):
-        """
-        Return a dict of config options for a given hostname.
-
-        The host-matching rules of OpenSSH's C{ssh_config} man page are used,
-        which means that all configuration options from matching host
-        specifications are merged, with more specific hostmasks taking
-        precedence.  In other words, if C{"Port"} is set under C{"Host *"}
-        and also C{"Host *.example.com"}, and the lookup is for
-        C{"ssh.example.com"}, then the port entry for C{"Host *.example.com"}
-        will win out.
-
-        The keys in the returned dict are all normalized to lowercase (look for
-        C{"port"}, not C{"Port"}.  No other processing is done to the keys or
-        values.
-
-        @param hostname: the hostname to lookup
-        @type hostname: str
-        """
-        matches = [
-            x for x in self._config if fnmatch.fnmatch(hostname, x['host'])]
-        # sort in order of shortest match (usually '*') to longest
-        matches.sort(lambda x,y: cmp(len(x['host']), len(y['host'])))
-        ret = {}
-        for m in matches:
-            ret.update(m)
-        del ret['host']
-        return ret
-
-# END paramiko config fork
-#############################################################################

=== modified file 'lib/devscripts/ec2test/testrunner.py'
--- lib/devscripts/ec2test/testrunner.py	2009-09-27 00:27:17 +0000
+++ lib/devscripts/ec2test/testrunner.py	2009-09-27 20:39:13 +0000
@@ -368,14 +368,13 @@
 
 
     def configure_system(self):
-        user_connection = self._instance.connect_as_user()
+        user_connection = self._instance.connect()
         as_user = user_connection.perform
-        user_sftp = user_connection.ssh.open_sftp()
         # Set up bazaar.conf with smtp information if necessary
         if self.email or self.message:
-            as_user('mkdir /home/%(USER)s/.bazaar')
-            bazaar_conf_file = user_sftp.open(
-                "/home/%(USER)s/.bazaar/bazaar.conf" % self.vals, 'w')
+            as_user('mkdir .bazaar')
+            bazaar_conf_file = user_connection.sftp.open(
+                ".bazaar/bazaar.conf", 'w')
             bazaar_conf_file.write(
                 'smtp_server = %(smtp_server)s\n' % self.vals)
             if self.vals['smtp_username']:
@@ -387,25 +386,18 @@
             bazaar_conf_file.close()
         # Copy remote ec2-remote over
         self.log('Copying ec2test-remote.py to remote machine.\n')
-        user_sftp.put(
+        user_connection.sftp.put(
             os.path.join(os.path.dirname(os.path.realpath(__file__)),
                          'ec2test-remote.py'),
             '/var/launchpad/ec2test-remote.py')
-        user_sftp.close()
         # Set up launchpad login and email
         as_user('bzr launchpad-login %(launchpad-login)s')
         user_connection.close()
 
     def prepare_tests(self):
-        user_connection = self._instance.connect_as_user()
+        user_connection = self._instance.connect()
         # Clean up the test branch left in the instance image.
         user_connection.perform('rm -rf /var/launchpad/test')
-        # get newest sources
-        user_connection.run_with_ssh_agent(
-            "rsync -avp --partial --delete "
-            "--filter='P *.o' --filter='P *.pyc' --filter='P *.so' "
-            "devpad.canonical.com:/code/rocketfuel-built/launchpad/sourcecode/* "
-            "/var/launchpad/sourcecode/")
         # Get trunk.
         user_connection.run_with_ssh_agent(
             'bzr branch %(trunk_branch)s /var/launchpad/test')
@@ -415,6 +407,10 @@
                 'cd /var/launchpad/test; bzr merge %(branch)s')
         else:
             self.log('(Testing trunk, so no branch merge.)')
+        # get newest sources
+        user_connection.run_with_ssh_agent(
+            "/var/launchpad/test/utilities/update-sourcecode "
+            "/var/launchpad/sourcecode")
         # Get any new sourcecode branches as requested
         for dest, src in self.branches:
             fulldest = os.path.join('/var/launchpad/test/sourcecode', dest)
@@ -455,20 +451,18 @@
         p('mv /var/launchpad/download-cache /var/launchpad/tmp/download-cache')
         if (self.include_download_cache_changes and
             self.download_cache_additions):
-            sftp = user_connection.ssh.open_sftp()
             root = os.path.realpath(
                 os.path.join(self.original_branch, 'download-cache'))
             for info in self.download_cache_additions:
                 src = os.path.join(root, info[0])
                 self.log('Copying %s to remote machine.\n' % (src,))
-                sftp.put(
+                user_connection.sftp.put(
                     src,
                     os.path.join('/var/launchpad/tmp/download-cache', info[0]))
-            sftp.close()
         p('/var/launchpad/test/utilities/link-external-sourcecode '
           '-p/var/launchpad/tmp -t/var/launchpad/test'),
         # set up database
-        p('/var/launchpad/test/utilities/launchpad-database-setup %(USER)s')
+        p('/var/launchpad/test/utilities/launchpad-database-setup $USER')
         # close ssh connection
         user_connection.close()
 
@@ -476,7 +470,7 @@
         """Turn ec2 instance into a demo server."""
         self.configure_system()
         self.prepare_tests()
-        user_connection = self._instance.connect_as_user()
+        user_connection = self._instance.connect()
         p = user_connection.perform
         p('make -C /var/launchpad/test schema')
         p('mkdir -p /var/tmp/bazaar.launchpad.dev/static')
@@ -510,7 +504,7 @@
     def run_tests(self):
         self.configure_system()
         self.prepare_tests()
-        user_connection = self._instance.connect_as_user()
+        user_connection = self._instance.connect()
 
         # Make sure we activate the failsafe --shutdown feature.  This will
         # make the server shut itself down after the test run completes, or
@@ -580,11 +574,9 @@
 
             # deliver results as requested
             if self.file:
-                sftp = user_connection.ssh.open_sftp()
                 self.log(
                     'Writing abridged test results to %s.\n' % self.file)
-                sftp.get('/var/www/summary.log', self.file)
-                sftp.close()
+                user_connection.sftp.get('/var/www/summary.log', self.file)
         user_connection.close()
 
     def get_remote_test_options(self):

=== modified file 'utilities/update-sourcecode'
--- utilities/update-sourcecode	2009-09-27 00:27:17 +0000
+++ utilities/update-sourcecode	2009-09-27 20:39:13 +0000
@@ -5,10 +5,12 @@
 
 """Update the sourcecode managed in sourcecode/."""
 
-import _pythonpath
-
+import os
 import sys
 
+sys.path.append(
+    os.path.join(os.path.dirname(os.path.dirname(__file__)), 'lib'))
+
 from devscripts import sourcecode