Merge lp:~allenap/launchpad/update-ec2-image into lp:launchpad

[Gavin Panella]

[r=abentley][ui=none][no-qa] New "images" command for bin/ec2 to display all current test images.

Add a new command to display all the usable test images for bin/ec2:

{{{
$ bin/ec2 help images
Purpose: Display all available images.
Usage: ec2 images

Options:
  --usage Show usage message and options.
  -v, --verbose Display more information.
  -q, --quiet Only display errors and warnings.
  -h, --help Show help message.

Description:
  The first in the list is the default image.

$ bin/ec2 images
  Rev AMI Owner ID Owner
  499 ami-0288626b 366009196755 salgado
  120 ami-217f9448 038531743404 jelmer
  119 ami-2d8e6044 366009196755 salgado
  117 ami-a9749bc0 036590675370 jml
  116 ami-5d1df134 366009196755 salgado
  114 ami-35806c5c 889698597288 henninge
  113 ami-a909e4c0 200337130613 mwhudson
  112 ami-9212f0fb 200337130613 mwhudson
  111 ami-d228cabb 200337130613 mwhudson
  102 ami-748f6d1d 255383312499 gary
  100 ami-22a1424b 200337130613 mwhudson
}}}

It also adds me to VALID_AMI_OWNERS.

[Aaron Bentley (abentley)]

VALID_AMI_OWNERS should come after EC2Account in the __all__.

It looks as if it would be simpler to use dict.setdefault rather than sorting, groupby, etc.

e.g. instead of

  search_results.append((revision, image)), make search_results a dict and
do
  search_results.setdefault(revision, []).append(image).

And then instead of
 # Sort and group by revision.
 get_revision = itemgetter(0)
 search_results.sort(key=get_revision, reverse=True)
 for revision, group in groupby(search_results, get_revision):
 yield revision, [image for (revision, image) in group]

do

 return sorted(search_results.items())

review: Approve

=== modified file 'lib/devscripts/ec2test/account.py'
--- lib/devscripts/ec2test/account.py	2010-07-30 15:52:05 +0000
+++ lib/devscripts/ec2test/account.py	2010-11-22 17:04:20 +0000
@@ -6,30 +6,33 @@
 __metaclass__ = type
 __all__ = [
     'EC2Account',
+    'VALID_AMI_OWNERS',
     ]
 
+from collections import defaultdict
 import cStringIO
+from datetime import datetime
+from operator import itemgetter
 import re
 import sys
 import urllib
 
-from datetime import datetime
-
 from boto.exception import EC2ResponseError
 from devscripts.ec2test.session import EC2SessionName
-
 import paramiko
 
-VALID_AMI_OWNERS = (
-    '255383312499', # gary
-    '559320013529', # flacoste
-    '200337130613', # mwhudson
-    '889698597288', # henninge
-    '366009196755', # salgado
-    '036590675370', # jml
-    '038531743404', # jelmer
+
+VALID_AMI_OWNERS = {
+    '255383312499': 'gary',
+    '559320013529': 'flacoste',
+    '200337130613': 'mwhudson',
+    '889698597288': 'henninge',
+    '366009196755': 'salgado',
+    '036590675370': 'jml',
+    '038531743404': 'jelmer',
+    '444667466231': 'allenap',
     # ...anyone else want in on the fun?
-    )
+    }
 
 AUTH_FAILURE_MESSAGE = """\
 POSSIBLE CAUSES OF ERROR:
@@ -160,6 +163,28 @@
         self.delete_previous_security_groups()
         self.delete_previous_key_pairs()
 
+    def find_images(self):
+        # We are trying to find an image that has a location that matches a
+        # regex (see definition of _image_match, above). Part of that regex is
+        # expected to be an integer with the semantics of a revision number.
+        # The image location with the highest revision number is the one that
+        # should be chosen. Because AWS does not guarantee that two images
+        # cannot share a location string, we need to make sure that the search
+        # result for this image is unique, or throw an error because the
+        # choice of image is ambiguous.
+        results = defaultdict(list)
+
+        # Find the images with the highest revision numbers and locations that
+        # match the regex.
+        images = self.conn.get_all_images(owners=tuple(VALID_AMI_OWNERS))
+        for image in images:
+            match = self._image_match(image.location)
+            if match is not None:
+                revision = int(match.group(1))
+                results[revision].append(image)
+
+        return sorted(results.iteritems(), key=itemgetter(0), reverse=True)
+
     def acquire_image(self, machine_id):
         """Get the image.
 
@@ -181,48 +206,20 @@
             # they can deal with it.
             return self.conn.get_image(machine_id)
 
-        # We are trying to find an image that has a location that matches a
-        # regex (see definition of _image_match, above). Part of that regex is
-        # expected to be an integer with the semantics of a revision number.
-        # The image location with the highest revision number is the one that
-        # should be chosen. Because AWS does not guarantee that two images
-        # cannot share a location string, we need to make sure that the search
-        # result for this image is unique, or throw an error because the
-        # choice of image is ambiguous.
-        search_results = None
-
-        # Find the images with the highest revision numbers and locations that
-        # match the regex.
-        for image in self.conn.get_all_images(owners=VALID_AMI_OWNERS):
-            match = self._image_match(image.location)
-            if match:
-                revision = int(match.group(1))
-                if (search_results is None
-                    or search_results['revision'] < revision):
-                    # Then we have our first, highest match.
-                    search_results = {'revision': revision, 'images': [image]}
-                elif search_results['revision'] == revision:
-                    # Another image that matches and is equally high.
-                    search_results['images'].append(image)
-
-        # No matching image.
-        if search_results is None:
+        images_by_revision = self.find_images()
+        if len(images_by_revision) == 0:
             raise RuntimeError(
                 "You don't have access to a test-runner image.\n"
                 "Request access and try again.\n")
 
-        # More than one matching image.
-        if len(search_results['images']) > 1:
+        revision, images = images_by_revision[0]
+        if len(images) > 1:
             raise ValueError(
-                ('more than one image of revision %(revision)d found: '
-                 '%(images)r') % search_results)
+                'More than one image of revision %d found: %r' % (
+                    revision, images))
 
-        # We could put a minimum image version number check here.
-        image = search_results['images'][0]
-        self.log(
-            'Using machine image version %d\n'
-            % (search_results['revision'],))
-        return image
+        self.log('Using machine image version %d\n' % revision)
+        return images[0]
 
     def get_instance(self, instance_id):
         """Look in all of our reservations for an instance with the given ID.

=== modified file 'lib/devscripts/ec2test/builtins.py'
--- lib/devscripts/ec2test/builtins.py	2010-11-05 16:54:24 +0000
+++ lib/devscripts/ec2test/builtins.py	2010-11-22 17:04:20 +0000
@@ -8,24 +8,30 @@
 
 import os
 import pdb
+import socket
 import subprocess
 
 from bzrlib.bzrdir import BzrDir
 from bzrlib.commands import Command
 from bzrlib.errors import BzrCommandError
 from bzrlib.help import help_commands
-from bzrlib.option import ListOption, Option
-
-import socket
-
+from bzrlib.option import (
+    ListOption,
+    Option,
+    )
 from devscripts import get_launchpad_root
-
+from devscripts.ec2test.account import VALID_AMI_OWNERS
 from devscripts.ec2test.credentials import EC2Credentials
 from devscripts.ec2test.instance import (
-    AVAILABLE_INSTANCE_TYPES, DEFAULT_INSTANCE_TYPE, EC2Instance)
+    AVAILABLE_INSTANCE_TYPES,
+    DEFAULT_INSTANCE_TYPE,
+    EC2Instance,
+    )
 from devscripts.ec2test.session import EC2SessionName
-from devscripts.ec2test.testrunner import EC2TestRunner, TRUNK_BRANCH
-
+from devscripts.ec2test.testrunner import (
+    EC2TestRunner,
+    TRUNK_BRANCH,
+    )
 
 # Options accepted by more than one command.
 
@@ -188,7 +194,6 @@
     return branches, test_branch
 
 
-
 DEFAULT_TEST_OPTIONS = '--subunit -vvv'
 
 
@@ -409,7 +414,8 @@
                 "Commit text not specified. Use --commit-text, or specify a "
                 "message on the merge proposal.")
         if rollback and (no_qa or incremental):
-            print "--rollback option used. Ignoring --no-qa and --incremental."
+            print (
+                "--rollback option used. Ignoring --no-qa and --incremental.")
         try:
             commit_message = mp.build_commit_message(
                 commit_text, testfix, no_qa, incremental, rollback=rollback)
@@ -613,6 +619,25 @@
         instance.bundle(ami_name, credentials)
 
 
+class cmd_images(EC2Command):
+    """Display all available images.
+
+    The first in the list is the default image.
+    """
+
+    def run(self):
+        credentials = EC2Credentials.load_from_file()
+        session_name = EC2SessionName.make(EC2TestRunner.name)
+        account = credentials.connect(session_name)
+        format = "%5s  %-12s  %-12s  %s\n"
+        self.outf.write(format % ("Rev", "AMI", "Owner ID", "Owner"))
+        for revision, images in account.find_images():
+            for image in images:
+                self.outf.write(format % (
+                        revision, image.id, image.ownerId,
+                        VALID_AMI_OWNERS.get(image.ownerId, "unknown")))
+
+
 class cmd_help(EC2Command):
     """Show general help or help for a command."""