Merge lp:~mars/launchpad/fix-ec2-shutdown-617598 into lp:launchpad

[Māris Fogels]

Backed out r11224. Fixes a problem where EC2 instances where only shutting down after a full 8 hours. Dropped the EC2 failsafe timeout to 5 hours from 8.

Hi,

This branch fixes a problem with competing shutdown calls in the ec2 testrunner stepping on each other's toes, causing the instance to keep running for a full 8 hours. See bug 617598 for details.

On salgado's advice this backs out the failsafe shutdown code just in case someone else writes a shutdown call somewhere in the future and encounters this same problem. I backed out r11224 of devel in it's entirety.

While making the fix I dropped the failsafe timeout from 8 hours to 5. The suite runs inside four hours, so this should not be a problem.

I will run 'ec2 land' in order to test this change.

Maris

[Guilherme Salgado (salgado)]

review: Approve

[Jonathan Lange (jml)]

That's the code we had before! I think it's well worth adding to the comment, explaining why we are using 'at' rather than giving a time to 'shutdown'.

[Jonathan Lange (jml)]

My bad. My email client didn't show me that the code had been updated based on (IRC?) reviewer feedback. Maybe that's a bug in Launchpad.

[Guilherme Salgado (salgado)]

On Fri, 2010-08-13 at 23:52 +0000, Jonathan Lange wrote:
> My bad. My email client didn't show me that the code had been updated based on (IRC?) reviewer feedback. Maybe that's a bug in Launchpad.

Yeah, I think I've filed a bug some time ago about having email
notifications sent out when there's a new revision added to a branch
that is proposed for merging. Can't seem to find it now, though.

=== modified file 'lib/devscripts/ec2test/builtins.py'
--- lib/devscripts/ec2test/builtins.py	2010-07-29 19:52:31 +0000
+++ lib/devscripts/ec2test/builtins.py	2010-08-13 22:06:46 +0000
@@ -307,7 +307,7 @@
             open_browser=open_browser, pqm_email=pqm_email,
             include_download_cache_changes=include_download_cache_changes,
             instance=instance, launchpad_login=instance._launchpad_login,
-            timeout=480)
+            timeout=300)
 
         instance.set_up_and_run(postmortem, attached, runner.run_tests)
 

=== modified file 'lib/devscripts/ec2test/testrunner.py'
--- lib/devscripts/ec2test/testrunner.py	2010-07-25 10:43:37 +0000
+++ lib/devscripts/ec2test/testrunner.py	2010-08-13 22:06:46 +0000
@@ -318,7 +318,18 @@
     def configure_system(self):
         user_connection = self._instance.connect()
         if self.timeout is not None:
-            user_connection.perform("sudo -b shutdown -h +%s" % self.timeout)
+            # Activate a fail-safe shutdown just in case something goes
+            # really wrong with the server or suite.
+            #
+            # We need to use a call to /usr/bin/at here instead of a call to
+            # /sbin/shutdown because the test suite already uses the shutdown
+            # command after the suite finishes. If we called shutdown
+            # here, it would prevent the end-of-suite shutdown from executing,
+            # leaving the server running until the failsafe finally activates.
+            # See bug 617598 for the details.
+            user_connection.perform(
+                "echo sudo shutdown -h now | at today + %d minutes"
+                % self.timeout)
         as_user = user_connection.perform
         # Set up bazaar.conf with smtp information if necessary
         if self.email or self.message: