Launchpad itself

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

  1. fix-ec2-shutdown-617598
  2. Merge into devel
Proposed by Māris Fogels
Status: Merged
Approved by: Māris Fogels
Approved revision: no longer in the source branch.
Merged at revision: 11347
Proposed branch: lp:~mars/launchpad/fix-ec2-shutdown-617598
Merge into: lp:launchpad
Diff against target: 36 lines (+13/-2)
2 files modified
lib/devscripts/ec2test/builtins.py (+1/-1)
lib/devscripts/ec2test/testrunner.py (+12/-1)
To merge this branch: bzr merge lp:~mars/launchpad/fix-ec2-shutdown-617598
Reviewer Review Type Date Requested Status
Guilherme Salgado (community) Approve
Review via email: mp+32633@code.launchpad.net

Commit message

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.

Description of the change

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

To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) :
review: Approve
Revision history for this message
Jonathan Lange (jml) wrote :

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'.

Revision history for this message
Jonathan Lange (jml) 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.

Revision history for this message
Guilherme Salgado (salgado) wrote :

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.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== 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 @@
307 open_browser=open_browser, pqm_email=pqm_email,307 open_browser=open_browser, pqm_email=pqm_email,
308 include_download_cache_changes=include_download_cache_changes,308 include_download_cache_changes=include_download_cache_changes,
309 instance=instance, launchpad_login=instance._launchpad_login,309 instance=instance, launchpad_login=instance._launchpad_login,
310 timeout=480)310 timeout=300)
311311
312 instance.set_up_and_run(postmortem, attached, runner.run_tests)312 instance.set_up_and_run(postmortem, attached, runner.run_tests)
313313
314314
=== 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 @@
318 def configure_system(self):318 def configure_system(self):
319 user_connection = self._instance.connect()319 user_connection = self._instance.connect()
320 if self.timeout is not None:320 if self.timeout is not None:
321 user_connection.perform("sudo -b shutdown -h +%s" % self.timeout)321 # Activate a fail-safe shutdown just in case something goes
322 # really wrong with the server or suite.
323 #
324 # We need to use a call to /usr/bin/at here instead of a call to
325 # /sbin/shutdown because the test suite already uses the shutdown
326 # command after the suite finishes. If we called shutdown
327 # here, it would prevent the end-of-suite shutdown from executing,
328 # leaving the server running until the failsafe finally activates.
329 # See bug 617598 for the details.
330 user_connection.perform(
331 "echo sudo shutdown -h now | at today + %d minutes"
332 % self.timeout)
322 as_user = user_connection.perform333 as_user = user_connection.perform
323 # Set up bazaar.conf with smtp information if necessary334 # Set up bazaar.conf with smtp information if necessary
324 if self.email or self.message:335 if self.email or self.message: