launchpad-reviewers team mailing list archive

[Gavin Panella <gavin.panella@xxxxxxxxxxxxx>]

Gavin Panella has proposed merging lp:~allenap/launchpad/create-bucket-first into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers)


When calling bin/ec2 update-image, the given AMI name is actually an S3 destination bucket (this was confusing at first). This model is perhaps a bit confusing, but instead of trying to fix that I've just added a prerequisite check to create the bucket of the given name. This saves a long update-image run ending with an upload failure (with a not very informative error message to compound things).
-- 
https://code.launchpad.net/~allenap/launchpad/create-bucket-first/+merge/41630
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~allenap/launchpad/create-bucket-first into lp:launchpad/devel.

=== modified file 'lib/devscripts/ec2test/builtins.py'
--- lib/devscripts/ec2test/builtins.py	2010-11-22 16:23:02 +0000
+++ lib/devscripts/ec2test/builtins.py	2010-11-23 17:33:28 +0000
@@ -570,8 +570,8 @@
         instance = EC2Instance.make(
             session_name, instance_type, machine,
             credentials=credentials)
-        instance.check_bundling_prerequisites()
-
+        instance.check_bundling_prerequisites(
+            ami_name, credentials)
         instance.set_up_and_run(
             postmortem, True, self.update_image, instance,
             extra_update_image_command, ami_name, credentials, public)

=== modified file 'lib/devscripts/ec2test/credentials.py'
--- lib/devscripts/ec2test/credentials.py	2009-09-18 01:33:42 +0000
+++ lib/devscripts/ec2test/credentials.py	2010-11-23 17:33:28 +0000
@@ -12,11 +12,10 @@
 import os
 
 import boto
-
 from bzrlib.errors import BzrCommandError
-
 from devscripts.ec2test.account import EC2Account
 
+
 class CredentialsError(BzrCommandError):
     """Raised when AWS credentials could not be loaded."""
 
@@ -62,3 +61,10 @@
         """
         conn = boto.connect_ec2(self.identifier, self.secret)
         return EC2Account(name, conn)
+
+    def connect_s3(self):
+        """Connect to S3 with these credentials.
+
+        :return: A `boto.s3.connection.S3Connection` with these credentials.
+        """
+        return boto.connect_s3(self.identifier, self.secret)

=== modified file 'lib/devscripts/ec2test/instance.py'
--- lib/devscripts/ec2test/instance.py	2010-10-14 21:28:04 +0000
+++ lib/devscripts/ec2test/instance.py	2010-11-23 17:33:28 +0000
@@ -19,11 +19,9 @@
 import traceback
 
 from bzrlib.errors import BzrCommandError
-
+from devscripts.ec2test.session import EC2SessionName
 import paramiko
 
-from devscripts.ec2test.session import EC2SessionName
-
 
 DEFAULT_INSTANCE_TYPE = 'c1.xlarge'
 AVAILABLE_INSTANCE_TYPES = ('m1.large', 'm1.xlarge', 'c1.xlarge')
@@ -497,7 +495,7 @@
                 '%r must match a single %s file' % (pattern, file_kind))
         return matches[0]
 
-    def check_bundling_prerequisites(self):
+    def check_bundling_prerequisites(self, name, credentials):
         """Check, as best we can, that all the files we need to bundle exist.
         """
         if subprocess.call(['which', 'ec2-register']):
@@ -518,6 +516,10 @@
             local_ec2_dir, 'cert-*.pem', 'certificate')
         self.local_pk = self._check_single_glob_match(
             local_ec2_dir, 'pk-*.pem', 'private key')
+        # The bucket `name` needs to exist and be accessible. We create it
+        # here to reserve the name. If the bucket already exists and conforms
+        # to the above requirements, this is a no-op.
+        credentials.connect_s3().create_bucket(name)
 
     def bundle(self, name, credentials):
         """Bundle, upload and register the instance as a new AMI.