Merge lp:~allenap/launchpad/ec2-test-race-bug-422433 into lp:launchpad

[Gavin Panella]

[Gavin Panella (allenap)]

This branch attempts to resolve a race condition when starting two or
more ec2test instances at similar times. What can happen is that,
during setup, one session can delete the key pair for the other
session, or delete the security group that the other session has just
created, because a static name ("ec2-test-runner") is used to name
these objects within AWS.

Now, with this branch, a session-specific name is used to name key
pairs and security groups so that sessions don't clobber each
other. The name contains some random data to give a good guarantee of
uniqueness.

However, one useful feature of using a static name is that old stuff
gets cleared up, so the new session name also contains a expiry
timestamp after which time the object can be deleted. The methods
delete_previous_security_groups() and delete_previous_key_pairs()
respect this timestamp.

I cleaned up a big pile of lint in revision 9611, and it's basically
noise next to this feature (though it still needs review). I've
prepared two diffs to help review, one containing only the lint and
another with only the feature changes.

== The features ==

Diff: http://pastebin.ubuntu.com/283667/

lib/devscripts/ec2test/builtins.py

  Create EC2SessionName instances based on EC2TestRunner.name instead
  of passing the name in directly.

lib/devscripts/ec2test/instance.py

  Check that the name is an instance of EC2SessionName, and call new
  method account.collect_garbage() in place of just
  account.delete_previous_key_pairs(). Incidentally, the former calls
  the latter.

lib/devscripts/ec2test/account.py

  Don't try to delete old security groups in
  acquire_security_group(). Removing old security groups is now done
  in delete_previous_security_groups().

  The method delete_previous_security_groups() and
  delete_previous_key_pairs() are very similar in the way they handle
  expiry timestamps.

lib/devscripts/ec2test/session.py
lib/devscripts/ec2test/tests/test_session.py

  Implementation and tests for EC2SessionName. This is a subclass of
  str with a few useful properties and a classmethod to aid in
  generating a good and unique name.

lib/devscripts/ec2test/utils.py
lib/devscripts/ec2test/tests/test_utils.py

  A few functions used to help with generating and parsing session
  names, and their tests.

  make_random_string() is a little unfortunate because there's a uuid
  module in Python 2.5 and beyond that does a better job of
  this. However, this script needs to support Python 2.4, so it'll
  stay for now. It does no harm really I guess.

== The lint fixes ==

http://pastebin.ubuntu.com/283676/

There is some outstanding lint in this branch:

  lib/devscripts/ec2test/builtins.py
    223: [W0102, cmd_test.run] Dangerous default value [] as argument
    289: [W0102, cmd_demo.run] Dangerous default value [] as argument
    363: [W0102, cmd_update_image.run] Dangerous default value [] as argument

  lib/devscripts/ec2test/instance.py
    408: [W0703, EC2Instance.set_up_and_run] Catch "Exception"

There aren't any tests for these scripts other than the ones I've
written in the branch, so I'm going to leave this lint in place
because changing them could affect behaviour. I'm build...

[Abel Deuring (adeuring)]

Hi Gavin,

a very nice branch! Just one suggestion:

> + def delete_previous_security_groups(self):
> + """Delete previously used security groups, if found."""
> + def try_delete_group(group):
> + try:
> + group.delete()
> + except EC2ResponseError, e:
> + if e.code != 'InvalidGroup.InUse':
> + raise
> + self.log('Cannot delete; security group '
> + '%r in use.\n' % group.name)
> + now = datetime.utcnow()
> + for group in self.conn.get_all_security_groups():
> + session_name = EC2SessionName(group.name)
> + if session_name in (self.name, self.name.base):
> + self.log('Deleting security group %r\n' % group.name)
> + try_delete_group(group)
> + elif session_name.base == self.name.base:
> + if session_name.expires is None:
> + self.log('Found security group %r without creation '
> + 'date; leaving.\n' % group.name)
> + elif session_name.expires >= now:
> + self.log('Found recent security group %r; '
> + 'leaving\n' % group.name)
> + else:
> + self.log('Deleting expired security '
> + 'group %r\n' % group.name)
> + try_delete_group(group)
> + else:
> + self.log('Found other security group %r; '
> + 'leaving.\n' % group.name)
> +

> + def delete_previous_key_pairs(self):
> + """Delete previously used keypairs, if found."""
> + def try_delete_key_pair(key_pair):
> + try:
> + key_pair.delete()
> + except EC2ResponseError, e:
> + if e.code != 'InvalidKeyPair.NotFound':
> + if e.code == 'AuthFailure':
> + # Inserted because of previous support issue.
> + self.log(
> + 'POSSIBLE CAUSES OF ERROR:\n'
> + ' Did you sign up for EC2?\n'
> + ' Did you put a credit card number in your AWS '
> + 'account?\n'
> + 'Please doublecheck before reporting a '
> + 'problem.\n')
> + raise
> + self.log('Cannot delete; key pair not '
> + 'found %r\n' % key_pair.name)
> + now = datetime.utcnow()
> + for key_pair in self.conn.get_all_key_pairs():
> + session_name = EC2SessionName(key_pair.name)
> + if session_name in (self.name, self.name.base):
> + self.log('Deleting key pair %r\n' % key_pair.name)
> + try_delete_key_pair(key_pair)
> + elif session_name.base == self.name.base:
> + if session_name.expires is None:
> + self.log('Found key pair %r without creation date; '
> + 'leaving.\n' % key_pair.name)
> + elif session_n...

review: Approve

[Gavin Panella (allenap)]

Thanks for the review!

> Hi Gavin,
>
> a very nice branch! Just one suggestion:
>
...
>
> The for loops in both methods are nearly identical; the only differences I see
> are that the log messages contain "key pair" or "secuity group", and that
> try_delete_key_pair() or try_delete_security_group() is called. Would it make
> sense to move the code starting at "if session_name" and ending at "else:
> self.log(..)" into a common method?

Good point. I've done this, and also ditched a lot of the noisy
logging; it now says something only if a key pair or security group is
deleted or if the deletion fails.

Incremental diff: http://pastebin.ubuntu.com/283801/

Thanks, Gavin.

[Abel Deuring (adeuring)]

Hi Gavin,

On 02.10.2009 14:35, Gavin Panella wrote:
> Thanks for the review!
>
>> Hi Gavin,
>>
>> a very nice branch! Just one suggestion:
>>
> ...
>> The for loops in both methods are nearly identical; the only differences I see
>> are that the log messages contain "key pair" or "secuity group", and that
>> try_delete_key_pair() or try_delete_security_group() is called. Would it make
>> sense to move the code starting at "if session_name" and ending at "else:
>> self.log(..)" into a common method?
>
> Good point. I've done this, and also ditched a lot of the noisy
> logging; it now says something only if a key pair or security group is
> deleted or if the deletion fails.
>
> Incremental diff: http://pastebin.ubuntu.com/283801/

thanks for this change! Looks good

Abel

=== modified file 'lib/devscripts/ec2test/account.py'
--- lib/devscripts/ec2test/account.py	2009-09-29 15:48:49 +0000
+++ lib/devscripts/ec2test/account.py	2009-10-02 12:34:16 +0000
@@ -13,7 +13,10 @@
 import sys
 import urllib
 
+from datetime import datetime
+
 from boto.exception import EC2ResponseError
+from devscripts.ec2test.session import EC2SessionName
 
 import paramiko
 
@@ -24,6 +27,13 @@
     # ...anyone else want in on the fun?
     )
 
+AUTH_FAILURE_MESSAGE = """\
+POSSIBLE CAUSES OF ERROR:
+- Did you sign up for EC2?
+- Did you put a credit card number in your AWS account?
+Please double-check before reporting a problem.
+"""
+
 
 def get_ip():
     """Uses AWS checkip to obtain this machine's IP address.
@@ -64,44 +74,29 @@
         sys.stdout.write(msg)
         sys.stdout.flush()
 
+    def _find_expired_artifacts(self, artifacts):
+        now = datetime.utcnow()
+        for artifact in artifacts:
+            session_name = EC2SessionName(artifact.name)
+            if (session_name in (self.name, self.name.base) or (
+                    session_name.base == self.name.base and
+                    session_name.expires is not None and
+                    session_name.expires < now)):
+                yield artifact
+
     def acquire_security_group(self, demo_networks=None):
         """Get a security group with the appropriate configuration.
 
         "Appropriate" means configured to allow this machine to connect via
         SSH, HTTP and HTTPS.
 
-        If a group is already configured with this name for this connection,
-        then re-use that. Otherwise, create a new security group and configure
-        it appropriately.
-
         The name of the security group is the `EC2Account.name` attribute.
 
         :return: A boto security group.
         """
         if demo_networks is None:
             demo_networks = []
-        try:
-            group = self.conn.get_all_security_groups(self.name)[0]
-        except EC2ResponseError, e:
-            if e.code != 'InvalidGroup.NotFound':
-                raise
-        else:
-            # If an existing security group was configured, try deleting it
-            # since our external IP might have changed.
-            try:
-                group.delete()
-            except EC2ResponseError, e:
-                if e.code != 'InvalidGroup.InUse':
-                    raise
-                # Otherwise, it means that an instance is already using
-                # it, so simply re-use it. It's unlikely that our IP changed!
-                #
-                # XXX: JonathanLange 2009-06-05: If the security group exists
-                # already, verify that the current IP is permitted; if it is
-                # not, make an INFO log and add the current IP.
-                self.log("Security group already in use, so reusing.\n")
-                return group
-
+        # Create the security group.
         security_group = self.conn.create_security_group(
             self.name, 'Authorization to access the test runner instance.')
         # Authorize SSH and HTTP.
@@ -117,34 +112,49 @@
             security_group.authorize('tcp', 443, 443, network)
         return security_group
 
+    def delete_previous_security_groups(self):
+        """Delete previously used security groups, if found."""
+        expired_groups = self._find_expired_artifacts(
+            self.conn.get_all_security_groups())
+        for group in expired_groups:
+            try:
+                group.delete()
+            except EC2ResponseError, e:
+                if e.code != 'InvalidGroup.InUse':
+                    raise
+                self.log('Cannot delete; security group '
+                         '%r in use.\n' % group.name)
+            else:
+                self.log('Deleted security group %r.' % group.name)
+
     def acquire_private_key(self):
         """Create & return a new key pair for the test runner."""
         key_pair = self.conn.create_key_pair(self.name)
         return paramiko.RSAKey.from_private_key(
             cStringIO.StringIO(key_pair.material.encode('ascii')))
 
-    def delete_previous_key_pair(self):
-        """Delete previously used keypair, if it exists."""
-        try:
-            # Only one keypair will match 'self.name' since it's a unique
-            # identifier.
-            key_pairs = self.conn.get_all_key_pairs(self.name)
-            assert len(key_pairs) == 1, (
-                "Should be only one keypair, found %d (%s)"
-                % (len(key_pairs), key_pairs))
-            key_pair = key_pairs[0]
-            key_pair.delete()
-        except EC2ResponseError, e:
-            if e.code != 'InvalidKeyPair.NotFound':
-                if e.code == 'AuthFailure':
-                    # Inserted because of previous support issue.
-                    self.log(
-                        'POSSIBLE CAUSES OF ERROR:\n'
-                        '  Did you sign up for EC2?\n'
-                        '  Did you put a credit card number in your AWS '
-                        'account?\n'
-                        'Please doublecheck before reporting a problem.\n')
-                raise
+    def delete_previous_key_pairs(self):
+        """Delete previously used keypairs, if found."""
+        expired_key_pairs = self._find_expired_artifacts(
+            self.conn.get_all_key_pairs())
+        for key_pair in expired_key_pairs:
+            try:
+                key_pair.delete()
+            except EC2ResponseError, e:
+                if e.code != 'InvalidKeyPair.NotFound':
+                    if e.code == 'AuthFailure':
+                        # Inserted because of previous support issue.
+                        self.log(AUTH_FAILURE_MESSAGE)
+                    raise
+                self.log('Cannot delete; key pair not '
+                         'found %r\n' % key_pair.name)
+            else:
+                self.log('Deleted key pair %r.' % key_pair.name)
+
+    def collect_garbage(self):
+        """Remove any old keys and security groups."""
+        self.delete_previous_security_groups()
+        self.delete_previous_key_pairs()
 
     def acquire_image(self, machine_id):
         """Get the image.
 
=== modified file 'lib/devscripts/ec2test/builtins.py'
--- lib/devscripts/ec2test/builtins.py	2009-09-30 12:57:13 +0000
+++ lib/devscripts/ec2test/builtins.py	2009-10-02 12:34:16 +0000
@@ -23,6 +23,7 @@
 from devscripts.ec2test.credentials import EC2Credentials
 from devscripts.ec2test.instance import (
     AVAILABLE_INSTANCE_TYPES, DEFAULT_INSTANCE_TYPE, EC2Instance)
+from devscripts.ec2test.session import EC2SessionName
 from devscripts.ec2test.testrunner import EC2TestRunner, TRUNK_BRANCH
 
 
@@ -168,52 +169,52 @@
             help=('Store abridged test results in FILE.')),
         ListOption(
             'email', short_name='e', argname='EMAIL', type=str,
-            help=('Email address to which results should be mailed.  Defaults to '
-                  'the email address from `bzr whoami`. May be supplied multiple '
-                  'times. The first supplied email address will be used as the '
-                  'From: address.')),
+            help=('Email address to which results should be mailed. Defaults '
+                  'to the email address from `bzr whoami`. May be supplied '
+                  'multiple times. The first supplied email address will be '
+                  'used as the From: address.')),
         Option(
             'noemail', short_name='n',
             help=('Do not try to email results.')),
         Option(
             'test-options', short_name='o', type=str,
-            help=('Test options to pass to the remote test runner.  Defaults to '
-                  "``-o '-vv'``.  For instance, to run specific tests, you might "
-                  "use ``-o '-vvt my_test_pattern'``.")),
+            help=('Test options to pass to the remote test runner. Defaults '
+                  "to ``-o '-vv'``.  For instance, to run specific tests, "
+                  "you might use ``-o '-vvt my_test_pattern'``.")),
         Option(
             'submit-pqm-message', short_name='s', type=str, argname="MSG",
-            help=('A pqm message to submit if the test run is successful.  If '
-                  'provided, you will be asked for your GPG passphrase before '
-                  'the test run begins.')),
+            help=('A pqm message to submit if the test run is successful. '
+                  'If provided, you will be asked for your GPG passphrase '
+                  'before the test run begins.')),
         Option(
             'pqm-public-location', type=str,
-            help=('The public location for the pqm submit, if a pqm message is '
-                  'provided (see --submit-pqm-message).  If this is not provided, '
-                  'for local branches, bzr configuration is consulted; for '
-                  'remote branches, it is assumed that the remote branch *is* '
-                  'a public branch.')),
+            help=('The public location for the pqm submit, if a pqm message '
+                  'is provided (see --submit-pqm-message).  If this is not '
+                  'provided, for local branches, bzr configuration is '
+                  'consulted; for remote branches, it is assumed that the '
+                  'remote branch *is* a public branch.')),
         Option(
             'pqm-submit-location', type=str,
-            help=('The submit location for the pqm submit, if a pqm message is '
-                  'provided (see --submit-pqm-message).  If this option is not '
-                  'provided, the script will look for an explicitly specified '
-                  'launchpad branch using the -b/--branch option; if that branch '
-                  'was specified and is owned by the launchpad-pqm user on '
-                  'launchpad, it is used as the pqm submit location. Otherwise, '
-                  'for local branches, bzr configuration is consulted; for '
-                  'remote branches, it is assumed that the submit branch is %s.'
-                  % (TRUNK_BRANCH,))),
+            help=('The submit location for the pqm submit, if a pqm message '
+                  'is provided (see --submit-pqm-message).  If this option '
+                  'is not provided, the script will look for an explicitly '
+                  'specified launchpad branch using the -b/--branch option; '
+                  'if that branch was specified and is owned by the '
+                  'launchpad-pqm user on launchpad, it is used as the pqm '
+                  'submit location. Otherwise, for local branches, bzr '
+                  'configuration is consulted; for remote branches, it is '
+                  'assumed that the submit branch is %s.' % (TRUNK_BRANCH,))),
         Option(
             'pqm-email', type=str,
-            help=('Specify the email address of the PQM you are submitting to. '
-                  'If the branch is local, then the bzr configuration is '
+            help=('Specify the email address of the PQM you are submitting '
+                  'to. If the branch is local, then the bzr configuration is '
                   'consulted; for remote branches "Launchpad PQM '
                   '<[e-mail]>" is used by default.')),
         postmortem_option,
         Option(
             'headless',
-            help=('After building the instance and test, run the remote tests '
-                  'headless.  Cannot be used with postmortem '
+            help=('After building the instance and test, run the remote '
+                  'tests headless.  Cannot be used with postmortem '
                   'or file.')),
         debug_option,
         Option(
@@ -252,9 +253,9 @@
                 'You have specified no way to get the results '
                 'of your headless test run.')
 
-
+        session_name = EC2SessionName.make(EC2TestRunner.name)
         instance = EC2Instance.make(
-            EC2TestRunner.name, instance_type, machine)
+            session_name, instance_type, machine)
 
         runner = EC2TestRunner(
             test_branch, email=email, file=file,
@@ -381,8 +382,9 @@
         branches, test_branch = _get_branches_and_test_branch(
             trunk, branch, test_branch)
 
+        session_name = EC2SessionName.make(EC2TestRunner.name)
         instance = EC2Instance.make(
-            EC2TestRunner.name, instance_type, machine, demo)
+            session_name, instance_type, machine, demo)
 
         runner = EC2TestRunner(
             test_branch, branches=branches,
@@ -435,8 +437,9 @@
         ListOption(
             'extra-update-image-command', type=str,
             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.')),
+                  '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 '
@@ -453,8 +456,9 @@
 
         credentials = EC2Credentials.load_from_file()
 
+        session_name = EC2SessionName.make(EC2TestRunner.name)
         instance = EC2Instance.make(
-            EC2TestRunner.name, instance_type, machine,
+            session_name, instance_type, machine,
             credentials=credentials)
         instance.check_bundling_prerequisites()
 
@@ -489,7 +493,8 @@
         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')
+            'bzr pull -d /var/launchpad/download-cache '
+            'lp:lp-source-dependencies')
         if public:
             update_sourcecode_options = '--public-only'
         else:
 
=== modified file 'lib/devscripts/ec2test/instance.py'
--- lib/devscripts/ec2test/instance.py	2009-09-27 20:36:18 +0000
+++ lib/devscripts/ec2test/instance.py	2009-10-02 12:34:16 +0000
@@ -24,6 +24,7 @@
 import paramiko
 
 from devscripts.ec2test.credentials import EC2Credentials
+from devscripts.ec2test.session import EC2SessionName
 
 
 DEFAULT_INSTANCE_TYPE = 'c1.xlarge'
@@ -159,6 +160,18 @@
 """
 
 
+postmortem_banner = """\
+Postmortem Console. EC2 instance is not yet dead.
+It will shut down when you exit this prompt (CTRL-D)
+
+Tab-completion is enabled.
+EC2Instance is available as `instance`.
+Also try these:
+  http://%(dns)s/current_test.log
+  ssh -A %(dns)s
+"""
+
+
 class EC2Instance:
     """A single EC2 instance."""
 
@@ -169,6 +182,7 @@
 
         :param name: The name to use for the key pair and security group for
             the instance.
+        :type name: `EC2SessionName`
         :param instance_type: One of the AVAILABLE_INSTANCE_TYPES.
         :param machine_id: The AMI to use, or None to do the usual regexp
             matching.  If you put 'based-on:' before the AMI id, it is assumed
@@ -179,6 +193,7 @@
             to allow access to the instance.
         :param credentials: An `EC2Credentials` object.
         """
+        assert isinstance(name, EC2SessionName)
         if instance_type not in AVAILABLE_INSTANCE_TYPES:
             raise ValueError('unknown instance_type %s' % (instance_type,))
 
@@ -199,7 +214,7 @@
         # We always recreate the keypairs because there is no way to
         # programmatically retrieve the private key component, unless we
         # generate it.
-        account.delete_previous_key_pair()
+        account.collect_garbage()
 
         if machine_id and machine_id.startswith('based-on:'):
             from_scratch = True
@@ -320,7 +335,8 @@
         """
         authorized_keys_file = conn.sftp.open(remote_filename, 'w')
         authorized_keys_file.write(
-            "%s %s\n" % (self._user_key.get_name(), self._user_key.get_base64()))
+            "%s %s\n" % (self._user_key.get_name(),
+                         self._user_key.get_base64()))
         authorized_keys_file.close()
 
     def _ensure_ec2test_user_has_keys(self, connection=None):
@@ -345,8 +361,8 @@
             if our_connection:
                 connection.close()
             self.log(
-                'You can now use ssh -A ec2test@%s to log in the instance.\n' %
-                self.hostname)
+                '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):
@@ -390,25 +406,17 @@
             try:
                 return func(*args, **kw)
             except Exception:
-                # When running in postmortem mode, it is really helpful to see if
-                # there are any exceptions before it waits in the console (in the
-                # finally block), and you can't figure out why it's broken.
+                # When running in postmortem mode, it is really helpful to see
+                # if there are any exceptions before it waits in the console
+                # (in the finally block), and you can't figure out why it's
+                # broken.
                 traceback.print_exc()
         finally:
             try:
                 if postmortem:
                     console = code.InteractiveConsole(locals())
-                    console.interact((
-                        'Postmortem Console.  EC2 instance is not yet dead.\n'
-                        'It will shut down when you exit this prompt (CTRL-D).\n'
-                        '\n'
-                        'Tab-completion is enabled.'
-                        '\n'
-                        'EC2Instance is available as `instance`.\n'
-                        'Also try these:\n'
-                        '  http://%(dns)s/current_test.log\n'
-                        '  ssh -A %(dns)s') %
-                                     {'dns': self.hostname})
+                    console.interact(
+                        postmortem_banner % {'dns': self.hostname})
                     print 'Postmortem console closed.'
             finally:
                 if shutdown:
 
=== added file 'lib/devscripts/ec2test/session.py'
--- lib/devscripts/ec2test/session.py	1970-01-01 00:00:00 +0000
+++ lib/devscripts/ec2test/session.py	2009-10-02 12:34:16 +0000
@@ -0,0 +1,90 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Code to represent a single session of EC2 use."""
+
+__metaclass__ = type
+__all__ = [
+    'EC2SessionName',
+    ]
+
+from datetime import datetime, timedelta
+
+from devscripts.ec2test.utils import (
+    find_datetime_string, make_datetime_string, make_random_string)
+
+
+DEFAULT_LIFETIME = timedelta(hours=6)
+
+
+class EC2SessionName(str):
+    """A name for an EC2 session.
+
+    This is used when naming key pairs and security groups, so it's
+    useful to be unique. However, to aid garbage collection of old key
+    pairs and security groups, the name contains a common element and
+    an expiry timestamp. The form taken should always be:
+
+      <base-name>/<expires-timestamp>/<random-data>
+
+    None of the parts should contain forward-slashes, and the
+    timestamp should be acceptable input to `find_datetime_string`.
+
+    `EC2SessionName.make()` will generate a suitable name given a
+    suitable base name.
+    """
+
+    @classmethod
+    def make(cls, base, expires=None):
+        """Create an `EC2SessionName`.
+
+        This checks that `base` does not contain a forward-slash, and
+        provides some convenient functionality for `expires`:
+
+        - If `expires` is None, it defaults to now (UTC) plus
+          `DEFAULT_LIFETIME`.
+
+        - If `expires` is a `datetime`, it is converted to a timestamp
+          in the correct form.
+
+        - If `expires` is a `timedelta`, it is added to now (UTC) then
+          converted to a timestamp.
+
+        - Otherwise `expires` is assumed to be a string, so is checked
+          for the absense of forward-slashes, and that a correctly
+          formed timestamp can be discovered.
+
+        """
+        assert '/' not in base
+        if expires is None:
+            expires = DEFAULT_LIFETIME
+        if isinstance(expires, timedelta):
+            expires = datetime.utcnow() + expires
+        if isinstance(expires, datetime):
+            expires = make_datetime_string(expires)
+        else:
+            assert '/' not in expires
+            assert find_datetime_string(expires) is not None
+        rand = make_random_string(8)
+        return cls("%s/%s/%s" % (base, expires, rand))
+
+    @property
+    def base(self):
+        parts = self.split('/')
+        if len(parts) != 3:
+            return None
+        return parts[0]
+
+    @property
+    def expires(self):
+        parts = self.split('/')
+        if len(parts) != 3:
+            return None
+        return find_datetime_string(parts[1])
+
+    @property
+    def rand(self):
+        parts = self.split('/')
+        if len(parts) != 3:
+            return None
+        return parts[2]
 
=== added directory 'lib/devscripts/ec2test/tests'
=== added file 'lib/devscripts/ec2test/tests/__init__.py'
--- lib/devscripts/ec2test/tests/__init__.py	1970-01-01 00:00:00 +0000
+++ lib/devscripts/ec2test/tests/__init__.py	2009-10-02 12:34:16 +0000
@@ -0,0 +1,2 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
 
=== added file 'lib/devscripts/ec2test/tests/test_session.py'
--- lib/devscripts/ec2test/tests/test_session.py	1970-01-01 00:00:00 +0000
+++ lib/devscripts/ec2test/tests/test_session.py	2009-10-02 12:34:16 +0000
@@ -0,0 +1,69 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test the session module."""
+
+__metaclass__ = type
+
+import re
+import unittest
+
+from datetime import datetime, timedelta
+
+from devscripts.ec2test import session
+
+
+class TestEC2SessionName(unittest.TestCase):
+    """Tests for EC2SessionName."""
+
+    def test_make(self):
+        # EC2SessionName.make() is the most convenient way to create
+        # valid names.
+        name = session.EC2SessionName.make("fred")
+        check = re.compile(
+            r'^fred/\d{4}-\d{2}-\d{2}-\d{4}/[0-9a-zA-Z]{8}$').match
+        self.failIf(check(name) is None, "Did not match %r" % name)
+        possible_expires = [
+            None, '1986-04-26-0123', timedelta(hours=10),
+            datetime(1986, 04, 26, 1, 23)
+            ]
+        for expires in possible_expires:
+            name = session.EC2SessionName.make("fred", expires)
+            self.failIf(check(name) is None, "Did not match %r" % name)
+
+    def test_properties(self):
+        # A valid EC2SessionName has properies to access the three
+        # components of its name.
+        base = "fred"
+        timestamp = datetime(1986, 4, 26, 1, 23)
+        timestamp_string = '1986-04-26-0123'
+        rand = 'abcdef123456'
+        name = session.EC2SessionName(
+            "%s/%s/%s" % (base, timestamp_string, rand))
+        self.failUnlessEqual(base, name.base)
+        self.failUnlessEqual(timestamp, name.expires)
+        self.failUnlessEqual(rand, name.rand)
+
+    def test_invalid_base(self):
+        # If the given base contains a forward-slash, an
+        # AssertionError should be raised.
+        self.failUnlessRaises(
+            AssertionError, session.EC2SessionName.make, "forward/slash")
+
+    def test_invalid_timestamp(self):
+        # If the given expiry timestamp contains a forward-slash, an
+        # AssertionError should be raised.
+        self.failUnlessRaises(
+            AssertionError, session.EC2SessionName.make, "fred", "/")
+        # If the given expiry timestamp does not contain a timestamp
+        # in the correct form, an AssertionError should be raised.
+        self.failUnlessRaises(
+            AssertionError, session.EC2SessionName.make, "fred", "1986.04.26")
+
+    def test_form_not_correct(self):
+        # If the form of the string is not base/timestamp/rand then
+        # the corresponding properties should all return None.
+        broken_name = session.EC2SessionName('bob')
+        self.failUnless(broken_name.base is None)
+        self.failUnless(broken_name.expires is None)
+        self.failUnless(broken_name.rand is None)
 
=== added file 'lib/devscripts/ec2test/tests/test_utils.py'
--- lib/devscripts/ec2test/tests/test_utils.py	1970-01-01 00:00:00 +0000
+++ lib/devscripts/ec2test/tests/test_utils.py	2009-10-02 12:34:16 +0000
@@ -0,0 +1,61 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""Test the utils module."""
+
+__metaclass__ = type
+
+import unittest
+
+from datetime import datetime
+
+from devscripts.ec2test import utils
+
+
+class TestDateTimeUtils(unittest.TestCase):
+    """Tests for date/time related utilities."""
+
+    example_date = datetime(1986, 4, 26, 1, 23)
+    example_date_string = '1986-04-26-0123'
+    example_date_text = (
+        'blah blah foo blah 23545 646 ' +
+        example_date_string + ' 435 blah')
+
+    def test_make_datetime_string(self):
+        self.failUnlessEqual(
+            self.example_date_string,
+            utils.make_datetime_string(self.example_date))
+
+    def test_find_datetime_string(self):
+        self.failUnlessEqual(
+            self.example_date,
+            utils.find_datetime_string(self.example_date_string))
+        self.failUnlessEqual(
+            self.example_date,
+            utils.find_datetime_string(self.example_date_text))
+
+
+class TestRandomUtils(unittest.TestCase):
+    """Tests for randomness related utilities."""
+
+    hex_chars = frozenset('0123456789abcdefABCDEF')
+
+    def test_make_random_string(self):
+        rand_a = utils.make_random_string()
+        rand_b = utils.make_random_string()
+        self.failIfEqual(rand_a, rand_b)
+        self.failUnlessEqual(32, len(rand_a))
+        self.failUnlessEqual(32, len(rand_b))
+        self.failUnless(self.hex_chars.issuperset(rand_a))
+        self.failUnless(self.hex_chars.issuperset(rand_b))
+
+    def test_make_random_string_with_length(self):
+        for length in (8, 16, 64):
+            rand = utils.make_random_string(length)
+            self.failUnlessEqual(length, len(rand))
+            self.failUnless(self.hex_chars.issuperset(rand))
+
+    def test_make_random_string_with_bad_length(self):
+        # length must be a multiple of 2.
+        self.failUnlessRaises(
+            AssertionError, utils.make_random_string, 15)
 
=== added file 'lib/devscripts/ec2test/utils.py'
--- lib/devscripts/ec2test/utils.py	1970-01-01 00:00:00 +0000
+++ lib/devscripts/ec2test/utils.py	2009-10-02 12:34:16 +0000
@@ -0,0 +1,55 @@
+# Copyright 2009 Canonical Ltd.  This software is licensed under the
+# GNU Affero General Public License version 3 (see the file LICENSE).
+
+"""General useful stuff."""
+
+__metaclass__ = type
+__all__ = [
+    'find_datetime_string',
+    'make_datetime_string',
+    'make_random_string',
+    ]
+
+
+import binascii
+import datetime
+import os
+import re
+
+
+def make_datetime_string(when=None):
+    """Generate a simple formatted date and time string.
+
+    This is intended to be embedded in text to be later found by
+    `find_datetime_string`.
+    """
+    if when is None:
+        when = datetime.datetime.utcnow()
+    return when.strftime('%Y-%m-%d-%H%M')
+
+
+re_find_datetime = re.compile(
+    r'(\d{4})-(\d{2})-(\d{2})-(\d{2})(\d{2})')
+
+def find_datetime_string(text):
+    """Search for a simple date and time in arbitrary text.
+
+    The format searched for is %Y-%m-%d-%H%M - the same as produced by
+    `make_datetime_string`.
+    """
+    match = re_find_datetime.search(text)
+    if match is None:
+        return None
+    else:
+        return datetime.datetime(
+            *(int(part) for part in match.groups()))
+
+
+def make_random_string(length=32):
+    """Return a simple random UUID.
+
+    The uuid module is only available in Python 2.5 and above, but a
+    simple non-RFC-compliant hack here is sufficient.
+    """
+    assert length % 2 == 0, "length must be a multiple of 2"
+    return binascii.hexlify(os.urandom(length/2))